[PATCH] Make variable argument intrinsics behave correctly in a Win64 CC function.

[Charles Davis]

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:
> 
>   lang=llvm
>   %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).

Chip

> 
> -Eli

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130921/29bfc8c4/attachment.html>