mirror of
https://github.com/github/codeql.git
synced 2026-04-30 03:05:15 +02:00
Ruby: Fix bad join in access predicate
Joining on variable name alone is a bad thing:
```
[2022-01-25 11:13:20] (228s) Tuple counts for Variable::Cached::access#ff#shared/3@868b54tu after 3m37s:
112554 ~0% {3} r1 = JOIN Variable::VariableReal::getNameImpl_dispred#ff WITH Variable::VariableReal::getDeclaringScopeImpl_dispred#ff ON FIRST 1 OUTPUT Lhs.1, Lhs.0 'arg2', Rhs.1 'arg1'
561015756 ~1% {3} r2 = JOIN r1 WITH Variable::variableName#ff_10#join_rhs ON FIRST 1 OUTPUT Rhs.1 'arg0', Lhs.2 'arg1', Lhs.1 'arg2'
return r2
```
This change ensures that we join on name and scope simultaneously.
This commit is contained in:
@@ -106,20 +106,23 @@ private predicate scopeDefinesParameterVariable(
|
||||
)
|
||||
}
|
||||
|
||||
private string variableName(Ruby::AstNode i) {
|
||||
result = i.(Ruby::Identifier).getValue()
|
||||
or
|
||||
exists(Ruby::KeywordPattern p | i = p.getKey() and not exists(p.getValue()) |
|
||||
result = i.(Ruby::String).getChild(0).(Ruby::StringContent).getValue() or
|
||||
result = i.(Ruby::HashKeySymbol).getValue()
|
||||
pragma[nomagic]
|
||||
private string variableNameInScope(Ruby::AstNode i, Scope::Range scope) {
|
||||
scope = scopeOf(i) and
|
||||
(
|
||||
result = i.(Ruby::Identifier).getValue()
|
||||
or
|
||||
exists(Ruby::KeywordPattern p | i = p.getKey() and not exists(p.getValue()) |
|
||||
result = i.(Ruby::String).getChild(0).(Ruby::StringContent).getValue() or
|
||||
result = i.(Ruby::HashKeySymbol).getValue()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `name` is assigned in `scope` at `i`. */
|
||||
private predicate scopeAssigns(Scope::Range scope, string name, Ruby::AstNode i) {
|
||||
(explicitAssignmentNode(i, _) or implicitAssignmentNode(i)) and
|
||||
name = variableName(i) and
|
||||
scope = scopeOf(i)
|
||||
name = variableNameInScope(i, scope)
|
||||
}
|
||||
|
||||
cached
|
||||
@@ -306,13 +309,18 @@ private module Cached {
|
||||
i = any(Ruby::WhileModifier x).getBody()
|
||||
}
|
||||
|
||||
pragma[nomagic]
|
||||
private predicate hasScopeAndName(VariableReal variable, Scope::Range scope, string name) {
|
||||
variable.getNameImpl() = name and
|
||||
scope = variable.getDeclaringScopeImpl()
|
||||
}
|
||||
|
||||
cached
|
||||
predicate access(Ruby::AstNode access, VariableReal variable) {
|
||||
exists(string name |
|
||||
variable.getNameImpl() = name and
|
||||
name = variableName(access)
|
||||
exists(string name, Scope::Range scope |
|
||||
pragma[only_bind_into](name) = variableNameInScope(access, scope)
|
||||
|
|
||||
variable.getDeclaringScopeImpl() = scopeOf(access) and
|
||||
hasScopeAndName(variable, scope, name) and
|
||||
not access.getLocation().strictlyBefore(variable.getLocationImpl()) and
|
||||
// In case of overlapping parameter names, later parameters should not
|
||||
// be considered accesses to the first parameter
|
||||
@@ -321,8 +329,8 @@ private module Cached {
|
||||
else any()
|
||||
or
|
||||
exists(Scope::Range declScope |
|
||||
variable.getDeclaringScopeImpl() = declScope and
|
||||
inherits(scopeOf(access), name, declScope)
|
||||
hasScopeAndName(variable, declScope, pragma[only_bind_into](name)) and
|
||||
inherits(scope, name, declScope)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user