[llvm] r194428 - Fix the recently added anyregcc convention to handle spilled operands.

Andrew Trick atrick at apple.com
Mon Nov 11 19:47:41 PST 2013 

On Nov 11, 2013, at 7:11 PM, Sean Silva <silvas at purdue.edu> wrote:

>    StartIdx = isPatchPoint ?
> -             StartIdx + MI->getOperand(StartIdx+3).getImm() + 5 :
> +             StartIdx + NumCallArgs + 5 :
>               StartIdx + 2;
> 
> Magic numbers?

Yes, I absolutely hate referring to operands by position. I was also thinking of fixing this when I fixed the bug today, but haven’t figured out the right way to do it yet.
-Andy

> On Mon, Nov 11, 2013 at 5:40 PM, Andrew Trick <atrick at apple.com> wrote:
> Author: atrick
> Date: Mon Nov 11 16:40:25 2013
> New Revision: 194428
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=194428&view=rev
> Log:
> Fix the recently added anyregcc convention to handle spilled operands.
> 
> Fixes <rdar://15432754> [JS] Assertion: "Folded a def to a non-store!"
> 
> The primary purpose of anyregcc is to prevent a patchpoint's call
> arguments and return value from being spilled. They must be available
> in a register, although the calling convention does not pin the
> register. It's up to the front end to avoid using this convention for
> calls with more arguments than allocatable registers.
> 
> What happens otherwise? UB? Do we expose an API to indicate the maximum number available? This sounds ripe for turning into an exploitable vulnerability without some way for the API user to check their assumptions about how many registers LLVM will provide them with.
> 
> Also, btw, anyregcc is not documented in LangRef. Please update the documentation.
> 
> -- Sean Silva
>  
> 
> Modified:
>     llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
>     llvm/trunk/test/CodeGen/X86/anyregcc.ll
> 
> Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=194428&r1=194427&r2=194428&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Mon Nov 11 16:40:25 2013
> @@ -4207,10 +4207,19 @@ static MachineInstr* foldPatchpoint(Mach
>    MachineInstrBuilder MIB(MF, NewMI);
> 
>    bool isPatchPoint = MI->getOpcode() == TargetOpcode::PATCHPOINT;
> +  // For PatchPoint, the call args are not foldable.
> +  unsigned NumCallArgs = MI->getOperand(StartIdx+3).getImm();
>    StartIdx = isPatchPoint ?
> -             StartIdx + MI->getOperand(StartIdx+3).getImm() + 5 :
> +             StartIdx + NumCallArgs + 5 :
>               StartIdx + 2;
> 
> +  // Return false if any operands requested for folding are not foldable (not
> +  // part of the stackmap's live values).
> +  for (SmallVectorImpl<unsigned>::const_iterator I = Ops.begin(), E = Ops.end();
> +       I != E; ++I) {
> +    if (*I < StartIdx)
> +      return 0;
> +  }
>    // No need to fold return, the meta data, and function arguments
>    for (unsigned i = 0; i < StartIdx; ++i)
>      MIB.addOperand(MI->getOperand(i));
> 
> Modified: llvm/trunk/test/CodeGen/X86/anyregcc.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/anyregcc.ll?rev=194428&r1=194427&r2=194428&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/anyregcc.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/anyregcc.ll Mon Nov 11 16:40:25 2013
> @@ -8,11 +8,11 @@
>  ; Num Constants
>  ; CHECK-NEXT:   .long   0
>  ; Num Callsites
> -; CHECK-NEXT:   .long   6
> +; CHECK-NEXT:   .long   7
> 
>  ; test
>  ; CHECK-NEXT:   .long   0
> -; CHECK-NEXT:   .long   L{{.*}}-_test
> +; CHECK-LABEL:  .long   L{{.*}}-_test
>  ; CHECK-NEXT:   .short  0
>  ; 3 locations
>  ; CHECK-NEXT:   .short  3
> @@ -39,7 +39,7 @@ entry:
> 
>  ; property access 1 - %obj is an anyreg call argument and should therefore be in a register
>  ; CHECK-NEXT:   .long   1
> -; CHECK-NEXT:   .long   L{{.*}}-_property_access1
> +; CHECK-LABEL:  .long   L{{.*}}-_property_access1
>  ; CHECK-NEXT:   .short  0
>  ; 2 locations
>  ; CHECK-NEXT:   .short  2
> @@ -62,7 +62,7 @@ entry:
> 
>  ; property access 2 - %obj is an anyreg call argument and should therefore be in a register
>  ; CHECK-NEXT:   .long   2
> -; CHECK-NEXT:   .long   L{{.*}}-_property_access2
> +; CHECK-LABEL:  .long   L{{.*}}-_property_access2
>  ; CHECK-NEXT:   .short  0
>  ; 2 locations
>  ; CHECK-NEXT:   .short  2
> @@ -86,7 +86,7 @@ entry:
> 
>  ; property access 3 - %obj is a frame index
>  ; CHECK-NEXT:   .long   3
> -; CHECK-NEXT:   .long   L{{.*}}-_property_access3
> +; CHECK-LABEL:  .long   L{{.*}}-_property_access3
>  ; CHECK-NEXT:   .short  0
>  ; 2 locations
>  ; CHECK-NEXT:   .short  2
> @@ -110,7 +110,7 @@ entry:
> 
>  ; anyreg_test1
>  ; CHECK-NEXT:   .long   4
> -; CHECK-NEXT:   .long   L{{.*}}-_anyreg_test1
> +; CHECK-LABEL:  .long   L{{.*}}-_anyreg_test1
>  ; CHECK-NEXT:   .short  0
>  ; 14 locations
>  ; CHECK-NEXT:   .short  14
> @@ -193,7 +193,7 @@ entry:
> 
>  ; anyreg_test2
>  ; CHECK-NEXT:   .long   5
> -; CHECK-NEXT:   .long   L{{.*}}-_anyreg_test2
> +; CHECK-LABEL:  .long   L{{.*}}-_anyreg_test2
>  ; CHECK-NEXT:   .short  0
>  ; 14 locations
>  ; CHECK-NEXT:   .short  14
> @@ -274,6 +274,35 @@ entry:
>    ret i64 %ret
>  }
> 
> +; Test spilling the return value of an anyregcc call.
> +;
> +; <rdar://problem/15432754> [JS] Assertion: "Folded a def to a non-store!"
> +;
> +; CHECK-LABEL: .long 12
> +; CHECK-LABEL: .long L{{.*}}-_patchpoint_spilldef
> +; CHECK-NEXT: .short 0
> +; CHECK-NEXT: .short 3
> +; Loc 0: Register (some register that will be spilled to the stack)
> +; CHECK-NEXT: .byte  1
> +; CHECK-NEXT: .byte  0
> +; CHECK-NEXT: .short {{[0-9]+}}
> +; CHECK-NEXT: .long  0
> +; Loc 1: Register RDI
> +; CHECK-NEXT: .byte  1
> +; CHECK-NEXT: .byte  0
> +; CHECK-NEXT: .short 5
> +; CHECK-NEXT: .long  0
> +; Loc 1: Register RSI
> +; CHECK-NEXT: .byte  1
> +; CHECK-NEXT: .byte  0
> +; CHECK-NEXT: .short 4
> +; CHECK-NEXT: .long  0
> +define i64 @patchpoint_spilldef(i64 %p1, i64 %p2, i64 %p3, i64 %p4) {
> +entry:
> +  %result = tail call anyregcc i64 (i32, i32, i8*, i32, ...)* @llvm.experimental.patchpoint.i64(i32 12, i32 15, i8* inttoptr (i64 0 to i8*), i32 2, i64 %p1, i64 %p2)
> +  tail call void asm sideeffect "nop", "~{ax},~{bx},~{cx},~{dx},~{bp},~{si},~{di},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15}"() nounwind
> +  ret i64 %result
> +}
> +
>  declare void @llvm.experimental.patchpoint.void(i32, i32, i8*, i32, ...)
>  declare i64 @llvm.experimental.patchpoint.i64(i32, i32, i8*, i32, ...)
> -
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

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

More information about the llvm-commits mailing list