Skip to content

Commit 0367e2c

Browse files
committed
C front-end: fix processing of alignment and packing attributes
Packing needs to be evaluated as part of the definition of the type while alignment applies also when using a type. The position of the attribute also requires extra care as some positions are accepted by GCC (and warned about by Clang) while actually being ignored. Fixes: #8443
1 parent 50be009 commit 0367e2c

File tree

4 files changed

+274
-3
lines changed

4 files changed

+274
-3
lines changed
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
#include <stdint.h>
2+
3+
struct S1
4+
{
5+
uint8_t c;
6+
int32_t i;
7+
} __attribute__((packed));
8+
struct S1 s1;
9+
10+
struct S1a
11+
{
12+
uint8_t c;
13+
int32_t i;
14+
} __attribute__((packed)) __attribute__((aligned(16)));
15+
struct S1a s1a;
16+
17+
struct __attribute__((packed)) S2
18+
{
19+
uint8_t c;
20+
int32_t i;
21+
};
22+
struct S2 s2;
23+
24+
struct __attribute__((packed)) __attribute__((aligned(16))) S2a
25+
{
26+
uint8_t c;
27+
int32_t i;
28+
};
29+
struct S2a s2a;
30+
31+
// "packed" will be ignored
32+
__attribute__((packed)) struct S3
33+
{
34+
uint8_t c;
35+
int32_t i;
36+
};
37+
struct S3 s3;
38+
39+
// both "packed" and "aligned"
40+
__attribute__((packed)) __attribute__((aligned(16))) struct S3a
41+
{
42+
uint8_t c;
43+
int32_t i;
44+
};
45+
struct S3a s3a;
46+
47+
struct S4
48+
{
49+
uint8_t c;
50+
int32_t i;
51+
} __attribute__((packed)) s4;
52+
53+
struct S4a
54+
{
55+
uint8_t c;
56+
int32_t i;
57+
} __attribute__((packed)) __attribute__((aligned(16))) s4a;
58+
59+
struct __attribute__((packed)) S5
60+
{
61+
uint8_t c;
62+
int32_t i;
63+
} s5;
64+
65+
struct __attribute__((packed)) __attribute__((aligned(16))) S5a
66+
{
67+
uint8_t c;
68+
int32_t i;
69+
} s5a;
70+
71+
typedef struct __attribute__((packed)) S6
72+
{
73+
uint8_t c;
74+
int32_t i;
75+
} s6;
76+
77+
typedef struct __attribute__((packed)) __attribute__((aligned(16))) S6a
78+
{
79+
uint8_t c;
80+
int32_t i;
81+
} s6a;
82+
83+
// "packed" will be ignored in the following by both GCC and Clang
84+
struct S7
85+
{
86+
uint8_t c;
87+
int32_t i;
88+
} s7 __attribute__((packed));
89+
90+
// "packed" will be ignored in the following by both GCC and Clang
91+
struct S7a
92+
{
93+
uint8_t c;
94+
int32_t i;
95+
} s7a __attribute__((packed)) __attribute__((aligned(16)));
96+
97+
typedef struct T1
98+
{
99+
uint8_t c;
100+
int32_t i;
101+
} __attribute__((packed)) T1_t;
102+
103+
typedef struct T1a
104+
{
105+
uint8_t c;
106+
int32_t i;
107+
} __attribute__((packed)) __attribute__((aligned(16))) T1a_t;
108+
109+
typedef struct __attribute__((packed)) T2
110+
{
111+
uint8_t c;
112+
int32_t i;
113+
} T2_t;
114+
115+
typedef struct __attribute__((packed)) __attribute__((aligned(16))) T2a
116+
{
117+
uint8_t c;
118+
int32_t i;
119+
} T2a_t;
120+
121+
struct U
122+
{
123+
uint8_t c;
124+
int32_t i;
125+
};
126+
127+
struct S
128+
{
129+
uint8_t c;
130+
// attribute ignored by GCC
131+
struct S1 __attribute((packed)) i1;
132+
struct U __attribute((packed)) i2;
133+
uint8_t c2;
134+
// alignment has to be a power of 2
135+
// struct S2 __attribute((aligned(5))) i2_5;
136+
struct S2a __attribute((aligned(8))) i3;
137+
uint8_t c3;
138+
struct S3a __attribute((aligned(8))) i4;
139+
uint8_t c4;
140+
T1a_t __attribute((aligned(8))) i5;
141+
T1_t __attribute((aligned(8))) i6;
142+
};
143+
144+
int32_t main()
145+
{
146+
_Static_assert(sizeof(struct S1) == 5, "");
147+
_Static_assert(sizeof(s1) == 5, "");
148+
_Static_assert(sizeof(struct S1a) == 16, "");
149+
_Static_assert(sizeof(s1a) == 16, "");
150+
151+
_Static_assert(sizeof(struct S2) == 5, "");
152+
_Static_assert(sizeof(s2) == 5, "");
153+
_Static_assert(sizeof(struct S2a) == 16, "");
154+
_Static_assert(sizeof(s2a) == 16, "");
155+
156+
_Static_assert(sizeof(struct S3) == 8, "");
157+
_Static_assert(sizeof(s3) == 8, "");
158+
_Static_assert(sizeof(struct S3a) == 8, "");
159+
_Static_assert(sizeof(s3a) == 8, "");
160+
161+
_Static_assert(sizeof(struct S4) == 5, "");
162+
_Static_assert(sizeof(s4) == 5, "");
163+
_Static_assert(sizeof(struct S4a) == 16, "");
164+
_Static_assert(sizeof(s4a) == 16, "");
165+
166+
_Static_assert(sizeof(struct S5) == 5, "");
167+
_Static_assert(sizeof(s5) == 5, "");
168+
_Static_assert(sizeof(struct S5a) == 16, "");
169+
_Static_assert(sizeof(s5a) == 16, "");
170+
171+
_Static_assert(sizeof(struct S6) == 5, "");
172+
_Static_assert(sizeof(s6) == 5, "");
173+
_Static_assert(sizeof(struct S6a) == 16, "");
174+
_Static_assert(sizeof(s6a) == 16, "");
175+
176+
_Static_assert(sizeof(struct S7) == 8, "");
177+
_Static_assert(sizeof(s7) == 8, "");
178+
_Static_assert(sizeof(struct S7a) == 8, "");
179+
_Static_assert(sizeof(s7a) == 8, "");
180+
181+
_Static_assert(sizeof(struct T1) == 5, "");
182+
_Static_assert(sizeof(T1_t) == 5, "");
183+
_Static_assert(sizeof(struct T1a) == 16, "");
184+
_Static_assert(sizeof(T1a_t) == 16, "");
185+
186+
_Static_assert(sizeof(struct T2) == 5, "");
187+
_Static_assert(sizeof(T2_t) == 5, "");
188+
_Static_assert(sizeof(struct T2a) == 16, "");
189+
_Static_assert(sizeof(T2a_t) == 16, "");
190+
191+
_Static_assert(sizeof(struct S) == 96, "");
192+
return 0;
193+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CORE gcc-only
2+
main.c
3+
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
--
7+
^warning: ignoring
8+
^CONVERSION ERROR$

src/ansi-c/c_typecheck_type.cpp

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ void c_typecheck_baset::typecheck_type(typet &type)
7272
{
7373
typecheck_expr(alignment);
7474
make_constant(alignment);
75+
const auto align_int = numeric_cast<mp_integer>(alignment);
76+
if(
77+
!align_int.has_value() ||
78+
power(2, align_int->floorPow2()) != *align_int)
79+
{
80+
throw errort().with_location(type.source_location())
81+
<< "alignment is not a positive power of 2";
82+
}
7583
}
7684
}
7785

