Skip to content

Commit

Permalink
Merge pull request #17421 from jketema/fix-except-inconsistency
Browse files Browse the repository at this point in the history 
C++: Fix IR inconsistency due to throwing `__except` block
  • Loading branch information
@jketema
jketema authored Sep 10, 2024
2 parents ba5218d + 5f4fee0 commit 4c8aec0
Show file tree
Hide file tree
Showing 10 changed files with 311 additions and 333 deletions.
7 changes: 6 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,13 @@ class TranslatedFunction extends TranslatedRootElement, TTranslatedFunction {
(
// Only generate the `Unwind` instruction if there is any exception
// handling present in the function.
exists(TryStmt try | try.getEnclosingFunction() = func) or
exists(TryOrMicrosoftTryStmt try | try.getEnclosingFunction() = func)
or
exists(ThrowExpr throw | throw.getEnclosingFunction() = func)
or
exists(FunctionCall call | call.getEnclosingFunction() = func |
getTranslatedExpr(call).(TranslatedCallExpr).mayThrowException()
)
)
or
tag = AliasedUseTag() and
Expand Down
23 changes: 9 additions & 14 deletions cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
tag = TryExceptCompareOneBranch() and
opcode instanceof Opcode::ConditionalBranch and
resultType = getVoidType()
or
// unwind stack
tag = UnwindTag() and
opcode instanceof Opcode::Unwind and
resultType = getVoidType()
}

final override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
Expand Down Expand Up @@ -156,7 +151,7 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
// TODO: This is not really correct. The semantics of `EXCEPTION_CONTINUE_EXECUTION` is that
// we should continue execution at the point where the exception occurred. But we don't have
// any instruction to model this behavior.
result = this.getInstruction(UnwindTag())
result = this.getExceptionSuccessorInstruction(any(GotoEdge edge))
or
kind instanceof FalseEdge and
result = this.getInstruction(TryExceptGenerateZero())
Expand All @@ -176,7 +171,7 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
tag = TryExceptCompareZeroBranch() and
(
kind instanceof TrueEdge and
result = this.getInstruction(UnwindTag())
result = this.getExceptionSuccessorInstruction(any(GotoEdge edge))
or
kind instanceof FalseEdge and
result = this.getInstruction(TryExceptGenerateOne())
Expand All @@ -196,10 +191,6 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
tag = TryExceptCompareOneBranch() and
kind instanceof TrueEdge and
result = this.getTranslatedHandler().getFirstInstruction(any(GotoEdge edge))
or
// Unwind -> Parent
tag = UnwindTag() and
result = this.getParent().getChildSuccessor(this, kind)
}

override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
Expand All @@ -215,8 +206,6 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,

override Instruction getALastInstructionInternal() {
result = this.getTranslatedHandler().getALastInstruction()
or
result = this.getInstruction(UnwindTag())
}

private TranslatedExpr getTranslatedCondition() {
Expand All @@ -236,6 +225,12 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
}

final override Function getFunction() { result = tryExcept.getEnclosingFunction() }

override Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
// A throw from within a `__except` block flows to the handler for the parent of
// the `__try`.
result = this.getParent().getParent().getExceptionSuccessorInstruction(kind)
}
}

