<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Nov 12, 2013, at 6:09 PM, Sean Silva <<a href="mailto:silvas@purdue.edu">silvas@purdue.edu</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="gmail_extra"><br class="Apple-interchange-newline"><br><br><div class="gmail_quote">On Tue, Nov 12, 2013 at 7:42 PM, Andrew Trick<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><br><div><div class="im"><div>On Nov 12, 2013, at 1:10 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;">On Mon, Nov 11, 2013 at 7:47 PM, Andrew Trick <<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>> wrote:<br><blockquote type="cite"><br>On Nov 11, 2013, at 7:11 PM, Sean Silva <<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>> wrote:<br><br> StartIdx = isPatchPoint ?<br>- StartIdx + MI->getOperand(StartIdx+3).getImm() + 5 :<br>+ StartIdx + NumCallArgs + 5 :<br> StartIdx + 2;<br><br>Magic numbers?<br><br><br>Yes, I absolutely hate referring to operands by position. I was also<br>thinking of fixing this when I fixed the bug today, but haven’t figured out<br>the right way to do it yet.<br></blockquote><br>Enum along with the instruction?<br></div></blockquote><div><br></div></div>:) That’s what I’ve done in other compilers. So how do we do it with LLVM target opcodes? Any examples?</div><div><div class="im"><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;">Also the documentation as pointed out by Sean. Please do document the<br>cases he describes and probably update the code to fail in some sort<br>of spectacular manner.<br></div></blockquote><div><br></div></div><div>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:</div><div><br></div>"LLVM ERROR: ran out of registers during register allocation"</div></div></blockquote><div><br></div><div>Is this one of those checks that evaporates in release builds? </div></div></div></div></blockquote><div dir="auto"><br></div><div dir="auto">You will see this error in release builds. It triggers the LLVM fatal error handler.</div><br><blockquote type="cite"><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="gmail_extra"><div class="gmail_quote"><div>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</div></div></div></div></blockquote><div><br></div><div>Unless I'm missing something, the worst case is that you die in a fire while compiling. This is harmless from a security standpoint.</div><div><br></div><div>Even if this wasn't true, I still wouldn't be trippin'. Of course if you have a JIT then it carries security vulnerabilities. There are many, many ways in which JITs have to be careful to ensure that they don't generate wrong code that is exploitable. I don't think there is any way that LLVM could validate or verify that the IR it receives doesn't contain exploitable nonsense. A far bigger worry than patchpoint argument overflow would be that the generated IR has a buffer overflow.</div><br><blockquote type="cite"><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="gmail_extra"><div class="gmail_quote"><div>, 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)</div></div></div></div></blockquote><div><br></div><div>JSC won't have any tests for this. In JSC there is no such thing as an inline cache (i.e. patchpoint) that uses AnyRegCC and has a variable number of arguments. I suspect that this can be generalized to other clients of patchpoints. To me, the only reason why it's vararg on the LLVM side is to ensure that LLVM doesn't have to know anything about the source language construct that caused it.</div><div><br></div><div>As an example, in JS, inline caches arise from heap access productions, like:</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>o.f = v</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>v = o.f</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>o[i] = v</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>v = o[i]</div><div><br></div><div>and so on. Notice that in each of these there is never more than three "operands" to the production:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span></div><div><span class="Apple-tab-span" style="white-space: pre;"> </span>o.f = v // two operands (o, v)</div><div><span class="Apple-tab-span" style="white-space: pre;"> </span>v = o.f // one operand (o)</div><div><span class="Apple-tab-span" style="white-space: pre;"> </span>o[i] = v // three operands (o, i, v)</div><div><span class="Apple-tab-span" style="white-space: pre;"> </span>v = o[i] // two operands (o, i)</div><div><br></div><div>It makes no sense for JSC to test for an inline cache that has more than three operands because there is no such construct in JavaScript. There is no way for our frontend to generate a patchpoint that isn't for one of these constructs. Generalizing again, I don't think that other languages have constructs that would benefit from IC and that use more than three or so operands.</div><div><br></div><div>And FWIW, JSC's FTL JIT only uses inline caches for the o.f case, so two operands is the max. AFAICT, array accesses benefit much more from speculative type inference than from self-modifying code.</div><div><br></div><div>For calls, which do have arbitrary numbers of arguments, we do use inline caches but we never use AnyRegCC - we use WebKitJS instead. In general, I don't think it would make sense for anyone to use AnyRegCC for a patchpoint that is being used to implement calls, since for calls you want LLVM to put the call arguments into your calling convention's places instead of putting them "anywhere".</div><div><br></div><div>So - in the particular case of JSC there is no way for a patchpoint to have more than 2 arguments right now or 3 arguments in any hypothetical future. In the general case I don't see a practical application of patchpoint+AnyRegCC that involves more than a small constant number of arguments (and that constant is 3 for any dynamic language construct I can think of right now).</div><div><br></div><div>OTOH, since patchpoint AnyRegCC argument overflow causes the fatal error handler to be triggered, you could sort of imagine writing a test for this on the LLVM side.</div><br><blockquote type="cite"><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div><br></div><div>If a front end requests anyregcc on a call that is not a patchpoint they get:</div><div><br></div><div>"The AnyReg calling convention is only supported by the stackmap and patchpoint intrinsics."</div><div><div><br></div><div>It would be so great to be able to check these docs in:</div><a href="http://llvm-reviews.chandlerc.com/D1981" target="_blank">http://llvm-reviews.chandlerc.com/D1981</a></div><div><br></div><div>You and Sean are the only reviewers I’m not sure about.</div></div></blockquote><div><br></div><div>Oof, sorry about dropping that on the floor!</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div><br></div><div>-Andy</div><div><div class="h5"><div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><br>-eric<br><br><blockquote type="cite">-Andy<br><br>On Mon, Nov 11, 2013 at 5:40 PM, Andrew Trick <<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>> wrote:<br><blockquote type="cite"><br>Author: atrick<br>Date: Mon Nov 11 16:40:25 2013<br>New Revision: 194428<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=194428&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=194428&view=rev</a><br>Log:<br>Fix the recently added anyregcc convention to handle spilled operands.<br><br>Fixes <<a>rdar://15432754</a>> [JS] Assertion: "Folded a def to a non-store!"<br><br>The primary purpose of anyregcc is to prevent a patchpoint's call<br>arguments and return value from being spilled. They must be available<br>in a register, although the calling convention does not pin the<br>register. It's up to the front end to avoid using this convention for<br>calls with more arguments than allocatable registers.<br></blockquote><br><br>What happens otherwise? UB? Do we expose an API to indicate the maximum<br>number available? This sounds ripe for turning into an exploitable<br>vulnerability without some way for the API user to check their assumptions<br>about how many registers LLVM will provide them with.<br><br>Also, btw, anyregcc is not documented in LangRef. Please update the<br>documentation.<br><br>-- Sean Silva<br><br><blockquote type="cite"><br><br>Modified:<br> llvm/trunk/lib/Target/X86/X86InstrInfo.cpp<br> llvm/trunk/test/CodeGen/X86/anyregcc.ll<br><br>Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp<br>URL:<br><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=194428&r1=194427&r2=194428&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=194428&r1=194427&r2=194428&view=diff</a><br><br>==============================================================================<br>--- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)<br>+++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Mon Nov 11 16:40:25 2013<br>@@ -4207,10 +4207,19 @@ static MachineInstr* foldPatchpoint(Mach<br> MachineInstrBuilder MIB(MF, NewMI);<br><br> bool isPatchPoint = MI->getOpcode() == TargetOpcode::PATCHPOINT;<br>+ // For PatchPoint, the call args are not foldable.<br>+ unsigned NumCallArgs = MI->getOperand(StartIdx+3).getImm();<br> StartIdx = isPatchPoint ?<br>- StartIdx + MI->getOperand(StartIdx+3).getImm() + 5 :<br>+ StartIdx + NumCallArgs + 5 :<br> StartIdx + 2;<br><br>+ // Return false if any operands requested for folding are not foldable<br>(not<br>+ // part of the stackmap's live values).<br>+ for (SmallVectorImpl<unsigned>::const_iterator I = Ops.begin(), E =<br>Ops.end();<br>+ I != E; ++I) {<br>+ if (*I < StartIdx)<br>+ return 0;<br>+ }<br> // No need to fold return, the meta data, and function arguments<br> for (unsigned i = 0; i < StartIdx; ++i)<br> MIB.addOperand(MI->getOperand(i));<br><br>Modified: llvm/trunk/test/CodeGen/X86/anyregcc.ll<br>URL:<br><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/anyregcc.ll?rev=194428&r1=194427&r2=194428&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/anyregcc.ll?rev=194428&r1=194427&r2=194428&view=diff</a><br><br>==============================================================================<br>--- llvm/trunk/test/CodeGen/X86/anyregcc.ll (original)<br>+++ llvm/trunk/test/CodeGen/X86/anyregcc.ll Mon Nov 11 16:40:25 2013<br>@@ -8,11 +8,11 @@<br>; Num Constants<br>; CHECK-NEXT: .long 0<br>; Num Callsites<br>-; CHECK-NEXT: .long 6<br>+; CHECK-NEXT: .long 7<br><br>; test<br>; CHECK-NEXT: .long 0<br>-; CHECK-NEXT: .long L{{.*}}-_test<br>+; CHECK-LABEL: .long L{{.*}}-_test<br>; CHECK-NEXT: .short 0<br>; 3 locations<br>; CHECK-NEXT: .short 3<br>@@ -39,7 +39,7 @@ entry:<br><br>; property access 1 - %obj is an anyreg call argument and should<br>therefore be in a register<br>; CHECK-NEXT: .long 1<br>-; CHECK-NEXT: .long L{{.*}}-_property_access1<br>+; CHECK-LABEL: .long L{{.*}}-_property_access1<br>; CHECK-NEXT: .short 0<br>; 2 locations<br>; CHECK-NEXT: .short 2<br>@@ -62,7 +62,7 @@ entry:<br><br>; property access 2 - %obj is an anyreg call argument and should<br>therefore be in a register<br>; CHECK-NEXT: .long 2<br>-; CHECK-NEXT: .long L{{.*}}-_property_access2<br>+; CHECK-LABEL: .long L{{.*}}-_property_access2<br>; CHECK-NEXT: .short 0<br>; 2 locations<br>; CHECK-NEXT: .short 2<br>@@ -86,7 +86,7 @@ entry:<br><br>; property access 3 - %obj is a frame index<br>; CHECK-NEXT: .long 3<br>-; CHECK-NEXT: .long L{{.*}}-_property_access3<br>+; CHECK-LABEL: .long L{{.*}}-_property_access3<br>; CHECK-NEXT: .short 0<br>; 2 locations<br>; CHECK-NEXT: .short 2<br>@@ -110,7 +110,7 @@ entry:<br><br>; anyreg_test1<br>; CHECK-NEXT: .long 4<br>-; CHECK-NEXT: .long L{{.*}}-_anyreg_test1<br>+; CHECK-LABEL: .long L{{.*}}-_anyreg_test1<br>; CHECK-NEXT: .short 0<br>; 14 locations<br>; CHECK-NEXT: .short 14<br>@@ -193,7 +193,7 @@ entry:<br><br>; anyreg_test2<br>; CHECK-NEXT: .long 5<br>-; CHECK-NEXT: .long L{{.*}}-_anyreg_test2<br>+; CHECK-LABEL: .long L{{.*}}-_anyreg_test2<br>; CHECK-NEXT: .short 0<br>; 14 locations<br>; CHECK-NEXT: .short 14<br>@@ -274,6 +274,35 @@ entry:<br> ret i64 %ret<br>}<br><br>+; Test spilling the return value of an anyregcc call.<br>+;<br>+; <<a href="rdar://problem/15432754">rdar://problem/15432754</a>> [JS] Assertion: "Folded a def to a<br>non-store!"<br>+;<br>+; CHECK-LABEL: .long 12<br>+; CHECK-LABEL: .long L{{.*}}-_patchpoint_spilldef<br>+; CHECK-NEXT: .short 0<br>+; CHECK-NEXT: .short 3<br>+; Loc 0: Register (some register that will be spilled to the stack)<br>+; CHECK-NEXT: .byte 1<br>+; CHECK-NEXT: .byte 0<br>+; CHECK-NEXT: .short {{[0-9]+}}<br>+; CHECK-NEXT: .long 0<br>+; Loc 1: Register RDI<br>+; CHECK-NEXT: .byte 1<br>+; CHECK-NEXT: .byte 0<br>+; CHECK-NEXT: .short 5<br>+; CHECK-NEXT: .long 0<br>+; Loc 1: Register RSI<br>+; CHECK-NEXT: .byte 1<br>+; CHECK-NEXT: .byte 0<br>+; CHECK-NEXT: .short 4<br>+; CHECK-NEXT: .long 0<br>+define i64 @patchpoint_spilldef(i64 %p1, i64 %p2, i64 %p3, i64 %p4) {<br>+entry:<br>+ %result = tail call anyregcc i64 (i32, i32, i8*, i32, ...)*<br>@llvm.experimental.patchpoint.i64(i32 12, i32 15, i8* inttoptr (i64 0 to<br>i8*), i32 2, i64 %p1, i64 %p2)<br>+ tail call void asm sideeffect "nop",<br>"~{ax},~{bx},~{cx},~{dx},~{bp},~{si},~{di},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15}"()<br>nounwind<br>+ ret i64 %result<br>+}<br>+<br>declare void @llvm.experimental.patchpoint.void(i32, i32, i8*, i32, ...)<br>declare i64 @llvm.experimental.patchpoint.i64(i32, i32, i8*, i32, ...)<br>-<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></blockquote></div><br></div></div></div></blockquote></div><br></div></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;">llvm-commits mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;"><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"></blockquote></div><br></body></html>