Bug 53900 - [regression] Too optimistic on a alignment assert
Summary: [regression] Too optimistic on a alignment assert
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-09 14:57 UTC by Gael Guennebaud
Modified: 2015-02-19 15:26 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
an example triggering the issue. (246 bytes, text/x-c++src)
2012-07-09 14:57 UTC, Gael Guennebaud
Details
Another demonstration of the issue using std::vector (218 bytes, text/x-c++src)
2012-07-10 11:09 UTC, Gael Guennebaud
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gael Guennebaud 2012-07-09 14:57:43 UTC
Created attachment 27766 [details]
an example triggering the issue.

Since gcc 4.7 the assertion of the attached piece of code is resolved at compile-time while there is no guaranty, for instance when using a non-aligned memory allocator, or on some systems where function arguments cannot be aligned.

Here is the output with gcc-4.7:

$ g++-4.7 -m32 alignedassert.cpp && ./a.out
0x8321008
0x832100c

no assertion!

And with gcc 4.6:

$ g++-4.6 -m32 alignedassert.cpp && ./a.out
0x9322008
a.out: alignedassert.cpp:12: Foo::Foo(): Assertion `(std::ptrdiff_t(array) & std::ptrdiff_t(0xf))==0' failed.
Aborted


Or without the -m32 flag:

$ g++-4.7 alignedassert.cpp && ./a.out
0xde3010
0xde3014

$ g++-4.6 alignedassert.cpp && ./a.out
0x1f03010
0x1f03014
a.out: alignedassert.cpp:12: Foo::Foo(): Assertion `(std::ptrdiff_t(array) & std::ptrdiff_t(0xf))==0' failed.
Aborted
Comment 1 Richard Biener 2012-07-09 15:23:36 UTC
You tell the compiler array is aligned, so what do you expect?
Comment 2 Gael Guennebaud 2012-07-09 16:12:07 UTC
The problem is that it is not guaranteed to be effectively aligned, and it is nice to be able to detect when this happens to either abort with a clear message, trigger an exception, or even properly handle this case by disabling at runtime some optimizations.

Anyway, if you disagree, I'm sure I'll find a workaround using, e.g., a non inlined intermediate function masking the origin of the pointer.
Comment 3 Richard Biener 2012-07-09 16:18:58 UTC
Well, I suggest you instead do

struct Foo
{
  float array[4];
  Foo()
  {
    std::cout << array << "\n";
    // check the object is really aligned
    if (std::ptrdiff_t(array) & std::ptrdiff_t(0xf)) != 0)
      assert (false);
    <...code assuming array is aligned...>
  }
};

and the compiler will figure out that array is aligned in code dominated
by the alignment check.
Comment 4 Gael Guennebaud 2012-07-10 11:09:16 UTC
Created attachment 27770 [details]
Another demonstration of the issue using std::vector

Note that when we declare:

__attribute__((aligned(16))) float array[4];

we request GCC to give us an aligned array, and we are not telling gcc that array is aligned that is slightly different. I attached another example demonstrating the problem:

a simple declaration like:

 std::vector<Foo> vec(5);

generates unaligned arrays that are not caught by assertion because it has been removed by gcc 4.7. Outputs are as follow:

$ g++-4.7 -m32 alignedassert.cpp && ./a.out
ctor 0xffdc58f0
copy 0x9f72008
copy 0x9f72018
copy 0x9f72028
copy 0x9f72038
copy 0x9f72048
ctor 0xffdc5900

g++-4.6  -m32 alignedassert.cpp && ./a.out
ctor 0xff8e19e0
copy 0x98e2008
a.out: alignedassert.cpp:18: Foo::Foo(const Foo&): Assertion `(std::ptrdiff_t(array) % std::ptrdiff_t(16))==0' failed.
Aborted
Comment 5 Kacper Kowalik 2012-12-28 17:47:12 UTC
(In reply to comment #1)
> You tell the compiler array is aligned, so what do you expect?

This regression was introduced by:
http://gcc.gnu.org/viewcvs?view=revision&revision=172424

2011-04-14  Richard Guenther  <rguenther@suse.de>

	* tree.h (get_object_alignment_1): Declare.
	* builtins.c (get_object_alignment_1): Split out worker from ...
	(get_object_alignment): ... here.
	* fold-const.c (get_pointer_modulus_and_residue): Use
	get_object_alignment_1.

	* gcc.dg/fold-bitand-4.c: Move ...
	* c-c++-common/fold-bitand-4.c: ... here.  Adjust slightly.

Could you please verify that it's intended behaviour and/or indicate if provided testcase is a valid/invalid code?
Thank you in advance,
Kacper Kowalik
Comment 6 Andrew Pinski 2013-01-07 04:25:38 UTC
>  __attribute__((aligned(16))) float array[4];

Explicitly says the array is aligned to 16 bytes which means it is undefined by the C/C++ standard what happens if you a pointer to the struct which contains this array.  As this pointer will always be aligned by definition of the struct (and any other use of it is undefined), then by definition std::ptrdiff_t(array) will always have the lower 8bits be zero.
Comment 7 Christoph Hertzberg 2013-01-07 10:19:13 UTC
(In reply to comment #6)
> >  __attribute__((aligned(16))) float array[4];
> 
> Explicitly says the array is aligned to 16 bytes which means it is undefined by
> the C/C++ standard what happens if you a pointer to the struct which contains
> this array.  As this pointer will always be aligned by definition of the struct
> (and any other use of it is undefined), then by definition
> std::ptrdiff_t(array) will always have the lower 8bits be zero.

So does that mean that new and std::allocator act undefined for aligned objects?
Is there anything wrong with Gael's second test case?
Comment 8 Andrew Pinski 2013-01-07 10:21:44 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > >  __attribute__((aligned(16))) float array[4];
> > 
> > Explicitly says the array is aligned to 16 bytes which means it is undefined by
> > the C/C++ standard what happens if you a pointer to the struct which contains
> > this array.  As this pointer will always be aligned by definition of the struct
> > (and any other use of it is undefined), then by definition
> > std::ptrdiff_t(array) will always have the lower 8bits be zero.
> 
> So does that mean that new and std::allocator act undefined for aligned
> objects?
> Is there anything wrong with Gael's second test case?

It is rather a bug in libstdc++ which should be reported separately though it needs an ABI change to work correctly.
Comment 9 Benoit Jacob 2015-02-19 15:26:58 UTC
(In reply to Andrew Pinski from comment #8)
> It is rather a bug in libstdc++ which should be reported separately though
> it needs an ABI change to work correctly.

Filed bug 65122 for that.