abstract class TranslatedStmt extends TranslatedElement, TTranslatedStmt {
Expand Down Expand Up @@ -583,7 +578,7 @@ class TranslatedNoValueReturnStmt extends TranslatedReturnStmt, TranslatedVariab
/**
* A C/C++ `try` statement, or a `__try __except` or `__try __finally` statement.
*/
private class TryOrMicrosoftTryStmt extends Stmt {
class TryOrMicrosoftTryStmt extends Stmt {
TryOrMicrosoftTryStmt() {
this instanceof TryStmt or
this instanceof MicrosoftTryStmt
Expand Down
56 changes: 33 additions & 23 deletions cpp/ql/test/library-tests/ir/ir/aliased_ir.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3167,41 +3167,45 @@ ir.c:
# 36| v36_3(void) = Call[ExRaiseAccessViolation] : func:r36_1, 0:r36_2
# 36| m36_4(unknown) = ^CallSideEffect : ~m32_4
# 36| m36_5(unknown) = Chi : total:m32_4, partial:m36_4
#-----| Exception -> Block 3
#-----| Exception -> Block 4

# 39| Block 1
# 32| Block 1
# 32| v32_5(void) = Unwind :
# 32| v32_6(void) = AliasedUse : ~m40_5
# 32| v32_7(void) = ExitFunction :

# 39| Block 2
# 39| r39_1(int) = Constant[0] :
# 39| r39_2(bool) = CompareEQ : r38_2, r39_1
# 39| r39_2(bool) = CompareEQ : r38_1, r39_1
# 39| v39_3(void) = ConditionalBranch : r39_2
#-----| False (back edge) -> Block 2
#-----| True -> Block 5
#-----| False -> Block 3
#-----| True -> Block 6

# 39| Block 2
# 39| Block 3
# 39| r39_4(int) = Constant[1] :
# 39| r39_5(bool) = CompareEQ : r38_2, r39_4
# 39| r39_5(bool) = CompareEQ : r38_1, r39_4
# 39| v39_6(void) = ConditionalBranch : r39_5
#-----| False -> Block 5
#-----| True (back edge) -> Block 4

# 38| Block 3
# 38| m38_1(unknown) = Phi : from 0:~m36_5, from 4:~m40_5
# 38| r38_2(int) = Constant[1] :
# 39| r39_7(int) = Constant[-1] :
# 39| r39_8(bool) = CompareEQ : r38_2, r39_7
# 39| v39_9(void) = ConditionalBranch : r39_8
#-----| False (back edge) -> Block 1
#-----| False -> Block 6
#-----| True -> Block 5

# 40| Block 4
# 38| Block 4
# 38| r38_1(int) = Constant[1] :
# 39| r39_7(int) = Constant[-1] :
# 39| r39_8(bool) = CompareEQ : r38_1, r39_7
# 39| v39_9(void) = ConditionalBranch : r39_8
#-----| False -> Block 2
#-----| True -> Block 6

# 40| Block 5
# 40| r40_1(glval<unknown>) = FunctionAddress[ExRaiseAccessViolation] :
# 40| r40_2(int) = Constant[1] :
# 40| v40_3(void) = Call[ExRaiseAccessViolation] : func:r40_1, 0:r40_2
# 40| m40_4(unknown) = ^CallSideEffect : ~m38_1
# 40| m40_5(unknown) = Chi : total:m38_1, partial:m40_4
#-----| Exception (back edge) -> Block 3
# 40| m40_4(unknown) = ^CallSideEffect : ~m36_5
# 40| m40_5(unknown) = Chi : total:m36_5, partial:m40_4
#-----| Exception -> Block 1

# 32| Block 5
# 32| v32_5(void) = Unreached :
# 32| Block 6
# 32| v32_8(void) = Unreached :

# 44| void try_with_finally()
# 44| Block 0
Expand Down Expand Up @@ -3261,6 +3265,12 @@ ir.c:
# 81| v81_3(void) = Call[ExRaiseAccessViolation] : func:r81_1, 0:r81_2
# 81| m81_4(unknown) = ^CallSideEffect : ~m80_4
# 81| m81_5(unknown) = Chi : total:m80_4, partial:m81_4
#-----| Exception -> Block 1

# 80| Block 1
# 80| v80_5(void) = Unwind :
# 80| v80_6(void) = AliasedUse : ~m81_5
# 80| v80_7(void) = ExitFunction :

ir.cpp:
# 1| void Constants()
Expand Down
20 changes: 0 additions & 20 deletions cpp/ql/test/library-tests/ir/ir/aliased_ssa_consistency.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,8 @@ sideEffectWithoutPrimary
instructionWithoutSuccessor
| ir.c:62:5:62:26 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' has no successors in function '$@'. | ir.c:57:6:57:30 | void throw_in_try_with_finally() | void throw_in_try_with_finally() |
| ir.c:73:5:73:26 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' has no successors in function '$@'. | ir.c:70:6:70:39 | void throw_in_try_with_throw_in_finally() | void throw_in_try_with_throw_in_finally() |
| ir.c:81:3:81:24 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' has no successors in function '$@'. | ir.c:80:6:80:27 | void raise_access_violation() | void raise_access_violation() |
ambiguousSuccessors
unexplainedLoop
| ir.c:38:13:38:37 | Constant: 1 | Instruction 'Constant: 1' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:38:13:38:37 | Phi: 1 | Instruction 'Phi: 1' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | CompareEQ: { ... } | Instruction 'CompareEQ: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | CompareEQ: { ... } | Instruction 'CompareEQ: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | CompareEQ: { ... } | Instruction 'CompareEQ: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | ConditionalBranch: { ... } | Instruction 'ConditionalBranch: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | ConditionalBranch: { ... } | Instruction 'ConditionalBranch: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | ConditionalBranch: { ... } | Instruction 'ConditionalBranch: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Instruction 'Constant: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Instruction 'Constant: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Instruction 'Constant: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | Call: call to ExRaiseAccessViolation | Instruction 'Call: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | CallSideEffect: call to ExRaiseAccessViolation | Instruction 'CallSideEffect: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | FunctionAddress: call to ExRaiseAccessViolation | Instruction 'FunctionAddress: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:28:40:28 | Constant: 1 | Instruction 'Constant: 1' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
unnecessaryPhiInstruction
memoryOperandDefinitionIsUnmodeled
operandAcrossFunctions
Expand All @@ -37,9 +20,6 @@ containsLoopOfForwardEdges
missingIRType
multipleIRTypes
lostReachability
| ir.c:39:3:41:3 | Constant: { ... } | Block 'Constant: { ... }' is not reachable by traversing only forward edges in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Block 'Constant: { ... }' is not reachable by traversing only forward edges in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | FunctionAddress: call to ExRaiseAccessViolation | Block 'FunctionAddress: call to ExRaiseAccessViolation' is not reachable by traversing only forward edges in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
backEdgeCountMismatch
useNotDominatedByDefinition
switchInstructionWithoutDefaultEdge
Expand Down
20 changes: 0 additions & 20 deletions cpp/ql/test/library-tests/ir/ir/aliased_ssa_consistency_unsound.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,8 @@ sideEffectWithoutPrimary
instructionWithoutSuccessor
| ir.c:62:5:62:26 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' has no successors in function '$@'. | ir.c:57:6:57:30 | void throw_in_try_with_finally() | void throw_in_try_with_finally() |
| ir.c:73:5:73:26 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' has no successors in function '$@'. | ir.c:70:6:70:39 | void throw_in_try_with_throw_in_finally() | void throw_in_try_with_throw_in_finally() |
| ir.c:81:3:81:24 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' has no successors in function '$@'. | ir.c:80:6:80:27 | void raise_access_violation() | void raise_access_violation() |
ambiguousSuccessors
unexplainedLoop
| ir.c:38:13:38:37 | Constant: 1 | Instruction 'Constant: 1' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:38:13:38:37 | Phi: 1 | Instruction 'Phi: 1' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | CompareEQ: { ... } | Instruction 'CompareEQ: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | CompareEQ: { ... } | Instruction 'CompareEQ: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | CompareEQ: { ... } | Instruction 'CompareEQ: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | ConditionalBranch: { ... } | Instruction 'ConditionalBranch: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | ConditionalBranch: { ... } | Instruction 'ConditionalBranch: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | ConditionalBranch: { ... } | Instruction 'ConditionalBranch: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Instruction 'Constant: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Instruction 'Constant: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Instruction 'Constant: { ... }' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | Call: call to ExRaiseAccessViolation | Instruction 'Call: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | CallSideEffect: call to ExRaiseAccessViolation | Instruction 'CallSideEffect: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | Chi: call to ExRaiseAccessViolation | Instruction 'Chi: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | FunctionAddress: call to ExRaiseAccessViolation | Instruction 'FunctionAddress: call to ExRaiseAccessViolation' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:28:40:28 | Constant: 1 | Instruction 'Constant: 1' is part of an unexplained loop in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
unnecessaryPhiInstruction
memoryOperandDefinitionIsUnmodeled
operandAcrossFunctions
Expand All @@ -37,9 +20,6 @@ containsLoopOfForwardEdges
missingIRType
multipleIRTypes
lostReachability
| ir.c:39:3:41:3 | Constant: { ... } | Block 'Constant: { ... }' is not reachable by traversing only forward edges in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:39:3:41:3 | Constant: { ... } | Block 'Constant: { ... }' is not reachable by traversing only forward edges in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
| ir.c:40:5:40:26 | FunctionAddress: call to ExRaiseAccessViolation | Block 'FunctionAddress: call to ExRaiseAccessViolation' is not reachable by traversing only forward edges in function '$@'. | ir.c:32:6:32:32 | void unexplained_loop_regression() | void unexplained_loop_regression() |
backEdgeCountMismatch
useNotDominatedByDefinition
switchInstructionWithoutDefaultEdge
Expand Down
Loading

0 comments on commit 4c8aec0

Please sign in to comment.