[llvm] r253222 - [ARM] Prevent use of a value pointed by end() iterator when placing a jump table

[David Blaikie via llvm-commits]

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

On Tue, Nov 17, 2015 at 12:37 AM, Petr Pavlu <petr.pavlu at arm.com> wrote:

> When I add the suggested test assertion, the following tests fail because
> they hit it:
> Failing Tests (4):
>     LLVM :: CodeGen/ARM/2008-04-11-PHIofImpDef.ll
>     LLVM :: CodeGen/ARM/2011-12-19-sjlj-clobber.ll
>     LLVM :: CodeGen/ARM/eh-dispcont.ll
>     LLVM :: CodeGen/ARM/sjljehprepare-lower-empty-struct.ll
>
> > From: David Blaikie [mailto:dblaikie at gmail.com]
> > Sent: 16 November 2015 16:58
> > To: Petr Pavlu
> > Cc: llvm-commits
> > Subject: Re: [llvm] r253222 - [ARM] Prevent use of a value pointed by
> end() iterator when placing a jump table
> >
> > Is this covered by existing tests? (if I replaced the if/continue with
> assert (!= end) would it fire on the existing regression suite?)
> >
> > On Mon, Nov 16, 2015 at 8:41 AM, Petr Pavlu via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> > Author: petr.pavlu
> > Date: Mon Nov 16 10:41:13 2015
> > New Revision: 253222
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=253222&view=rev
> > Log:
> > [ARM] Prevent use of a value pointed by end() iterator when placing a
> jump table
> >
> > Function ARMConstantIslands::doInitialJumpTablePlacement() iterates over
> all
> > basic blocks in a machine function. It calls `MI =
> MBB.getLastNonDebugInstr()`
> > to get the last instruction in each block and then uses MI->getOpcode()
> to
> > decide what to do. If getLastNonDebugInstr() returns MBB.end() (for
> example,
> > when the block does not contain any instructions) then calling
> getOpcode() on
> > this value is incorrect. Avoid this problem by checking the result of
> > getLastNonDebugInstr().
> >
> > Differential Revision: http://reviews.llvm.org/D14694
> >
> > Modified:
> >    llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
> >
> > Modified: llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp?rev=253222&r1=253221&r2=253222&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp (original)
> > +++ llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp Mon Nov 16
> 10:41:13 2015
> > @@ -589,6 +589,8 @@ void ARMConstantIslands::doInitialJumpTa
> >   MachineBasicBlock *LastCorrectlyNumberedBB = nullptr;
> >   for (MachineBasicBlock &MBB : *MF) {
> >     auto MI = MBB.getLastNonDebugInstr();
> > +    if (MI == MBB.end())
> > +      continue;
> >
> >     unsigned JTOpcode;
> >     switch (MI->getOpcode()) {
> >
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151117/de766262/attachment.html>