-
Notifications
You must be signed in to change notification settings - Fork 63
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
Consider using a different type than MemType
in typeOfSetupValue
#1876
Labels
subsystem: crucible-llvm
Issues related to LLVM bitcode verification with crucible-llvm
tech debt
Issues that document or involve technical debt
type: enhancement
Issues describing an improvement to an existing feature or capability
Milestone
Comments
RyanGlScott
added
subsystem: crucible-llvm
Issues related to LLVM bitcode verification with crucible-llvm
tech debt
Issues that document or involve technical debt
labels
May 31, 2023
RyanGlScott
added a commit
that referenced
this issue
May 31, 2023
This patch adds support for LLVM 15 and 16 by adding support for opaque pointers, which are described in https://llvm.org/docs/OpaquePointers.html. I have also added a test case involving LLVM bitcode using opaque pointers to kick the tires and ensure that the basics work as expected. For the most part, this is a straightforward process, as most uses of pointer types in SAW already do not care about pointee types. There are some exceptions, however: * The `typeOfSetupValue` function, as well as several places that use this function, scrutinize pointee types of pointers, which would appear to fly in the face of opaque pointers. I attempt to explain in #1876 which this is actually OK for now (although a bit awkward). * The `llvm_boilerplate`/skeleton machinery does not support opaque pointers at all. See #1877. This patch also bumps the following submodules to bring in support for opaque pointers: * `llvm-pretty`: GaloisInc/llvm-pretty#110 * `llvm-pretty-bc-parser`: GaloisInc/llvm-pretty-bc-parser#221 * `crucible`: GaloisInc/crucible#1085 This also bumps the `what4` submodule to bring in the changes from GaloisInc/what4#234. This isn't necessary to support opaque pointers, but it _is_ necessary to support a build plan involving `tasty-sugar-2.2.*`, which `llvm-pretty-bc-parser`'s test suite now requires.
Interesting. Thanks for the very detailed and clear explanation! |
RyanGlScott
added a commit
that referenced
this issue
Jun 1, 2023
This patch adds support for LLVM 15 and 16 by adding support for opaque pointers, which are described in https://llvm.org/docs/OpaquePointers.html. I have also added a test case involving LLVM bitcode using opaque pointers to kick the tires and ensure that the basics work as expected. For the most part, this is a straightforward process, as most uses of pointer types in SAW already do not care about pointee types. There are some exceptions, however: * The `typeOfSetupValue` function, as well as several places that use this function, scrutinize pointee types of pointers, which would appear to fly in the face of opaque pointers. I attempt to explain in #1876 which this is actually OK for now (although a bit awkward). * The `llvm_boilerplate`/skeleton machinery does not support opaque pointers at all. See #1877. * The `llvm_fresh_expanded_val` command does not support opaque pointers at all. See #1879. This patch also bumps the following submodules to bring in support for opaque pointers: * `llvm-pretty`: GaloisInc/llvm-pretty#110 * `llvm-pretty-bc-parser`: GaloisInc/llvm-pretty-bc-parser#221 * `crucible`: GaloisInc/crucible#1085 This also bumps the `what4` submodule to bring in the changes from GaloisInc/what4#234. This isn't necessary to support opaque pointers, but it _is_ necessary to support a build plan involving `tasty-sugar-2.2.*`, which `llvm-pretty-bc-parser`'s test suite now requires.
yav
pushed a commit
that referenced
this issue
Jun 16, 2023
This patch adds support for LLVM 15 and 16 by adding support for opaque pointers, which are described in https://llvm.org/docs/OpaquePointers.html. I have also added a test case involving LLVM bitcode using opaque pointers to kick the tires and ensure that the basics work as expected. For the most part, this is a straightforward process, as most uses of pointer types in SAW already do not care about pointee types. There are some exceptions, however: * The `typeOfSetupValue` function, as well as several places that use this function, scrutinize pointee types of pointers, which would appear to fly in the face of opaque pointers. I attempt to explain in #1876 which this is actually OK for now (although a bit awkward). * The `llvm_boilerplate`/skeleton machinery does not support opaque pointers at all. See #1877. * The `llvm_fresh_expanded_val` command does not support opaque pointers at all. See #1879. This patch also bumps the following submodules to bring in support for opaque pointers: * `llvm-pretty`: GaloisInc/llvm-pretty#110 * `llvm-pretty-bc-parser`: GaloisInc/llvm-pretty-bc-parser#221 * `crucible`: GaloisInc/crucible#1085 This also bumps the `what4` submodule to bring in the changes from GaloisInc/what4#234. This isn't necessary to support opaque pointers, but it _is_ necessary to support a build plan involving `tasty-sugar-2.2.*`, which `llvm-pretty-bc-parser`'s test suite now requires.
sauclovian-g
added
the
type: enhancement
Issues describing an improvement to an existing feature or capability
label
Nov 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
subsystem: crucible-llvm
Issues related to LLVM bitcode verification with crucible-llvm
tech debt
Issues that document or involve technical debt
type: enhancement
Issues describing an improvement to an existing feature or capability
I have recently updated SAW to work with LLVM's opaque pointers, an LLVM language change that replaces pointers with explicit pointee types (e.g.,
i32*
) with opaque pointers that lack pointee types. In an opaque pointer setting, all pointers have the typeptr
, and LLVM determines how to interpret the underlying memory based on the types of instructions that the pointers are used in. For more information, see the following links:llvm-pretty
PR to support opaque pointers: More complete support for opaque pointers (required for LLVM 15+) llvm-pretty#110llvm-pretty-bc-parser
PR to support opaque pointers: More complete support for opaque pointers (required for LLVM 15+) llvm-pretty-bc-parser#221crucible
PR to support opaque pointers: More complete support for opaque pointers (required for LLVM 15+) crucible#1085For the most part, updating code to support opaque pointers means auditing every use of non-opaque pointers (e.g.,
llvm-pretty
'sPtrTo
andcrucible-llvm
'sPtrType
) and making sure that the same code would work even if an opaque pointer were used instead. While that is generally possible in a SAW context, there is a place where this is not so straightforward: determining the types ofSetupValue
s. To see what I mean, look at this case in thetypeOfSetupValue
function:saw-script/src/SAWScript/Crucible/LLVM/ResolveSetupValue.hs
Lines 491 to 499 in 79f69bf
This pattern-matches on a
PtrType
value and then proceeds by cases on the underlying pointee type. Usually, this is a red flag, as such code would fail to work in an opaque pointer setting. At the same time, it's not so obvious how to write this code ifPtrType symTy
were replaced withPtrOpaque
. What would we use for thesymTy
in the code that follows?In fact, there is something more interesting going on. I claim that this use of
PtrType
is ever-so-slightly different from most other uses ofPtrType
in that it is specifically referring to the type of a SAW allocation, not just any old pointer type. Unlike LLVM pointers in general, which might lack pointee types, SAW allocations are always declared with an explicit pointee type via thellvm_alloc*
andllvm_fresh_pointer
family of commands. (I am not proposing that we changellvm_alloc*
andllvm_fresh_pointer
to get rid of their pointee type arguments, as this is pretty deeply baked into SAW's design.)As such, it is always fine to scrutinize a SAW allocation's pointee type.
typeOfSetupValue
isn't the only place where we scrutinize SAW allocations' pointee types, either. See also the following places in the code:saw-script/src/SAWScript/Crucible/LLVM/ResolveSetupValue.hs
Lines 563 to 567 in 79f69bf
saw-script/src/SAWScript/Crucible/LLVM/ResolveSetupValue.hs
Lines 394 to 395 in 79f69bf
saw-script/src/SAWScript/Crucible/LLVM/Builtins.hs
Lines 2686 to 2688 in 79f69bf
Note that all of these places are preceded by uses of
typeOfSetupValue
or a derived function.This current status quo technically works, but it is a bit uncomfortable, as it relies on the
llvm-pretty
/crucible-llvm
ASTs having special support for non-opaque pointers, a language feature which no longer exists in recent versions of LLVM. While it is unlikely that we will removePtrTo
/PtrType
any time soon (if ever), it is worth thinking about how the code above could be made to work in a world where the ASTs only supported non-opaque pointers.One way to do so would be to change the return type of
typeOfSetupValue
to use a slightly richer type thanMemType
. We could have it instead returnSetupMemType
, which might look roughly like:This would allow us to express the fact that SAW allocations have explicit pointee types while keeping it separate from
MemType
. Of course, we'd have to refactor quite a bit of code to adapt to this change of data type, including conversions fromSetupMemType
toMemType
and vice versa. For now, I am not going to perform this refactoring, as the current status quo works well enough, and it is unclear how much work the refactoring would take.The text was updated successfully, but these errors were encountered: