-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
C#: Move implicit entry definitions inside method bodies in SSA construction #16875
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,24 @@ import csharp | |
*/ | ||
module Ssa { | ||
private import internal.SsaImpl as SsaImpl | ||
private import semmle.code.csharp.internal.Location | ||
|
||
pragma[nomagic] | ||
private predicate assignableDefinitionLocalScopeVariable( | ||
AssignableDefinition ad, LocalScopeVariable v, Callable c | ||
) { | ||
ad.getTarget() = v and | ||
ad.getEnclosingCallable() = c | ||
} | ||
|
||
pragma[nomagic] | ||
private predicate localScopeSourceVariable( | ||
SourceVariables::LocalScopeSourceVariable sv, LocalScopeVariable v, Callable c1, Callable c2 | ||
) { | ||
sv.getAssignable() = v and | ||
sv.getEnclosingCallable() = c1 and | ||
v.getCallable() = c2 | ||
} | ||
|
||
/** | ||
* A variable that can be SSA converted. | ||
|
@@ -34,11 +52,10 @@ module Ssa { | |
or | ||
// Local variable declaration without initializer | ||
not exists(result.getTargetAccess()) and | ||
this = | ||
any(SourceVariables::LocalScopeSourceVariable v | | ||
result.getTarget() = v.getAssignable() and | ||
result.getEnclosingCallable() = v.getEnclosingCallable() | ||
) | ||
exists(LocalScopeVariable v, Callable c | | ||
assignableDefinitionLocalScopeVariable(result, v, c) and | ||
localScopeSourceVariable(this, v, c, _) | ||
Comment on lines
+56
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you need to force the join order for performance reasons? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
) | ||
} | ||
|
||
/** | ||
|
@@ -507,9 +524,7 @@ module Ssa { | |
override Element getElement() { result = ad.getElement() } | ||
|
||
override string toString() { | ||
if this.getADefinition() instanceof AssignableDefinitions::ImplicitParameterDefinition | ||
then result = SsaImpl::getToStringPrefix(this) + "SSA param(" + this.getSourceVariable() + ")" | ||
else result = SsaImpl::getToStringPrefix(this) + "SSA def(" + this.getSourceVariable() + ")" | ||
result = SsaImpl::getToStringPrefix(this) + "SSA def(" + this.getSourceVariable() + ")" | ||
} | ||
|
||
override Location getLocation() { result = ad.getLocation() } | ||
|
@@ -537,7 +552,7 @@ module Ssa { | |
|
||
/** | ||
* An SSA definition representing the implicit initialization of a variable | ||
* at the beginning of a callable. Either the variable is a local scope variable | ||
* at the beginning of a callable. Either a parameter, a local scope variable | ||
* captured by the callable, or a field or property accessed inside the callable. | ||
*/ | ||
class ImplicitEntryDefinition extends ImplicitDefinition { | ||
|
@@ -551,7 +566,7 @@ module Ssa { | |
/** Gets the callable that this entry definition belongs to. */ | ||
final Callable getCallable() { result = this.getBasicBlock().getCallable() } | ||
|
||
override Callable getElement() { result = this.getCallable() } | ||
override Element getElement() { result = this.getCallable() } | ||
|
||
override string toString() { | ||
if this.getSourceVariable().getAssignable() instanceof LocalScopeVariable | ||
|
@@ -566,6 +581,54 @@ module Ssa { | |
override Location getLocation() { result = this.getCallable().getLocation() } | ||
} | ||
|
||
private module NearestLocationInput implements NearestLocationInputSig { | ||
class C = ImplicitParameterDefinition; | ||
|
||
predicate relevantLocations(ImplicitParameterDefinition def, Location l1, Location l2) { | ||
not def.getBasicBlock() instanceof ControlFlow::BasicBlocks::EntryBlock and | ||
l1 = def.getParameter().getALocation() and | ||
l2 = def.getBasicBlock().getLocation() | ||
} | ||
} | ||
|
||
pragma[nomagic] | ||
private predicate implicitEntryDef(ImplicitEntryDefinition def, SourceVariable v, Callable c) { | ||
v = def.getSourceVariable() and | ||
c = def.getCallable() | ||
} | ||
|
||
/** | ||
* An SSA definition representing the implicit initialization of a parameter | ||
* at the beginning of a callable. | ||
*/ | ||
class ImplicitParameterDefinition extends ImplicitEntryDefinition { | ||
private Parameter p; | ||
|
||
ImplicitParameterDefinition() { | ||
exists(SourceVariable sv, Callable c | | ||
implicitEntryDef(this, sv, c) and | ||
localScopeSourceVariable(sv, p, _, c) | ||
) | ||
} | ||
|
||
/** Gets the parameter that this entry definition represents. */ | ||
Parameter getParameter() { result = p } | ||
|
||
override Element getElement() { result = this.getParameter() } | ||
|
||
override string toString() { | ||
result = SsaImpl::getToStringPrefix(this) + "SSA param(" + this.getParameter() + ")" | ||
} | ||
|
||
override Location getLocation() { | ||
not NearestLocation<NearestLocationInput>::nearestLocation(this, _, _) and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why doesn't this give us the exact location whe the is only a single implementation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the
constraint in |
||
result = p.getLocation() | ||
or | ||
// multi-bodied method: use matching parameter location | ||
NearestLocation<NearestLocationInput>::nearestLocation(this, result, _) | ||
} | ||
} | ||
|
||
/** | ||
* An SSA definition representing the potential definition of a variable | ||
* via a call. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, so it is possible to declare bindingsets for member predicates?
Or is the bindingset just declared because you want to use the
inline_late
pragma?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's needed for
inline_late
.