Skip to content

ci: fix windows-build with GCC v12.x #3864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

dscho
Copy link
Member

@dscho dscho commented May 23, 2022

A recent update of GCC in Git for Windows' SDK (a subset of which is used in Git's CI/PR builds) broke the build. Lets' fix it.

This is a companion of gitgitgadget#1238.

@dscho dscho changed the title ci: fix windows-build with GCC v12.x ci: fix windows-build with GCC v12.x May 23, 2022
@dscho dscho self-assigned this May 23, 2022
@dscho dscho force-pushed the fix-win-build-with-gcc-12-on-top-of-v2.36.1 branch from 275e29f to 365889e Compare May 23, 2022 23:46
dscho added 3 commits May 24, 2022 15:58
Git for Windows' SDK recently upgraded to GCC v12.x which points out
that the `pos` variable might be used even after the corresponding
memory was `realloc()`ed and therefore potentially no longer valid.

Since a subset of this SDK is used in Git's CI/PR builds, we need to fix
this to continue to be able to benefit from the CI/PR runs.

Note: This bug has been with us since 2a6b149 (mingw: avoid using
strbuf in syslog, 2011-10-06), and while it looks tempting to replace
the hand-rolled string manipulation with a `strbuf`-based one, that
commit's message explains why we cannot do that: The `syslog()` function
is called as part of the function in `daemon.c` which is set as the
`die()` routine, and since `strbuf_grow()` can call that function if it
runs out of memory, this would cause a nasty infinite loop that we do
not want to re-introduce.

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
GCC v12.x complains thusly:

compat/nedmalloc/nedmalloc.c: In function 'DestroyCaches':
compat/nedmalloc/nedmalloc.c:326:12: error: the comparison will always
                              evaluate as 'true' for the address of 'caches'
                              will never be NULL [-Werror=address]
  326 |         if(p->caches)
      |            ^
compat/nedmalloc/nedmalloc.c:196:22: note: 'caches' declared here
  196 |         threadcache *caches[THREADCACHEMAXCACHES];
      |                      ^~~~~~

... and it is correct, of course.

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Technically, the pointer difference `end - start` _could_ be negative,
and when cast to an (unsigned) `size_t` that would cause problems. In
this instance, the symptom is:

dir.c: In function 'git_url_basename':
dir.c:3087:13: error: 'memchr' specified bound [9223372036854775808, 0]
       exceeds maximum object size 9223372036854775807
       [-Werror=stringop-overread]
    CC ewah/bitmap.o
 3087 |         if (memchr(start, '/', end - start) == NULL
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

While it is a bit far-fetched to think that `end` (which is defined as
`repo + strlen(repo)`) and `start` (which starts at `repo` and never
steps beyond the NUL terminator) could result in such a negative
difference, GCC has no way of knowing that.

See also https://gcc.gnu.org/bugzilla//show_bug.cgi?id=85783.

Let's just add a safety check, primarily for GCC's benefit.

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
@dscho dscho force-pushed the fix-win-build-with-gcc-12-on-top-of-v2.36.1 branch from 365889e to 402b28e Compare May 27, 2022 20:47
This branch brings a couple of fixes to be able to compile Git in
`DEVELOPER=1` mode again.

The only remaining issue is a local variable's address in
`run_active_slot()` in `http.c` that is assigned to a struct that is
still valid after the function scope was left. Junio wanted to do this
differently from my proposed solution, and we'll merge Junio's
work-around next.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the fix-win-build-with-gcc-12-on-top-of-v2.36.1 branch from 402b28e to 78f62fb Compare May 27, 2022 20:55
@dscho dscho added this to the Next release milestone May 27, 2022
In http.c, the run_active_slot() function allows the given "slot" to
make progress by calling step_active_slots() in a loop repeatedly,
and the loop is not left until the request held in the slot
completes.

Ages ago, we used to use the slot->in_use member to get out of the
loop, which misbehaved when the request in "slot" completes (at
which time, the result of the request is copied away from the slot,
and the in_use member is cleared, making the slot ready to be
reused), and the "slot" gets reused to service a different request
(at which time, the "slot" becomes in_use again, even though it is
for a different request).  The loop terminating condition mistakenly
thought that the original request has yet to be completed.

Today's code, after baa7b67 (HTTP slot reuse fixes, 2006-03-10)
fixed this issue, uses a separate "slot->finished" member that is
set in run_active_slot() to point to an on-stack variable, and the
code that completes the request in finish_active_slot() clears the
on-stack variable via the pointer to signal that the particular
request held by the slot has completed.  It also clears the in_use
member (as before that fix), so that the slot itself can safely be
reused for an unrelated request.

One thing that is not quite clean in this arrangement is that,
unless the slot gets reused, at which point the finished member is
reset to NULL, the member keeps the value of &finished, which
becomes a dangling pointer into the stack when run_active_slot()
returns.  Clear the finished member before the control leaves the
function, which has a side effect of unconfusing compilers like
recent GCC 12 that is over-eager to warn against such an assignment.

Signed-off-by: Junio C Hamano <[email protected]>
This work-around shuts up GCC when it complains about a local variable's
address being assigned to a struct that lives on after returning from
the function.

Technically, GCC is wrong not to complain that the pointer could have
been copied and used later, but hey, the code compiles so we're happy.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the fix-win-build-with-gcc-12-on-top-of-v2.36.1 branch from 78f62fb to bc65862 Compare May 30, 2022 11:21
@dscho dscho merged commit 158a93f into git-for-windows:main May 30, 2022
@dscho dscho deleted the fix-win-build-with-gcc-12-on-top-of-v2.36.1 branch May 30, 2022 13:11
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 30, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 31, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 31, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request May 31, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 1, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 1, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 7, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 7, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 7, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 7, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 8, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 8, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 8, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 9, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 9, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 9, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 10, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 10, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 10, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 10, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 10, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 13, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 14, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
git-for-windows-ci pushed a commit that referenced this pull request Jun 14, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 14, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 15, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 18, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 23, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jun 27, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
dscho added a commit that referenced this pull request Jul 9, 2022
…of-v2.36.1

ci: fix `windows-build` with GCC v12.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants