[codegen] Fix crash in codegen caused by pointer calculation overflow #115791
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
Fixes llvm#48168 Under certain conditions, the front end does not detect a possible overflow address calculations until codegen. This change emits a warning instead of allowing the compiler to crash. clang: <root>/clang/lib/CodeGen/CGExprScalar.cpp:5834: llvm::Value* clang::CodeGen::CodeGenFunction::EmitCheckedInBoundsGEP( llvm::Type*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, bool, bool, clang::SourceLocation, const llvm::Twine&): Assertion `(!isa<llvm::Constant>(EvaluatedGEP.TotalOffset) || EvaluatedGEP.OffsetOverflows == Builder.getFalse()) && "If the offset got constant-folded, we don't expect that there was an " "overflow."' failed. 0. Program arguments: clang -c --target=x86_64-- -fsanitize=undefined ubsan-emit-bounds-check-crash-x86.c 1. <eof> parser at end of file 2. ubsan-emit-bounds-check-crash-x86.c:4:5: LLVM IR generation of declaration 'main' 3. ubsan-emit-bounds-check-crash-x86.c:4:5: Generating code for declaration 'main' ... llvm#9 <addr> clang::CodeGen::CodeGenFunction::EmitCheckedInBoundsGEP(clang::CodeGen::Address, llvm::ArrayRef<llvm::Value*>, llvm::Type*, bool, bool, clang::SourceLocation, clang::CharUnits, llvm::Twine const&) llvm::ArrayRef<llvm::Value*>, clang::QualType, bool, bool, clang::SourceLocation, clang::QualType*, clang::Expr const*, llvm::Twine const&) CGExpr.cpp:0:0 bool)
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (vabridgers) ChangesFixes #48168 Under certain conditions, the front end does not detect a possible overflow address calculations until codegen. This change emits a warning instead of allowing the compiler to crash. Full diff: https://github.com/llvm/llvm-project/pull/115791.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 287d911e10ba58..5ed865820a9151 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -5831,10 +5831,15 @@ CodeGenFunction::EmitCheckedInBoundsGEP(llvm::Type *ElemTy, Value *Ptr,
GEPOffsetAndOverflow EvaluatedGEP =
EmitGEPOffsetInBytes(Ptr, GEPVal, getLLVMContext(), CGM, Builder);
- assert((!isa<llvm::Constant>(EvaluatedGEP.TotalOffset) ||
- EvaluatedGEP.OffsetOverflows == Builder.getFalse()) &&
- "If the offset got constant-folded, we don't expect that there was an "
- "overflow.");
+ if (!(!isa<llvm::Constant>(EvaluatedGEP.TotalOffset) ||
+ EvaluatedGEP.OffsetOverflows == Builder.getFalse())) {
+ DiagnosticsEngine &Diags = CGM.getDiags();
+ unsigned DiagID = Diags.getCustomDiagID(
+ DiagnosticsEngine::Error, "Expression caused pointer calculation "
+ "overflow during code generation");
+ Diags.Report(Loc, DiagID);
+ return GEPVal;
+ }
auto *Zero = llvm::ConstantInt::getNullValue(IntPtrTy);
diff --git a/clang/test/CodeGen/ubsan-emit-bounds-check-crash-msp.c b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-msp.c
new file mode 100644
index 00000000000000..b4da2f2b7ee720
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-msp.c
@@ -0,0 +1,7 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang -c -fsanitize=undefined -Wno-tentative-definition-array -Wno-return-type -Wno-unused-value -Wno-array-bounds -Xclang -verify --target=msp430-- %s
+int a;
+_Complex double b[1][1];
+void c(void) {
+ b[a][8920]; // expected-error {{Expression caused pointer calculation overflow during code generation}}
+}
diff --git a/clang/test/CodeGen/ubsan-emit-bounds-check-crash-x86.c b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-x86.c
new file mode 100644
index 00000000000000..63bda82c14ae20
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-x86.c
@@ -0,0 +1,7 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -c -Wno-tentative-definition-array -Wno-return-type -Wno-unused-value -Wno-array-bounds -Xclang -verify --target=x86_64-- -fsanitize=undefined %s
+int **a[];
+int main() {
+ (*a)[3300220222222200000]; // expected-error {{Expression caused pointer calculation overflow during code generation}}
+ return 0;
+}
|
|
@llvm/pr-subscribers-clang-codegen Author: None (vabridgers) ChangesFixes #48168 Under certain conditions, the front end does not detect a possible overflow address calculations until codegen. This change emits a warning instead of allowing the compiler to crash. Full diff: https://github.com/llvm/llvm-project/pull/115791.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 287d911e10ba58..5ed865820a9151 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -5831,10 +5831,15 @@ CodeGenFunction::EmitCheckedInBoundsGEP(llvm::Type *ElemTy, Value *Ptr,
GEPOffsetAndOverflow EvaluatedGEP =
EmitGEPOffsetInBytes(Ptr, GEPVal, getLLVMContext(), CGM, Builder);
- assert((!isa<llvm::Constant>(EvaluatedGEP.TotalOffset) ||
- EvaluatedGEP.OffsetOverflows == Builder.getFalse()) &&
- "If the offset got constant-folded, we don't expect that there was an "
- "overflow.");
+ if (!(!isa<llvm::Constant>(EvaluatedGEP.TotalOffset) ||
+ EvaluatedGEP.OffsetOverflows == Builder.getFalse())) {
+ DiagnosticsEngine &Diags = CGM.getDiags();
+ unsigned DiagID = Diags.getCustomDiagID(
+ DiagnosticsEngine::Error, "Expression caused pointer calculation "
+ "overflow during code generation");
+ Diags.Report(Loc, DiagID);
+ return GEPVal;
+ }
auto *Zero = llvm::ConstantInt::getNullValue(IntPtrTy);
diff --git a/clang/test/CodeGen/ubsan-emit-bounds-check-crash-msp.c b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-msp.c
new file mode 100644
index 00000000000000..b4da2f2b7ee720
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-msp.c
@@ -0,0 +1,7 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang -c -fsanitize=undefined -Wno-tentative-definition-array -Wno-return-type -Wno-unused-value -Wno-array-bounds -Xclang -verify --target=msp430-- %s
+int a;
+_Complex double b[1][1];
+void c(void) {
+ b[a][8920]; // expected-error {{Expression caused pointer calculation overflow during code generation}}
+}
diff --git a/clang/test/CodeGen/ubsan-emit-bounds-check-crash-x86.c b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-x86.c
new file mode 100644
index 00000000000000..63bda82c14ae20
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-emit-bounds-check-crash-x86.c
@@ -0,0 +1,7 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -c -Wno-tentative-definition-array -Wno-return-type -Wno-unused-value -Wno-array-bounds -Xclang -verify --target=x86_64-- -fsanitize=undefined %s
+int **a[];
+int main() {
+ (*a)[3300220222222200000]; // expected-error {{Expression caused pointer calculation overflow during code generation}}
+ return 0;
+}
|
|
I know the diagnostic could probably be better, just trying to get the code review party started. This is not ideal, but better than a crash. Thanks. |
vabridgers
changed the title
There was a problem hiding this comment.
CodeGen should not produce errors for overflows which do not occur at compile-time according to the language semantics. This is happening at runtime, so we need to just eat the error; we can emit an unconditional call to the ubsan error handler. We are not allowed to reject valid programs.
Ideally Sema should produce a warning so users aren't caught off-guard.
|
@efriedma-quic , thanks for the comments. I'll explore a more appropriate solution. |
|
Closing this review since this approach is not appropriate. |
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.
Fixes #48168
This crash was found by internal randomized testing. Under certain conditions, the front end does not detect a possible overflow address calculations until codegen. This change emits a warning instead of allowing the compiler to crash.