Skip to content

Commit 601e317

Browse files
authored
Merge pull request #19833 from paldepind/rust/overloaded-index
Rust: Add type inference for overloaded index expressions
2 parents d428eae + 153e91b commit 601e317

File tree

6 files changed

+190
-216
lines changed

6 files changed

+190
-216
lines changed

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,13 @@ final class ArgumentPosition extends ParameterPosition {
145145
* as the synthetic `ReceiverNode` is the argument for the `self` parameter.
146146
*/
147147
predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ParameterPosition pos) {
148-
call.getPositionalArgument(pos.getPosition()) = arg
149-
or
150-
call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
148+
// TODO: Handle index expressions as calls in data flow.
149+
not call.getCall() instanceof IndexExpr and
150+
(
151+
call.getPositionalArgument(pos.getPosition()) = arg
152+
or
153+
call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
154+
)
151155
}
152156

153157
/** Provides logic related to SSA. */
@@ -959,7 +963,11 @@ private module Cached {
959963

960964
cached
961965
newtype TDataFlowCall =
962-
TCall(CallCfgNode c) { Stages::DataFlowStage::ref() } or
966+
TCall(CallCfgNode c) {
967+
Stages::DataFlowStage::ref() and
968+
// TODO: Handle index expressions as calls in data flow.
969+
not c.getCall() instanceof IndexExpr
970+
} or
963971
TSummaryCall(
964972
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
965973
) {

rust/ql/lib/codeql/rust/dataflow/internal/Node.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,11 @@ newtype TNode =
472472
getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
473473
]
474474
} or
475-
TReceiverNode(CallCfgNode mc, Boolean isPost) { mc.getCall().receiverImplicitlyBorrowed() } or
475+
TReceiverNode(CallCfgNode mc, Boolean isPost) {
476+
mc.getCall().receiverImplicitlyBorrowed() and
477+
// TODO: Handle index expressions as calls in data flow.
478+
not mc.getCall() instanceof IndexExpr
479+
} or
476480
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
477481
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
478482
TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c, _) } or

rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,20 @@ module Impl {
160160
pos.asPosition() = 0 and result = super.getOperand(1)
161161
}
162162
}
163+
164+
private class IndexCall extends Call instanceof IndexExpr {
165+
override string getMethodName() { result = "index" }
166+
167+
override Trait getTrait() { result.getCanonicalPath() = "core::ops::index::Index" }
168+
169+
override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
170+
pos.isSelf() and certain = true
171+
}
172+
173+
override Expr getArgument(ArgumentPosition pos) {
174+
pos.isSelf() and result = super.getBase()
175+
or
176+
pos.asPosition() = 0 and result = super.getIndex()
177+
}
178+
}
163179
}

