Skip to content

Commit c51be1b

Browse files
authored
[clang][bytecode] Fix checking for integer overflow (#137962)
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.
1 parent d0a9727 commit c51be1b

File tree

5 files changed

+23
-5
lines changed

5 files changed

+23
-5
lines changed

clang/lib/AST/ByteCode/ByteCodeEmitter.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class ByteCodeEmitter {
6060

6161
/// We're always emitting bytecode.
6262
bool isActive() const { return true; }
63+
bool checkingForUndefinedBehavior() const { return false; }
6364

6465
/// Callback for local registration.
6566
Local createLocal(Descriptor *D);

clang/lib/AST/ByteCode/Compiler.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -866,12 +866,14 @@ bool Compiler<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
866866

867867
// Assignments require us to evalute the RHS first.
868868
if (BO->getOpcode() == BO_Assign) {
869-
// We don't support assignments in C.
870-
if (!Ctx.getLangOpts().CPlusPlus)
871-
return this->emitInvalid(BO);
872869

873870
if (!visit(RHS) || !visit(LHS))
874871
return false;
872+
873+
// We don't support assignments in C.
874+
if (!Ctx.getLangOpts().CPlusPlus && !this->emitInvalid(BO))
875+
return false;
876+
875877
if (!this->emitFlip(*LT, *RT, BO))
876878
return false;
877879
} else {
@@ -2367,8 +2369,19 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
23672369
return false;
23682370
}
23692371

2370-
if (!this->visitBool(Condition))
2372+
if (!this->visitBool(Condition)) {
2373+
// If the condition failed and we're checking for undefined behavior
2374+
// (which only happens with EvalEmitter) check the TrueExpr and FalseExpr
2375+
// as well.
2376+
if (this->checkingForUndefinedBehavior()) {
2377+
if (!this->discard(TrueExpr))
2378+
return false;
2379+
if (!this->discard(FalseExpr))
2380+
return false;
2381+
}
23712382
return false;
2383+
}
2384+
23722385
if (!this->jumpFalse(LabelFalse))
23732386
return false;
23742387
if (!visitChildExpr(TrueExpr))

clang/lib/AST/ByteCode/EvalEmitter.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ class EvalEmitter : public SourceMapper {
6969
/// Since expressions can only jump forward, predicated execution is
7070
/// used to deal with if-else statements.
7171
bool isActive() const { return CurrentLabel == ActiveLabel; }
72+
bool checkingForUndefinedBehavior() const {
73+
return S.checkingForUndefinedBehavior();
74+
}
7275

7376
/// Callback for registering a local.
7477
Local createLocal(Descriptor *D);

clang/lib/AST/ByteCode/Interp.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1336,7 +1336,7 @@ static bool getField(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
13361336
return false;
13371337
}
13381338

1339-
if (Off > Ptr.block()->getSize())
1339+
if ((Ptr.getByteOffset() + Off) >= Ptr.block()->getSize())
13401340
return false;
13411341

13421342
S.Stk.push<Pointer>(Ptr.atField(Off));

clang/test/Sema/integer-overflow.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu
2+
// RUN: %clang_cc1 %s -verify -fsyntax-only -triple x86_64-pc-linux-gnu -fexperimental-new-constant-interpreter
23
typedef unsigned long long uint64_t;
34
typedef unsigned int uint32_t;
45

0 commit comments

Comments
 (0)