@@ -786,6 +794,29 @@ void c_typecheck_baset::typecheck_vector_type(typet &type)
786794
type = new_type.with_source_location(source_location);
787795
}
788796

797+
/// Adjust \p alignment to be the least common multiple with \p other_alignment,
798+
/// if both of them are non-nil. If exactly one of them is non-nil, set
799+
/// \p alignment to that value.
800+
static void align_to(exprt &alignment, const exprt &other_alignment)
801+
{
802+
if(other_alignment.is_nil())
803+
return;
804+
else if(alignment.is_nil())
805+
alignment = other_alignment;
806+
else
807+
{
808+
const auto a = numeric_cast<mp_integer>(alignment);
809+
CHECK_RETURN(a.has_value());
810+
const auto other = numeric_cast<mp_integer>(other_alignment);
811+
CHECK_RETURN(other.has_value());
812+
813+
// alignments are powers of two, so taking the maximum is sufficient for it
814+
// will be equal to the least common multiple
815+
if(a < other)
816+
alignment = other_alignment;
817+
}
818+
}
819+
789820
void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
790821
{
791822
// These get replaced by symbol types later.
@@ -804,7 +835,7 @@ void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
804835
remove_qualifiers.write(type);
805836

806837
bool is_packed = type.get_bool(ID_C_packed);
807-
irept alignment = type.find(ID_C_alignment);
838+
exprt alignment = static_cast<const exprt &>(type.find(ID_C_alignment));
808839

809840
if(type.find(ID_tag).is_nil())
810841
{
@@ -895,6 +926,10 @@ void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
895926
throw errort().with_location(type.source_location())
896927
<< "redefinition of body of '" << s_it->second.pretty_name << "'";
897928
}
929+
930+
align_to(
931+
alignment,
932+
static_cast<const exprt &>(s_it->second.type.find(ID_C_alignment)));
898933
}
899934
}
900935

