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

Pointer cast #1564

Merged
merged 10 commits into from
Feb 9, 2022
Merged

Pointer cast #1564

merged 10 commits into from
Feb 9, 2022

Conversation

robdockins
Copy link
Contributor

This PR implements a new llvm_cast_pointer operation for setup blocks. It allows the user to specify that a pointer should be treated as a pointer to a given llvm type, despite the information contained in its initial allocation site.

This is primarily intended to make it easier to work with unions, as the type information provided by the LLVM bitcode in these cases is not terribly useful.

This produces a new setup value that points to the same location,
but is assumed to point to values of the provided type. This is useful
when, e.g., verifing code that contains `union` values inside
structs, as the type provided by the bitcode will not accurately
reflect all the intended uses.  It may also be useful for other
"polymorphic" C programming styles.
It is only used in the LLVM mode, and I'd like to make some changes.
It is unclear that having this be shared code is worthwhile.
This check is necessarily approximate, and the system may miss
incompatible resolved values if overlapping regions are
resolved through different pointer casts.

This should not be a soundness issue, as the verification will
fail at a later point instead (albeit, with less-useful information
provided to the user).
This plays more nicely with `llvm_field`, as alias information
is retained longer.
…re function,

similar to `llvm_field`, `llvm_elem`, etc.
@robdockins robdockins marked this pull request as ready for review February 9, 2022 00:52
@robdockins
Copy link
Contributor Author

I plan to add a unit test, but I think this is otherwise ready to go.

Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, aside from some documentation-related requests.

src/SAWScript/Crucible/Common/MethodSpec.hs Outdated Show resolved Hide resolved
src/SAWScript/Crucible/LLVM/MethodSpecIR.hs Outdated Show resolved Hide resolved
src/SAWScript/Crucible/LLVM/MethodSpecIR.hs Outdated Show resolved Hide resolved
src/SAWScript/Crucible/LLVM/MethodSpecIR.hs Show resolved Hide resolved
src/SAWScript/Crucible/LLVM/MethodSpecIR.hs Outdated Show resolved Hide resolved
src/SAWScript/Crucible/LLVM/ResolveSetupValue.hs Outdated Show resolved Hide resolved
src/SAWScript/Interpreter.hs Show resolved Hide resolved
@robdockins
Copy link
Contributor Author

I'm not sure I have the necessary context to pipe the new pointer cast form through the Python client code.

Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I have the necessary context to pipe the new pointer cast form through the Python client code.

I've opened #1575 to track this.

@robdockins robdockins added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Feb 9, 2022
@mergify mergify bot merged commit 4af8f26 into master Feb 9, 2022
@mergify mergify bot deleted the pointer-cast branch February 9, 2022 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants