[Bug libstdc++/64798] New: [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[Bug libstdc++/64798] New: [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[ro at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

            Bug ID: 64798
           Summary: [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ro at gcc dot gnu.org
                CC: rguenth at gcc dot gnu.org
              Host: sparc*-sun-solaris2.*
            Target: sparc*-sun-solaris2.*
             Build: sparc*-sun-solaris2.*

Between 20150116 (r219745) and 20150123 (r220039),
g++.old-deja/g++.eh/badalloc1.C
started to FAIL on 32-bit Solaris/SPARC:

FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++11 execution test
FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++14 execution test
FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++98 execution test

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1 (LWP 1)]
__cxxabiv1::__cxa_throw (obj=0x2195c <arena+92>, 
    tinfo=0x73908 <typeinfo for int>, dest=0x0)
    at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:76
76       
__GXX_INIT_PRIMARY_EXCEPTION_CLASS(header->exc.unwindHeader.exception_class);
1: x/i $pc
=> 0xff220314 <__cxxabiv1::__cxa_throw(void*, std::type_info*, void
(*)(void*))+96>:    sttw  %g2, [ %i0 + -24 ]
(gdb) where
#0  __cxxabiv1::__cxa_throw (obj=0x2195c <arena+92>, 
    tinfo=0x73908 <typeinfo for int>, dest=0x0)
    at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:76
#1  0x00011114 in fn_throw ()
    at
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C:98
#2  0x000112f8 in main ()
    at
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C:129
(gdb) p $i0-24
$3 = 137540

The SEGV happens because the sttw target needs to be 8-byte aligned, but is
not.

  Rainer

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[ro at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |5.0

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #1 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 26 Jan 2015, ro at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798
> 
>             Bug ID: 64798
>            Summary: [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs
>            Product: gcc
>            Version: 5.0
>             Status: UNCONFIRMED
>           Severity: normal
>           Priority: P3
>          Component: libstdc++
>           Assignee: unassigned at gcc dot gnu.org
>           Reporter: ro at gcc dot gnu.org
>                 CC: rguenth at gcc dot gnu.org
>               Host: sparc*-sun-solaris2.*
>             Target: sparc*-sun-solaris2.*
>              Build: sparc*-sun-solaris2.*
> 
> Between 20150116 (r219745) and 20150123 (r220039),
> g++.old-deja/g++.eh/badalloc1.C
> started to FAIL on 32-bit Solaris/SPARC:
> 
> FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++11 execution test
> FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++14 execution test
> FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++98 execution test
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 1 (LWP 1)]
> __cxxabiv1::__cxa_throw (obj=0x2195c <arena+92>, 
>     tinfo=0x73908 <typeinfo for int>, dest=0x0)
>     at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:76
> 76       
> __GXX_INIT_PRIMARY_EXCEPTION_CLASS(header->exc.unwindHeader.exception_class);
> 1: x/i $pc
> => 0xff220314 <__cxxabiv1::__cxa_throw(void*, std::type_info*, void
> (*)(void*))+96>:    sttw  %g2, [ %i0 + -24 ]
> (gdb) where
> #0  __cxxabiv1::__cxa_throw (obj=0x2195c <arena+92>, 
>     tinfo=0x73908 <typeinfo for int>, dest=0x0)
>     at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:76
> #1  0x00011114 in fn_throw ()
>     at
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C:98
> #2  0x000112f8 in main ()
>     at
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C:129
> (gdb) p $i0-24
> $3 = 137540
> 
> The SEGV happens because the sttw target needs to be 8-byte aligned, but is
> not.

Does malloc return 8-byte aligned memory?  Is __alignof__

      struct free_entry {
        std::size_t size;
        free_entry *next;
      };

less than 8?

Thanks,
Richard.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #2 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 26 Jan 2015, Richard Biener wrote:

> On Mon, 26 Jan 2015, ro at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798
> > 
> >             Bug ID: 64798
> >            Summary: [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs
> >            Product: gcc
> >            Version: 5.0
> >             Status: UNCONFIRMED
> >           Severity: normal
> >           Priority: P3
> >          Component: libstdc++
> >           Assignee: unassigned at gcc dot gnu.org
> >           Reporter: ro at gcc dot gnu.org
> >                 CC: rguenth at gcc dot gnu.org
> >               Host: sparc*-sun-solaris2.*
> >             Target: sparc*-sun-solaris2.*
> >              Build: sparc*-sun-solaris2.*
> > 
> > Between 20150116 (r219745) and 20150123 (r220039),
> > g++.old-deja/g++.eh/badalloc1.C
> > started to FAIL on 32-bit Solaris/SPARC:
> > 
> > FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++11 execution test
> > FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++14 execution test
> > FAIL: g++.old-deja/g++.eh/badalloc1.C  -std=c++98 execution test
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 1 (LWP 1)]
> > __cxxabiv1::__cxa_throw (obj=0x2195c <arena+92>, 
> >     tinfo=0x73908 <typeinfo for int>, dest=0x0)
> >     at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:76
> > 76       
> > __GXX_INIT_PRIMARY_EXCEPTION_CLASS(header->exc.unwindHeader.exception_class);
> > 1: x/i $pc
> > => 0xff220314 <__cxxabiv1::__cxa_throw(void*, std::type_info*, void
> > (*)(void*))+96>:    sttw  %g2, [ %i0 + -24 ]
> > (gdb) where
> > #0  __cxxabiv1::__cxa_throw (obj=0x2195c <arena+92>, 
> >     tinfo=0x73908 <typeinfo for int>, dest=0x0)
> >     at /vol/gcc/src/hg/trunk/local/libstdc++-v3/libsupc++/eh_throw.cc:76
> > #1  0x00011114 in fn_throw ()
> >     at
> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C:98
> > #2  0x000112f8 in main ()
> >     at
> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.old-deja/g++.eh/badalloc1.C:129
> > (gdb) p $i0-24
> > $3 = 137540
> > 
> > The SEGV happens because the sttw target needs to be 8-byte aligned, but is
> > not.
> 
> Does malloc return 8-byte aligned memory?  Is __alignof__
> 
>       struct free_entry {
>         std::size_t size;
>         free_entry *next;
>       };
> 
> less than 8?

Ah - the issue might be that g++.old-deja/g++.eh/badalloc1.C does

extern "C" void *malloc (size_t size)
{
  object *p = reinterpret_cast<object *>(&arena[pos]);

  if (fail)
    return 0;

  p->size = size;
  size = (size + __alignof__(object) - 1) & - __alignof__(object);
  pos += size + sizeof(object);

thus it aligns to 'object' but 'object' is

struct object
{
  size_t size __attribute__((aligned));
};

Richard

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-01-26
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine, but can you confirm those findings?

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #4 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #1 from rguenther at suse dot de <rguenther at suse dot de> ---
[...]
> Does malloc return 8-byte aligned memory?  Is __alignof__

It does, according to libc sources exactly for the case at hand:

 * Alignment (ALIGN) changed to 8 for SPARC ldd/std.

>       struct free_entry {
>         std::size_t size;
>         free_entry *next;
>       };
>
> less than 8?

Yup: 4 in the (failing) 32-bit case.

    Rainer

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 34591
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34591&action=edit
patch

Ok, so it was bogus to assume size_t would cover the largest alignment needed.  
Can test check the attached patch please?

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
__BIGGEST_ALIGNMENT__ is unnecessarily large, e.g. on i?86-linux, it is 16,
while you only need to guarantee 8-byte alignment.
Isn't the bug just in the badalloc1.C testcase, if it provides its own malloc,
it should IMHO guarantee the system malloc alignments (generally, something
like alignment of
union U
{
  long long ll;
  double d;
  long double ld;
  void *p;
};
.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[ro at CeBiTec dot Uni-Bielefeld.DE]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #7 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
> Created attachment 34591
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34591&action=edit
> patch
>
> Ok, so it was bogus to assume size_t would cover the largest alignment needed.  
> Can test check the attached patch please?

Initial testing (just rebuilding libstdc++ and re-running the test)
looked good.  I'll fire off a full bootstrap for good measure, but that
will take a bit.

Thanks.
        Rainer

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 27 Jan 2015, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> __BIGGEST_ALIGNMENT__ is unnecessarily large, e.g. on i?86-linux, it is 16,
> while you only need to guarantee 8-byte alignment.
> Isn't the bug just in the badalloc1.C testcase, if it provides its own malloc,
> it should IMHO guarantee the system malloc alignments (generally, something
> like alignment of
> union U
> {
>   long long ll;
>   double d;
>   long double ld;
>   void *p;
> };
> .

no, badalloc1.C provides __BIGGEST_ALIGNMENT__ aligned memory.  The bug
is in the EH allocator which allocates the extra size_t entry aligned
but the EH object itself only size_t aligned.  It's documented
that __attribute__((aligned)) aligns to the biggest type so using
__BIGGEST_ALIGNMENT__ looks correct here - this is also what malloc ()
guarantees for alignment, no?  Do we have any other means of getting
the same alignment as malloc () provides?

Btw, it still wastes a lot less space than the previous allocator.

Bug in the patch anyway and the following hunk is also needed:

@@ -185,7 +186,7 @@ namespace
     {
       __gnu_cxx::__scoped_lock sentry(emergency_mutex);
       allocated_entry *e = reinterpret_cast <allocated_entry *>
-       (reinterpret_cast <char *> (data) - sizeof (std::size_t));
+       (reinterpret_cast <char *> (data) - offsetof (allocated_entry, 
data));
       std::size_t sz = e->size;
       if (!first_free_entry)
        {

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34591|0                           |1
        is obsolete|                            |

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 34592
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34592&action=edit
updated patch

Patch with missing hunk

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
__BIGGEST_ALIGNMENT__ has nothing to do with alignment of malloc returned
memory.
The union I've mentioned is a reasonable guess (something actually mandated by
the standard), glibc right now guarantees 2 * sizeof (void *) (which is wrong
for ppc32).  For malloc alignment, we've added an attribute for that, but glibc
maintainers haven't added it to their headers :( and obviously other libcs
neither.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 27 Jan 2015, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798
> 
> --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> __BIGGEST_ALIGNMENT__ has nothing to do with alignment of malloc returned
> memory.
> The union I've mentioned is a reasonable guess (something actually mandated by
> the standard), glibc right now guarantees 2 * sizeof (void *) (which is wrong
> for ppc32).  For malloc alignment, we've added an attribute for that, but glibc
> maintainers haven't added it to their headers :( and obviously other libcs
> neither.

I can drop __BIGGEST_ALIGNMENT__ and use alignof (allocated_entry::data)
which is aligned via __attribute__((aligned)) by the patch (same trick
that badalloc1.C uses).  That attribute trick is documented as

"Alternatively, you can leave out the alignment factor
and just ask the compiler to align a variable or field to the
default alignment for the target architecture you are compiling for.
The default alignment is sufficient for all scalar types, but may not be
enough for all vector types on a target that supports vector operations.
The default alignment is fixed for a particular target ABI."

but then

"GCC also provides a target specific macro @code{__BIGGEST_ALIGNMENT__},
which is the largest alignment ever used for any data type on the
target machine you are compiling for."

and the following example suggests that the first doesn't necessarily
matches the second.  Though testing on x86_64 reveals that
__attribute__((aligned)) aligns to 16 bytes as well.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 34593
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34593&action=edit
further updated patch

Ceases use of __BIGGEST_ALIGNMENT__.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think aligned attribute is similarly useless for this, I'd really go for
testing alignments you need, rather than expecting some magic value.  I'm not
aware of any target that would have bigger alignment for say
char/short/int/long than long long, ditto with float/double, so I'd just check
maximum of
long long, long double, void * alignment.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #14 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 27 Jan 2015, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798
> 
> --- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> --- I 
> think aligned attribute is similarly useless for this, I'd really go for 
> testing alignments you need, rather than expecting some magic value.  
> I'm not aware of any target that would have bigger alignment for say 
> char/short/int/long than long long, ditto with float/double, so I'd just 
> check maximum of long long, long double, void * alignment.

For the actual bug I can use __alignof__ (__cxa_refcounted_exception),
but that again ends up using __attribute__((__aligned__)) via the
_Unwind_Exception member:

struct _Unwind_Exception
{
  _Unwind_Exception_Class exception_class;
  _Unwind_Exception_Cleanup_Fn exception_cleanup;

#if !defined (__USING_SJLJ_EXCEPTIONS__) && defined (__SEH__)
  _Unwind_Word private_[6];
#else
  _Unwind_Word private_1;
  _Unwind_Word private_2;
#endif

  /* @@@ The IA-64 ABI says that this structure must be double-word 
aligned.
     Taking that literally does not make much sense generically.  Instead 
we
     provide the maximum alignment required by any type for the machine.  
*/
} __attribute__((__aligned__));

so I think that using __attribute__((__aligned__)) is very much the
correct and easiest thing to use.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If _Unwind_Exception already uses it, sure.  Sorry, didn't know that.

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Wed Jan 28 09:53:39 2015
New Revision: 220201

URL: https://gcc.gnu.org/viewcvs?rev=220201&root=gcc&view=rev
Log:
2015-01-28  Richard Biener  <rguenther@suse.de>

    PR libstdc++/64798
    * libsupc++/eh_alloc.cc (struct allocated_entry): Align
    data member.
    (pool::allocate): Adjust allocation size and alignment to
    that change.
    (pool::free): Adjust pointer offsetting.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/libsupc++/eh_alloc.cc

[Bug libstdc++/64798] [5 regression] g++.old-deja/g++.eh/badalloc1.C FAILs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64798

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
Should be fixed now.