[Clang] Use correct base expression for counted_by field (#73168) #73465
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The base expression of the counted_by field may not be the outermost enclosing struct, but could be a sub-struct. Use the latter in those cases. Closes llvm#73168
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Bill Wendling (bwendling) ChangesI'll add some testcases later today. Patch is 31.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73465.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index c83ea966fdeadc6..1d02b0fc59b9b0e 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -916,7 +916,7 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
// Build a load of the counted_by field.
bool IsSigned = CountedByFD->getType()->isSignedIntegerType();
- const Expr *CountedByExpr = BuildCountedByFieldExpr(Base, CountedByFD);
+ Expr *CountedByExpr = BuildCountedByFieldExpr(Base, CountedByFD);
Value *CountedByInst = EmitAnyExprToTemp(CountedByExpr).getScalarVal();
llvm::Type *CountedByTy = CountedByInst->getType();
@@ -945,7 +945,7 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
: Builder.CreateZExtOrTrunc(FAMSize, ResType);
Value *Res = FAMSize;
- if (const auto *DRE = dyn_cast<DeclRefExpr>(Base)) {
+ if (isa<DeclRefExpr>(Base)) {
// The whole struct is specificed in the __bdos.
const RecordDecl *OuterRD =
CountedByFD->getDeclContext()->getOuterLexicalRecordContext();
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index a75f630e1a4c767..60dc5d68a223dd8 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -940,7 +940,7 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction &CGF,
if (const ValueDecl *VD = CGF.FindCountedByField(Base)) {
IndexedType = Base->getType();
- const Expr *E = CGF.BuildCountedByFieldExpr(Base, VD);
+ Expr *E = CGF.BuildCountedByFieldExpr(Base, VD);
return CGF.EmitAnyExprToTemp(E).getScalarVal();
}
}
@@ -956,21 +956,41 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction &CGF,
return nullptr;
}
-const Expr *
-CodeGenFunction::BuildCountedByFieldExpr(const Expr *Base,
- const ValueDecl *CountedByVD) {
+Expr *CodeGenFunction::BuildCountedByFieldExpr(const Expr *Base,
+ const ValueDecl *CountedByVD) {
// Find the outer struct expr (i.e. p in p->a.b.c.d).
Expr *CountedByExpr = const_cast<Expr *>(Base)->IgnoreParenImpCasts();
+ // Get the enclosing struct, but not the outermost enclosing struct.
+ const DeclContext *DC = CountedByVD->getLexicalDeclContext();
+ const RecordDecl *CountedByRD = dyn_cast<RecordDecl>(DC);
+ if (!CountedByRD)
+ return nullptr;
+
// Work our way up the expression until we reach the DeclRefExpr.
- while (!isa<DeclRefExpr>(CountedByExpr))
- if (const auto *ME = dyn_cast<MemberExpr>(CountedByExpr))
- CountedByExpr = ME->getBase()->IgnoreParenImpCasts();
+ while (auto *ME = dyn_cast<MemberExpr>(CountedByExpr)) {
+ CountedByExpr = ME->getBase()->IgnoreImpCasts();
+
+ // Use the base of an ArraySubscriptExpr.
+ while (auto *ASE = dyn_cast<ArraySubscriptExpr>(CountedByExpr))
+ CountedByExpr = ASE->getBase()->IgnoreImpCasts();
+
+ QualType Ty = CountedByExpr->getType();
+ if (Ty->isPointerType())
+ Ty = Ty->getPointeeType();
+
+ // Stop when we reach the struct containing the counted_by field.
+ if (CountedByRD == Ty->getAsRecordDecl())
+ break;
+ }
+
+ ASTContext &Ctx = getContext();
- // Add back an implicit cast to create the required pr-value.
- CountedByExpr = ImplicitCastExpr::Create(
- getContext(), CountedByExpr->getType(), CK_LValueToRValue, CountedByExpr,
- nullptr, VK_PRValue, FPOptionsOverride());
+ if (!CountedByExpr->isPRValue())
+ // Add an implicit cast to create the required pr-value.
+ CountedByExpr = ImplicitCastExpr::Create(
+ Ctx, CountedByExpr->getType(), CK_LValueToRValue, CountedByExpr,
+ nullptr, VK_PRValue, FPOptionsOverride());
if (const auto *IFD = dyn_cast<IndirectFieldDecl>(CountedByVD)) {
// The counted_by field is inside an anonymous struct / union. The
@@ -978,15 +998,13 @@ CodeGenFunction::BuildCountedByFieldExpr(const Expr *Base,
// easily. (Yay!)
for (NamedDecl *ND : IFD->chain()) {
auto *VD = cast<ValueDecl>(ND);
- CountedByExpr =
- MemberExpr::CreateImplicit(getContext(), CountedByExpr,
- CountedByExpr->getType()->isPointerType(),
- VD, VD->getType(), VK_LValue, OK_Ordinary);
+ CountedByExpr = MemberExpr::CreateImplicit(
+ Ctx, CountedByExpr, CountedByExpr->getType()->isPointerType(), VD,
+ VD->getType(), VK_LValue, OK_Ordinary);
}
} else {
CountedByExpr = MemberExpr::CreateImplicit(
- getContext(), const_cast<Expr *>(CountedByExpr),
- CountedByExpr->getType()->isPointerType(),
+ Ctx, CountedByExpr, CountedByExpr->getType()->isPointerType(),
const_cast<ValueDecl *>(CountedByVD), CountedByVD->getType(), VK_LValue,
OK_Ordinary);
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 618e78809db408b..7dc4d2812ab245d 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3032,8 +3032,7 @@ class CodeGenFunction : public CodeGenTypeCache {
const ValueDecl *FindCountedByField(const Expr *Base);
/// Build an expression accessing the "counted_by" field.
- const Expr *BuildCountedByFieldExpr(const Expr *Base,
- const ValueDecl *CountedByVD);
+ Expr *BuildCountedByFieldExpr(const Expr *Base, const ValueDecl *CountedByVD);
llvm::Value *EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
bool isInc, bool isPre);
diff --git a/clang/test/CodeGen/attr-counted-by.c b/clang/test/CodeGen/attr-counted-by.c
index 5cefff0e6f1cd5c..97226a24f5d62fe 100644
--- a/clang/test/CodeGen/attr-counted-by.c
+++ b/clang/test/CodeGen/attr-counted-by.c
@@ -61,14 +61,14 @@ struct anon_struct {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED:%.*]], ptr [[P]], i64 0, i32 1
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa [[TBAA2:![0-9]+]]
-// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP3:%.*]] = icmp ult i64 [[TMP1]], [[TMP2]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP3]], label [[CONT7:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7:![0-9]+]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize [[META6:![0-9]+]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP3:%.*]] = icmp ult i64 [[TMP1]], [[TMP2]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP3]], label [[CONT7:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7:![0-9]+]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: [[TMP4:%.*]] = zext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 [[TMP4]]) #[[ATTR4:[0-9]+]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP4:%.*]] = zext i32 [[INDEX]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 [[TMP4]]) #[[ATTR4:[0-9]+]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont7:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED]], ptr [[P]], i64 0, i32 2, i64 [[TMP1]]
// SANITIZE-WITH-ATTR-NEXT: store i32 [[VAL]], ptr [[ARRAYIDX]], align 4, !tbaa [[TBAA2]]
@@ -107,12 +107,12 @@ void test1(struct annotated *p, int index, int val) {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED:%.*]], ptr [[P]], i64 0, i32 1
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa [[TBAA2]]
-// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP1]], [[INDEX]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP1]], [[INDEX]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB4:[0-9]+]], i64 [[INDEX]]) #[[ATTR4]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB4:[0-9]+]], i64 [[INDEX]]) #[[ATTR4]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont12:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED]], ptr [[P]], i64 0, i32 2, i64 [[INDEX]]
// SANITIZE-WITH-ATTR-NEXT: [[DOTINV:%.*]] = icmp slt i32 [[TMP0]], 0
@@ -156,12 +156,12 @@ void test2(struct annotated *p, size_t index) {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED:%.*]], ptr [[P]], i64 0, i32 1
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa [[TBAA2]]
-// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP1]], [[INDEX]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP1]], [[INDEX]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB5:[0-9]+]], i64 [[INDEX]]) #[[ATTR4]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB5:[0-9]+]], i64 [[INDEX]]) #[[ATTR4]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont12:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED]], ptr [[P]], i64 0, i32 2, i64 [[INDEX]]
// SANITIZE-WITH-ATTR-NEXT: [[DOTINV:%.*]] = icmp slt i32 [[TMP0]], 0
@@ -211,14 +211,14 @@ void test3(struct annotated *p, size_t index) {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED:%.*]], ptr [[P]], i64 0, i32 1
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa [[TBAA2]]
-// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP3:%.*]] = icmp ult i64 [[TMP1]], [[TMP2]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP3]], label [[CONT13:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = zext i32 [[TMP0]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP3:%.*]] = icmp ult i64 [[TMP1]], [[TMP2]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP3]], label [[CONT13:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: [[TMP4:%.*]] = zext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB6:[0-9]+]], i64 [[TMP4]]) #[[ATTR4]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP4:%.*]] = zext i32 [[INDEX]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB6:[0-9]+]], i64 [[TMP4]]) #[[ATTR4]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont13:
// SANITIZE-WITH-ATTR-NEXT: [[TMP5:%.*]] = icmp sgt i32 [[TMP0]], 2
// SANITIZE-WITH-ATTR-NEXT: [[TMP6:%.*]] = shl i32 [[TMP0]], 2
@@ -229,14 +229,14 @@ void test3(struct annotated *p, size_t index) {
// SANITIZE-WITH-ATTR-NEXT: store i32 [[CONV3]], ptr [[ARRAYIDX]], align 4, !tbaa [[TBAA2]]
// SANITIZE-WITH-ATTR-NEXT: [[TMP9:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa [[TBAA2]]
// SANITIZE-WITH-ATTR-NEXT: [[ADD:%.*]] = add nsw i32 [[INDEX]], 1
-// SANITIZE-WITH-ATTR-NEXT: [[TMP10:%.*]] = sext i32 [[ADD]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP11:%.*]] = zext i32 [[TMP9]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP12:%.*]] = icmp ult i64 [[TMP10]], [[TMP11]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP12]], label [[CONT34:%.*]], label [[HANDLER_OUT_OF_BOUNDS29:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP10:%.*]] = sext i32 [[ADD]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP11:%.*]] = zext i32 [[TMP9]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP12:%.*]] = icmp ult i64 [[TMP10]], [[TMP11]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP12]], label [[CONT34:%.*]], label [[HANDLER_OUT_OF_BOUNDS29:%.*]], !prof [[PROF7]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds29:
-// SANITIZE-WITH-ATTR-NEXT: [[TMP13:%.*]] = zext i32 [[ADD]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB7:[0-9]+]], i64 [[TMP13]]) #[[ATTR4]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP13:%.*]] = zext i32 [[ADD]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB7:[0-9]+]], i64 [[TMP13]]) #[[ATTR4]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont34:
// SANITIZE-WITH-ATTR-NEXT: [[TMP14:%.*]] = icmp sgt i32 [[TMP9]], 3
// SANITIZE-WITH-ATTR-NEXT: [[TMP15:%.*]] = shl i32 [[TMP9]], 2
@@ -247,14 +247,14 @@ void test3(struct annotated *p, size_t index) {
// SANITIZE-WITH-ATTR-NEXT: store i32 [[CONV20]], ptr [[ARRAYIDX32]], align 4, !tbaa [[TBAA2]]
// SANITIZE-WITH-ATTR-NEXT: [[TMP18:%.*]] = load i32, ptr [[COUNT]], align 8, !tbaa [[TBAA2]]
// SANITIZE-WITH-ATTR-NEXT: [[ADD45:%.*]] = add nsw i32 [[INDEX]], 2
-// SANITIZE-WITH-ATTR-NEXT: [[TMP19:%.*]] = sext i32 [[ADD45]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP20:%.*]] = zext i32 [[TMP18]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP21:%.*]] = icmp ult i64 [[TMP19]], [[TMP20]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP21]], label [[CONT56:%.*]], label [[HANDLER_OUT_OF_BOUNDS51:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP19:%.*]] = sext i32 [[ADD45]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP20:%.*]] = zext i32 [[TMP18]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP21:%.*]] = icmp ult i64 [[TMP19]], [[TMP20]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP21]], label [[CONT56:%.*]], label [[HANDLER_OUT_OF_BOUNDS51:%.*]], !prof [[PROF7]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds51:
-// SANITIZE-WITH-ATTR-NEXT: [[TMP22:%.*]] = zext i32 [[ADD45]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB8:[0-9]+]], i64 [[TMP22]]) #[[ATTR4]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP22:%.*]] = zext i32 [[ADD45]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB8:[0-9]+]], i64 [[TMP22]]) #[[ATTR4]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont56:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX54:%.*]] = getelementptr inbounds [[STRUCT_ANNOTATED]], ptr [[P]], i64 0, i32 2, i64 [[TMP19]]
// SANITIZE-WITH-ATTR-NEXT: [[TMP23:%.*]] = sub nsw i32 [[TMP18]], [[FAM_IDX]]
@@ -346,13 +346,13 @@ void test4(struct annotated *p, int index, int fam_idx) {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = getelementptr inbounds [[STRUCT_ANON_STRUCT:%.*]], ptr [[P]], i64 0, i32 1
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = load i64, ptr [[COUNT]], align 8, !tbaa [[TBAA8:![0-9]+]]
-// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP0]], [[TMP1]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP0]], [[TMP1]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: [[TMP3:%.*]] = zext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB9:[0-9]+]], i64 [[TMP3]]) #[[ATTR4]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP3:%.*]] = zext i32 [[INDEX]] to i64, !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB9:[0-9]+]], i64 [[TMP3]]) #[[ATTR4]], !nosanitize [[META6]]
+// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META6]]
// SANITIZE-WITH-ATTR: cont12:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAY:%.*]] = getelementptr inbounds [[STRUCT_ANON_STRUCT]], ptr [[P]], i64 1
// SANITIZE-WITH-ATTR-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [0 x i32], ptr [[ARRAY]], i64 0, i64 [[TMP1]]
@@ -407,13 +407,13 @@ void test5(struct anon_struct *p, int index) {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNT:%.*]] = getelementptr inbounds [[STRUCT_ANON_STRUCT:%.*]], ptr [[P]], i64 0, i32 1
// SANITIZE-WITH-ATTR-NEXT: [[TMP0:%.*]] = load i64, ptr [[COUNT]], align 8, !tbaa [[TBAA8]]
-// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ugt i64 [[TMP0]], [[TMP1]], !nosanitize !6
-// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2]], label [[CONT12:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF7]], !nosanitize !6
+// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = sext i32 [[INDEX]] to i64, !nosanitize [[META6]]
+// SANITIZE...
[truncated]
|
bwendling
requested review from
kees,
nickdesaulniers,
AaronBallman and
nathanchance
clang/lib/CodeGen/CGBuiltin.cpp
Outdated
| @@ -916,7 +916,7 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type, | |||
|
|
|||
| // Build a load of the counted_by field. | |||
| bool IsSigned = CountedByFD->getType()->isSignedIntegerType(); | |||
| const Expr *CountedByExpr = BuildCountedByFieldExpr(Base, CountedByFD); | |||
| Expr *CountedByExpr = BuildCountedByFieldExpr(Base, CountedByFD); | |||
There was a problem hiding this comment.
Why did we need to drop the const here?
There was a problem hiding this comment.
Because it's not needed, and doesn't make too much sense, because we're building the expression at this point.
There was a problem hiding this comment.
I'm not opposed but it doesn't seem like a useful change to me -- for starters, we shouldn't be generating new AST nodes at codegen time (this will mean consumers of the AST won't ever see this information), but we especially shouldn't need to mutate the pointed-to AST nodes from CodeGen.
There was a problem hiding this comment.
Okay. I can pull back that part of the patch. It's more of a "cleanup" for this fix anyway. :-)
There was a problem hiding this comment.
Thanks! At some point we should see if we can hoist the AST generation out of codegen too but that's very orthogonal to your current patch.
There was a problem hiding this comment.
for starters, we shouldn't be generating new AST nodes at codegen time
Yeah, I should have caught that before approving the initial implementation. I think it would have been better to generate the corresponding GEP manually here. Instead, Bill is synthesizing an expression via Expr which is why he's running into this const confusion.
i.e. if we have:
struct annotated {
int count;
int array[] __counted_by(count);
};
int foo (struct annotated *a, size_t i) { return a->array[i]; }the current implementation is turning that into:
struct annotated {
int count;
int array[] __counted_by(count);
};
int foo (struct annotated *a, size_t i) {
if (i > a->count)
__builtin_ubsan_trap();
return a->array[i];
}at the AST (Expr) level rather than [llvm] IR level, because it's perhaps simpler/easier to generate a->count (for arbitrarily complex counted by definitions) and then rely on existing machinery to codegen the correct GEP.
But, I now believe we should have generated the GEP, and not synthesized the Expr.
There was a problem hiding this comment.
Sure, that would be fine. However, dealing with all of the forms that the expression can take is a nightmare that the ::Emit*Expr all ready deal with. I don't think it's as simple as "find the GEP of the FAM and then replace the indices with that of the counted_by field."
Or perhaps I'm wrong? @AaronBallman You'd know more about it than I would. Are the ::Emit*Expr methods going to generate more than a load and GEP for this?
|
For what it's worth, this resolves both the issues that I reported at #73168, at least when building the full two files that I noticed had a problem before. I can do more builds later if necessary. |
|
Closing in favor of #73730. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'll add some testcases later today.