Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brodes/seh flow overhaul2 #17676

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
652b74d
Altering TExceptionEdge to include SEH and regular C++ exceptions, an…
bdrodes Oct 1, 2024
82cdd02
Overhaul of IR exception handling to account for SEH and C++ exceptions.
bdrodes Oct 2, 2024
a818346
Fixing bug in translated call edge generation.
bdrodes Oct 3, 2024
a2ede08
Finalizing a model for throwing and non-throwing functions.
bdrodes Oct 7, 2024
0ed72d1
Propagating the exception kind into getExceptionSuccessorInstruction
bdrodes Oct 7, 2024
e3d0015
updating getNextHandler logic.
bdrodes Oct 7, 2024
7a3ab8c
Review updates to EdgeKind.qll
bdrodes Oct 10, 2024
d0a54a3
Review updates.
bdrodes Oct 10, 2024
fcff607
PR review corrections.
bdrodes Oct 15, 2024
fdf5ac6
Formatting.
bdrodes Oct 23, 2024
a053519
Removing SEH load exceptions for var args and lambda translations
bdrodes Oct 23, 2024
e86c764
Updated comments and formatting.
bdrodes Oct 23, 2024
a40343e
Updating comments.
bdrodes Oct 23, 2024
cc4b280
Comments and changed char pred (moved up to abstract class)
bdrodes Oct 23, 2024
fb16db2
Formatting.
bdrodes Oct 23, 2024
781a9b7
Typo
bdrodes Oct 23, 2024
30810ce
Merge branch 'main' into brodes/seh_flow_overhaul2
bdrodes Oct 23, 2024
a498c87
adding change log
bdrodes Oct 24, 2024
aeeafbc
Merge branch 'brodes/seh_flow_overhaul2' of https://github.com/micros…
bdrodes Oct 24, 2024
61b1fef
Comments
bdrodes Oct 24, 2024
f85ba7a
Updating docs
bdrodes Oct 24, 2024
9dc776e
Type in comments.
bdrodes Oct 24, 2024
3f01e4d
changing isSEH to isSeh to match conventions.
bdrodes Oct 24, 2024
222c9f2
Style changes.
bdrodes Oct 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
private import TranslatedExpr
private import TranslatedFunction
private import DefaultOptions as DefaultOptions
private import EdgeKind

/**
* Gets the `CallInstruction` from the `TranslatedCallExpr` for the specified expression.
Expand Down Expand Up @@ -84,30 +85,28 @@
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) |

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// 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.
(
// 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
Expand Down Expand Up @@ -146,7 +145,7 @@
* and `neverRaiseException` may conflict (e.g., all hold for a given target).
* Conflicting results are resolved during IR generation.
*/
abstract predicate alwaysRaiseException(boolean isSEH);

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.

/**
* The call target is known to conditionally raise an exception.
Expand All @@ -154,15 +153,15 @@
* and `neverRaiseException` may conflict (e.g., all hold for a given target).
* Conflicting results are resolved during IR generation.
*/
abstract predicate mayRaiseException(boolean isSEH);

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.

/**
* The call target is known to never raise an exception.
* Note that `alwaysRaiseException`, `mayRaiseException`,
* and `neverRaiseException` may conflict (e.g., all hold for a given target).
* Conflicting results are resolved during IR generation.
*/
abstract predicate neverRaiseException(boolean isSEH);

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.

