From d0a54a30f58ca9837208429a0a6427b8e8b738d7 Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Thu, 10 Oct 2024 14:36:43 -0400 Subject: [PATCH] Review updates. --- .../raw/internal/TranslatedCall.qll | 19 +++---- .../internal/TranslatedDeclarationEntry.qll | 11 +--- .../raw/internal/TranslatedExpr.qll | 54 ++++--------------- .../raw/internal/TranslatedFunction.qll | 4 +- 4 files changed, 21 insertions(+), 67 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll index 870c81bcfa3e..28c57b345af8 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll @@ -10,6 +10,7 @@ private import TranslatedElement private import TranslatedExpr private import TranslatedFunction private import DefaultOptions as DefaultOptions +private import EdgeKind /** * Gets the `CallInstruction` from the `TranslatedCallExpr` for the specified expression. @@ -84,11 +85,7 @@ abstract class TranslatedCall extends TranslatedExpr { this.getEnclosingFunction().getFunction() = instr.getEnclosingFunction() ) else - exists(boolean isSEH | - isSEH = true and kind.(ExceptionEdge).isSEH() - or - isSEH = false and not kind.(ExceptionEdge).isSEH() - | + exists(boolean isSEH | kind = exceptionEdge(isSEH) | // Call throw behavior is resolved from most restricted to least restricted in order // if there are conflicting throwing specificaitons for a function. // Enumerating all scenarios to be explicit. @@ -96,18 +93,20 @@ abstract class TranslatedCall extends TranslatedExpr { // If the call is known to never throw, regardless of other defined throwing behavior, // do not generate any exception edges, only an ordinary successor if this.(TranslatedCallExpr).neverRaiseException(isSEH) - then result = this.getParent().getChildSuccessor(this, kind) + then result = this.getParent().getChildSuccessor(this, any(GotoEdge edge)) else // If the call is known to always throw, regardless of other defined throwing behavior, // only generate an exception edge. if this.(TranslatedCallExpr).alwaysRaiseException(isSEH) - then result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), isSEH) + then + result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), isSEH) else if this.(TranslatedCallExpr).mayRaiseException(isSEH) then ( // if the call is known to conditionally throw, generate both an exception edge and an // ordinary successor - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), isSEH) + result = + this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), isSEH) or result = this.getParent().getChildSuccessor(this, kind) ) else @@ -432,9 +431,7 @@ class TranslatedExprCall extends TranslatedCallExpr { // For SEH exceptions, use the defined ThrowingFunction behavior and // if no throwing function is found, assume a conditional SEH exception // see `mayRaiseException` - exists(NonThrowingFunction f | f = expr.getTarget() | - f.isSEH() and isSEH = true - ) + exists(NonThrowingFunction f | f = expr.getTarget() | f.isSEH() and isSEH = true) } } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedDeclarationEntry.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedDeclarationEntry.qll index e10de508e874..1dc1eed9f6a3 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedDeclarationEntry.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedDeclarationEntry.qll @@ -168,10 +168,6 @@ class TranslatedStaticLocalVariableDeclarationEntry extends TranslatedDeclaratio ( kind instanceof GotoEdge and result = this.getInstruction(DynamicInitializationConditionalBranchTag()) - or - // All load instructions may throw an SEH exception - kind.(ExceptionEdge).isSEH() and - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) ) or tag = DynamicInitializationConditionalBranchTag() and @@ -188,12 +184,7 @@ class TranslatedStaticLocalVariableDeclarationEntry extends TranslatedDeclaratio result = this.getInstruction(DynamicInitializationFlagStoreTag()) or tag = DynamicInitializationFlagStoreTag() and - ( - result = this.getParent().getChildSuccessor(this, kind) - or - // All store instructions may throw an SEH exception - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and kind.(ExceptionEdge).isSEH() - ) + result = this.getParent().getChildSuccessor(this, kind) } final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll index 9bb62b400e5e..68f341950372 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll @@ -15,6 +15,7 @@ private import TranslatedInitialization private import TranslatedStmt private import TranslatedGlobalVar private import IRConstruction +private import EdgeKind import TranslatedCall /** @@ -280,13 +281,7 @@ class TranslatedConditionValue extends TranslatedCoreExpr, ConditionContext, ) or tag = ConditionValueResultLoadTag() and - ( - result = this.getParent().getChildSuccessor(this, kind) - or - // All load instructions could throw an SEH exception - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and - kind.(ExceptionEdge).isSEH() - ) + result = this.getParent().getChildSuccessor(this, kind) } override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) { @@ -943,13 +938,7 @@ class TranslatedThisExpr extends TranslatedNonConstantExpr { result = this.getInstruction(ThisLoadTag()) or tag = ThisLoadTag() and - ( - result = this.getParent().getChildSuccessor(this, kind) - or - // All load instructions could throw an SEH exception - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and - kind.(ExceptionEdge).isSEH() - ) + result = this.getParent().getChildSuccessor(this, kind) } final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { @@ -2681,11 +2670,6 @@ abstract class TranslatedConditionalExpr extends TranslatedNonConstantExpr { result = this.getInstruction(ConditionValueResultTempAddressTag()) ) or - // All load instructions could throw an SEH exception - (tag = ConditionValueTrueStoreTag() or tag = ConditionValueFalseStoreTag()) and - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and - kind.(ExceptionEdge).isSEH() - or not this.elseIsVoid() and kind instanceof GotoEdge and ( @@ -2712,13 +2696,7 @@ abstract class TranslatedConditionalExpr extends TranslatedNonConstantExpr { or (not expr.hasLValueToRValueConversion() or not isExtractorFrontendVersion65OrHigher()) and tag = ConditionValueResultLoadTag() and - ( - result = this.getParent().getChildSuccessor(this, kind) - or - // All load instructions could throw an SEH exception - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and - kind.(ExceptionEdge).isSEH() - ) + result = this.getParent().getChildSuccessor(this, kind) ) } @@ -3084,8 +3062,8 @@ class TranslatedDestructorsAfterThrow extends TranslatedElement, TTranslatedDest // And otherwise, exit this element with an exceptional edge not exists(this.getChild(id + 1)) and kind instanceof ExceptionEdge and - not kind.(ExceptionEdge).isSEH() and - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), true) + kind = exceptionEdge(false) and + result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), false) ) } @@ -3124,8 +3102,8 @@ abstract class TranslatedThrowExpr extends TranslatedNonConstantExpr { or not exists(this.getDestructors()) and kind instanceof ExceptionEdge and - not kind.(ExceptionEdge).isSEH() and - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), true) + kind = exceptionEdge(false) and + result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), false) ) } @@ -3413,13 +3391,7 @@ class TranslatedVarArgsStart extends TranslatedNonConstantExpr { result = this.getVAList().getFirstInstruction(kind) or tag = VarArgsVAListStoreTag() and - ( - result = this.getParent().getChildSuccessor(this, kind) - or - // All store instructions could throw an SEH exception - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and - kind.(ExceptionEdge).isSEH() - ) + result = this.getParent().getChildSuccessor(this, kind) } final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { @@ -3489,13 +3461,7 @@ class TranslatedVarArg extends TranslatedNonConstantExpr { final override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) { tag = VarArgsVAListLoadTag() and - ( - kind instanceof GotoEdge and result = this.getInstruction(VarArgsArgAddressTag()) - or - // All load instructions could throw an SEH exception - result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge e), true) and - kind.(ExceptionEdge).isSEH() - ) + (kind instanceof GotoEdge and result = this.getInstruction(VarArgsArgAddressTag())) or tag = VarArgsArgAddressTag() and kind instanceof GotoEdge and diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll index d41a18c25b11..b647bbab2122 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll @@ -230,8 +230,8 @@ class TranslatedFunction extends TranslatedRootElement, TTranslatedFunction { final override Instruction getExceptionSuccessorInstruction(EdgeKind kind, boolean isSEH) { // only unwind for C++ exceptions since SEH exceptions are too verbose - // and would generate unwind for all functions. - isSEH = false and + // and would generate unwind for all functions. + isSEH = false and result = this.getInstruction(UnwindTag()) and kind instanceof GotoEdge }