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

Andrew Trick atrick at apple.com
Tue Nov 12 16:42:06 PST 2013 

On Nov 12, 2013, at 1:10 PM, Eric Christopher <echristo at gmail.com> wrote:

> On Mon, Nov 11, 2013 at 7:47 PM, Andrew Trick <atrick at apple.com> wrote:
>> 
>> 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.
> 
> Enum along with the instruction?

:) That’s what I’ve done in other compilers. So how do we do it with LLVM target opcodes? Any examples?

> Also the documentation as pointed out by Sean. Please do document the
> cases he describes and probably update the code to fail in some sort
> of spectacular manner.

anyregcc is meant to be used for patching code sequences in place of a call where we just need 2-3 incoming values guaranteed to be in registers. If a JIT does ask for more register args than the platform can handle, which would be completely insane, then they get this spectacular error:

"LLVM ERROR: ran out of registers during register allocation"

If a front end requests anyregcc on a call that is not a patchpoint they get:

"The AnyReg calling convention is only supported by the stackmap and patchpoint intrinsics."

It would be so great to be able to check these docs in:
http://llvm-reviews.chandlerc.com/D1981

You and Sean are the only reviewers I’m not sure about.

-Andy

> 
> -eric
> 
>> -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
>> 
>> 
>> 
>> 
>> _______________________________________________
>> 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/20131112/a3c083c1/attachment.html>

More information about the llvm-commits mailing list