Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C front-end: fix processing of alignment and packing attributes #8454

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

C front-end: fix processing of alignment and packing attributes #8454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 193 additions & 0 deletions regression/ansi-c/gcc_attributes16/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
#include <stdint.h>

struct S1
{
uint8_t c;
int32_t i;
} __attribute__((packed));
struct S1 s1;

struct S1a
{
uint8_t c;
int32_t i;
} __attribute__((packed)) __attribute__((aligned(16)));
struct S1a s1a;

struct __attribute__((packed)) S2
{
uint8_t c;
int32_t i;
};
struct S2 s2;

struct __attribute__((packed)) __attribute__((aligned(16))) S2a
{
uint8_t c;
int32_t i;
};
struct S2a s2a;

// "packed" will be ignored
__attribute__((packed)) struct S3
{
uint8_t c;
int32_t i;
};
struct S3 s3;

// both "packed" and "aligned"
__attribute__((packed)) __attribute__((aligned(16))) struct S3a
{
uint8_t c;
int32_t i;
};
struct S3a s3a;

struct S4
{
uint8_t c;
int32_t i;
} __attribute__((packed)) s4;

struct S4a
{
uint8_t c;
int32_t i;
} __attribute__((packed)) __attribute__((aligned(16))) s4a;

struct __attribute__((packed)) S5
{
uint8_t c;
int32_t i;
} s5;

struct __attribute__((packed)) __attribute__((aligned(16))) S5a
{
uint8_t c;
int32_t i;
} s5a;

typedef struct __attribute__((packed)) S6
{
uint8_t c;
int32_t i;
} s6;

typedef struct __attribute__((packed)) __attribute__((aligned(16))) S6a
{
uint8_t c;
int32_t i;
} s6a;

// "packed" will be ignored in the following by both GCC and Clang
struct S7
{
uint8_t c;
int32_t i;
} s7 __attribute__((packed));

// "packed" will be ignored in the following by both GCC and Clang
struct S7a
{
uint8_t c;
int32_t i;
} s7a __attribute__((packed)) __attribute__((aligned(16)));

typedef struct T1
{
uint8_t c;
int32_t i;
} __attribute__((packed)) T1_t;

typedef struct T1a
{
uint8_t c;
int32_t i;
} __attribute__((packed)) __attribute__((aligned(16))) T1a_t;

typedef struct __attribute__((packed)) T2
{
uint8_t c;
int32_t i;
} T2_t;

typedef struct __attribute__((packed)) __attribute__((aligned(16))) T2a
{
uint8_t c;
int32_t i;
} T2a_t;

struct U
{
uint8_t c;
int32_t i;
};

struct S
{
uint8_t c;
// attribute ignored by GCC
struct S1 __attribute((packed)) i1;
struct U __attribute((packed)) i2;
uint8_t c2;
// alignment has to be a power of 2
// struct S2 __attribute((aligned(5))) i2_5;
struct S2a __attribute((aligned(8))) i3;
uint8_t c3;
struct S3a __attribute((aligned(8))) i4;
uint8_t c4;
T1a_t __attribute((aligned(8))) i5;
T1_t __attribute((aligned(8))) i6;
};

int32_t main()
{
_Static_assert(sizeof(struct S1) == 5, "");
_Static_assert(sizeof(s1) == 5, "");
_Static_assert(sizeof(struct S1a) == 16, "");
_Static_assert(sizeof(s1a) == 16, "");

_Static_assert(sizeof(struct S2) == 5, "");
_Static_assert(sizeof(s2) == 5, "");
_Static_assert(sizeof(struct S2a) == 16, "");
_Static_assert(sizeof(s2a) == 16, "");

_Static_assert(sizeof(struct S3) == 8, "");
_Static_assert(sizeof(s3) == 8, "");
_Static_assert(sizeof(struct S3a) == 8, "");
_Static_assert(sizeof(s3a) == 8, "");

_Static_assert(sizeof(struct S4) == 5, "");
_Static_assert(sizeof(s4) == 5, "");
_Static_assert(sizeof(struct S4a) == 16, "");
_Static_assert(sizeof(s4a) == 16, "");

_Static_assert(sizeof(struct S5) == 5, "");
_Static_assert(sizeof(s5) == 5, "");
_Static_assert(sizeof(struct S5a) == 16, "");
_Static_assert(sizeof(s5a) == 16, "");

_Static_assert(sizeof(struct S6) == 5, "");
_Static_assert(sizeof(s6) == 5, "");
_Static_assert(sizeof(struct S6a) == 16, "");
_Static_assert(sizeof(s6a) == 16, "");

_Static_assert(sizeof(struct S7) == 8, "");
_Static_assert(sizeof(s7) == 8, "");
_Static_assert(sizeof(struct S7a) == 8, "");
_Static_assert(sizeof(s7a) == 8, "");

_Static_assert(sizeof(struct T1) == 5, "");
_Static_assert(sizeof(T1_t) == 5, "");
_Static_assert(sizeof(struct T1a) == 16, "");
_Static_assert(sizeof(T1a_t) == 16, "");

_Static_assert(sizeof(struct T2) == 5, "");
_Static_assert(sizeof(T2_t) == 5, "");
_Static_assert(sizeof(struct T2a) == 16, "");
_Static_assert(sizeof(T2a_t) == 16, "");

_Static_assert(sizeof(struct S) == 96, "");
return 0;
}
8 changes: 8 additions & 0 deletions regression/ansi-c/gcc_attributes16/test.desc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
CORE gcc-only
main.c

