[clang][bytecode] Fix checking for integer overflow #137962
Merged
Conversation
This file contains hidden or 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
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesWe need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases. Full diff: https://github.com/llvm/llvm-project/pull/137962.diff 5 Files Affected:
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
index 5c7482923386e..9e9dd5e87cd7a 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
@@ -60,6 +60,7 @@ class ByteCodeEmitter {
/// We're always emitting bytecode.
bool isActive() const { return true; }
+ bool checkingForUndefinedBehavior() const { return false; }
/// Callback for local registration.
Local createLocal(Descriptor *D);
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 9a1e61b54b8be..b4ac832946233 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -866,12 +866,14 @@ bool Compiler<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
// Assignments require us to evalute the RHS first.
if (BO->getOpcode() == BO_Assign) {
- // We don't support assignments in C.
- if (!Ctx.getLangOpts().CPlusPlus)
- return this->emitInvalid(BO);
if (!visit(RHS) || !visit(LHS))
return false;
+
+ // We don't support assignments in C.
+ if (!Ctx.getLangOpts().CPlusPlus && !this->emitInvalid(BO))
+ return false;
+
if (!this->emitFlip(*LT, *RT, BO))
return false;
} else {
@@ -2364,8 +2366,19 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
return false;
}
- if (!this->visitBool(Condition))
+ if (!this->visitBool(Condition)) {
+ // If the condition failed and we're checking for undefined behavior
+ // (which only happens with EvalEmitter) check the TrueExpr and FalseExpr
+ // as well.
+ if (this->checkingForUndefinedBehavior()) {
+ if (!this->discard(TrueExpr))
+ return false;
+ if (!this->discard(FalseExpr))
+ return false;
+ }
return false;
+ }
+
if (!this->jumpFalse(LabelFalse))
return false;
if (!visitChildExpr(TrueExpr))
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h b/clang/lib/AST/ByteCode/EvalEmitter.h
index f53f86c31ec1e..18adf8656c0d5 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.h
+++ b/clang/lib/AST/ByteCode/EvalEmitter.h
@@ -69,6 +69,9 @@ class EvalEmitter : public SourceMapper {
/// Since expressions can only jump forward, predicated execution is
/// used to deal with if-else statements.
bool isActive() const { return CurrentLabel == ActiveLabel; }
+ bool checkingForUndefinedBehavior() const {
+ return S.checkingForUndefinedBehavior();
+ }
/// Callback for registering a local.
Local createLocal(Descriptor *D);
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 4d89f23401db5..40a6965b426ed 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1336,7 +1336,7 @@ static bool getField(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
return false;
}
- if (Off > Ptr.block()->getSize())
+ if ((Ptr.getByteOffset() + Off) >= Ptr.block()->getSize())
return false;
S.Stk.push<Pointer>(Ptr.atField(Off));
diff --git a/clang/test/Sema/integer-overflow.c b/clang/test/Sema/integer-overflow.c
index 3141443c73305..30a47aa5f6ad6 100644
--- a/clang/test/Sema/integer-overflow.c
+++ b/clang/test/Sema/integer-overflow.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu -fexperimental-new-constant-interpreter
typedef unsigned long long uint64_t;
typedef unsigned int uint32_t;
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/8388 Here is the relevant piece of the build log for the reference |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.
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.
We need to evaluate both the True/False expressions of a conditional operator as well as the LHS/RHS of a binary operator in more cases.