[PATCH] Make variable argument intrinsics behave correctly in a Win64 CC function.
cdavis5x at gmail.com
Sat Sep 21 11:54:45 PDT 2013
On Sep 16, 2013, at 4:57 PM, Eli Friedman wrote:
> On Fri, Sep 13, 2013 at 11:38 AM, Charles Davis <cdavis5x at gmail.com> wrote:
> Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:1941
> @@ +1940,3 @@
> + if (Cast->getSrcTy() ==
> + Type::getInt8PtrTy(Cast->getContext())->getPointerTo())
> + return;
> Evgeniy Stepanov wrote:
> > Evgeniy Stepanov wrote:
> > > This looks very fragile.
> > > Why can't you decide this based on the calling convention of the instrumented function?
> > OK, va_list may be from a function with a different calling convention.
> > I don't see a better way to detect this, but... how does va_copy work in this case? It gets plain i8* as an argument.
> Remember how LLVM IR works--it's single-static assignment. All instructions are values. The IR for a Win64 ABI `va_copy` typically looks something like this:
> %ap = alloca i8*, align 8
> %cp = alloca i8*, align 8
> %ap1 = bitcast i8** %ap to i8*
> %cp1 = bitcast i8** %cp to i8*
> call void @llvm.va_copy(i8* %cp1, i8* %ap1)
> So, I `dyn_cast` the argument to an `llvm::BitCastInst` (and hope that the optimizer didn't make the bitcast go away--yes, this is very fragile, but I can't think of any better way to do this), and check if the source type is a `i8**` (the argument to the `bitcast` is a pointer to the `va_list` itself). If it is, we assume it's a Win64 ABI `va_copy`, else, a System V ABI `va_copy`.
> Now that I think about it, it might be better if I just add a new intrinsic (`llvm.ms_va_copy`, maybe?) to handle Win64 ABI `va_copy` calls, but when I tried to do it this way, I couldn't work out how to use the generic lowering code (that I pulled out of `LegalizeDAG`) to lower it. (In retrospect, I should probably have asked about it earlier on the list.)
> If you can't reuse the code, it's fine... it's only 10 lines.
> Alternatively, you might want to consider modifying the existing va_copy variadic intrinsic to use llvm_anyptr_ty.
Done (cf. D1737).
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits