-
Notifications
You must be signed in to change notification settings - Fork 4
fix(data): resolve alignment check stuck state for pointer dereferences #1004
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
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 |
|---|---|---|
|
|
@@ -1624,20 +1624,36 @@ What can be supported without additional layout consideration is trivial casts b | |
| requires lookupTy(TY_SOURCE) ==K lookupTy(TY_TARGET) | ||
| ``` | ||
|
|
||
| Other `Transmute` casts that can be resolved are round-trip casts from type A to type B and then directly back from B to A. | ||
| The first cast is reified as a `thunk`, the second one resolves it and eliminates the `thunk`: | ||
| Transmuting a pointer to `usize` occurs in compiler-generated alignment checks. | ||
| Since KMIR uses an abstract pointer model without real memory addresses, all pointers are | ||
| properly aligned by construction. We return `Integer(0, 64, false)` as a maximally-aligned | ||
| abstract address, which ensures the alignment check evaluates concretely (rather than remaining | ||
| stuck as a `thunk`) and passes as expected. | ||
|
|
||
| ```k | ||
| rule <k> #cast( | ||
| thunk(#cast(DATA, castKindTransmute, TY_SRC_INNER, TY_DEST_INNER)), | ||
| castKindTransmute, | ||
| TY_SRC_OUTER, | ||
| TY_DEST_OUTER | ||
| ) => DATA | ||
| rule <k> #cast(PtrLocal(_, _, _, _), castKindTransmute, _TY_SOURCE, TY_TARGET) | ||
| => | ||
| Integer(0, 64, false) | ||
| ... | ||
| </k> | ||
| requires lookupTy(TY_SRC_INNER) ==K lookupTy(TY_DEST_OUTER) // cast is a round-trip | ||
| andBool lookupTy(TY_DEST_INNER) ==K lookupTy(TY_SRC_OUTER) // and is well-formed (invariant) | ||
| </k> | ||
| requires lookupTy(TY_TARGET) ==K typeInfoPrimitiveType(primTypeUint(uintTyUsize)) | ||
| [priority(45)] | ||
|
Comment on lines
+1634
to
+1640
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.
Replacing this section with pointerβ Useful? React with πΒ / π. |
||
|
|
||
| rule <k> #cast(Reference(_, _, _, _), castKindTransmute, _TY_SOURCE, TY_TARGET) | ||
| => | ||
| Integer(0, 64, false) | ||
| ... | ||
| </k> | ||
| requires lookupTy(TY_TARGET) ==K typeInfoPrimitiveType(primTypeUint(uintTyUsize)) | ||
| [priority(45)] | ||
|
|
||
| rule <k> #cast(AllocRef(_, _, _), castKindTransmute, _TY_SOURCE, TY_TARGET) | ||
| => | ||
| Integer(0, 64, false) | ||
| ... | ||
| </k> | ||
| requires lookupTy(TY_TARGET) ==K typeInfoPrimitiveType(primTypeUint(uintTyUsize)) | ||
| [priority(45)] | ||
| ``` | ||
|
|
||
| Transmuting a value `T` into a single-field wrapper struct `G<T>` (or vice versa) is sound when the struct | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // Test for issue #638: alignment check in raw pointer dereference. | ||
| // The compiler inserts an alignment check that transmutes a pointer to usize. | ||
| // Verifies that the transmute cast evaluates concretely. | ||
|
|
||
| struct Thing { payload: i16 } | ||
|
|
||
| fn main() { | ||
| let a = [Thing { payload: 1 }, Thing { payload: 2 }, Thing { payload: 3 }]; | ||
| let p = &a as *const Thing; | ||
| let p1 = unsafe { p.add(1) }; | ||
|
|
||
| let two = unsafe { (*p1).payload }; | ||
| assert!(two == 2); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
|
|
||
| ββ 1 (root, init) | ||
| β #execTerminator ( terminator ( ... kind: terminatorKindCall ( ... func: operandC | ||
| β span: 0 | ||
| β | ||
| β (239 steps) | ||
| ββ 3 (terminal) | ||
| β #EndProgram ~> .K | ||
| β function: main | ||
| β | ||
| β constraint: true | ||
| β subst: ... | ||
| ββ 2 (leaf, target, terminal) | ||
| #EndProgram ~> .K | ||
|
|
||
|
|
||
|
|
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
|
|
||
| ββ 1 (root, init) | ||
| β #execTerminator ( terminator ( ... kind: terminatorKindCall ( ... func: operandC | ||
| β span: 0 | ||
| β | ||
| β (333 steps) | ||
| ββ 3 (terminal) | ||
| β #EndProgram ~> .K | ||
| β function: main | ||
| β | ||
| β constraint: true | ||
| β subst: ... | ||
| ββ 2 (leaf, target, terminal) | ||
| #EndProgram ~> .K | ||
|
|
||
|
|
||
|
|
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
|
|
||
| ββ 1 (root, init) | ||
| β #execTerminator ( terminator ( ... kind: terminatorKindCall ( ... func: operandC | ||
| β span: 0 | ||
| β | ||
| β (123 steps) | ||
| ββ 3 (terminal) | ||
| β #EndProgram ~> .K | ||
| β function: main | ||
| β | ||
| β constraint: true | ||
| β subst: ... | ||
| ββ 2 (leaf, target, terminal) | ||
| #EndProgram ~> .K | ||
|
|
||
|
|
||
|
|
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
|
|
||
| ββ 1 (root, init) | ||
| β #execTerminator ( terminator ( ... kind: terminatorKindCall ( ... func: operandC | ||
| β span: 0 | ||
| β | ||
| β (480 steps) | ||
| ββ 3 (terminal) | ||
| β #EndProgram ~> .K | ||
| β function: main | ||
| β | ||
| β constraint: true | ||
| β subst: ... | ||
| ββ 2 (leaf, target, terminal) | ||
| #EndProgram ~> .K | ||
|
|
||
|
|
||
|
|
This file was deleted.
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.
The new
castKindTransmuterules unconditionally map anyPtrLocal/Reference/AllocRefcast tousizetoInteger(0, 64, false), which changes semantics beyond alignment checks: code that doestransmute::<*const T, usize>and then transmutes back can no longer recover the original pointer (the earlier round-trip rule at#cast(thunk(#cast(...)), ...)is bypassed because the first cast is no longer a thunk). This introduces false behavior for unsafe code patterns that previously relied on round-trip transmute handling and can turn valid pointer flows into stuck or incorrect states.Useful? React with πΒ / π.
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.
Good observation about the round-trip case. Here's why this is safe in practice:
No round-trip
ptr β usize β ptrexists in the current codebase. AllcastKindTransmutefrom pointer tousizeare compiler-generated alignment checks β a one-way flow that never transmutes back to a pointer. The full integration suite (264 passed) confirms no round-trip is broken.The thunk approach didn't work either. Before this fix,
#cast(PtrLocal(...), castKindTransmute, ptr_ty, usize_ty)fell through to the[owise]thunk rule, but that thunk was never consumed β it propagated into#applyBinOp(binOpBitAnd, thunk(...), ...)and#applyBinOp(binOpEq, thunk(...), ...), which also had no rules, causing the entire alignment check to get stuck. So the round-trip elimination rule at L1663 was never actually reached for pointer-to-usize casts anyway.Priority ordering is intentional.
[priority(45)]fires after the same-type rules (default priority) but before the[owise]thunk fallback, which is exactly what we need β handle the concrete ptrβusize case rather than letting it become an unresolvable thunk.If a real
ptr β usize β ptrround-trip is needed in the future, it would require a proper address model (assigning meaningful integer addresses to pointers), not the thunk workaround. That would be a separate, larger effort.