Question about alignment when the packed struct type's field is accessed.

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. 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? After Mem2Reg pass, this problem is
disappeared. If clang considers this code as problem, we would like to
make a patch to fix it.

Thanks,
JinGu Kang

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?

Hi Richard,

I really appreciate your detailed response. I am finding information about this kind of code.

> How do you propose to fix it?

I have no detailed idea. but I intended to generalize this problem and propagate alignment information with some analyses. because clang also supports analyses and uses them to issue warnings. It's just brain storming phase. Your comment is really helpful for me.

Thanks for your great reponse,
JinGu Kang