rust/ql/lib/codeql/rust/internal/TypeInference.qll

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -772,45 +772,48 @@ private Type inferCallExprBaseType(AstNode n, TypePath path) {
772772
n = a.getNodeAt(apos) and
773773
result = CallExprBaseMatching::inferAccessType(a, apos, path0)
774774
|
775-
(
775+
if
776776
apos.isBorrowed(true)
777777
or
778-
// The desugaring of the unary `*e` is `*Deref::deref(&e)`. To handle the
779-
// deref expression after the call we must strip a `&` from the type at
780-
// the return position.
781-
apos.isReturn() and a instanceof DerefExpr
782-
) and
783-
path0.isCons(TRefTypeParameter(), path)
784-
or
785-
apos.isBorrowed(false) and
786-
exists(Type argType | argType = inferType(n) |
787-
if argType = TRefType()
778+
// The desugaring of the unary `*e` is `*Deref::deref(&e)` and the
779+
// desugaring of `a[b]` is `*Index::index(&a, b)`. To handle the deref
780+
// expression after the call we must strip a `&` from the type at the
781+
// return position.
782+
apos.isReturn() and
783+
(a instanceof DerefExpr or a instanceof IndexExpr)
784+
then path0.isCons(TRefTypeParameter(), path)
785+
else
786+
if apos.isBorrowed(false)
788787
then
789-
path = path0 and
790-
path0.isCons(TRefTypeParameter(), _)
791-
or
792-
// adjust for implicit deref
793-
not path0.isCons(TRefTypeParameter(), _) and
794-
not (path0.isEmpty() and result = TRefType()) and
795-
path = TypePath::cons(TRefTypeParameter(), path0)
796-
else (
797-
not (
798-
argType.(StructType).asItemNode() instanceof StringStruct and
799-
result.(StructType).asItemNode() instanceof Builtins::Str
800-
) and
801-
(
802-
not path0.isCons(TRefTypeParameter(), _) and
803-
not (path0.isEmpty() and result = TRefType()) and
804-
path = path0
805-
or
806-
// adjust for implicit borrow
807-
path0.isCons(TRefTypeParameter(), path)
788+
exists(Type argType | argType = inferType(n) |
789+
if argType = TRefType()
790+
then
791+
path = path0 and
792+
path0.isCons(TRefTypeParameter(), _)
793+
or
794+
// adjust for implicit deref
795+
not path0.isCons(TRefTypeParameter(), _) and
796+
not (path0.isEmpty() and result = TRefType()) and
797+
path = TypePath::cons(TRefTypeParameter(), path0)
798+
else (
799+
not (
800+
argType.(StructType).asItemNode() instanceof StringStruct and
801+
result.(StructType).asItemNode() instanceof Builtins::Str
802+
) and
803+
(
804+
not path0.isCons(TRefTypeParameter(), _) and
805+
not (path0.isEmpty() and result = TRefType()) and
806+
path = path0
807+
or
808+
// adjust for implicit borrow
809+
path0.isCons(TRefTypeParameter(), path)
810+
)
811+
)
808812
)
813+
else (
814+
not apos.isBorrowed(_) and
815+
path = path0
809816
)
810-
)
811-
or
812-
not apos.isBorrowed(_) and
813-
path = path0
814817
)
815818
}
816819

@@ -1046,8 +1049,8 @@ private class Vec extends Struct {
10461049
*/
10471050
pragma[nomagic]
10481051
private Type inferIndexExprType(IndexExpr ie, TypePath path) {
1049-
// TODO: Should be implemented as method resolution, using the special
1050-
// `std::ops::Index` trait.
1052+
// TODO: Method resolution to the `std::ops::Index` trait can handle the
1053+
// `Index` instances for slices and arrays.
10511054
exists(TypePath exprPath, Builtins::BuiltinType t |
10521055
TStruct(t) = inferType(ie.getIndex()) and
10531056
(

rust/ql/test/library-tests/type-inference/main.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,21 +1856,27 @@ mod indexers {
18561856

18571857
// MyVec::index
18581858
fn index(&self, index: usize) -> &Self::Output {
1859-
&self.data[index] // $ fieldof=MyVec
1859+
&self.data[index] // $ fieldof=MyVec method=index
18601860
}
18611861
}
18621862

18631863
fn analyze_slice(slice: &[S]) {
1864-
let x = slice[0].foo(); // $ method=foo type=x:S
1864+
// NOTE: `slice` gets the spurious type `[]` because the desugaring of
1865+
// the index expression adds an implicit borrow. `&slice` has the type
1866+
// `&&[S]`, but the `index` methods takes a `&[S]`, so Rust adds an
1867+
// implicit dereference. We cannot currently handle a position that is
1868+
// both implicitly dereferenced and implicitly borrowed, so the extra
1869+
// type sneaks in.
1870+
let x = slice[0].foo(); // $ method=foo type=x:S method=index SPURIOUS: type=slice:[]
18651871
}
18661872

18671873
pub fn f() {
18681874
let mut vec = MyVec::new(); // $ type=vec:T.S
18691875
vec.push(S); // $ method=push
1870-
vec[0].foo(); // $ MISSING: method=foo -- type inference does not support the `Index` trait yet
1876+
vec[0].foo(); // $ method=MyVec::index method=foo
18711877

18721878
let xs: [S; 1] = [S];
1873-
let x = xs[0].foo(); // $ method=foo type=x:S
1879+
let x = xs[0].foo(); // $ method=foo type=x:S method=index
18741880

18751881
analyze_slice(&xs);
18761882
}

0 commit comments

Comments
 (0)