Created attachment 26147 [details] packed.c test case with output in a comment I've seen this problem with gcc 4.5.1 on SPARC Solaris 9. I presume it will affect other versions of gcc on other platforms with strict alignment requirements (unlike x86, which supports misaligned accesses in hardware as I understand it). I think it also applies to "#pragma pack". The gcc extension __attribute__(packed), which applied to a struct, has the following semantics (quoting the gcc documentation for 4.5.2): The `packed' attribute specifies that a variable or structure field should have the smallest possible alignment--one byte for a variable, and one bit for a field, unless you specify a larger value with the `aligned' attribute. When a program accesses a misaligned member of a packed struct, the compiler generates whatever code is necessary to read or write the correct value. If the address of a misaligned member is stored in a pointer object, dereferencing that pointer doesn't give the compiler an opportunity to generate that extra code. The attached program demonstrates the problem, and includes (as a comment) the output I get on Ubuntu x86 (ok) and Solaris 9 SPARC (bus error). See also http://stackoverflow.com/questions/8568432/is-gccs-attribute-packed-pragma-pack-unsafe/ I don't believe it would be practical to fix this (though there might be some clever solution I haven't thought of). But at least I suggest mentioning this issue in the documentation.
A commenter on stackoverflow points out that a possible fix would be to permit the address of a member of a packed structure to be assigned only to a pointer object that is itself declared "packed".
I think this is just undefined at runtime which means we can only warn about it. Which we do with -Walign
I see no "-Walign" option, either in the versions of gcc I'm using or in the online documentation. Were you thinking of a different option? What I'm suggesting, primarily, is that the issue should be mentioned in the documentation of the "packed" attribute (and probably of "pragma #pack" as well).
The point is that even if you use sth like typedef int myint __attribute__((aligned(1))); to capture the misaligned pointer to the packed structure element: myint *p = &s->i; then accesses like '*p' will still assume an _aligned_ int at 'p' for STRICT_ALIGNMENT targets. That's a long-long-long-standing bug and a cause of major headache for more modern GCCs even ... The testcase with using a 'int *' pointer is indeed invalid though.
> The point is that even if you use sth like > > typedef int myint __attribute__((aligned(1))); > > to capture the misaligned pointer to the packed structure element: > > myint *p = &s->i; > > then accesses like '*p' will still assume an _aligned_ int at 'p' for > STRICT_ALIGNMENT targets. > > That's a long-long-long-standing bug and a cause of major headache for > more modern GCCs even ... That's a limitation rather than a bug. Clearly, on strict-alignment targets, you must know what you're doing when you start to misalign things. As for typedef int myint __attribute__((aligned(1))); that's an abomination I don't even want to know of ;-)
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > --- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 10:52:19 UTC --- > > The point is that even if you use sth like > > > > typedef int myint __attribute__((aligned(1))); > > > > to capture the misaligned pointer to the packed structure element: > > > > myint *p = &s->i; > > > > then accesses like '*p' will still assume an _aligned_ int at 'p' for > > STRICT_ALIGNMENT targets. > > > > That's a long-long-long-standing bug and a cause of major headache for > > more modern GCCs even ... > > That's a limitation rather than a bug. Clearly, on strict-alignment targets, > you must know what you're doing when you start to misalign things. As for > > typedef int myint __attribute__((aligned(1))); > > that's an abomination I don't even want to know of ;-) Huh, it's not. It's the same as a packed struct or enum type. Why can't you strict-align people simply fix this case? Richard.
> Huh, it's not. It's the same as a packed struct or enum type. No, it isn't, the mode is integral instead of BLKmode. In Ada we do support misaligned integers, but we simply wrap them in a BLKmode record. > Why can't you strict-align people simply fix this case? Because integral modes are naturally aligned. The only reasonable way to support the aforementioned abomination is to use the Ada approach.
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > --- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 11:18:24 UTC --- > > Huh, it's not. It's the same as a packed struct or enum type. > > No, it isn't, the mode is integral instead of BLKmode. In Ada we do support > misaligned integers, but we simply wrap them in a BLKmode record. > > > Why can't you strict-align people simply fix this case? > > Because integral modes are naturally aligned. The only reasonable way to > support the aforementioned abomination is to use the Ada approach. You mean that handling the TYPE_ALIGN != MODE_ALIGN case when expanding a MEM_REF (thus, INDIRECT_REF on old branches) won't work? Why not? You'd simply have to emit the same RTL as when expanding that wrapped struct case. Richard.
> You mean that handling the TYPE_ALIGN != MODE_ALIGN case when > expanding a MEM_REF (thus, INDIRECT_REF on old branches) won't work? But you cannot have TYPE_ALIGN < MODE_ALIGN (TYPE_MODE).
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > --- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 11:34:00 UTC --- > > You mean that handling the TYPE_ALIGN != MODE_ALIGN case when > > expanding a MEM_REF (thus, INDIRECT_REF on old branches) won't work? > > But you cannot have TYPE_ALIGN < MODE_ALIGN (TYPE_MODE). You can. Just check what you get with that aligned(1) int typedef. For vector types you can anyways (by design) to allow unaligned vector moves. Is it documented anywhere that you can't take the address of an unaligned structure member (given the struct is packed) on STRICT_ALIGNMENT targets (or, when it's a vector component even on non-STRICT_ALIGNMENT targets)? Why does the C frontend not warn for these cases (unconditionally?)? I don't see at all that we can't support expanding such moves properly. After all we can extract an INT mode value from unaligned memory (we are never generating a mem:BLK with INT mode size in that case anyway, or a MEM:SI with MEM_ALIGN less than SImode align anyway)
> You can. Just check what you get with that aligned(1) int typedef. Well, we're going in circles as this example precisely doesn't work. > Is it documented anywhere that you can't take the address of > an unaligned structure member (given the struct is packed) on > STRICT_ALIGNMENT targets (or, when it's a vector component even > on non-STRICT_ALIGNMENT targets)? Why does the C frontend not > warn for these cases (unconditionally?)? Good question, but for a C maintainer. The C front-end would have implemented something for a long time if it had cared about the issue, but apparently not. In Ada we do care since Ada 2005, so we have implemented the necessary support.
What Ada does looks just like a workaround for what should be done properly in the expander. So no, IMHO we shouldn't be changing all other FEs and the middle-end (when it wants to generate them e.g. for memcpy) to do what Ada does, but we should change the expander. It has all the information to perform the unaligned reads/writes, it just doesn't use them.
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > --- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 12:25:13 UTC --- > > You can. Just check what you get with that aligned(1) int typedef. > > Well, we're going in circles as this example precisely doesn't work. > > > Is it documented anywhere that you can't take the address of > > an unaligned structure member (given the struct is packed) on > > STRICT_ALIGNMENT targets (or, when it's a vector component even > > on non-STRICT_ALIGNMENT targets)? Why does the C frontend not > > warn for these cases (unconditionally?)? > > Good question, but for a C maintainer. The C front-end would have implemented > something for a long time if it had cared about the issue, but apparently not. > In Ada we do care since Ada 2005, so we have implemented the necessary support. Fact is, the middle-end needs a way to support this (well, or "wants"). Otherwise stripping off component-refs does not work even for the "long time working cases". "Fixes" to avoid stripping them away are not really fixes but workarounds around fixing this long-time issue. But I see you are not going to work on fixing the expansion side (which I hoped, since you have plenty of experience in this area and strict-alignment targets)
> What Ada does looks just like a workaround for what should be done properly in > the expander. So no, IMHO we shouldn't be changing all other FEs and the > middle-end (when it wants to generate them e.g. for memcpy) to do what Ada > does, but we should change the expander. Of course I have the exactly opposite viewpoint, since I think that we should keep the invariants we have: integral modes are naturally aligned and TYPE_ALIGN >= MODE_ALIGN (TYPE_MODE). Breaking them for a few pathological cases that the C compiler has shun for years doesn't seem the best course of action.
I believe the discussion here is missing the point. Currently (at least with version 4.5 and ARM, which I am currently using) the situation is that the compiler generates broken code WITHOUT COMMENT when the address of something like an integer in a packed structure is assigned to a int* . Since the pointer type does not provide any information about the actual alignment of the integer, it is obviously impractical to fix when accessing the integer and we certainly do not want to have extra code inserted for handling the possibility that the int may not be aligned correctly. Therefore the compiler MUST see this as a type conflict and at least warn that the address of the member in a packed struct is incompatible with the pointer type. I have several projects which require the use of packed structures and I simply cannot rule out that that I have not at some point accidentally assigned the address of a member variable to a pointer. It would therefore be very handy if the compiler could point this out. -Wcast-align certainly doesn't do this.
(In reply to comment #14) > > What Ada does looks just like a workaround for what should be done properly in > > the expander. So no, IMHO we shouldn't be changing all other FEs and the > > middle-end (when it wants to generate them e.g. for memcpy) to do what Ada > > does, but we should change the expander. > > Of course I have the exactly opposite viewpoint, since I think that we should > keep the invariants we have: integral modes are naturally aligned and > TYPE_ALIGN >= MODE_ALIGN (TYPE_MODE). Breaking them for a few pathological > cases that the C compiler has shun for years doesn't seem the best course of > action. Well. I'm fine with forcing BLKmode for unaligned accesses, but not sure if that is less invasive. You'd basically have to change modes whenever more precise knowledge about alignment appears ...
(In reply to comment #15) > I believe the discussion here is missing the point. Currently (at least with > version 4.5 and ARM, which I am currently using) the situation is that the > compiler generates broken code WITHOUT COMMENT when the address of something > like an integer in a packed structure is assigned to a int* . Since the pointer > type does not provide any information about the actual alignment of the > integer, it is obviously impractical to fix when accessing the integer and we > certainly do not want to have extra code inserted for handling the possibility > that the int may not be aligned correctly. Therefore the compiler MUST see this > as a type conflict and at least warn that the address of the member in a packed > struct is incompatible with the pointer type. > > I have several projects which require the use of packed structures and I simply > cannot rule out that that I have not at some point accidentally assigned the > address of a member variable to a pointer. It would therefore be very handy if > the compiler could point this out. -Wcast-align certainly doesn't do this. Because there is no cast involved. And that's because the types of the fields are "wrong" as well.
Not sure if I can agree with (or understand) this comment. If we use my example of the address of an int in a packed structure being assigned to an int* then one could argue that there is a kind of implicit cast happening. The pointer is to an integer WITH the default alignment which would be 4 on the ARM system I work with. The int in the struct has an alignment of 1 and so the two types need to be regarded as being incompatible and there should be an unconditional warning or at least -Wcast-align should enable such a warning. I don't know what BLKmode is but if you're speaking of making an int* type generally able to access the int regardless of its alignment, I think this is the wrong way to go because code size will explode and efficiency will go out the window. However if the int* would be declared with some special attribute then, sure, generate whatever code that is necessary to access the int and the above mentioned warning can be suppressed in this case (wouldn't the code generated be much the same code that is used to directly access the int in the packed struct anyway?) However this would be the second (optional) step, the first thing is that the compiler needs to emit a warning explaining that the code that it is generating is broken.
> Not sure if I can agree with (or understand) this comment. If we use my example > of the address of an int in a packed structure being assigned to an int* then > one could argue that there is a kind of implicit cast happening. The pointer is > to an integer WITH the default alignment which would be 4 on the ARM system I > work with. The int in the struct has an alignment of 1 and so the two types > need to be regarded as being incompatible and there should be an unconditional > warning or at least -Wcast-align should enable such a warning. Yes, a warning should probably be implemented in the C family of compilers, but this is a very old issue and nobody is really interested. Note that taking the address of the field is OK, the problem is dereferencing it.
On Wed, 3 Apr 2013, ebotcazou at gcc dot gnu.org wrote: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > --- Comment #19 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-04-03 07:29:51 UTC --- > > Not sure if I can agree with (or understand) this comment. If we use my example > > of the address of an int in a packed structure being assigned to an int* then > > one could argue that there is a kind of implicit cast happening. The pointer is > > to an integer WITH the default alignment which would be 4 on the ARM system I > > work with. The int in the struct has an alignment of 1 and so the two types > > need to be regarded as being incompatible and there should be an unconditional > > warning or at least -Wcast-align should enable such a warning. > > Yes, a warning should probably be implemented in the C family of compilers, but > this is a very old issue and nobody is really interested. Note that taking the > address of the field is OK, the problem is dereferencing it. One of the C frontend issues is that the type of the address of the field of the packed struct is int *, not int attribute((aligned(1))) *. And this is so because nothing adjusts the type of the FIELD_DECL to be a less aligned type. That is, we have <field_decl 0x7ffff6d245f0 i type <integer_type 0x7ffff6d175e8 int public SI size <integer_cst 0x7ffff6d1a0c0 constant 32> unit size <integer_cst 0x7ffff6d1a0e0 constant 4> align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8 precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max <integer_cst 0x7ffff6d1a080 2147483647> pointer_to_this <pointer_type 0x7ffff6d1f2a0>> packed SI file t.c line 2 col 9 size <integer_cst 0x7ffff6d1a0c0 32> unit size <integer_cst 0x7ffff6d1a0e0 4> align 8 offset_align 128 offset <integer_cst 0x7ffff6d02d80 type <integer_type 0x7ffff6d17000 sizetype> constant 0> bit offset <integer_cst 0x7ffff6d02e00 type <integer_type 0x7ffff6d170a8 bitsizetype> constant 0> context <record_type 0x7ffff6e1c3f0 Foo>> and alignof (x.i) and alignof (*&x.i) would not agree (if the latter were not folded to x.i) for struct Foo { int i; } __attribute__((packed)) ; struct Foo x; for example (surprisingly) __alignof__ (*(char *)&x.i) is 4 while __alignof__ (*&x.i) is 1 (due to folding) and __alignof__ (x.i) is 1 as well. I'm quite sure that "fixing" this will have loads of fallout throughout the frontend(s if 'fixed' in stor-layout.c). Which is why this issue is so old and unfixed.
> One of the C frontend issues is that the type of the address of > the field of the packed struct is int *, not int attribute((aligned(1))) > *. And this is so because nothing adjusts the type of the FIELD_DECL > to be a less aligned type. That is, we have > > <field_decl 0x7ffff6d245f0 i > type <integer_type 0x7ffff6d175e8 int public SI > size <integer_cst 0x7ffff6d1a0c0 constant 32> > unit size <integer_cst 0x7ffff6d1a0e0 constant 4> > align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8 > precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max <integer_cst > 0x7ffff6d1a080 2147483647> > pointer_to_this <pointer_type 0x7ffff6d1f2a0>> > packed SI file t.c line 2 col 9 size <integer_cst 0x7ffff6d1a0c0 32> > unit size <integer_cst 0x7ffff6d1a0e0 4> > align 8 offset_align 128 > offset <integer_cst 0x7ffff6d02d80 type <integer_type 0x7ffff6d17000 > sizetype> constant 0> > bit offset <integer_cst 0x7ffff6d02e00 type <integer_type > 0x7ffff6d170a8 bitsizetype> constant 0> context <record_type > 0x7ffff6e1c3f0 Foo>> This is on x86, right? If the alignment of the field cannot be guaranteed to be that of its type, then it should be made a bit-field. Maybe it's already made a bit-field on strict-alignment targets.
On Wed, 3 Apr 2013, ebotcazou at gcc dot gnu.org wrote: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > --- Comment #21 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-04-03 08:51:44 UTC --- > > One of the C frontend issues is that the type of the address of > > the field of the packed struct is int *, not int attribute((aligned(1))) > > *. And this is so because nothing adjusts the type of the FIELD_DECL > > to be a less aligned type. That is, we have > > > > <field_decl 0x7ffff6d245f0 i > > type <integer_type 0x7ffff6d175e8 int public SI > > size <integer_cst 0x7ffff6d1a0c0 constant 32> > > unit size <integer_cst 0x7ffff6d1a0e0 constant 4> > > align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8 > > precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max <integer_cst > > 0x7ffff6d1a080 2147483647> > > pointer_to_this <pointer_type 0x7ffff6d1f2a0>> > > packed SI file t.c line 2 col 9 size <integer_cst 0x7ffff6d1a0c0 32> > > unit size <integer_cst 0x7ffff6d1a0e0 4> > > align 8 offset_align 128 > > offset <integer_cst 0x7ffff6d02d80 type <integer_type 0x7ffff6d17000 > > sizetype> constant 0> > > bit offset <integer_cst 0x7ffff6d02e00 type <integer_type > > 0x7ffff6d170a8 bitsizetype> constant 0> context <record_type > > 0x7ffff6e1c3f0 Foo>> > > This is on x86, right? If the alignment of the field cannot be guaranteed to > be that of its type, then it should be made a bit-field. Maybe it's already > made a bit-field on strict-alignment targets. Note the FIELD_DECL is perfectly ok (align 8), it is its TREE_TYPE that is "bogus", and this type is used when building the pointer type used for taking the address of it (so you could argue _that_ is the bug - it shouldn't literally take TREE_TYPE of a FIELD_DECL when building the address of a COMPONENT_REF - the COMPONENT_REF surely only caring about the type of the value not the storage). Richard.
FYI: I have asked the llvm folks to add a warning to their compiler for the when a pointer to a member of a packed struct is assigned to an "ordinary" pointer with higher alignment guarantees. Clearly, I agree with comment #18 that compilers should warn about this. See https://llvm.org/bugs/show_bug.cgi?id=22821
Comment #4 mentions typedef int myint __attribute__((aligned(1))); That shouldn't even work. The GCC documentation on Type Attributes mentions that "The aligned attribute can only increase the alignment". It goes on to mention the packed attribute, which can be used to decrease alignment. But as far as I know, that attributes was designed for structs. Anyhow, it seems that the aligned attribute is intended for increasing the alignment only - not for decreasing. Yet, when I checked __alignof(myint) with both gcc and clang, it was in fact decreased from 4 to 1. Not sure why. That seems to contradict the documentation.
(In reply to Sven from comment #24) Agreed. This is the subject of bug 69502.
(In reply to Keith Thompson from comment #3) > I see no "-Walign" option, either in the versions of gcc I'm using or in the > online documentation. Were you thinking of a different option? > Maybe -Wpacked?
(In reply to Eric Gallager from comment #26) > (In reply to Keith Thompson from comment #3) > > I see no "-Walign" option, either in the versions of gcc I'm using or in the > > online documentation. Were you thinking of a different option? > > > > Maybe -Wpacked? gcc 8 adds -Wpacked-not-aligned; does that fix this bug?
(In reply to Eric Gallager from comment #27) > gcc 8 adds -Wpacked-not-aligned; does that fix this bug? I couldn't find documentation on what this switch is supposed to do. Can you point me in the right direction? Is there some commit explaining -Wpacked-not-aligned? FYI: the LLVM developers have added -Waddress-of-packed-member which addresses the issue discussed here.
(In reply to Sven from comment #28) > (In reply to Eric Gallager from comment #27) > > gcc 8 adds -Wpacked-not-aligned; does that fix this bug? > > I couldn't find documentation on what this switch is supposed to do. Can you > point me in the right direction? Is there some commit explaining > -Wpacked-not-aligned? > I don't remember the exact commit number but HJ Lu added it; I added him on cc with my previous comment, so maybe he can explain.
(In reply to Eric Gallager from comment #29) > > I don't remember the exact commit number but HJ Lu added it; I added him on > cc with my previous comment, so maybe he can explain. It is r251180.
https://gcc.gnu.org/viewcvs/gcc/trunk/?view=log&pathrev=251180 I am reading the commit message, and the example doesn't make any sense. The aligned attribute is for providing additional alignment guarantees in addition to the types default alignment. In particular, the documentation of the aligned type attribute specifically states, that this attribute cannot decrease alignment. It can only _increase_ alignment. In particular, aligned(4) in combination with a 64bit type makes little sense.
(In reply to Sven from comment #31) > https://gcc.gnu.org/viewcvs/gcc/trunk/?view=log&pathrev=251180 > > I am reading the commit message, and the example doesn't make any sense. > The aligned attribute is for providing additional alignment guarantees in > addition to the types default alignment. In particular, the documentation of > the aligned type attribute specifically states, that this attribute cannot > decrease alignment. It can only _increase_ alignment. > > In particular, aligned(4) in combination with a 64bit type makes little > sense. long long is aligned to 4 bytes in struct for i386.
[hjl@gnu-skl-1 gcc]$ cat x.c #include <stdio.h> #include <stddef.h> typedef int aligned_int __attribute__((warn_if_not_aligned(4))); int main(void) { struct foo { char c; aligned_int x; } __attribute__((packed)); struct foo arr[2] = { { 'a', 10 }, {'b', 20 } }; int *p0 = &arr[0].x; int *p1 = &arr[1].x; printf("sizeof(struct foo) = %d\n", (int)sizeof(struct foo)); printf("offsetof(struct foo, c) = %d\n", (int)offsetof(struct foo, c)); printf("offsetof(struct foo, x) = %d\n", (int)offsetof(struct foo, x)); printf("arr[0].x = %d\n", arr[0].x); printf("arr[1].x = %d\n", arr[1].x); printf("p0 = %p\n", (void*)p0); printf("p1 = %p\n", (void*)p1); printf("*p0 = %d\n", *p0); printf("*p1 = %d\n", *p1); return 0; } [hjl@gnu-skl-1 gcc]$ ./xgcc -B./ -S x.c -Wall x.c: In function \u2018main\u2019: x.c:9:5: warning: alignment 1 of \u2018struct foo\u2019 is less than 4 [-Wif-not-aligned] } __attribute__((packed)); ^ x.c:8:14: warning: \u2018x\u2019 offset 1 in \u2018struct foo\u2019 isn't aligned to 4 [-Wif-not-aligned] aligned_int x; ^ [hjl@gnu-skl-1 gcc]$
(In reply to H.J. Lu from comment #32) > long long is aligned to 4 bytes in struct for i386. Understood. So the aligned(4) was just added to explicitly restating the alignment? Anyhow, the two warnings added by that commit seem unrelated to this issue. The warnings added check the alignment of variables and members in memory. When they fall below the given threshold, a warning is issued. This bug report however is about a different issue. Every member of a packed struct has an alignment guarantee of 1 - no more than that. However, a regular int* variable guarantees, when dereferenced, guarantees an alignment of 4 (on i386 and arm, for example). So the following code should produce a warning: struct foo { int i; } __attribute__((packed)); struct foo *x; int *y = &(x->i); The issue becomes more obvious with this struct: struct foo2 { char c; int i; } __attribute__((packed)); However, both foo and foo2 only have an alignment guarantee of at most 1, so also the int member inside both structs has an alignment guarantee of at most 1. As mentioned in the initial post, gcc will generate the proper machine code to read an unaligned int (on arm for example) when using x->i directly. However, when using *y, gcc will assume 4 byte alignment. That is to be expected, hence gcc should warn about the fact, and the address of a (potentially) unaligned int is assigned to a regular int* pointer.
(In reply to Sven from comment #34) > That is to be > expected, hence gcc should warn about the fact, and the address of a > (potentially) unaligned int is assigned to a regular int* pointer. Sorry, typo: That is to be expected, hence gcc should warn about the fact that the address of a (potentially) unaligned int is assigned to a regular int* pointer.
FYI a test case that actually triggers SIGSEGV on x86 with gcc 7.2: https://godbolt.org/g/kPw6NJ Compiler emits a movaps instruction which is alignment-sensitive. A warning would be helpful. struct pair_t { char c; __int128_t i; } __attribute__((packed)); struct pair_t p = {0, 1}; __int128_t *addr = &p.i; int main() { *addr = ~(__int128_t)0; return (p.i != 1) ? 0 : 1; }
Created attachment 43120 [details] Add -Waddress-of-packed-member to C/C++ FE Please try this.
A patch is posted at https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01237.html
Thank you, this patch works for me. Gives a warning in the attached test case, but still allows to take an address of a packed struct members when the packed attribute is preserved in the pointer.
(In reply to Alexey Salmin from comment #39) > Thank you, this patch works for me. > > Gives a warning in the attached test case, but still allows to take an > addre of a packed struct members when the packed attribute is preserved in > the pointer. Do you have such a testcase with the packed attribute preserved in the pointer?
(In reply to Alexey Salmin from comment #39) > .. when the packed attribute is preserved in the pointer. What do you mean by that? GCC documentation explicitly forbids to use the packed attribute for anything but structs, unions, and enums.
Sorry for being unclear. When I need a pointer to an unaligned type I wrap it in a struct. E.g. a fix for SIGSEGV from comment#36 would look like this: struct pair_t { char c; __int128_t i; } __attribute__((packed)); typedef struct unaligned_int128_t_ { __int128_t value; } __attribute__((packed)) unaligned_int128_t; struct pair_t p = {0, 1}; unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i); int main() { addr->value = ~(__int128_t)0; return (p.i != 1) ? 0 : 1; } I was trying to say that I'm glad that the "address-of-packed-member" warning isn't triggered by this code. It still relies on the "address of packed member" but in a safe way.
(In reply to Alexey Salmin from comment #42) > > I was trying to say that I'm glad that the "address-of-packed-member" > warning isn't triggered by this code. It still relies on the "address of > packed member" but in a safe way. Please try the current patch: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01535.html
(In reply to Alexey Salmin from comment #42) > typedef struct unaligned_int128_t_ { > __int128_t value; > } __attribute__((packed)) unaligned_int128_t; You can combine the packed attribute with the aligned attribute. Then you can define one struct with aligned(4) and one with aligned(8). Does the warning trigger if you cast between those types? Or does the cast simply override the warning because it's now the programmers responsibility to make sure that the alignment is correct?
(In reply to Sven from comment #44) > > You can combine the packed attribute with the aligned attribute. Then you > can define one struct with aligned(4) and one with aligned(8). Does the > warning trigger if you cast between those types? Or does the cast simply > override the warning because it's now the programmers responsibility to make > sure that the alignment is correct? Yes, my patch handles it correctly: [hjl@gnu-tools-1 pr51628]$ cat g3.i struct pair_t { __int128_t x; __int128_t i; } __attribute__((packed, aligned (4))); typedef struct unaligned_int128_t_ { __int128_t value; } __attribute__((packed, aligned (8))) unaligned_int128_t; struct pair_t p = {0, 1}; unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i); int main() { addr->value = ~(__int128_t)0; return (p.i != 1) ? 0 : 1; } [hjl@gnu-tools-1 pr51628]$ /export/build/gnu/gcc-test/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-test/build-x86_64-linux/gcc/ -O2 -S g3.i g3.i:11:28: warning: initialization of ‘unaligned_int128_t *’ {aka ‘struct unaligned_int128_t_ *’} from address of packed member of ‘struct pair_t’ may result in an unaligned pointer value [-Waddress-of-packed-member] unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i); ^ [hjl@gnu-tools-1 pr51628]$ cat g4.i struct pair_t { __int128_t x; __int128_t i; } __attribute__((packed, aligned (8))); typedef struct unaligned_int128_t_ { __int128_t value; } __attribute__((packed, aligned (4))) unaligned_int128_t; struct pair_t p = {0, 1}; unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i); int main() { addr->value = ~(__int128_t)0; return (p.i != 1) ? 0 : 1; } [hjl@gnu-tools-1 pr51628]$ /export/build/gnu/gcc-test/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-test/build-x86_64-linux/gcc/ -O2 -S g4.i [hjl@gnu-tools-1 pr51628]$
Tested the latest patch, behavior looks very reasonable even in tricky cases. 1) No warning, gcd(12, 8) == 4 struct tuple_t { char c[12]; __int128_t i; } __attribute__((packed, aligned(8))); typedef struct unaligned_int128_t_ { __int128_t value; } __attribute__((packed, aligned(4))) unaligned_int128_t; struct tuple_t p = {0, 1}; unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i); 2) Gives a warning, gcd(10, 8) < 4 struct tuple_t { char c[10]; __int128_t i; } __attribute__((packed, aligned(8))); typedef struct unaligned_int128_t_ { __int128_t value; } __attribute__((packed, aligned(4))) unaligned_int128_t; struct tuple_t p = {0, 1}; unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);
Hi, everyone I wonder if this issue has to do with the bug-like problem I encountered when accessing an unaligned stand-alone global variable (rather than a member of a packed struct). A test case is as follows: char g_c = 'x'; int g_d __attribute__((aligned(1))) = 13; int main(void) { g_c = 'z'; //================ g_d = 33; // Crash on, in my case, ARM Cortex-M0 //================ return 0; } Due to the presence of g_c, g_d is at an odd address, as the map file indicates: .data.g_c 0x20000020 0x1 ./src/test.o 0x20000020 g_c .data.g_d 0x20000021 0x4 ./src/test.o 0x20000021 g_d The generated assembly, however, accesses g_d as if it were properly aligned: 8:../src/test.c **** //================ 9:../src/test.c **** g_d = 33; 52 .loc 1 9 0 53 000a 044B ldr r3, .L2+4 54 000c 2122 movs r2, #33 55 000e 1A60 str r2, [r3] 10:../src/test.c **** //================ BTW: I'm using GNU ARM Embedded Toolchain 7-2017-q4-major and on Windows 7. Any ideas?
On Thu, 12 Apr 2018, dingcurie at icloud dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628 > > W.H. Ding <dingcurie at icloud dot com> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |dingcurie at icloud dot com > > --- Comment #47 from W.H. Ding <dingcurie at icloud dot com> --- > Hi, everyone > > I wonder if this issue has to do with the bug-like problem I encountered when > accessing an unaligned stand-alone global variable (rather than a member of a > packed struct). A test case is as follows: > > char g_c = 'x'; > int g_d __attribute__((aligned(1))) = 13; > > int main(void) > { > g_c = 'z'; > //================ > g_d = 33; // Crash on, in my case, ARM Cortex-M0 > //================ > return 0; > } > > Due to the presence of g_c, g_d is at an odd address, as the map file > indicates: > > .data.g_c 0x20000020 0x1 ./src/test.o > 0x20000020 g_c > .data.g_d 0x20000021 0x4 ./src/test.o > 0x20000021 g_d > > The generated assembly, however, accesses g_d as if it were properly aligned: > > 8:../src/test.c **** //================ > 9:../src/test.c **** g_d = 33; > 52 .loc 1 9 0 > 53 000a 044B ldr r3, .L2+4 > 54 000c 2122 movs r2, #33 > 55 000e 1A60 str r2, [r3] > 10:../src/test.c **** //================ > > BTW: I'm using GNU ARM Embedded Toolchain 7-2017-q4-major and on Windows 7. > > Any ideas? It looks like RTL expansion is broken here (also on x86_64): ;; g_d = 33; (insn 6 5 0 (set (mem/c:SI (symbol_ref:DI ("g_d") [flags 0x2] <var_decl 0x7fea38d38ea0 g_d>) [1 g_d+0 S4 A32]) (const_int 33 [0x21])) "t.c":7 -1 (nil)) notice how the MEM has 32bit alignment. This is probably because while g_d has a DECL_ALIGN of 8 it has a type with TYPE_ALIGN of 32. IIRC there's related bugs for the FEs that taking the address of g_d results in a 32bit aligned int * pointer and thus *&g_d has different alignment than g_d. A workaround is to place such variables in a struct I guess. The "bug" is in set_mem_attributes_minus_bitpos (when you ignore the FE bug of mismatch of decl and type alignment): /* We can set the alignment from the type if we are making an object or if this is an INDIRECT_REF. */ if (objectp || TREE_CODE (t) == INDIRECT_REF) attrs.align = MAX (attrs.align, TYPE_ALIGN (type)); a workaround would be to do the following as in the !TYPE_P path we already use the type to derive alignment when allowed. Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c (revision 259337) +++ gcc/emit-rtl.c (working copy) @@ -1985,9 +1985,8 @@ set_mem_attributes_minus_bitpos (rtx ref able to simply always use TYPE_ALIGN? */ } - /* We can set the alignment from the type if we are making an object or if - this is an INDIRECT_REF. */ - if (objectp || TREE_CODE (t) == INDIRECT_REF) + /* We can set the alignment from the type if we are making an object. */ + if (objectp && TYPE_P (t)) attrs.align = MAX (attrs.align, TYPE_ALIGN (type)); /* If the size is known, we can set that. */
(In reply to W.H. Ding from comment #47) > Hi, everyone > > I wonder if this issue has to do with the bug-like problem I encountered > when accessing an unaligned stand-alone global variable (rather than a > member of a packed struct). A test case is as follows: > > char g_c = 'x'; > int g_d __attribute__((aligned(1))) = 13; > > int main(void) > { > g_c = 'z'; > //================ > g_d = 33; // Crash on, in my case, ARM Cortex-M0 > //================ > return 0; > } This doesn't work. The aligned attribute is for providing additional alignment hints. The GCC documentation clearly states, that aligned can increase the alignment. So g_d is still 4 byte aligned, and correctly so. Also, you cannot use the packed attribute (which reduces the alignment to 1) for simple types. You can only use it for structs. Try a packed struct that contains a single int. That will work.
(In reply to Sven from comment #49) > This doesn't work. The aligned attribute is for providing additional > alignment hints. The GCC documentation clearly states, that aligned can > increase the alignment. So g_d is still 4 byte aligned, and correctly so. Submitted too soon. Should have been "The GCC documentation clearly states that the aligned attribute can only increase the alignment"
(In reply to Sven from comment #49) > This doesn't work. The aligned attribute is for providing additional > alignment hints. The GCC documentation clearly states, that aligned can > increase the alignment. So g_d is still 4 byte aligned, and correctly so. It's really confusing about using 'aligned' attributes in various situations. Under the title "Specifying Attributes of Variables", the doc says "When used as part of a typedef, the aligned attribute can both increase and decrease alignment" without an example. While under the title "Specifying Attributes of Types", the doc gives an example of typedef but states at the end "The aligned attribute can only increase alignment." So perhaps, my test case should have been: typedef int unaligned_int __attribute__((aligned(1))); char g_c = 'x'; unaligned_int g_d = 13; int main(void) { g_c = 'z'; //================ g_d = 33; // Crash on, in my case, ARM Cortex-M0 //================ return 0; } But the result is the same. After all, the 'aligned' attribute took effect in either case since g_d is at an odd address as a consequence.
(In reply to rguenther@suse.de from comment #48) So, is there an old bug that covers my problem, or should I file a new one? Thank you.
latest version of patch adding -Waddress-of-packed-member is here: https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01452.html (In reply to W.H. Ding from comment #52) > (In reply to rguenther@suse.de from comment #48) > > So, is there an old bug that covers my problem, or should I file a new one? > Thank you. If there is an old bug, I don't know of it. I'd just file a new one and we can close it as a duplicate if in fact someone can find an old bug it's a dup of
Author: hjl Date: Thu Dec 20 21:41:48 2018 New Revision: 267313 URL: https://gcc.gnu.org/viewcvs?rev=267313&root=gcc&view=rev Log: C/C++: Add -Waddress-of-packed-member When address of packed member of struct or union is taken, it may result in an unaligned pointer value. This patch adds -Waddress-of-packed-member to check alignment at pointer assignment and warn unaligned address as well as unaligned pointer: $ cat x.i struct pair_t { char c; int i; } __attribute__ ((packed)); extern struct pair_t p; int *addr = &p.i; $ gcc -O2 -S x.i x.i:8:13: warning: taking address of packed member of ‘struct pair_t’ may result in an unaligned pointer value [-Waddress-of-packed-member] 8 | int *addr = &p.i; | ^ $ cat c.i struct B { int i; }; struct C { struct B b; } __attribute__ ((packed)); long* g8 (struct C *p) { return p; } $ gcc -O2 -S c.i -Wno-incompatible-pointer-types c.i: In function ‘g8’: c.i:4:18: warning: converting a packed ‘struct C *’ pointer (alignment 1) to ‘long int *’ (alignment 8) may may result in an unaligned pointer value [-Waddress-of-packed-member] 4 | long* g8 (struct C *p) { return p; } | ^ c.i:2:8: note: defined here 2 | struct C { struct B b; } __attribute__ ((packed)); | ^ $ This warning is enabled by default. Since read_encoded_value_with_base in unwind-pe.h has union unaligned { void *ptr; unsigned u2 __attribute__ ((mode (HI))); unsigned u4 __attribute__ ((mode (SI))); unsigned u8 __attribute__ ((mode (DI))); signed s2 __attribute__ ((mode (HI))); signed s4 __attribute__ ((mode (SI))); signed s8 __attribute__ ((mode (DI))); } __attribute__((__packed__)); _Unwind_Internal_Ptr result; and GCC warns: gcc/libgcc/unwind-pe.h:210:37: warning: taking address of packed member of 'union unaligned' may result in an unaligned pointer value [-Waddress-of-packed-member] result = (_Unwind_Internal_Ptr) u->ptr; ^ we need to add GCC pragma to ignore -Waddress-of-packed-member. gcc/ PR c/51628 * doc/invoke.texi: Document -Wno-address-of-packed-member. gcc/c-family/ PR c/51628 * c-common.h (warn_for_address_or_pointer_of_packed_member): New. * c-warn.c (check_alignment_of_packed_member): New function. (check_address_of_packed_member): Likewise. (check_and_warn_address_of_packed_member): Likewise. (warn_for_address_or_pointer_of_packed_member): Likewise. * c.opt: Add -Wno-address-of-packed-member. gcc/c/ PR c/51628 * c-typeck.c (convert_for_assignment): Call warn_for_address_or_pointer_of_packed_member. gcc/cp/ PR c/51628 * call.c (convert_for_arg_passing): Call warn_for_address_or_pointer_of_packed_member. * typeck.c (convert_for_assignment): Likewise. gcc/testsuite/ PR c/51628 * c-c++-common/pr51628-1.c: New test. * c-c++-common/pr51628-2.c: Likewise. * c-c++-common/pr51628-3.c: Likewise. * c-c++-common/pr51628-4.c: Likewise. * c-c++-common/pr51628-5.c: Likewise. * c-c++-common/pr51628-6.c: Likewise. * c-c++-common/pr51628-7.c: Likewise. * c-c++-common/pr51628-8.c: Likewise. * c-c++-common/pr51628-9.c: Likewise. * c-c++-common/pr51628-10.c: Likewise. * c-c++-common/pr51628-11.c: Likewise. * c-c++-common/pr51628-12.c: Likewise. * c-c++-common/pr51628-13.c: Likewise. * c-c++-common/pr51628-14.c: Likewise. * c-c++-common/pr51628-15.c: Likewise. * c-c++-common/pr51628-26.c: Likewise. * c-c++-common/pr51628-27.c: Likewise. * c-c++-common/pr51628-28.c: Likewise. * c-c++-common/pr51628-29.c: Likewise. * c-c++-common/pr51628-30.c: Likewise. * c-c++-common/pr51628-31.c: Likewise. * c-c++-common/pr51628-32.c: Likewise. * gcc.dg/pr51628-17.c: Likewise. * gcc.dg/pr51628-18.c: Likewise. * gcc.dg/pr51628-19.c: Likewise. * gcc.dg/pr51628-20.c: Likewise. * gcc.dg/pr51628-21.c: Likewise. * gcc.dg/pr51628-22.c: Likewise. * gcc.dg/pr51628-23.c: Likewise. * gcc.dg/pr51628-24.c: Likewise. * gcc.dg/pr51628-25.c: Likewise. * c-c++-common/asan/misalign-1.c: Add -Wno-address-of-packed-member. * c-c++-common/asan/misalign-2.c: Likewise. * c-c++-common/ubsan/align-2.c: Likewise. * c-c++-common/ubsan/align-4.c: Likewise. * c-c++-common/ubsan/align-6.c: Likewise. * c-c++-common/ubsan/align-7.c: Likewise. * c-c++-common/ubsan/align-8.c: Likewise. * c-c++-common/ubsan/align-10.c: Likewise. * g++.dg/ubsan/align-2.C: Likewise. * gcc.target/i386/avx512bw-vmovdqu16-2.c: Likewise. * gcc.target/i386/avx512f-vmovdqu32-2.c: Likewise. * gcc.target/i386/avx512f-vmovdqu64-2.c: Likewise. * gcc.target/i386/avx512vl-vmovdqu16-2.c: Likewise. * gcc.target/i386/avx512vl-vmovdqu32-2.c: Likewise. * gcc.target/i386/avx512vl-vmovdqu64-2.c: Likewise. libgcc/ * unwind-pe.h (read_encoded_value_with_base): Add GCC pragma to ignore -Waddress-of-packed-member. Added: trunk/gcc/testsuite/c-c++-common/pr51628-1.c trunk/gcc/testsuite/c-c++-common/pr51628-10.c trunk/gcc/testsuite/c-c++-common/pr51628-11.c trunk/gcc/testsuite/c-c++-common/pr51628-12.c trunk/gcc/testsuite/c-c++-common/pr51628-13.c trunk/gcc/testsuite/c-c++-common/pr51628-14.c trunk/gcc/testsuite/c-c++-common/pr51628-15.c trunk/gcc/testsuite/c-c++-common/pr51628-16.c trunk/gcc/testsuite/c-c++-common/pr51628-2.c trunk/gcc/testsuite/c-c++-common/pr51628-26.c trunk/gcc/testsuite/c-c++-common/pr51628-27.c trunk/gcc/testsuite/c-c++-common/pr51628-28.c trunk/gcc/testsuite/c-c++-common/pr51628-29.c trunk/gcc/testsuite/c-c++-common/pr51628-3.c trunk/gcc/testsuite/c-c++-common/pr51628-30.c trunk/gcc/testsuite/c-c++-common/pr51628-31.c trunk/gcc/testsuite/c-c++-common/pr51628-32.c trunk/gcc/testsuite/c-c++-common/pr51628-4.c trunk/gcc/testsuite/c-c++-common/pr51628-5.c trunk/gcc/testsuite/c-c++-common/pr51628-6.c trunk/gcc/testsuite/c-c++-common/pr51628-7.c trunk/gcc/testsuite/c-c++-common/pr51628-8.c trunk/gcc/testsuite/c-c++-common/pr51628-9.c trunk/gcc/testsuite/gcc.dg/pr51628-17.c trunk/gcc/testsuite/gcc.dg/pr51628-18.c trunk/gcc/testsuite/gcc.dg/pr51628-19.c trunk/gcc/testsuite/gcc.dg/pr51628-20.c trunk/gcc/testsuite/gcc.dg/pr51628-21.c trunk/gcc/testsuite/gcc.dg/pr51628-22.c trunk/gcc/testsuite/gcc.dg/pr51628-23.c trunk/gcc/testsuite/gcc.dg/pr51628-24.c trunk/gcc/testsuite/gcc.dg/pr51628-25.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-common.h trunk/gcc/c-family/c-warn.c trunk/gcc/c-family/c.opt trunk/gcc/c/ChangeLog trunk/gcc/c/c-typeck.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/cp/typeck.c trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/asan/misalign-1.c trunk/gcc/testsuite/c-c++-common/asan/misalign-2.c trunk/gcc/testsuite/c-c++-common/ubsan/align-10.c trunk/gcc/testsuite/c-c++-common/ubsan/align-2.c trunk/gcc/testsuite/c-c++-common/ubsan/align-4.c trunk/gcc/testsuite/c-c++-common/ubsan/align-6.c trunk/gcc/testsuite/c-c++-common/ubsan/align-7.c trunk/gcc/testsuite/c-c++-common/ubsan/align-8.c trunk/gcc/testsuite/g++.dg/ubsan/align-2.C trunk/gcc/testsuite/gcc.target/i386/avx512bw-vmovdqu16-2.c trunk/gcc/testsuite/gcc.target/i386/avx512f-vmovdqu32-2.c trunk/gcc/testsuite/gcc.target/i386/avx512f-vmovdqu64-2.c trunk/gcc/testsuite/gcc.target/i386/avx512vl-vmovdqu16-2.c trunk/gcc/testsuite/gcc.target/i386/avx512vl-vmovdqu32-2.c trunk/gcc/testsuite/gcc.target/i386/avx512vl-vmovdqu64-2.c trunk/libgcc/ChangeLog trunk/libgcc/unwind-pe.h
Fixed for GCC 9.
The updated testcase pr51628-10 fails at execution on aarch64 FAIL: c-c++-common/pr51628-10.c -Wc++-compat execution test
(In reply to Christophe Lyon from comment #56) > The updated testcase pr51628-10 fails at execution on aarch64 > FAIL: c-c++-common/pr51628-10.c -Wc++-compat execution test Do you know why it fails?
No, I haven't reproduced it manually yet. The log only says: Execution returned 1
A missing warning: [hjl@gnu-cfl-1 pr51628-7]$ cat pr51628-33.c struct pair_t { char x; int i[4]; } __attribute__ ((packed, aligned (4))); extern struct pair_t p; extern void bar (int *); void foo (struct pair_t *p) { bar (p ? p->i : (int *) 0); } [hjl@gnu-cfl-1 pr51628-7]$ /export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/ -O2 -S pr51628-33.c [hjl@gnu-cfl-1 pr51628-7]$
Author: jakub Date: Wed Jan 16 14:18:47 2019 New Revision: 267970 URL: https://gcc.gnu.org/viewcvs?rev=267970&root=gcc&view=rev Log: PR c/51628 PR target/88682 * c-c++-common/pr51628-10.c (unaligned_int128_t): Add may_alias attribute. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/pr51628-10.c
Author: hjl Date: Fri Jan 18 13:05:18 2019 New Revision: 268075 URL: https://gcc.gnu.org/viewcvs?rev=268075&root=gcc&view=rev Log: c-family: Update unaligned adress of packed member check Check unaligned pointer conversion and strip NOPS. gcc/c-family/ PR c/51628 PR c/88664 * c-common.h (warn_for_address_or_pointer_of_packed_member): Remove the boolean argument. * c-warn.c (check_address_of_packed_member): Renamed to ... (check_address_or_pointer_of_packed_member): This. Also warn pointer conversion. (check_and_warn_address_of_packed_member): Renamed to ... (check_and_warn_address_or_pointer_of_packed_member): This. Also warn pointer conversion. (warn_for_address_or_pointer_of_packed_member): Remove the boolean argument. Don't check pointer conversion here. gcc/c PR c/51628 PR c/88664 * c-typeck.c (convert_for_assignment): Upate the warn_for_address_or_pointer_of_packed_member call. gcc/cp PR c/51628 PR c/88664 * call.c (convert_for_arg_passing): Upate the warn_for_address_or_pointer_of_packed_member call. * typeck.c (convert_for_assignment): Likewise. gcc/testsuite/ PR c/51628 PR c/88664 * c-c++-common/pr51628-33.c: New test. * c-c++-common/pr51628-35.c: New test. * c-c++-common/pr88664-1.c: Likewise. * c-c++-common/pr88664-2.c: Likewise. * gcc.dg/pr51628-34.c: Likewise. Added: trunk/gcc/testsuite/c-c++-common/pr51628-33.c trunk/gcc/testsuite/c-c++-common/pr51628-35.c trunk/gcc/testsuite/c-c++-common/pr88664-1.c trunk/gcc/testsuite/c-c++-common/pr88664-2.c trunk/gcc/testsuite/gcc.dg/pr51628-34.c Modified: trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-common.h trunk/gcc/c-family/c-warn.c trunk/gcc/c/ChangeLog trunk/gcc/c/c-typeck.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/cp/typeck.c trunk/gcc/testsuite/ChangeLog