[LLVMbugs] [Bug 20810] New: inconsistent llvm_unreachable handling causes infinite loops in release builds

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=20810

            Bug ID: 20810
           Summary: inconsistent llvm_unreachable handling causes infinite
                    loops in release builds
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: Support Libraries
          Assignee: unassignedbugs at nondot.org
          Reporter: dberlin at dberlin.org
                CC: llvmbugs at cs.uiuc.edu
    Classification: Unclassified

Reproduction:

Build with CMAKE_BUILD_TYPE=Release on Linux with compiler set to G++ and using
g++ > 4.5.


run llvm-lit on /test/CodeGen/R600/infinite-loop-evergreen.ll

It will take forever (not to *execute*, but to *compile*)

Here is what happens:
lib/Target/R600/AMDILCFGStructurizer.cpp has the following code at line 1737:

 MachineBasicBlock::iterator I = BranchMI;
  unsigned ImmReg = FuncRep->getRegInfo().createVirtualRegister(I32RC);
  llvm_unreachable("Extra register needed to handle CFG");
  MachineInstr *NewMI = insertInstrBefore(I, AMDGPU::BRANCH_COND_i32);
  MachineInstrBuilder MIB(*FuncRep, NewMI);
  MIB.addMBB(LoopHeader);
  MIB.addReg(ImmReg, false);
  SHOWNEWINSTR(NewMI);
  BranchMI->eraseFromParent();
  LoopLatch->addSuccessor(DummyExitBlk);


It expects the llvm_unreachable to abort (as do a lot of other places in the
compiler), but when it doesn't, it just happily continues on (going around and
around forever, it turns out)

include/Support/ErrorHandling.h has this:

/// Use this instead of assert(0).  It conveys intent more clearly and
/// allows compilers to omit some unnecessary code.
#ifndef NDEBUG
#define llvm_unreachable(msg) \
  ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
#elif defined(LLVM_BUILTIN_UNREACHABLE)
#define llvm_unreachable(msg) LLVM_BUILTIN_UNREACHABLE
#else
#define llvm_unreachable(msg) ::llvm::llvm_unreachable_internal()
#endif

(Note the request to use instead of assert(0) )


llvm_unreachable_internal prints a message, calls *abort*, then calls
LLVM_BUILTIN_UNREACHABLE if it is defined.

But as per above, if LLVM_BUILTIN_UNREACHABLE and NDEBUG are defined, we just
call LLVM_BUILTIN_UNREACHABLE

LLVM_BUILTIN_UNREACHABLE is defined as:

/// LLVM_BUILTIN_UNREACHABLE - On compilers which support it, expands
/// to an expression which states that it is undefined behavior for the
/// compiler to reach this point.  Otherwise is not defined.
#if __has_builtin(__builtin_unreachable) || __GNUC_PREREQ(4, 5)
# define LLVM_BUILTIN_UNREACHABLE __builtin_unreachable()
#elif defined(_MSC_VER)
# define LLVM_BUILTIN_UNREACHABLE __assume(false)
#endif


Thus, this llvm_unreachable expression transforms to builtin_unreachable on GCC
4.5 or above (I expect windows has a similar problem unless assume also
aborts).

However, builtin_unreachable *does not abort*. In fact, in most cases, it does
nothing.  GCC only defines it as a hint to the compiler.


If we expect llvm_unreachable to abort, the right solution is to just remove
the line
#elif defined(LLVM_BUILTIN_UNREACHABLE)
#define llvm_unreachable(msg) LLVM_BUILTIN_UNREACHABLE

It will then always do the right thing in llvm_unreachable_internal (including
calling the unreachable builtin).

If we don't expect it to abort, then the code in places like above needs to be
modified to abort rather than infinite loop forever.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20140829/89d27710/attachment.html>