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

[Paul Carroll <Paul_Carroll at mentor dot com>]

I originally submitted this patch back in May as part of Bugzilla 
#21513.  That issue was never pursued to completion.

There is an open issue filed against the Gcc Middle-End - Bugzilla #61118.
In that issue, it is noted that the use of the pthread_cleanup_push 
macro from pthread.h will generate 2 warnings when -Wclobbered is set.
The warnings are for the '__cancel_routine' and '__cancel_arg' variables 
that are created as part of the macro definition.
The warning occurs because of the presence of a sigsetjmp() call after 
those 2 variables are defined.

For our customer, who compiles with -Werror, the presence of these 
warnings is unacceptable.

In the absence of a GCC fix, a solution is to modify the macro 
definitions in pthread.h so as to mark those variables as 'volatile'.
The changes would be made to both pthread_cleanup_push and 
pthread_cleanup_push_defer_np macro definitions.  The changes would make 
the macros look something like this:

# define pthread_cleanup_push(routine, arg) \
  do {                                                               \
    __pthread_unwind_buf_t __cancel_buf;                             \
    void (* volatile __cancel_routine) (void *) = (routine);         \
    void * volatile __cancel_arg = (arg);                            \
    :

Since those variables are now reloaded whenever they are used and thus 
cannot not be clobbered by the setjmp, the compilation is quiet.

Attempts were made to use '#pragma GCC diagnostic' to turn off the 
warning specifically for this macro expansion, but that had no effect.

And, as has been noted in the GCC Bugzilla issue, this issue only occurs 
when optimization is used and when files are preprocessed (i.e., not .i 
files).


The analysis behind this patch was noted by one of our engineers:

(a) The warning is a false positive. The _cancel_routine and cancel_arg 
variables will not in fact be modified between the _sigsetjmp call and 
the corresponding longjmp (if any) with the same jmp_buf.

(b) The compiler has no way of knowing that it is a false positive. The 
calls to the macros are within a loop and the compiler cannot tell that 
a longjmp does not occur that uses the jmp_buf from a previous iteration.

(c) As these warnings occur quite late and are based on when RTL pseudos 
for particular variables are live, they are very sensitive to details of 
source code and code generation.

(d) Occurring that late also means the macro expansion contexts that are 
available earlier in compilation are not available here, only the 
expansion-point location. This probably explains why diagnostic pragmas 
inside the cleanup macros do not serve to disable the warning.

(e) Going via preprocessed source simplifies the location information to 
the form that can be represented in .i files. By forcing every expanded 
token to be either a system header token or not, as marked in the .i 
file, it may well perturb details of when warnings occur for code coming 
partly from expansions of macros in system headers. The warning state in 
normal compilation matters more than the warning state when information 
has been discarded by going through a .i file.

(f) On that basis, although it is a workaround for a compiler limitation 
regarding diagnostic pragmas, adding volatile in pthread.h seems a 
reasonable approach to avoiding the warning.

Attachment: pthread_patch
Description: Text document