/**
* Gets the result type of the call.
Expand Down Expand Up @@ -363,7 +362,7 @@
tag = CallTargetTag() and result = expr.getTarget()
}

override predicate alwaysRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
exists(ThrowingFunction f | f = expr.getTarget() |
f.alwaysRaisesException() and f.isSEH() and isSEH = true
or
Expand All @@ -371,7 +370,7 @@
)
}

override predicate mayRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
exists(ThrowingFunction f | f = expr.getTarget() |
f.mayRaiseException() and f.isSEH() and isSEH = true
or
Expand All @@ -379,7 +378,7 @@
)
}

override predicate neverRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
exists(NonThrowingFunction f | f = expr.getTarget() |
f.isSEH() and isSEH = true
or
Expand All @@ -398,7 +397,7 @@
result = getTranslatedExpr(expr.getExpr().getFullyConverted())
}

override predicate alwaysRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// We assume that a call to a function pointer will not throw a CXX exception.
// This is not sound in general, but this will greatly reduce the number of
// exceptional edges.
Expand All @@ -410,7 +409,7 @@
)
}

override predicate mayRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// We assume that a call to a function pointer will not throw a CXX exception.
// This is not sound in general, but this will greatly reduce the number of
// exceptional edges.
Expand All @@ -425,16 +424,14 @@
isSEH = true
}

override predicate neverRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// We assume that a call to a function pointer will not throw a CXX exception.
// This is not sound in general, but this will greatly reduce the number of
// exceptional edges.
// 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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
private import TranslatedStmt
private import TranslatedGlobalVar
private import IRConstruction
private import EdgeKind
import TranslatedCall

/**
Expand Down Expand Up @@ -280,13 +281,7 @@
)
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) {
Expand Down Expand Up @@ -943,13 +938,7 @@
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) {
Expand Down Expand Up @@ -2404,16 +2393,16 @@
result = getTranslatedExpr(expr.getAllocatorCall().getArgument(index).getFullyConverted())
}

final override predicate mayRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// We assume that a call to `new` or `new[]` will never throw. This is not
// sound in general, but this will greatly reduce the number of exceptional
// edges.
none()
}

final override predicate alwaysRaiseException(boolean isSEH) { none() }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.

final override predicate neverRaiseException(boolean isSEH) { none() }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
}

TranslatedAllocatorCall getTranslatedAllocatorCall(NewOrNewArrayExpr newExpr) {
Expand Down Expand Up @@ -2479,16 +2468,16 @@
result = getTranslatedExpr(expr.getExprWithReuse().getFullyConverted())
}

final override predicate mayRaiseException(boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// We assume that a call to `delete` or `delete[]` will never throw. This is not
// sound in general, but this will greatly reduce the number of exceptional
// edges.
none()
}

final override predicate alwaysRaiseException(boolean isSEH) { none() }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.

final override predicate neverRaiseException(boolean isSEH) { none() }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
}

TranslatedDeleteOrDeleteArrayExpr getTranslatedDeleteOrDeleteArray(DeleteOrDeleteArrayExpr newExpr) {
Expand Down Expand Up @@ -2681,11 +2670,6 @@
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
(
Expand All @@ -2712,13 +2696,7 @@
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)
)
}

Expand Down Expand Up @@ -3084,8 +3062,8 @@
// And otherwise, exit this element with an exceptional edge
not exists(this.getChild(id + 1)) and
kind instanceof ExceptionEdge and
bdrodes marked this conversation as resolved.
Show resolved Hide resolved
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)
)
}

Expand Down Expand Up @@ -3124,8 +3102,8 @@
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)
)
}

Expand Down Expand Up @@ -3413,13 +3391,7 @@
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) {
Expand Down Expand Up @@ -3489,13 +3461,7 @@

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()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parenthesis here is also superfluous now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much for the formatting tool fixing that automatically.

Copy link
Contributor

@MathiasVP MathiasVP Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 Yeah, the autoformatter doesn't remove all superfluous parentheses (because that's apparently a super hard problem in general). By the way, it's still complaining about missing autoformat in a couple of places, though:

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll would change by autoformatting.
cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedInitialization.qll would change by autoformatting.
cpp/ql/lib/semmle/code/cpp/models/implementations/StructuredExceptionHandling.qll would change by autoformatting.
cpp/ql/lib/semmle/code/cpp/models/implementations/Strcpy.qll would change by autoformatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

or
tag = VarArgsArgAddressTag() and
kind instanceof GotoEdge and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@
)
}

final override Instruction getExceptionSuccessorInstruction(EdgeKind kind, boolean isSEH) {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in isSEH should be PascalCase/camelCase.
// 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
}
Expand Down
Loading