Re: [PATCH] pthread_cleanup_push macro generates warning when -Wclobbered is set

[Florian Weimer <fweimer at redhat dot com>]

On 11/15/2017 02:44 PM, Joseph Myers wrote:
> On Wed, 15 Nov 2017, Florian Weimer wrote:
>
> > > The "returns twice" information in GCC does not link the possible second
> > > return to the lifetime of a particular object.
> >
> > But the control flow in pthread_cleanup_push is such that for both returns,
> > local objects of limited scope are referenced immediately, so the compiler
> > could infer that.
>
> As I see it, the point of the warning is to say that a second return might
> access a variable that has (through being in another loop iteration, in
> this case) changed.  If we reason that such an access would be invalid if
> in another loop iteration, and so we can't be in another loop iteration,
> when would the warning ever occur?

I think the warning is completely bogus in this case, and for any 
properly matched use of pthread_cleanup_push and pthread_cleanup_pop, as 
far as the __cancel_routine and __cancel_arg variables are concerned.

The warning is valid in in other cases.  I think here it is valid 
because the lifetime of the variable a has not ended upon the second 
return from setjmp:

#include <setjmp.h>

extern jmp_buf buf;
extern void f1 (void);
extern void f2 (void);

int
f3 (void)
{
  int a = 1;
  if (setjmp (buf))
    return a;
  f1 ();
  ++a;
  f2 ();
  return a;
}

But as far as I can see, this is always a portability warning because 
GCC will put the variable on the stack to make sure that its value is in 
fact determinate.

> As far as the compiler can see, the __sigsetjmp call makes the jmp_buf
> contents escape, and at any subsequent point (before the function returns
> or a scope with a variably modified type is left) the function might
> return again - and if this is when the containing scope has reentered, the
> warning is meant to warn, not deduce that in fact that case does not
> occur.  Some more precise way of describing the possible times of a second
> return would be needed to make it valid not to warn.

Still not convinced about this, and what's worse, I don't understand why 
you come to a different conclusion.  This shouldn't be a matter of 
opinion. 8-/

> > However, I still don't see how this matters here.  __cancel_routine and
> > __cancel_arg are not addressable and not written to after initialization (and
> > that's also true for the future argument in Paul's reproducer). This means
> > that even with the POSIX interpretation if setjmp (where object lifetimes do
> > not float outwards, and the address of the jump buffer matters), there is no
> > wiggle room for the implementation to change the value of these variables.
>
> The end of the containing scope causes their values to be uninitialized.

They cease to exist completely once the block is exited.  Any further 
access should result in undefined behavior.

Thanks,
Florian