git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] http API: fix dangling pointer issue noted by GCC 12.0

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 27, 2022, 18:23 UTC
Message-ID
<xmqqv8y52g3a.fsf@gitster.g>
In-Reply-To
<220127.86mtjhdeme.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 28 quoted lines
>> If the motivation behind the original "do not get fooled by a reused
>> slot still working on somebody else's request---instead return when
>> our request is done" was indeed what I speculated, then the pointer
>> slot->finished when we leave this function should not matter to
>> anybody.  Would the following patch make the compiler realize that
>> we never smuggle a local variable's address out of this function via
>> a pointer in the structure?
>>
>>  http.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git c/http.c w/http.c
>> index 229da4d148..85437b1980 100644
>> --- c/http.c
>> +++ w/http.c
>> @@ -1367,6 +1367,9 @@ void run_active_slot(struct active_request_slot *slot)
>>  			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
>>  		}
>>  	}
>> +
>> +	if (slot->finished == &finished)
>> +		slot->finished = NULL;
>>  }
>>  
>>  static void release_active_slot(struct active_request_slot *slot)
>
> Yes, that does quiet it. The GCC warning is specifically about pointers
> that survive the exit of the function.
Thanks for a quick testing.

The real question is if this breaks anything. By definition, if this changes the behaviour, it is an indication that the code has been somehow depending on having a pointer to a stackframe that has already gone out of scope, which is a more serious problem. I am wondering if we need to dig further to find it out, and if so, how.

In any case, it looks like a more correct fix to address the "early GCC 12" problem, than reverting baa7b67d (HTTP slot reuse fixes, 2006-03-10), at least to me.

Unless we devise another way to address the "slot reuse" issue, or we come up with an explanation that the "slot reuse" issue is no longer possible in todays code, that is.

Thanks.
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 6 of 15 in “http API: fix dangling pointer issue noted by GCC 12.0”
  1. http API: fix dangling pointer issue noted by GCC 12.0Ævar Arnfjörð Bjarmason, Jan 26, 2022
  2. Taylor BlauJan 26, 2022
  3. Junio C HamanoJan 27, 2022
  4. Junio C HamanoJan 27, 2022
  5. Ævar Arnfjörð BjarmasonJan 27, 2022
  6. Junio C HamanoJan 27, 2022
  7. http API: fix dangling pointer issue noted by GCC 12.0Ævar Arnfjörð Bjarmason, Feb 25, 2022
  8. Junio C HamanoFeb 25, 2022
  9. Taylor BlauFeb 26, 2022
  10. http API: fix dangling pointer issue noted by GCC 12.0Ævar Arnfjörð Bjarmason, Mar 25, 2022
  11. Taylor BlauMar 25, 2022
  12. Junio C HamanoMar 26, 2022
  13. Ævar Arnfjörð BjarmasonApr 14, 2022
  14. Junio C HamanoApr 14, 2022
  15. Ævar Arnfjörð BjarmasonApr 15, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.