[Bug c/56434] New: document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[Bug c/56434] New: document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

             Bug #: 56434
           Summary: document that __attribute__((__malloc__)) assumes
                    returned pointer has BIGGEST_ALIGNMENT
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: chip@pobox.com


The docs say that __attribute__((__malloc__)) only has one effect: informing
the compiler that returned pointers do not alias other pointers.  But reading
the compiler output, and then reading gcc source code, proves that it also has
a second effect: informing the compiler that returned pointers are aligned to
BIGGEST_ALIGNMENT.  To quote expand_call:

          /* The return value from a malloc-like function is a pointer.  */
          if (TREE_CODE (rettype) == POINTER_TYPE)
            mark_reg_pointer (temp, BIGGEST_ALIGNMENT);

This should be added to the documentation.

As a side issue, BIGGEST_ALIGNMENT changes on the i386 target depending on
whether -mavx is specified (128 vs. 256).  Is it really a good idea for gcc to
assume different things about the behavior of malloc() depending on -mavx?  It
seems that perhaps an alignment of 128 should always be conferred on malloc on
the i386 platform, regardless of -mavx?

What would the new target macro be?  SMALLEST_BIGGEST_ALIGNMENT?  :)

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-*, i?86-*-*
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2013-02-25
          Component|c                           |rtl-optimization
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> 2013-02-25 15:19:17 UTC ---
We have introduced MALLOC_ABI_ALIGNMENT for this, the calls.c:expand_call use
should be fixed to use that I think.  Not sure what makes use of this reg-note,
but it's a possible wrong-code issue (at least since AVX on x86).

Confirmed.

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #2 from Chip Salzenberg <chip at pobox dot com> 2013-02-25 17:51:36 UTC ---
I detected this by observing inlined strlen() on a malloc'd pointer did not
first do an unaligned prologue.  I expected it to first advance by bytes until
it detected alignment, but it didn't do any of that; it leapt right into the
word-sized optimized loop.

This suggests that the compiler knows than an 8-byte-aligned (say) pointer has
its low seven bits off and will evaporate away any code that depends on them
being nonzero.  Or is the strlen inlining special-cased?

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #3 from Chip Salzenberg <chip at pobox dot com> 2013-02-25 17:54:23 UTC ---
I meant the low three bits off, for a maximum value of seven.  Of course.

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-25 18:36:49 UTC ---
MALLOC_ABI_ALIGNMENT isn't properly set on most targets though.

E.g. with glibc, the alignment of malloced memory is 2 * sizeof (void *) (right
now, might be bigger in the future), while MALLOC_ABI_ALIGNMENT is still 8
bits.

It is just fine to assume malloced memory must be at aligned enough for any of
the standard C types, so if this is say on x86_64 where long double is 16-byte
aligned, then there is really no reason to bother with aligning to 16-byte
boundaries.  If you have malloc implementation that doesn't align at least that
much, just fix it, or don't use malloc attribute for that.

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #5 from Chip Salzenberg <chip at pobox dot com> 2013-02-25 20:02:24 UTC ---
Indeed.  So MALLOC_ABI_ALIGNMENT should perhaps default to the largest
alignment of all the C89 types, with platform overrides as needed?

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> 2013-02-26 10:03:46 UTC ---
(In reply to comment #4)
> MALLOC_ABI_ALIGNMENT isn't properly set on most targets though.
> 
> E.g. with glibc, the alignment of malloced memory is 2 * sizeof (void *) (right
> now, might be bigger in the future), while MALLOC_ABI_ALIGNMENT is still 8
> bits.
> 
> It is just fine to assume malloced memory must be at aligned enough for any of
> the standard C types, so if this is say on x86_64 where long double is 16-byte
> aligned, then there is really no reason to bother with aligning to 16-byte
> boundaries.  If you have malloc implementation that doesn't align at least that
> much, just fix it, or don't use malloc attribute for that.

True, but:

config/i386/i386.h:#define BIGGEST_ALIGNMENT (TARGET_AVX ? 256 : 128)

I don't think glibc aligns to 32 bytes currently, so the mark_reg_pointer
is definitely wrong (with -mavx).  Or BIGGEST_ALIGNMENT is wrong ;)

MALLOC_ABI_ALIGNMENT default was set very conservatively (because we use it
to derive information about pointer values).  I didn't realize we already
do that via the calls.c code (and even wrong ...).

Feel free to implement a better default setting for MALLOC_ABI_ALIGNMENT.

> Indeed.  So MALLOC_ABI_ALIGNMENT should perhaps default to the largest
> alignment of all the C89 types, with platform overrides as needed?

But this information is not readily accessible (it was supposed to be
BIGGEST_ALIGNMENT, but -mavx changed this - not sure what the psABI
says and whether this makes current glibc non-conforming to the psABI).

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #7 from Chip Salzenberg <chip at pobox dot com> 2013-03-21 00:31:40 UTC ---
So ... is there still a question of the Right Thing here?  It seems that fixing
MALLOC_ABI_ALIGNMENT for the world, and ensuring that BIGGEST_ALIGNMENT never
affects the ABI, are the actions to take.  If this were done soon we could even
see it fixed for 4.8.0.  Help?

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> 2013-03-22 09:56:37 UTC ---
On Thu, 21 Mar 2013, chip at pobox dot com wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434
> 
> --- Comment #7 from Chip Salzenberg <chip at pobox dot com> 2013-03-21 00:31:40 UTC ---
> So ... is there still a question of the Right Thing here?  It seems that fixing
> MALLOC_ABI_ALIGNMENT for the world, and ensuring that BIGGEST_ALIGNMENT never
> affects the ABI, are the actions to take.  If this were done soon we could even
> see it fixed for 4.8.0.  Help?

The RTL code using BIGGEST_ALIGNMENT for the malloc result is simply
wrong and was wrong since forever.  What the replacement should be
is the question here, but as we use MALLOC_ABI_ALIGNMENT on the
tree level for _exactly the same purpose_ I think the answer is
obvious.

I will propose a patch.

Richard.

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-22 10:08:54 UTC ---
http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00864.html

waiting for comments.

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #10 from Chip Salzenberg <chip at pobox dot com> 2013-03-22 20:20:11 UTC ---
Thanks muchly.  Then MALLOC_ABI_ALIGNMENT will need fixing, as Jakub observes,
but that needed to happen anyway.

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |4.8.0
         Resolution|                            |FIXED

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-25 09:41:29 UTC ---
Author: rguenth
Date: Mon Mar 25 09:39:52 2013
New Revision: 197030

URL: http://gcc.gnu.org/viewcvs?rev=197030&root=gcc&view=rev
Log:
2013-03-25  Richard Biener  <rguenther@suse.de>

    PR middle-end/56434
    * calls.c (expand_call): Use MALLOC_ABI_ALIGNMENT to annotate
    the pointer returned by calls with ECF_MALLOC set.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/calls.c

[Bug rtl-optimization/56434] document that __attribute__((__malloc__)) assumes returned pointer has BIGGEST_ALIGNMENT

[chip at pobox dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56434

--- Comment #12 from Chip Salzenberg <chip at pobox dot com> 2013-03-25 19:15:10 UTC ---
Thank you.  I've filed #56726 with a patch to update MALLOC_ABI_ALIGNMENT on
i386.