: Fix failing test malloc/tst-interpose-nothread with GCC 7.

Message ID 639e8844-ca14-1873-2407-484f286974b1@linux.vnet.ibm.com
State Committed
Headers

Commit Message

Stefan Liebler March 21, 2017, 9:39 a.m. UTC
  Hi,

The test malloc/tst-interpose-nothread fails on s390x if built
with GCC 7 and recent glibc commit "Remove the str(n)dup inlines
from string/bits/string2.h. Although inlining"
(ae65d4f3c3995279ca458c460ebf8bab1885fa03) with output:
error: free: 0x3fffdffa010: invalid allocation index: 0 (not less than 0)

The destructor check_for_allocations in malloc/tst-interpose-aux.c is
called twice.  One time after the test-child-process has finished
successfully and once after the test-parent-process finishes.
During the latter invocation, allocation_index == 0.  GCC 7 is
now inlining the free function and calls unconditionally fail in
get_header as header->allocation_index (type == size_t) is always
 >= allocation_index (== 0).
Before the mentioned commit above, strdup was replaced by strlen,
malloc and memcpy.  The malloc call was also inlined and
allocation_index was set to one.

This patch moves the already existing compiler barrier before
the invocation of free.

Okay to commit?

ChangeLog:

	* malloc/tst-interpose-aux.c (check_for_allocations):
	Move compiler barrier before free.
  

Comments

Florian Weimer March 21, 2017, 10:06 a.m. UTC | #1
On 03/21/2017 10:39 AM, Stefan Liebler wrote:
>     	* malloc/tst-interpose-aux.c (check_for_allocations):
>     	Move compiler barrier before free.

Looks reasonable.  Thanks.

Florian
  
Stefan Liebler March 21, 2017, 3:43 p.m. UTC | #2
On 03/21/2017 11:06 AM, Florian Weimer wrote:
> On 03/21/2017 10:39 AM, Stefan Liebler wrote:
>>         * malloc/tst-interpose-aux.c (check_for_allocations):
>>         Move compiler barrier before free.
>
> Looks reasonable.  Thanks.
>
> Florian
>
Committed.

Thanks.
Stefan
  

Patch

commit 0d796fb327dc6e14e7888eb4a0531fdde4ce93b2
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Tue Mar 21 09:29:09 2017 +0100

    Fix failing test malloc/tst-interpose-nothread with GCC 7.
    
    The test malloc/tst-interpose-nothread fails on s390x if built
    with GCC 7 and glibc commit "Remove the str(n)dup inlines
    from string/bits/string2.h. Although inlining"
    (ae65d4f3c3995279ca458c460ebf8bab1885fa03) with output:
    error: free: 0x3fffdffa010: invalid allocation index: 0 (not less than 0)
    
    The destructor check_for_allocations in malloc/tst-interpose-aux.c is
    called twice.  One time after the test-child-process has finished successfully
    and once after the test-parent-process finishes.
    During the latter invocation, allocation_index = 0.  GCC 7 is now inlining the
    free function and calls unconditionally fail in get_header as
    header->allocation_index (type == size_t) is always >= allocation_index (= 0).
    Before the mentioned commit above, strdup was replaced by strlen, malloc and
    memcpy.  The malloc call was also inlined and allocation_index was set to one.
    
    This patch moves the already existing compiler barrier before the invocation
    of free.
    
    ChangeLog:
    
    	* malloc/tst-interpose-aux.c (check_for_allocations):
    	Move compiler barrier before free.

diff --git a/malloc/tst-interpose-aux.c b/malloc/tst-interpose-aux.c
index e80e979..68282b4 100644
--- a/malloc/tst-interpose-aux.c
+++ b/malloc/tst-interpose-aux.c
@@ -113,11 +113,11 @@  check_for_allocations (void)
     {
       /* Make sure that malloc is called at least once from libc.  */
       void *volatile ptr = strdup ("ptr");
-      free (ptr);
       /* Compiler barrier.  The strdup function calls malloc, which
          updates allocation_index, but strdup is marked __THROW, so
          the compiler could optimize away the reload.  */
       __asm__ volatile ("" ::: "memory");
+      free (ptr);
       /* If the allocation count is still zero, it means we did not
          interpose malloc successfully.  */
       if (allocation_index == 0)