Skip to content
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

Don't use projectLLVM_bv for zero-extending or truncating pointers #41

Closed
RyanGlScott opened this issue Jul 25, 2024 · 0 comments
Closed
Labels
bug Something isn't working

Comments

@RyanGlScott
Copy link
Collaborator

Several parts of stubs employ an anti-pattern where we use the projectLLVM_bv function immediately followed by llvmPointer_bv for the purposes of zero-extending or truncating a pointer value. Here is one part of stubs that does this:

-- Zero extend return value to fit in 32-bit register and update register
-- state.
extendResult
:: (1 WI.<= srcW, DMT.KnownNat srcW)
=> LCLM.LLVMPtr sym srcW
-> IO (LCLM.LLVMPtr sym 32)
extendResult res = do
asBv <- LCLM.projectLLVM_bv bak res
AO.bvToPtr sym asBv (PN.knownNat @32)

(Note that bvToPtr is defined in terms of llvmPointer_bv.)

This is wrong when dealing with pointer arguments, as projectLLVM_bv will throw an assertion failure when the argument is a pointer. It would be more correct to project out the block number and offset (without making an assertion), adjust the offset accordingly, then reconstruct a pointer with the same block number and the adjusted offset.

It's only by sheer luck that we've managed to avoid triggering these assertion failures up until now, but I have managed to trigger this error as part of ongoing work to support PowerPC (#38). As such, I plan to fix this as part of #38.

See also #40, which is about improving the error messages in situations where we really do need to assert that a pointer is a bitvector (unlike the scenario described in this issue, where we can work with both pointers and bitvectors alike).

@RyanGlScott RyanGlScott added the bug Something isn't working label Jul 25, 2024
RyanGlScott added a commit that referenced this issue Jul 26, 2024
Doing so is actively harmful when adjusting the size of pointers, as
`projectLLVM_bv` will throw an assertion failure if this happens. We now adjust
pointer sizes in an alternative, non-assertion-failure–throwing fashion.

After doing this, some functions in `Stubs.Override` (e.g., `bvToPtr`) no
longer serve a useful purpose, so I went ahead and removed them.

Fixes #41.
RyanGlScott added a commit that referenced this issue Aug 1, 2024
Doing so is actively harmful when adjusting the size of pointers, as
`projectLLVM_bv` will throw an assertion failure if this happens. We now adjust
pointer sizes in an alternative, non-assertion-failure–throwing fashion.

After doing this, some functions in `Stubs.Override` (e.g., `bvToPtr`) no
longer serve a useful purpose, so I went ahead and removed them.

Fixes #41.
RyanGlScott added a commit that referenced this issue Aug 1, 2024
Doing so is actively harmful when adjusting the size of pointers, as
`projectLLVM_bv` will throw an assertion failure if this happens. We now adjust
pointer sizes in an alternative, non-assertion-failure–throwing fashion.

After doing this, some functions in `Stubs.Override` (e.g., `bvToPtr`) no
longer serve a useful purpose, so I went ahead and removed them.

Fixes #41.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant