<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;"><div><br></div><div>Yikes, this completely escaped me. Shame on me!</div><div><br></div><div>Eric, thanks for jumping in and resolving the issue for Joerg in the short term. Looking at the problem, however, I think it is best to change to the source code by adding a no-inline attribute to vm_exec_core(), and fix more issues in the cloner.</div><div><br></div><div>We had some discussions why the compiler shouldn’t inline functions when it contains indirect branches and address of its labels are taken - at least unless there is analysis in place that determines it is safe to do so. Well, the issue here is related to that discussion: vm_exec_core(), where the address of label INSN_LABEL_finish is taken and assigned to a global variable, is inlined into rb_vm_get_insn. In the “good” the caller (rb_vm_get_insns_address_table()) takes the address of that label in vm_exec_core() [<font color="#b51a00">!!</font>] and stores it into a global.</div><div><br></div><div><div style="margin: 0px; font-family: Menlo;">; Function Attrs: nounwind ssp uwtable</div><div style="margin: 0px; font-family: Menlo;"><span style="color: #ce7924"> </span>define noalias i32* @rb_vm_get_insns_address_table() #0 {</div><div style="margin: 0px; font-family: Menlo;"><span style="color: #ce7924"> </span>entry:</div><div style="margin: 0px; font-family: Menlo;"><span style="color: #ce7924"> </span> store i32 ptrtoint (i8* blockaddress(<font color="#e32400">@vm_exec_core, %INSN_LABEL_finis</font>h) to i32), i32* @fi<span style="color: #ce7924"> </span>nish_insn_seq_0, align 4, !tbaa !1</div><div style="margin: 0px; font-family: Menlo;"><span style="color: #ce7924"> </span> ret i32* undef</div><div style="margin: 0px; font-family: Menlo; color: rgb(206, 121, 36);"> <span style="color: #000000">}</span></div></div><div style="margin: 0px; font-family: Menlo; color: rgb(206, 121, 36);"><span style="color: #000000"><br></span></div><div style="margin: 0px; font-family: Menlo;"><div style="margin: 0px;"><font color="#11053b">define i32 @vm_exec_core() #0 {</font></div><div style="margin: 0px;"><font color="#11053b">entry:</font></div><div style="margin: 0px;"><font color="#11053b"> store i32 ptrtoint (i8* blockaddress(@vm_exec_core, %INSN_LABEL_finish) to i32), i32* @fi nish_insn_seq_0, align 4, !tbaa !1</font></div><div style="margin: 0px;"><font color="#11053b"> br label %INSN_LABEL_finish</font></div><div style="margin: 0px;"><div style="margin: 0px;"><font color="#11053b">INSN_LABEL_finish: ; preds = %entry</font></div><div style="margin: 0px;"><font color="#11053b"> ret i32 undef</font></div></div></div><div style="margin: 0px; font-family: Menlo; color: rgb(206, 121, 36);"><span style="color: #000000"><br></span></div><div style="margin: 0px; font-family: Menlo; color: rgb(206, 121, 36);"><span style="color: #000000"><br></span></div><div style="margin: 0px;">With my patch the label became part of the callee (as it should be):</div><div style="margin: 0px;"><font face="Menlo"><br></font></div><div style="margin: 0px;"><div style="margin: 0px; font-family: Menlo;"> store i32 ptrtoint (i8* blockaddress(<font color="#e32400">@rb_vm_get_insns_address_table, %INSN_LABEL_finish.i</font><span style="color: #ce7924"> </span>) to i32), i32* @finish_insn_seq_0, align 4, !tbaa !1</div><div style="margin: 0px; font-family: Menlo;"><font color="#ce7924"> </font> br label %INSN_LABEL_finish.i</div></div><div><span style="color: #000000"><br></span></div><div>But then (still in the cloner) the branch is eliminated, the label removed and the final code reads:</div><div><br></div><div><div style="margin: 0px; font-family: Menlo;">define i32* @rb_vm_get_insns_address_table() #0 {</div><div style="margin: 0px; font-family: Menlo;">entry:</div><div style="margin: 0px; font-family: Menlo;"> store i32 <font color="#e32400">1</font>, i32* @finish_insn_seq_0, align 4, !tbaa !1 <font color="#4f7a28">// The jump to address 1 causes the seqfault</font></div><div style="margin: 0px; font-family: Menlo;"><span style="color: #ce7924"> </span> ret i32* undef</div><div style="margin: 0px; font-family: Menlo; color: rgb(206, 121, 36);"><span style="color: #000000">}</span></div></div><div style="margin: 0px; font-family: Menlo; color: rgb(206, 121, 36);"><span style="color: #000000"><br></span></div><div style="margin: 0px;">Conclusion:</div><div style="margin: 0px;">1) I suggest adding the no-inline attribute to vm_exec_core() to be on the safe side. Note the actual code may not segfault, but could be incorrect. </div><div style="margin: 0px;">2) Reapply the patch</div><div style="margin: 0px;">3) Suppress inlining when the address of a label is taken (until there is analysis in place that determines inlining is safe). See previous discussion in context with indirect branches. </div><div style="margin: 0px;">4) CloneFunction.cpp should not optimize away labels whose address is taken (another bug lurking in this code). Note that outside the cloner there is no issue since the label in vm_exec_core() is not touched.</div><div style="margin: 0px; font-family: Menlo;"><br></div><div>With the patch just taking care of 4) (and ignoring 3)) is probably enough since the label becomes part of the caller.</div><div><br></div><div>What do you think?</div><div><br></div><div>Cheers</div><div>Gerolf</div><div><br></div><div><br><div><div>On May 19, 2014, at 9:15 AM, Joerg Sonnenberger <<a href="mailto:joerg@britannica.bec.de">joerg@britannica.bec.de</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Mon, May 19, 2014 at 09:11:46AM -0700, Eric Christopher wrote:<br><blockquote type="cite">Hi Gerolf,<br><br>I've gone ahead and reverted this in r209135.<br><br>Joerg: What does Gerolf need to do other than compile the files? Link<br>them? Compile them together? A set of command lines might be good.<br></blockquote><br>Compile them together and run it. The broken version will hit the<br>segfault handler.<br><br>Joerg<br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><br></div></body></html>