Subject: Re: [PATCH] http.c: clear the 'finished' member once we are done with it

From: Junio C Hamano <[email-masked]>
Date: Tue, 24 May 2022 16:19:08 -0700

Carlo Marcelo Arenas Belón <[email-masked]> writes:

> On Tue, May 24, 2022 at 10:15:57AM -0700, Junio C Hamano wrote:
>> 
>> I _think_ we can even get away by not doing anything to
>> slot->finished at the end of run_active_slot(), as we are not
>> multi-threaded and the callee only returns to the caller, but if it
>> helps pleasing the warning compiler, I'd prefer the simplest
>> workaround, perhaps with an unconditional clearing there?
>
> Assuming that some overly clever compiler might optimize that out (either
> because it might think it is Undefined Behaviour or for other unknown
> reasons) then Ævar's version would be better for clearing the "warning".

You keep saying undefined behaviour but in this case I do not quite
see there is anything undefined.

The warning, as Ævar said in a message, is about storing an address
of an object on the stack in another object whose lifetime lasts
beyond the life of the stackframe.  If you dereference such a
pointer _after_ we return from run_active_slot() function, the
behaviour may indeed be undefined.

But if you recall one such call trace I walked through for Dscho in
another message this morning, we do not make such a dereferencing.
The run_active_slot() function sets up the slot with the pointer to
its on-stack variable in it, we make a call chain that is several
levels deep, and at some point in the call chain, the request that
is represented by the slot may complete and slot may be passed to
finish_active_slot(), which would update (*slot->finished) thus
modifying the on-stack variable of the run_active_slot() that we
will eventually return to.

Is such a use of the pointer in the structure a cause for an
undefined behaviour?