Skip to content

Commit 5bb6e0e

Browse files
keesgithub-actions[bot]
authored andcommitted
Automerge: [Clang][CodeGen] Fix __builtin_counted_by_ref for nested struct FAMs (#182575) (#182590)
GetCountedByFieldExprGEP() used getOuterLexicalRecordContext() to find the RecordDecl containing the counted_by count field. This walks up through all lexically enclosing records to find the outermost one, which is wrong when a struct with a counted_by FAM is defined nested inside another named struct. For example, when struct inner (containing the FAM) is defined inside struct outer, getOuterLexicalRecordContext() resolves to struct outer instead of struct inner. The StructAccessBase visitor then fails to match the base expression type (struct inner *) against the expected record (struct outer), returning nullptr. This nullptr propagates back as the GEP result, and the subsequent dereference in *__builtin_counted_by_ref() triggers an assertion failure in Address::getBasePointer(). Replace getOuterLexicalRecordContext() with a walk that only traverses anonymous structs and unions, which are transparent in C and must be walked past. Named nested structs are independently-addressable types, so the walk stops at them. Add a regression test for a FAM struct defined nested inside another struct. This also fixes __builtin_dynamic_object_size() for FAMs in nested structs, which was silently returning -1 (unknown) instead of computing the correct size. Update the attr-counted-by-pr88931.c test to reflect the now-correct dynamic object size calculation. Fixes #182575 Signed-off-by: Kees Cook <kees@kernel.org>
2 parents 9a7e0d8 + 09a3d83 commit 5bb6e0e

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1192,7 +1192,18 @@ static bool getGEPIndicesToField(CodeGenFunction &CGF, const RecordDecl *RD,
11921192

11931193
llvm::Value *CodeGenFunction::GetCountedByFieldExprGEP(
11941194
const Expr *Base, const FieldDecl *FAMDecl, const FieldDecl *CountDecl) {
1195-
const RecordDecl *RD = CountDecl->getParent()->getOuterLexicalRecordContext();
1195+
// Find the record containing the count field. Walk up through anonymous
1196+
// structs/unions (which are transparent in C) but stop at named records.
1197+
// Using getOuterLexicalRecordContext() here would be wrong because it walks
1198+
// past named nested structs to the outermost record, causing a crash when a
1199+
// struct with a counted_by FAM is defined nested inside another struct.
1200+
const RecordDecl *RD = CountDecl->getParent();
1201+
while (RD->isAnonymousStructOrUnion()) {
1202+
const auto *Parent = dyn_cast<RecordDecl>(RD->getLexicalParent());
1203+
if (!Parent)
1204+
break;
1205+
RD = Parent;
1206+
}
11961207

11971208
// Find the base struct expr (i.e. p in p->a.b.c.d).
11981209
const Expr *StructBase = StructAccessBase(RD).Visit(Base);

clang/test/CodeGen/attr-counted-by-pr88931.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ void init(void * __attribute__((pass_dynamic_object_size(0))));
1515
// CHECK-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
1616
// CHECK-NEXT: entry:
1717
// CHECK-NEXT: [[ARRAY:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 4
18-
// CHECK-NEXT: tail call void @init(ptr noundef nonnull [[ARRAY]], i64 noundef -1) #[[ATTR2:[0-9]+]]
18+
// CHECK-NEXT: [[COUNTED_BY_LOAD:%.*]] = load i32, ptr [[P]], align 4
19+
// CHECK-NEXT: [[COUNT:%.*]] = sext i32 [[COUNTED_BY_LOAD]] to i64
20+
// CHECK-NEXT: [[FLEXIBLE_ARRAY_MEMBER_SIZE:%.*]] = shl nsw i64 [[COUNT]], 2
21+
// CHECK-NEXT: [[TMP0:%.*]] = icmp sgt i32 [[COUNTED_BY_LOAD]], -1
22+
// CHECK-NEXT: [[TMP1:%.*]] = select i1 [[TMP0]], i64 [[FLEXIBLE_ARRAY_MEMBER_SIZE]], i64 0
23+
// CHECK-NEXT: tail call void @init(ptr noundef nonnull [[ARRAY]], i64 noundef [[TMP1]]) #[[ATTR2:[0-9]+]]
1924
// CHECK-NEXT: ret void
2025
//
2126
void test1(struct bar *p) {

clang/test/CodeGen/builtin-counted-by-ref.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,30 @@ struct d *test4(int size, int *data) {
233233
*__builtin_counted_by_ref(p->ptr) = size;
234234
return p;
235235
}
236+
237+
// Test for FAM struct defined nested inside another struct (issue #182575).
238+
// The nested struct is named (not anonymous), so getOuterLexicalRecordContext()
239+
// would incorrectly resolve to 'struct outer' instead of 'struct inner'.
240+
struct outer {
241+
struct inner {
242+
int counter;
243+
int ent[] __attribute__((counted_by(counter)));
244+
} *entries;
245+
};
246+
247+
// X86_64-LABEL: define dso_local ptr @test5(
248+
// X86_64-SAME: i32 noundef [[COUNT:%.*]]) #[[ATTR0]] {
249+
// X86_64: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds [[STRUCT_INNER:%.*]], ptr {{%.*}}, i32 0, i32 0
250+
// X86_64-NEXT: store i32 [[TMP1:%.*]], ptr [[DOT_COUNTED_BY_GEP]], align 4
251+
//
252+
// I386-LABEL: define dso_local ptr @test5(
253+
// I386-SAME: i32 noundef [[COUNT:%.*]]) #[[ATTR0]] {
254+
// I386: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds [[STRUCT_INNER:%.*]], ptr {{%.*}}, i32 0, i32 0
255+
// I386-NEXT: store i32 [[TMP1:%.*]], ptr [[DOT_COUNTED_BY_GEP]], align 4
256+
//
257+
struct inner *test5(int count) {
258+
struct inner *entries = __builtin_malloc(sizeof(*entries) + count * sizeof(*entries->ent));
259+
if (entries)
260+
*__builtin_counted_by_ref(entries->ent) = count;
261+
return entries;
262+
}

0 commit comments

Comments
 (0)