@@ -1666,14 +1701,19 @@ void c_typecheck_baset::typecheck_typedef_type(typet &type)
16661701

16671702
c_qualifierst c_qualifiers(type);
16681703
bool is_packed = type.get_bool(ID_C_packed);
1669-
irept alignment = type.find(ID_C_alignment);
1704+
exprt alignment = static_cast<const exprt &>(type.find(ID_C_alignment));
16701705

16711706
c_qualifiers += c_qualifierst(symbol.type);
16721707
type = symbol.type;
16731708
c_qualifiers.write(type);
16741709

16751710
if(is_packed)
16761711
type.set(ID_C_packed, true);
1712+
else
1713+
type.remove(ID_C_packed);
1714+
1715+
align_to(
1716+
alignment, static_cast<const exprt &>(symbol.type.find(ID_C_alignment)));
16771717
if(alignment.is_not_nil())
16781718
type.set(ID_C_alignment, alignment);
16791719

src/ansi-c/parser.y

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1276,7 +1276,37 @@ basic_type_specifier:
12761276
sue_declaration_specifier:
12771277
declaration_qualifier_list elaborated_type_name
12781278
{
1279-
$$=merge($1, $2);
1279+
// ignore packed or aligned attributes in this context (Clang warns
1280+
// that they will be ignored, GCC just silently ignores them)
1281+
if(parser_stack($1).id() == ID_packed ||
1282+
parser_stack($1).id() == ID_aligned)
1283+
{
1284+
$$=$2;
1285+
}
1286+
else if(parser_stack($1).id() == ID_merged_type)
1287+
{
1288+
auto &sub = parser_stack($1).get_sub();
1289+
irept::subt filtered_sub;
1290+
filtered_sub.reserve(sub.size());
1291+
for(const auto &s : sub)
1292+
{
1293+
if(s.id() != ID_packed && s.id() != ID_aligned)
1294+
filtered_sub.push_back(s);
1295+
}
1296+
if(filtered_sub.empty())
1297+
$$=$2;
1298+
else
1299+
{
1300+
if(filtered_sub.size() == 1)
1301+
parser_stack($1).swap(filtered_sub.front());
1302+
else
1303+
sub.swap(filtered_sub);
1304+
1305+
$$=merge($1, $2);
1306+
}
1307+
}
1308+
else
1309+
$$=merge($1, $2);
12801310
}
12811311
| sue_type_specifier storage_class gcc_type_attribute_opt
12821312
{

0 commit comments

Comments
 (0)