The pre-release GCC 12.0 development branch has a new warning about
dangling pointers in -Wall:
http.c: In function ‘run_active_slot’:
http.c:1332:24: error: storing the address of local variable ‘finished’ in ‘*slot.finished’ [-Werror=dangling-pointer=]
1332 | slot->finished = &finished;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~
http.c:1330:13: note: ‘finished’ declared here
1330 | int finished = 0;
| ^~~~~~~~
This is on a locally built "gcc (GCC) 12.0.1 20220120 (experimental)",
built from gcc.git's 8bc700f4c3f (Enhance vec_pack_trunc for integral
mode mask., 2022-01-17).
The GCC warning is specifically about pointers that survive the exit
of the function. See a comment added to
"pass_waccess::use_after_inval_p" in the GCC commit that added the
warning, or:
/* The use is one of a dangling pointer if a clobber of the variable
[the pointer points to] has not been found before the function exit
point. */
[...]
There's a few possible ways to fix this, but the simplest is to assign
NULL to "slot->finished" at the end of run_active_slot().
This isn't the only caller that assigns to "slot->finished", see see
the assignments in http-walker.c:process_alternates_response() and
http.c:finish_active_slot().
But those assignments are both to the pointer to our local variable
here, so this fix is correct. The only way that code in http-walker.c
could have done its assignments is to the pointer to this specific
variable.
It was suggested[2] to guard that with "if (slot->finished ==
&finished)", but that'll still trigger the warning.
1. https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9d6a0f388eb048f8d87f47af78f07b5ce513bfe6
2. https://lore.kernel.org/git/xmqq8rv2nggn.fsf@gitster.g/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
This should clarify the feedback on v2, sorry about the very late
re-roll.
I.e. I *meant* in v2 that it's the only assignment to slot->finished
itself is here, the other assignments in http-walker.c are to the
pointer to our variable here.
Range-diff against v2:
1: 777838267a5 ! 1: 69190804c67 http API: fix dangling pointer issue noted by GCC 12.0
@@ Commit message
[...]
There's a few possible ways to fix this, but the simplest is to assign
- NULL to "slot->finished" at the end of run_active_slot(), it's the
- only caller that ever assigns non-NULL to it. It was suggested[2] to
- guard that with "if (slot->finished == &finished)", but that'll still
- trigger the warning.
+ NULL to "slot->finished" at the end of run_active_slot().
+
+ This isn't the only caller that assigns to "slot->finished", see see
+ the assignments in http-walker.c:process_alternates_response() and
+ http.c:finish_active_slot().
+
+ But those assignments are both to the pointer to our local variable
+ here, so this fix is correct. The only way that code in http-walker.c
+ could have done its assignments is to the pointer to this specific
+ variable.
+
+ It was suggested[2] to guard that with "if (slot->finished ==
+ &finished)", but that'll still trigger the warning.
1. https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9d6a0f388eb048f8d87f47af78f07b5ce513bfe6
2. https://lore.kernel.org/git/xmqq8rv2nggn.fsf@gitster.g/
http.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/http.c b/http.c
index 229da4d1488..2f67fbb33cd 100644
--- a/http.c
+++ b/http.c
@@ -1367,6 +1367,7 @@ void run_active_slot(struct active_request_slot *slot)
select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
}
}
+ slot->finished = NULL;
}
static void release_active_slot(struct active_request_slot *slot)
--
2.35.1.1509.ge4eeb5bd39e
