From e2a43eeed6f30fcfcd01dfaee33bd3108f894847 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 5 Jul 2019 13:18:02 +0200 Subject: [PATCH 1/5] C++ IR: Tests with ErrorExpr --- .../library-tests/ir/ir/PrintAST.expected | 25 +++++++++++++++++++ .../ir/ir/aliased_ssa_sanity.expected | 1 + cpp/ql/test/library-tests/ir/ir/bad_asts.cpp | 8 +++++- .../test/library-tests/ir/ir/raw_ir.expected | 22 ++++++++++++++++ .../library-tests/ir/ir/raw_sanity.expected | 6 +++++ .../ir/ir/unaliased_ssa_sanity.expected | 1 + 6 files changed, 62 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected index ba49596f21f..b9ded77dbf2 100644 --- a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected +++ b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected @@ -190,6 +190,31 @@ bad_asts.cpp: # 27| Type = const Point & # 27| ValueCategory = prvalue(load) # 28| 1: return ... +# 30| void Bad::errorExpr() +# 30| params: +# 30| body: { ... } +# 31| 0: ExprStmt +#-----| 0: +#-----| Type = error +#-----| ValueCategory = prvalue(load) +# 32| 1: declaration +# 32| 0: definition of x +# 32| Type = int +# 32| init: initializer for x +# 32| expr: +# 32| Type = error +# 32| ValueCategory = prvalue +# 33| 2: ExprStmt +# 33| 0: ... = ... +# 33| Type = int +# 33| ValueCategory = lvalue +# 33| 0: x +# 33| Type = int +# 33| ValueCategory = lvalue +#-----| 1: +#-----| Type = error +#-----| ValueCategory = prvalue(load) +# 34| 3: return ... clang.cpp: # 5| int* globalIntAddress() # 5| params: diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected index 40bdb1e1909..8a0a1bd4777 100644 --- a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected +++ b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected @@ -4,6 +4,7 @@ duplicateOperand missingPhiOperand missingOperandType instructionWithoutSuccessor +| bad_asts.cpp:30:8:30:16 | UnmodeledDefinition: errorExpr | ambiguousSuccessors unexplainedLoop unnecessaryPhiInstruction diff --git a/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp b/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp index 6bd726959c7..d2920055506 100644 --- a/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp +++ b/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp @@ -1,4 +1,4 @@ -// semmle-extractor-options: -std=c++17 +// semmle-extractor-options: -std=c++17 --expect_errors // Test cases that illustrate known bad ASTs that we have to work around in IR generation. namespace Bad { @@ -26,4 +26,10 @@ namespace Bad { void CallCopyConstructor(const Point& a) { Point b = a; // Copy constructor contains literal expressions with no values. } + + void errorExpr() { + 0 = 1; + int x = 0[0]; + x = 1[1]; + } } diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected index a412d906023..0ab4cdd4de9 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected @@ -48,6 +48,28 @@ bad_asts.cpp: # 26| v0_13(void) = UnmodeledUse : mu* # 26| v0_14(void) = ExitFunction : +# 30| void Bad::errorExpr() +# 30| Block 0 +# 30| v0_0(void) = EnterFunction : +# 30| mu0_1(unknown) = AliasedDefinition : +# 30| mu0_2(unknown) = UnmodeledDefinition : + +#-----| Block 1 +#-----| r1_0(error) = Load : ~mu0_2 +# 33| r1_1(glval) = VariableAddress[x] : +# 33| mu1_2(int) = Store : &:r1_1, r1_0 +# 34| v1_3(void) = NoOp : +# 30| v1_4(void) = ReturnVoid : +# 30| v1_5(void) = UnmodeledUse : mu* +# 30| v1_6(void) = ExitFunction : + +#-----| Block 2 +#-----| r2_0(error) = Load : ~mu0_2 +# 32| r2_1(glval) = VariableAddress[x] : + +# 32| Block 3 +# 32| mu3_0(int) = Store : &:r2_1 + clang.cpp: # 5| int* globalIntAddress() # 5| Block 0 diff --git a/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected b/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected index 40bdb1e1909..faa6c2b5afb 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected @@ -1,9 +1,15 @@ missingOperand +| bad_asts.cpp:32:12:32:16 | Store: | Instruction 'Store' is missing an expected operand with tag 'StoreValue' in function '$@'. | bad_asts.cpp:30:8:30:16 | IR: errorExpr | void Bad::errorExpr() | +| file://:0:0:0:0 | Load: | Instruction 'Load' is missing an expected operand with tag 'Address' in function '$@'. | bad_asts.cpp:30:8:30:16 | IR: errorExpr | void Bad::errorExpr() | +| file://:0:0:0:0 | Load: | Instruction 'Load' is missing an expected operand with tag 'Address' in function '$@'. | bad_asts.cpp:30:8:30:16 | IR: errorExpr | void Bad::errorExpr() | unexpectedOperand duplicateOperand missingPhiOperand missingOperandType instructionWithoutSuccessor +| bad_asts.cpp:30:8:30:16 | UnmodeledDefinition: errorExpr | +| bad_asts.cpp:32:9:32:9 | VariableAddress: definition of x | +| bad_asts.cpp:32:12:32:16 | Store: | ambiguousSuccessors unexplainedLoop unnecessaryPhiInstruction diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected index 40bdb1e1909..8a0a1bd4777 100644 --- a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected +++ b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected @@ -4,6 +4,7 @@ duplicateOperand missingPhiOperand missingOperandType instructionWithoutSuccessor +| bad_asts.cpp:30:8:30:16 | UnmodeledDefinition: errorExpr | ambiguousSuccessors unexplainedLoop unnecessaryPhiInstruction From a86ddd50decb665243ba20ea83c3c69180cf759a Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 5 Jul 2019 13:45:14 +0200 Subject: [PATCH 2/5] C++ IR: Translate ErrorExpr to NoOp --- .../raw/internal/TranslatedExpr.qll | 33 +++++++++++++++++ .../ir/ir/aliased_ssa_sanity.expected | 1 - .../test/library-tests/ir/ir/raw_ir.expected | 35 +++++++++---------- .../library-tests/ir/ir/raw_sanity.expected | 6 ---- .../ir/ir/unaliased_ssa_sanity.expected | 1 - 5 files changed, 49 insertions(+), 27 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll index d1f85012d14..80f00a67268 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll @@ -2862,3 +2862,36 @@ class TranslatedStmtExpr extends TranslatedNonConstantExpr { result = getTranslatedStmt(expr.getStmt()) } } + +class TranslatedErrorExpr extends TranslatedSingleInstructionExpr { + override ErrorExpr expr; + + TranslatedErrorExpr() { + // The extractor deliberately emits an `ErrorExpr` as the first argument to + // the allocator call, if any, of a `NewOrNewArrayExpr`. That `ErrorExpr` + // should not be translated. + not exists(NewOrNewArrayExpr new | expr = new.getAllocatorCall().getArgument(0)) + } + + override final Instruction getFirstInstruction() { + result = getInstruction(OnlyInstructionTag()) + } + + override final TranslatedElement getChild(int id) { none() } + + override final Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { + tag = OnlyInstructionTag() and + result = getParent().getChildSuccessor(this) and + kind instanceof GotoEdge + } + + override final Instruction getChildSuccessor(TranslatedElement child) { none() } + + override final Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) { + none() + } + + override final Opcode getOpcode() { + result instanceof Opcode::NoOp + } +} diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected index 8a0a1bd4777..40bdb1e1909 100644 --- a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected +++ b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected @@ -4,7 +4,6 @@ duplicateOperand missingPhiOperand missingOperandType instructionWithoutSuccessor -| bad_asts.cpp:30:8:30:16 | UnmodeledDefinition: errorExpr | ambiguousSuccessors unexplainedLoop unnecessaryPhiInstruction diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected index 0ab4cdd4de9..4619b9bb9fa 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected @@ -50,25 +50,22 @@ bad_asts.cpp: # 30| void Bad::errorExpr() # 30| Block 0 -# 30| v0_0(void) = EnterFunction : -# 30| mu0_1(unknown) = AliasedDefinition : -# 30| mu0_2(unknown) = UnmodeledDefinition : - -#-----| Block 1 -#-----| r1_0(error) = Load : ~mu0_2 -# 33| r1_1(glval) = VariableAddress[x] : -# 33| mu1_2(int) = Store : &:r1_1, r1_0 -# 34| v1_3(void) = NoOp : -# 30| v1_4(void) = ReturnVoid : -# 30| v1_5(void) = UnmodeledUse : mu* -# 30| v1_6(void) = ExitFunction : - -#-----| Block 2 -#-----| r2_0(error) = Load : ~mu0_2 -# 32| r2_1(glval) = VariableAddress[x] : - -# 32| Block 3 -# 32| mu3_0(int) = Store : &:r2_1 +# 30| v0_0(void) = EnterFunction : +# 30| mu0_1(unknown) = AliasedDefinition : +# 30| mu0_2(unknown) = UnmodeledDefinition : +#-----| r0_3(glval) = NoOp : +#-----| r0_4(error) = Load : &:r0_3, ~mu0_2 +# 32| r0_5(glval) = VariableAddress[x] : +# 32| r0_6(error) = NoOp : +# 32| mu0_7(int) = Store : &:r0_5, r0_6 +#-----| r0_8(glval) = NoOp : +#-----| r0_9(error) = Load : &:r0_8, ~mu0_2 +# 33| r0_10(glval) = VariableAddress[x] : +# 33| mu0_11(int) = Store : &:r0_10, r0_9 +# 34| v0_12(void) = NoOp : +# 30| v0_13(void) = ReturnVoid : +# 30| v0_14(void) = UnmodeledUse : mu* +# 30| v0_15(void) = ExitFunction : clang.cpp: # 5| int* globalIntAddress() diff --git a/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected b/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected index faa6c2b5afb..40bdb1e1909 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_sanity.expected @@ -1,15 +1,9 @@ missingOperand -| bad_asts.cpp:32:12:32:16 | Store: | Instruction 'Store' is missing an expected operand with tag 'StoreValue' in function '$@'. | bad_asts.cpp:30:8:30:16 | IR: errorExpr | void Bad::errorExpr() | -| file://:0:0:0:0 | Load: | Instruction 'Load' is missing an expected operand with tag 'Address' in function '$@'. | bad_asts.cpp:30:8:30:16 | IR: errorExpr | void Bad::errorExpr() | -| file://:0:0:0:0 | Load: | Instruction 'Load' is missing an expected operand with tag 'Address' in function '$@'. | bad_asts.cpp:30:8:30:16 | IR: errorExpr | void Bad::errorExpr() | unexpectedOperand duplicateOperand missingPhiOperand missingOperandType instructionWithoutSuccessor -| bad_asts.cpp:30:8:30:16 | UnmodeledDefinition: errorExpr | -| bad_asts.cpp:32:9:32:9 | VariableAddress: definition of x | -| bad_asts.cpp:32:12:32:16 | Store: | ambiguousSuccessors unexplainedLoop unnecessaryPhiInstruction diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected index 8a0a1bd4777..40bdb1e1909 100644 --- a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected +++ b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected @@ -4,7 +4,6 @@ duplicateOperand missingPhiOperand missingOperandType instructionWithoutSuccessor -| bad_asts.cpp:30:8:30:16 | UnmodeledDefinition: errorExpr | ambiguousSuccessors unexplainedLoop unnecessaryPhiInstruction From 4324c97d398a783a639ac3a1d09b1d58db2ce35c Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 9 Jul 2019 13:14:45 +0200 Subject: [PATCH 3/5] C++: Use Opcode::Error for ErrorExpr translation --- .../semmle/code/cpp/ir/implementation/Opcode.qll | 2 ++ .../implementation/aliased_ssa/Instruction.qll | 16 ++++++++++++++++ .../cpp/ir/implementation/raw/Instruction.qll | 16 ++++++++++++++++ .../raw/internal/TranslatedExpr.qll | 2 +- .../implementation/unaliased_ssa/Instruction.qll | 16 ++++++++++++++++ cpp/ql/test/library-tests/ir/ir/raw_ir.expected | 6 +++--- 6 files changed, 54 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll index cf7cacdddd3..41ae2c07e21 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll @@ -1,6 +1,7 @@ private newtype TOpcode = TNoOp() or TUninitialized() or + TError() or TInitializeParameter() or TInitializeThis() or TEnterFunction() or @@ -147,6 +148,7 @@ abstract class BufferAccessOpcode extends MemoryAccessOpcode {} module Opcode { class NoOp extends Opcode, TNoOp { override final string toString() { result = "NoOp" } } class Uninitialized extends MemoryAccessOpcode, TUninitialized { override final string toString() { result = "Uninitialized" } } + class Error extends Opcode, TError { override final string toString() { result = "Error" } } class InitializeParameter extends MemoryAccessOpcode, TInitializeParameter { override final string toString() { result = "InitializeParameter" } } class InitializeThis extends Opcode, TInitializeThis { override final string toString() { result = "InitializeThis" } } class EnterFunction extends Opcode, TEnterFunction { override final string toString() { result = "EnterFunction" } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index 7ae52934a30..1492ca94671 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -702,6 +702,22 @@ class FieldAddressInstruction extends FieldInstruction { } } +/** + * An instruction that produces a well-defined but unknown result and has + * unknown side effects, including side effects that are not conservatively + * modeled in the SSA graph. + * + * This type of instruction appears when there is an `ErrorExpr` in the AST, + * meaning that the extractor could not understand the expression and therefore + * produced a partial AST. Queries that give alerts when some action is _not_ + * taken may want to ignore any function that contains an `ErrorInstruction`. + */ +class ErrorInstruction extends Instruction { + ErrorInstruction() { + getOpcode() instanceof Opcode::Error + } +} + class UninitializedInstruction extends VariableInstruction { UninitializedInstruction() { getOpcode() instanceof Opcode::Uninitialized diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index 7ae52934a30..1492ca94671 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -702,6 +702,22 @@ class FieldAddressInstruction extends FieldInstruction { } } +/** + * An instruction that produces a well-defined but unknown result and has + * unknown side effects, including side effects that are not conservatively + * modeled in the SSA graph. + * + * This type of instruction appears when there is an `ErrorExpr` in the AST, + * meaning that the extractor could not understand the expression and therefore + * produced a partial AST. Queries that give alerts when some action is _not_ + * taken may want to ignore any function that contains an `ErrorInstruction`. + */ +class ErrorInstruction extends Instruction { + ErrorInstruction() { + getOpcode() instanceof Opcode::Error + } +} + class UninitializedInstruction extends VariableInstruction { UninitializedInstruction() { getOpcode() instanceof Opcode::Uninitialized diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll index 80f00a67268..397b9fa04e0 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll @@ -2892,6 +2892,6 @@ class TranslatedErrorExpr extends TranslatedSingleInstructionExpr { } override final Opcode getOpcode() { - result instanceof Opcode::NoOp + result instanceof Opcode::Error } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index 7ae52934a30..1492ca94671 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -702,6 +702,22 @@ class FieldAddressInstruction extends FieldInstruction { } } +/** + * An instruction that produces a well-defined but unknown result and has + * unknown side effects, including side effects that are not conservatively + * modeled in the SSA graph. + * + * This type of instruction appears when there is an `ErrorExpr` in the AST, + * meaning that the extractor could not understand the expression and therefore + * produced a partial AST. Queries that give alerts when some action is _not_ + * taken may want to ignore any function that contains an `ErrorInstruction`. + */ +class ErrorInstruction extends Instruction { + ErrorInstruction() { + getOpcode() instanceof Opcode::Error + } +} + class UninitializedInstruction extends VariableInstruction { UninitializedInstruction() { getOpcode() instanceof Opcode::Uninitialized diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected index 4619b9bb9fa..d9b07b09635 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected @@ -53,12 +53,12 @@ bad_asts.cpp: # 30| v0_0(void) = EnterFunction : # 30| mu0_1(unknown) = AliasedDefinition : # 30| mu0_2(unknown) = UnmodeledDefinition : -#-----| r0_3(glval) = NoOp : +#-----| r0_3(glval) = Error : #-----| r0_4(error) = Load : &:r0_3, ~mu0_2 # 32| r0_5(glval) = VariableAddress[x] : -# 32| r0_6(error) = NoOp : +# 32| r0_6(error) = Error : # 32| mu0_7(int) = Store : &:r0_5, r0_6 -#-----| r0_8(glval) = NoOp : +#-----| r0_8(glval) = Error : #-----| r0_9(error) = Load : &:r0_8, ~mu0_2 # 33| r0_10(glval) = VariableAddress[x] : # 33| mu0_11(int) = Store : &:r0_10, r0_9 From 0889d5d27a22df56e4fddde0a54e2ce64bc6d580 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 9 Jul 2019 13:35:20 +0200 Subject: [PATCH 4/5] C++ IR: Improve ErrorExpr test The previous version of the test used `0 = 1;` to test an lvalue-typed `ErrorExpr`, but the extractor replaced the whole assignment expression with `ErrorExpr` instead of just the LHS. This variation of the test only leads to an `ErrorExpr` for the part of the syntax that's supposed to be an lvalue-typed expression, so that's an improvement. Unfortunately it still doesn't demonstrate that we can `Store` into an address computed by an `ErrorExpr`. --- .../library-tests/ir/ir/PrintAST.expected | 11 ++++--- cpp/ql/test/library-tests/ir/ir/bad_asts.cpp | 2 +- .../test/library-tests/ir/ir/raw_ir.expected | 33 ++++++++++--------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected index b9ded77dbf2..fb90a724eba 100644 --- a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected +++ b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected @@ -193,10 +193,13 @@ bad_asts.cpp: # 30| void Bad::errorExpr() # 30| params: # 30| body: { ... } -# 31| 0: ExprStmt -#-----| 0: -#-----| Type = error -#-----| ValueCategory = prvalue(load) +# 31| 0: declaration +# 31| 0: definition of intref +# 31| Type = int & +# 31| init: initializer for intref +# 31| expr: +# 31| Type = error +# 31| ValueCategory = prvalue # 32| 1: declaration # 32| 0: definition of x # 32| Type = int diff --git a/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp b/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp index d2920055506..dbb63b2c91a 100644 --- a/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp +++ b/cpp/ql/test/library-tests/ir/ir/bad_asts.cpp @@ -28,7 +28,7 @@ namespace Bad { } void errorExpr() { - 0 = 1; + int &intref = 0; int x = 0[0]; x = 1[1]; } diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected index d9b07b09635..c5ec1625798 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected @@ -50,22 +50,23 @@ bad_asts.cpp: # 30| void Bad::errorExpr() # 30| Block 0 -# 30| v0_0(void) = EnterFunction : -# 30| mu0_1(unknown) = AliasedDefinition : -# 30| mu0_2(unknown) = UnmodeledDefinition : -#-----| r0_3(glval) = Error : -#-----| r0_4(error) = Load : &:r0_3, ~mu0_2 -# 32| r0_5(glval) = VariableAddress[x] : -# 32| r0_6(error) = Error : -# 32| mu0_7(int) = Store : &:r0_5, r0_6 -#-----| r0_8(glval) = Error : -#-----| r0_9(error) = Load : &:r0_8, ~mu0_2 -# 33| r0_10(glval) = VariableAddress[x] : -# 33| mu0_11(int) = Store : &:r0_10, r0_9 -# 34| v0_12(void) = NoOp : -# 30| v0_13(void) = ReturnVoid : -# 30| v0_14(void) = UnmodeledUse : mu* -# 30| v0_15(void) = ExitFunction : +# 30| v0_0(void) = EnterFunction : +# 30| mu0_1(unknown) = AliasedDefinition : +# 30| mu0_2(unknown) = UnmodeledDefinition : +# 31| r0_3(glval) = VariableAddress[intref] : +# 31| r0_4(error) = Error : +# 31| mu0_5(int &) = Store : &:r0_3, r0_4 +# 32| r0_6(glval) = VariableAddress[x] : +# 32| r0_7(error) = Error : +# 32| mu0_8(int) = Store : &:r0_6, r0_7 +#-----| r0_9(glval) = Error : +#-----| r0_10(error) = Load : &:r0_9, ~mu0_2 +# 33| r0_11(glval) = VariableAddress[x] : +# 33| mu0_12(int) = Store : &:r0_11, r0_10 +# 34| v0_13(void) = NoOp : +# 30| v0_14(void) = ReturnVoid : +# 30| v0_15(void) = UnmodeledUse : mu* +# 30| v0_16(void) = ExitFunction : clang.cpp: # 5| int* globalIntAddress() From 70f81badcb8dfb8ac1e753ca8b64ea625ae085d5 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 10 Jul 2019 14:20:09 +0200 Subject: [PATCH 5/5] C++ IR: Move ErrorExpr filter to TranslatedElement The convention in the IR translation is to handle all ignored expressions in this central place. --- .../ir/implementation/raw/internal/TranslatedElement.qll | 8 +++++++- .../cpp/ir/implementation/raw/internal/TranslatedExpr.qll | 7 ------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll index 66d4dca4aba..199690f2761 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll @@ -84,7 +84,13 @@ private predicate ignoreExprOnly(Expr expr) { // Ignore the allocator call, because we always synthesize it. Don't ignore // its arguments, though, because we use them as part of the synthesis. newExpr.getAllocatorCall() = expr - ) or + ) + or + // The extractor deliberately emits an `ErrorExpr` as the first argument to + // the allocator call, if any, of a `NewOrNewArrayExpr`. That `ErrorExpr` + // should not be translated. + exists(NewOrNewArrayExpr new | expr = new.getAllocatorCall().getArgument(0)) + or not translateFunction(expr.getEnclosingFunction()) or // We do not yet translate destructors properly, so for now we ignore the diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll index 397b9fa04e0..73c0b8637af 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll @@ -2866,13 +2866,6 @@ class TranslatedStmtExpr extends TranslatedNonConstantExpr { class TranslatedErrorExpr extends TranslatedSingleInstructionExpr { override ErrorExpr expr; - TranslatedErrorExpr() { - // The extractor deliberately emits an `ErrorExpr` as the first argument to - // the allocator call, if any, of a `NewOrNewArrayExpr`. That `ErrorExpr` - // should not be translated. - not exists(NewOrNewArrayExpr new | expr = new.getAllocatorCall().getArgument(0)) - } - override final Instruction getFirstInstruction() { result = getInstruction(OnlyInstructionTag()) }