[cfe-dev] Question about alignment when the packed struct type's field is accessed.

[Richard Smith]

On Sun, Jun 23, 2013 at 4:09 AM, jingu <jingu at codeplay.com> wrote:
> Hi all,
>
> I have a question about alignment when the packed struct type's field is
> accessed.
>
> My colleague Fraser tested a code and the generated IR from clang is as
> following:
>
> ******* test source code *******
> typedef unsigned char uint8_t;
> typedef int int32_t;
>
> #pragma pack(push)
> #pragma pack(1)
> struct S0 {
> const uint8_t f_0;
> int32_t f_1;
> };
> #pragma pack(pop)
>
> static struct S0 g_var = {255UL, 1L};
>
> int main ()
> {
> int32_t *ptr = &g_var.f_1;
> int32_t test = *ptr;
>
> return 0;
> }
>
> ******* generated IR from clang *******
> %struct.S0 = type <{ i8, i32 }>
>
> @g_var = internal global %struct.S0 <{ i8 -1, i32 1 }>, align 1
>
> ; Function Attrs: nounwind
> define i32 @main() #0 {
> entry:
> %retval = alloca i32, align 4
> %ptr = alloca i32*, align 4
> %test = alloca i32, align 4
> store i32 0, i32* %retval
> store i32* getelementptr inbounds (%struct.S0* @g_var, i32 0, i32 1),
> i32** %ptr, align 4
> %0 = load i32** %ptr, align 4
> %1 = load i32* %0, align 4
> store i32 %1, i32* %test, align 4
> ret i32 0
> }
>
> We are wondering whether the alignment of this instruction "%1 = load
> i32* %0, align 4" is correct or not.

This is correct. The code generated to load a pointer is not dependent
on how the value in that pointer was originally computed. In this
case, you created a misaligned pointer when you took the address of a
member of a packed struct, and any attempt to perform a (naturally
aligned) load or store through such a pointer has undefined behavior.

> We thought "align 1" is correct
> because g_var is packed structype's variable. How do you feel about
> this? Is it problem or not?

It's a bug in the code. The 'packed' extension is a little broken --
taking the address of the struct member should give you some form of
"pointer to unaligned int" -- but Clang is implementing another
compiler's language extension here, so we can't really do much about
it. That said, we should probably issue a warning if the address of a
member of a packed struct is taken. Related GCC bugs:

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

> After Mem2Reg pass, this problem is
> disappeared. If clang considers this code as problem, we would like to
> make a patch to fix it.

How do you propose to fix it?