-
Notifications
You must be signed in to change notification settings - Fork 6
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
call
/callbr
's function type should not be a pointer type
#189
Comments
After some further investigation (prompted by the opaque pointer switchover—see #177), I've actually come to the opposite conclusion from this issue's title. Namely,
I had mistakenly assumed that #include <stdio.h>
void f() {
int (*p)(const char*, ...) = &printf;
p("%d\n", 0);
} What should the define dso_local void @f() #0 !dbg !17 {
entry:
%p = alloca ptr, align 8
call void @llvm.dbg.declare(metadata ptr %p, metadata !21, metadata !DIExpression()), !dbg !28
store ptr @printf, ptr %p, align 8, !dbg !28
%0 = load ptr, ptr %p, align 8, !dbg !29
%call = call i32 (ptr, ...) %0(ptr noundef @.str, i32 noundef 0), !dbg !29
ret void, !dbg !30
} In particular,
Long story short: in order to support opaque pointers in the future, we will need to change the |
invoke
's function should have a pointer typecall
/callbr
's function should not be a pointer type
call
/callbr
's function should not be a pointer typecall
/callbr
's function type should not be a pointer type
There are a handful of places in `crucible-llvm` where we assume that the `Call` and `CallBr` instructions have pointer types. As noted in GaloisInc/llvm-pretty-bc-parser#189, however, this is subtly incorrect. This patch: * Bumps the `llvm-pretty-bc-parser` submodule to bring in the changes from GaloisInc/llvm-pretty-bc-parser#207, which corrects this mistake, and * Changes the parts of `crucible-llvm` that match on pointer type to no longer do so.
There are a handful of places in `crucible-llvm` where we assume that the `Call` and `CallBr` instructions have pointer types. As noted in GaloisInc/llvm-pretty-bc-parser#189, however, this is subtly incorrect. This patch: * Bumps the `llvm-pretty-bc-parser` submodule to bring in the changes from GaloisInc/llvm-pretty-bc-parser#207, which corrects this mistake. * Bumps the `llvm-pretty` submodule to be in tandem with `llvm-pretty-bc-parser`. * Changes the parts of `crucible-llvm` that match on pointer type to no longer do so.
Previously, the code in `ppCallSym` assumed that the function type stored in a `call` instruction was a pointer type, but this should actually be a raw function type. See the discussion at GaloisInc/llvm-pretty-bc-parser#189 (comment) for more on this. We now assume the convention of raw function types in `call` instructions, as well as the related `callbr` and `invoke` instructions. This addresses one part of #102.
Previously, the code in `ppCallSym` assumed that the function type stored in a `call` instruction was a pointer type, but this should actually be a raw function type. See the discussion at GaloisInc/llvm-pretty-bc-parser#189 (comment) for more on this. We now assume the convention of raw function types in `call` instructions, as well as the related `callbr` and `invoke` instructions. This addresses one part of #102.
There are a handful of places in `crucible-llvm` where we assume that the `Call` and `CallBr` instructions have pointer types. As noted in GaloisInc/llvm-pretty-bc-parser#189, however, this is subtly incorrect. This patch: * Bumps the `llvm-pretty-bc-parser` submodule to bring in the changes from GaloisInc/llvm-pretty-bc-parser#207, which corrects this mistake. * Bumps the `llvm-pretty` submodule to be in tandem with `llvm-pretty-bc-parser`. Note that because this brings in the changes from GaloisInc/llvm-pretty#104, I opted to explicitly disallow LLVM's opaque pointers in `crucible-llvm` for now, but this is something that will likely change in subsequent patches. * Changes the parts of `crucible-llvm` that match on pointer types to no longer do so.
If you parse an
invoke
instruction, such as these one found incrux-llvm
'sinvoke-test.ll
test case, the resulting IR will be:The thing that is peculiar about this IR is that the argument type is a raw
FunTy
. Reading the LLVM Language Reference Manual entry forinvoke
, however, suggests that this should instead be aPtrTo
:Making this a
PtrTo
would also match the treatment forcall
andcallbr
, two closely related instructions. In fact, in the source of of LLVM, the classes forcall
,callbr
, andinvoke
are all subclasses of the sameCallBase
class, so they all store their function type in the same way.The text was updated successfully, but these errors were encountered: