[clang] [clang][bytecode] Fix checking for integer overflow (PR #137962)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/137962.diff


5 Files Affected:

- (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.h (+1) 
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+17-4) 
- (modified) clang/lib/AST/ByteCode/EvalEmitter.h (+3) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+1-1) 
- (modified) clang/test/Sema/integer-overflow.c (+1) 


``````````diff
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;
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/137962