[Bug nptl/11670] New: Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[Bug nptl/11670] New: Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[ceggers at gmx dot de]

Please consider the following code snippset:

--- snipp ---
my_ptr = malloc(100);
pthread_cleanup_push(my_handler, my_ptr);
...
pthread_cleanup_pop(1);
--- /snipp ---

With -Os optimisation, the C equivalent of the optimised code can look similar
to this (pure C, __EXCEPTIONS not defined):

--- snipp ---
/* # define pthread_cleanup_push(routine, arg) */
  do {
    __pthread_unwind_buf_t __cancel_buf;
[OPTIMISED AWAY]void (*__cancel_routine) (void *) = (routine);
    void *__cancel_arg = (arg);
    int not_first_call = __sigsetjmp ((struct __jmp_buf_tag *) (void *)
				      __cancel_buf.__cancel_jmp_buf, 0);
    if (__builtin_expect (not_first_call, 0))
      {
	my_handler (__cancel_arg);  [__cancel_routine has been replaced by a fixed
function call]
	__pthread_unwind_next (&__cancel_buf);
	/* NOTREACHED */
      }

    __pthread_register_cancel (&__cancel_buf);
    do {

...
<User code>
[somewhere here the stack memory for __cancel_arg is "reused" by another
variable, because ...]
<User code>
...

/* # define pthread_cleanup_pop(execute) */
      do { } while (0);/* Empty to allow label before pthread_cleanup_pop.  */
    } while (0);
    __pthread_unregister_cancel (&__cancel_buf);
    if (execute)
      my_handler (my_ptr);  [... __cancel_arg has been merged with my_ptr]
  } while (0)
--- /snipp ---

The compiler seems to recognize that the content of my_ptr and __cancel_arg is
always the same, so there's no need to keep both on the stack. __cancel_arg in
pthread_cleanup_pop() is replaced by my_ptr.

As long as no cancellation happens everything is fine. my_handler() is always
called with the correct argument. But in case of cancellation siglongjmp() may
be called from a position after the stack memory for __cancel_arg has been
"reused". In this case __sigsetjmp() returns and my_handler() is called with
something different than the original value of my_ptr.

The problem is not limited to __cancel_arg, the same could also happen to
__cancel_buf or __cancel_routine (if not replaced by a fixed function call). I
think there's no guarantee that none of the three stack variables is discarded
between __pthread_register_cancel() and __pthread_unregister_cancel().

As a first try to solve this I've modified the macros so that they are more
similar to the variant used when __EXCEPTIONS is defined (using the __cleanup__
attribute from GCC). The result seems to work for me, but it's in the nature of
compiler optimisations that problems seems to be "disappeared" if something is
altered a little bit ...

-- 
           Summary: Variables defined in pthread_cleanup_push() macro may be
                    optimised away which breaks __sigsetjmp
           Product: glibc
           Version: 2.12
            Status: NEW
          Severity: normal
          Priority: P2
         Component: nptl
        AssignedTo: drepper at redhat dot com
        ReportedBy: ceggers at gmx dot de
                CC: glibc-bugs at sources dot redhat dot com
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: arm-arm920t-linux-gnueabi


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[ceggers at gmx dot de]

------- Additional Comments From ceggers at gmx dot de  2010-06-05 07:22 -------
Created an attachment (id=4828)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4828&action=view)
Patch for pthread_cleanup_push() and pthread_cleanup_pop()


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[schwab at linux-m68k dot org]

------- Additional Comments From schwab at linux-m68k dot org  2010-06-07 08:40 -------
Which compiler are you using?  Since the variables are not modified between
setjmp and longjmp they must retain their values even after setjmp returns the
second time and the compiler is not allowed to reuse their stack location.  This
looks like a compiler bug.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[christian dot eggers at kathrein dot de]

------- Additional Comments From christian dot eggers at kathrein dot de  2010-06-07 12:05 -------
(In reply to comment #2)
> Which compiler are you using?  
I'm using gcc-4.4.4

> Since the variables are not modified between setjmp and longjmp they must
> retain their values even after setjmp returns the
> second time and the compiler is not allowed to reuse their stack location.  
> This looks like a compiler bug.
I think the compiler has no chance to detect that __sigsetjmp() may return a
second time (usual functions don't do that). I think that the "returns_twice"
attribute has to be added to the prototype of __sigsetjmp() in pthread.h.

The longjmp(3) manpage says that the values of automatic variables are ONLY
unspecified if they are changed between setjmp() and longjmp(). Maybe this
depends on the "returns_twice" attribute.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[schwab at linux-m68k dot org]

------- Additional Comments From schwab at linux-m68k dot org  2010-06-07 12:20 -------
GCC already knows that __sigsetjmp returns twice.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[schwab at linux-m68k dot org]

------- Additional Comments From schwab at linux-m68k dot org  2010-06-07 12:26 -------
Please report that as a compiler bug.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[christian dot eggers at kathrein dot de]

------- Additional Comments From christian dot eggers at kathrein dot de  2010-06-07 13:03 -------
(In reply to comment #4)
> GCC already knows that __sigsetjmp returns twice.
Are you sure?

# cd src/gcc-4.4.4
# find . | xargs grep __sigsetjmp
[some files under fixincludes]
<EOF>

I've no idea about GCC internals, but how does GCC know about the special
behaviour of __sigsetjmp()? Is this documented somewhere?

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[jakub at redhat dot com]

------- Additional Comments From jakub at redhat dot com  2010-06-07 13:12 -------
See special_function_p, it will skip over __ prefix and then match sigsetjmp.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11670

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug nptl/11670] Variables defined in pthread_cleanup_push() macro may be optimised away which breaks __sigsetjmp

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=11670

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.