Skip to content

Commit 5eea045

Browse files
tautschnigtautschnig
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 2212cd6 commit 5eea045

File tree

4 files changed

+274
-3
lines changed

4 files changed

+274
-3
lines changed

‎regression/ansi-c/gcc_attributes16/main.c

+193
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+
}

‎regression/ansi-c/gcc_attributes16/test.desc

+8
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

+42-2
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

@@ -726,6 +734,29 @@ void c_typecheck_baset::typecheck_vector_type(typet &type)
726734
type = new_type.with_source_location(source_location);
727735
}
728736

737+
/// Adjust \p alignment to be the least common multiple with \p other_alignment,
738+
/// if both of them are non-nil. If exactly one of them is non-nil, set
739+
/// \p alignment to that value.
740+
static void align_to(exprt &alignment, const exprt &other_alignment)
741+
{
742+
if(other_alignment.is_nil())
743+
return;
744+
else if(alignment.is_nil())
745+
alignment = other_alignment;
746+
else
747+
{
748+
const auto a = numeric_cast<mp_integer>(alignment);
749+
CHECK_RETURN(a.has_value());
750+
const auto other = numeric_cast<mp_integer>(other_alignment);
751+
CHECK_RETURN(other.has_value());
752+
753+
// alignments are powers of two, so taking the maximum is sufficient for it
754+
// will be equal to the least common multiple
755+
if(a < other)
756+
alignment = other_alignment;
757+
}
758+
}
759+
729760
void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
730761
{
731762
// These get replaced by symbol types later.
@@ -744,7 +775,7 @@ void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
744775
remove_qualifiers.write(type);
745776

746777
bool is_packed = type.get_bool(ID_C_packed);
747-
irept alignment = type.find(ID_C_alignment);
778+
exprt alignment = static_cast<const exprt &>(type.find(ID_C_alignment));
748779

749780
if(type.find(ID_tag).is_nil())
750781
{
@@ -835,6 +866,10 @@ void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
835866
throw errort().with_location(type.source_location())
836867
<< "redefinition of body of '" << s_it->second.pretty_name << "'";
837868
}
869+
870+
align_to(
871+
alignment,
872+
static_cast<const exprt &>(s_it->second.type.find(ID_C_alignment)));
838873
}
839874
}
840875

@@ -1606,14 +1641,19 @@ void c_typecheck_baset::typecheck_typedef_type(typet &type)
16061641

16071642
c_qualifierst c_qualifiers(type);
16081643
bool is_packed = type.get_bool(ID_C_packed);
1609-
irept alignment = type.find(ID_C_alignment);
1644+
exprt alignment = static_cast<const exprt &>(type.find(ID_C_alignment));
16101645

16111646
c_qualifiers += c_qualifierst(symbol.type);
16121647
type = symbol.type;
16131648
c_qualifiers.write(type);
16141649

16151650
if(is_packed)
16161651
type.set(ID_C_packed, true);
1652+
else
1653+
type.remove(ID_C_packed);
1654+
1655+
align_to(
1656+
alignment, static_cast<const exprt &>(symbol.type.find(ID_C_alignment)));
16171657
if(alignment.is_not_nil())
16181658
type.set(ID_C_alignment, alignment);
16191659

‎src/ansi-c/parser.y

+31-1
Original file line numberDiff line numberDiff line change
@@ -1243,7 +1243,37 @@ basic_type_specifier:
12431243
sue_declaration_specifier:
12441244
declaration_qualifier_list elaborated_type_name
12451245
{
1246-
$$=merge($1, $2);
1246+
// ignore packed or aligned attributes in this context (Clang warns
1247+
// that they will be ignored, GCC just silently ignores them)
1248+
if(parser_stack($1).id() == ID_packed ||
1249+
parser_stack($1).id() == ID_aligned)
1250+
{
1251+
$$=$2;
1252+
}
1253+
else if(parser_stack($1).id() == ID_merged_type)
1254+
{
1255+
auto &sub = parser_stack($1).get_sub();
1256+
irept::subt filtered_sub;
1257+
filtered_sub.reserve(sub.size());
1258+
for(const auto &s : sub)
1259+
{
1260+
if(s.id() != ID_packed && s.id() != ID_aligned)
1261+
filtered_sub.push_back(s);
1262+
}
1263+
if(filtered_sub.empty())
1264+
$$=$2;
1265+
else
1266+
{
1267+
if(filtered_sub.size() == 1)
1268+
parser_stack($1).swap(filtered_sub.front());
1269+
else
1270+
sub.swap(filtered_sub);
1271+
1272+
$$=merge($1, $2);
1273+
}
1274+
}
1275+
else
1276+
$$=merge($1, $2);
12471277
}
12481278
| sue_type_specifier storage_class gcc_type_attribute_opt
12491279
{

0 commit comments

Comments
 (0)