^EXIT=0$
^SIGNAL=0$
--
^warning: ignoring
^CONVERSION ERROR$
44 changes: 42 additions & 2 deletions src/ansi-c/c_typecheck_type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@
{
typecheck_expr(alignment);
make_constant(alignment);
const auto align_int = numeric_cast<mp_integer>(alignment);
if(
!align_int.has_value() ||
power(2, align_int->floorPow2()) != *align_int)
{
throw errort().with_location(type.source_location())
<< "alignment is not a positive power of 2";

Check warning on line 81 in src/ansi-c/c_typecheck_type.cpp

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/c_typecheck_type.cpp#L80-L81 

Added lines #L80 - L81 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a requirement in the standard?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C11 6.2.8 "Alignment of objects" includes the following sentence: "Every valid alignment value shall be a nonnegative integral power of two."

}
}
}

Expand Down Expand Up @@ -726,6 +734,29 @@
type = new_type.with_source_location(source_location);
}

/// Adjust \p alignment to be the least common multiple with \p other_alignment,
/// if both of them are non-nil. If exactly one of them is non-nil, set
/// \p alignment to that value.
static void align_to(exprt &alignment, const exprt &other_alignment)
{
if(other_alignment.is_nil())
return;

Check warning on line 743 in src/ansi-c/c_typecheck_type.cpp

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/c_typecheck_type.cpp#L743 

Added line #L743 was not covered by tests
else if(alignment.is_nil())
alignment = other_alignment;
else

Check warning on line 746 in src/ansi-c/c_typecheck_type.cpp

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/c_typecheck_type.cpp#L746 

Added line #L746 was not covered by tests
{
const auto a = numeric_cast<mp_integer>(alignment);
CHECK_RETURN(a.has_value());
const auto other = numeric_cast<mp_integer>(other_alignment);
CHECK_RETURN(other.has_value());

// alignments are powers of two, so taking the maximum is sufficient for it
// will be equal to the least common multiple
if(a < other)
alignment = other_alignment;
}
}

void c_typecheck_baset::typecheck_compound_type(struct_union_typet &type)
{
// These get replaced by symbol types later.
Expand All @@ -744,7 +775,7 @@
remove_qualifiers.write(type);

bool is_packed = type.get_bool(ID_C_packed);
irept alignment = type.find(ID_C_alignment);
exprt alignment = static_cast<const exprt &>(type.find(ID_C_alignment));

if(type.find(ID_tag).is_nil())
{
Expand Down Expand Up @@ -835,6 +866,10 @@
throw errort().with_location(type.source_location())
<< "redefinition of body of '" << s_it->second.pretty_name << "'";
}

align_to(
alignment,
static_cast<const exprt &>(s_it->second.type.find(ID_C_alignment)));
}
}

Expand Down Expand Up @@ -1606,14 +1641,19 @@

c_qualifierst c_qualifiers(type);
bool is_packed = type.get_bool(ID_C_packed);
irept alignment = type.find(ID_C_alignment);
exprt alignment = static_cast<const exprt &>(type.find(ID_C_alignment));

c_qualifiers += c_qualifierst(symbol.type);
type = symbol.type;
c_qualifiers.write(type);

if(is_packed)
type.set(ID_C_packed, true);
else
type.remove(ID_C_packed);

align_to(
alignment, static_cast<const exprt &>(symbol.type.find(ID_C_alignment)));
if(alignment.is_not_nil())
type.set(ID_C_alignment, alignment);

Expand Down
32 changes: 31 additions & 1 deletion src/ansi-c/parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,37 @@
sue_declaration_specifier:
declaration_qualifier_list elaborated_type_name
{
$$=merge($1, $2);
// ignore packed or aligned attributes in this context (Clang warns

Check warning on line 1246 in src/ansi-c/parser.y

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/parser.y#L1246 

Added line #L1246 was not covered by tests
// that they will be ignored, GCC just silently ignores them)
if(parser_stack($1).id() == ID_packed ||
parser_stack($1).id() == ID_aligned)
{

Check warning on line 1250 in src/ansi-c/parser.y

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/parser.y#L1250 

Added line #L1250 was not covered by tests
$$=$2;
}
else if(parser_stack($1).id() == ID_merged_type)
{

Check warning on line 1254 in src/ansi-c/parser.y

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/parser.y#L1254 

Added line #L1254 was not covered by tests
auto &sub = parser_stack($1).get_sub();
irept::subt filtered_sub;
filtered_sub.reserve(sub.size());
for(const auto &s : sub)
{
if(s.id() != ID_packed && s.id() != ID_aligned)
filtered_sub.push_back(s);
}
if(filtered_sub.empty())
$$=$2;
else
{

Check warning on line 1266 in src/ansi-c/parser.y

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/parser.y#L1265-L1266 

Added lines #L1265 - L1266 were not covered by tests
if(filtered_sub.size() == 1)
parser_stack($1).swap(filtered_sub.front());

Check warning on line 1268 in src/ansi-c/parser.y

View check run for this annotation

Codecov / codecov/patch

src/ansi-c/parser.y#L1268 

Added line #L1268 was not covered by tests
else
sub.swap(filtered_sub);

$$=merge($1, $2);
}
}
else
$$=merge($1, $2);
}
| sue_type_specifier storage_class gcc_type_attribute_opt
{
Expand Down
Loading