-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Type inference for raw pointers #20950
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
Conversation
9b884cb to
ea1b0a8
Compare
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.
Pull request overview
This PR implements comprehensive type inference support for Rust raw pointers by distinguishing between *const and *mut pointer types and adding proper type inference for raw borrows (&raw const / &raw mut) and raw pointer dereferences.
Key changes:
- Split the generic
Ptrbuiltin type intoPtrConstandPtrMutto distinguish between const and mutable raw pointers - Added type inference for raw borrow expressions (
&raw constand&raw mut) which were previously incorrectly typed as normal references - Implemented bidirectional type inference for raw pointer dereferences, supporting both reading from and writing to dereferenced pointers
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/tools/builtins/types.rs | Replaces Ptr struct with separate PtrConst and PtrMut structs to distinguish pointer types |
| rust/ql/lib/codeql/rust/frameworks/stdlib/Builtins.qll | Refactors PtrType into an abstract base class with PtrConstType and PtrMutType subclasses |
| rust/ql/lib/codeql/rust/internal/Type.qll | Implements the type hierarchy for raw pointers with separate PtrConstType and PtrMutType classes and relocates getPtrTypeParameter() |
| rust/ql/lib/codeql/rust/internal/TypeMention.qll | Updates PtrTypeReprMention to resolve correct pointer type based on const/mut qualifier |
| rust/ql/lib/codeql/rust/internal/TypeInference.qll | Adds inferRefExprType() for raw borrows, splits inferRefNodeType() into inferRefExprType() and inferRefPatType(), renames typeEqualityNonSymmetric() to typeEqualityAsymmetric(), and adds inferDereferencedExprPtrType() for deref type inference |
| rust/ql/lib/codeql/rust/internal/PathResolution.qll | Updates builtin resolution to distinguish const and mut pointer types |
| rust/ql/test/library-tests/type-inference/type-inference.ql | Adds inferCertainType query predicate and extracts relevantNode() helper |
| rust/ql/test/library-tests/type-inference/raw_pointer.rs | New comprehensive test file for raw pointer type inference scenarios |
| rust/ql/test/library-tests/elements/builtintypes/BuiltinTypes.expected | Updates expected output to reflect PtrConst instead of Ptr |
| rust/ql/test/query-tests/security/CWE-825/CONSISTENCY/PathResolutionConsistency.expected | Removes resolved method call inconsistencies for is_null() on raw pointers |
| rust/ql/test/query-tests/security/CWE-770/CONSISTENCY/PathResolutionConsistency.expected | Removes resolved method call inconsistencies for cast() on raw pointers |
| rust/ql/test/query-tests/security/CWE-696/CONSISTENCY/PathResolutionConsistency.expected | Removes resolved method call inconsistencies for is_null() on raw pointers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hvitved
left a comment
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.
Very nice, a few comments.
|
Because of the change in |
I'll try a new one with |
geoffw0
left a comment
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.
Changes and DCA LGTM.
| private Type inferDereferencedExprPtrType(AstNode n, TypePath path) { | ||
| exists(DerefExpr de, PtrType type, TypePath suffix | | ||
| de.getExpr() = n and | ||
| type = inferType(de.getExpr()) and |
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.
This is equivalent, assuming a DerefExpr only has one getExpr():
| type = inferType(de.getExpr()) and | |
| type = inferType(n) and |
481b9d5 to
27ddc81
Compare
This PR implements type inference for raw pointers. This amount to
*mutfrom*const.&raw. These are currently incorrectly given the type of normal borrows.*of raw pointers. This is a primitive operation, not a call to theDereftrait, so it needs special treatment.DCA
The DCA report looks surprisingly good. There's a 22% drop in path resolution inconsistencies and we're getting more types overall.
Most projects have no change in the percentage of resolved calls, but for
windows-rsclose to half of the previously unresolved calls are now resolved, leading to a large 7.4% point increase to the percentage of resolved call. I'm guessing that project does a lot of pointer work.