[llvm] r194428 - Fix the recently added anyregcc convention to handle spilled operands.
Sean Silva
silvas at purdue.edu
Tue Nov 12 18:09:55 PST 2013
On Tue, Nov 12, 2013 at 7:42 PM, Andrew Trick <atrick at apple.com> wrote:
>
> 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"
>
Is this one of those checks that evaporates in release builds? (Sorry for
nagging on this but I just want to be double-triple sure since a slip up in
this regard is basically a guaranteed to be an exploitable security
vulnerability of the worst kind, and it's not difficult to imagine a
scenario where this number gets goofed via a typo, or across architectures,
etc.; If it doesn't have one already, I would recommend making sure that
JSC has a test that pushes up against the limit of this number)
>
> 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.
>
Oof, sorry about dropping that on the floor!
-- Sean Silva
>
> -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/7b956b59/attachment.html>
More information about the llvm-commits
mailing list