<div dir="ltr">SGTM (bonus points would be to have a positive test for the behavior here - but since the current code is UB the observable behavior may be "exactly what you expect" which isn't helpful), please commit whenever you're ready </div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 17, 2015 at 12:37 AM, Petr Pavlu <span dir="ltr"><<a href="mailto:petr.pavlu@arm.com" target="_blank">petr.pavlu@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">When I add the suggested test assertion, the following tests fail because they hit it:<br>
Failing Tests (4):<br>
    LLVM :: CodeGen/ARM/2008-04-11-PHIofImpDef.ll<br>
    LLVM :: CodeGen/ARM/2011-12-19-sjlj-clobber.ll<br>
    LLVM :: CodeGen/ARM/eh-dispcont.ll<br>
    LLVM :: CodeGen/ARM/sjljehprepare-lower-empty-struct.ll<br>
<br>
> From: David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>]<br>
> Sent: 16 November 2015 16:58<br>
> To: Petr Pavlu<br>
> Cc: llvm-commits<br>
> Subject: Re: [llvm] r253222 - [ARM] Prevent use of a value pointed by end() iterator when placing a jump table<br>
<div class="HOEnZb"><div class="h5">><br>
> Is this covered by existing tests? (if I replaced the if/continue with assert (!= end) would it fire on the existing regression suite?)<br>
><br>
> On Mon, Nov 16, 2015 at 8:41 AM, Petr Pavlu via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: petr.pavlu<br>
> Date: Mon Nov 16 10:41:13 2015<br>
> New Revision: 253222<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=253222&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=253222&view=rev</a><br>
> Log:<br>
> [ARM] Prevent use of a value pointed by end() iterator when placing a jump table<br>
><br>
> Function ARMConstantIslands::doInitialJumpTablePlacement() iterates over all<br>
> basic blocks in a machine function. It calls `MI = MBB.getLastNonDebugInstr()`<br>
> to get the last instruction in each block and then uses MI->getOpcode() to<br>
> decide what to do. If getLastNonDebugInstr() returns MBB.end() (for example,<br>
> when the block does not contain any instructions) then calling getOpcode() on<br>
> this value is incorrect. Avoid this problem by checking the result of<br>
> getLastNonDebugInstr().<br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D14694" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14694</a><br>
><br>
> Modified:<br>
>    llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp<br>
><br>
> Modified: llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp?rev=253222&r1=253221&r2=253222&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp?rev=253222&r1=253221&r2=253222&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp (original)<br>
> +++ llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp Mon Nov 16 10:41:13 2015<br>
> @@ -589,6 +589,8 @@ void ARMConstantIslands::doInitialJumpTa<br>
>   MachineBasicBlock *LastCorrectlyNumberedBB = nullptr;<br>
>   for (MachineBasicBlock &MBB : *MF) {<br>
>     auto MI = MBB.getLastNonDebugInstr();<br>
> +    if (MI == MBB.end())<br>
> +      continue;<br>
><br>
>     unsigned JTOpcode;<br>
>     switch (MI->getOpcode()) {<br>
><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div>