I see the following crash:
16:10:04 ~/3rd/llvm/test> opt -newgvn CodeGen/X86/multiple-loop-post-inc.ll WARNING: You're attempting to print out a bitcode file. This is inadvisable as it may cause display problems. If you REALLY want to taste LLVM bitcode first-hand, you can force output with the `-f' option. opt: /llvm/lib/Transforms/Scalar/NewGVN.cpp:734: unsigned int (anonymous namespace)::NewGVN::InstrToDFSNum(const llvm::Value *) const: Assertion `isa<Instruction>(V) && "This should not be used for MemoryAccesses"' failed. #0 0x00007fe792dac8c9 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /llvm/build/../lib/Support/Unix/Signals.inc:398:11 #1 0x00007fe792dacae9 PrintStackTraceSignalHandler(void*) /llvm/build/../lib/Support/Unix/Signals.inc:462:1 #2 0x00007fe792da99a7 llvm::sys::RunSignalHandlers() /llvm/lib/Support/Signals.cpp:0:5 #3 0x00007fe792dace70 SignalHandler(int) /llvm/build/../lib/Support/Unix/Signals.inc:252:1 #4 0x00007fe792083890 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0xf890) #5 0x00007fe7916d6067 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x35067) #6 0x00007fe7916d7448 abort (/lib/x86_64-linux-gnu/libc.so.6+0x36448) #7 0x00007fe7916cf266 (/lib/x86_64-linux-gnu/libc.so.6+0x2e266) #8 0x00007fe7916cf312 (/lib/x86_64-linux-gnu/libc.so.6+0x2e312) #9 0x00007fe79328f4e5 (anonymous namespace)::NewGVN::InstrToDFSNum(llvm::Value const*) const /llvm/lib/Transforms/Scalar/NewGVN.cpp:735:12 #10 0x00007fe79329f393 void (anonymous namespace)::NewGVN::touchAndErase<llvm::DenseMap<llvm::Value const*, llvm::SmallPtrSet<llvm::Value*, 2u>, llvm::DenseMapInfo<llvm::Value const*>, llvm::detail::DenseMapPair<llvm::Value const*, llvm::SmallPtrSet<llvm::Value*, 2u> > >, llvm::Value*>(llvm::DenseMap<llvm::Value const*, llvm::SmallPtrSet<llvm::Value*, 2u>, llvm::DenseMapInfo<llvm::Value const*>, llvm::detail::DenseMapPair<llvm::Value const*, llvm::SmallPtrSet<llvm::Value*, 2u> > >&, llvm::Value* const&) /llvm/lib/Transforms/Scalar/NewGVN.cpp:1947:27 #11 0x00007fe79329ea05 (anonymous namespace)::NewGVN::markUsersTouched(llvm::Value*) /llvm/lib/Transforms/Scalar/NewGVN.cpp:1963:1 #12 0x00007fe793294c2a (anonymous namespace)::NewGVN::performCongruenceFinding(llvm::Instruction*, llvm::GVNExpression::Expression const*) /llvm/lib/Transforms/Scalar/NewGVN.cpp:2275:44 #13 0x00007fe793290c15 (anonymous namespace)::NewGVN::valueNumberInstruction(llvm::Instruction*) /llvm/lib/Transforms/Scalar/NewGVN.cpp:2814:5 #14 0x00007fe7932898c6 (anonymous namespace)::NewGVN::iterateTouchedInstructions() /llvm/lib/Transforms/Scalar/NewGVN.cpp:3092:7 #15 0x00007fe793287a70 (anonymous namespace)::NewGVN::runGVN() /llvm/lib/Transforms/Scalar/NewGVN.cpp:3166:3 #16 0x00007fe7932882d7 (anonymous namespace)::NewGVNLegacyPass::runOnFunction(llvm::Function&) /llvm/lib/Transforms/Scalar/NewGVN.cpp:3904:8 #17 0x00007fe794216caf llvm::FPPassManager::runOnFunction(llvm::Function&) /llvm/lib/IR/LegacyPassManager.cpp:1519:27 #18 0x00007fe794216fc5 llvm::FPPassManager::runOnModule(llvm::Module&) /llvm/lib/IR/LegacyPassManager.cpp:1540:16 #19 0x00007fe794217cc8 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /llvm/lib/IR/LegacyPassManager.cpp:1596:27 #20 0x00007fe794217286 llvm::legacy::PassManagerImpl::run(llvm::Module&) /llvm/lib/IR/LegacyPassManager.cpp:1699:16 #21 0x00007fe7942188b1 llvm::legacy::PassManager::run(llvm::Module&) /llvm/lib/IR/LegacyPassManager.cpp:1730:3 #22 0x0000000000270e4f (opt+0x270e4f) #23 0x00007fe7916c2b45 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b45) #24 0x0000000000244029 (opt+0x244029) Stack dump: 0. Program arguments: opt -newgvn CodeGen/X86/multiple-loop-post-inc.ll 1. Running pass 'Function Pass Manager' on module 'CodeGen/X86/multiple-loop-post-inc.ll'. 2. Running pass 'Global Value Numbering' on function '@foo' Aborted
The problem starts in makePossiblePhiOfOps at line 2522:
2505 Instruction *ValueOp = I->clone();
2506 auto Iter = TempToMemory.end();
2507 if (MemAccess)
2508 Iter = TempToMemory.insert({ValueOp, MemAccess}).first;
2509
2510 for (auto &Op : ValueOp->operands()) {
2511 Op = Op->DoPHITranslation(PHIBlock, PredBB);
2512 // When this operand changes, it could change whether there is a
2513 // leader for us or not.
2514 addAdditionalUsers(Op, I);
2515 }
2516 // Make sure it's marked as a temporary instruction.
2517 AllTempInstructions.insert(ValueOp);
2518 // and make sure anything that tries to add it's DFS number is
2519 // redirected to the instruction we are making a phi of ops
2520 // for.
2521 InstrDFS.insert({ValueOp, IDFSNum});
2522 const Expression *E = performSymbolicEvaluation(ValueOp, Visited);
2523 InstrDFS.erase(ValueOp);
2524 AllTempInstructions.erase(ValueOp);
2525 ValueOp->deleteValue();It's possible for ValueOp to be added as an additional user of some instruction
if, for instance, its sym eval were delegated to createExpression (which
checkSimplificationResults, which could addAdditionalUsers). Its pointer is kept
in AdditionalUsers[WhateverValueItSimplifiedTo].
The pointer remains after ValueOp deleted, leading to a use-after-free when
touchAndErase is called (frame 10 of the stack dump). In this particular case,
the memory previously occupied by ValueOp was reallocated to a llvm::Constant,
which breaks the implicit assumption by touchAndErase that all AdditionalUsers
are instructions:
void NewGVN::touchAndErase(Map &M, const KeyType &Key) {
const auto Result = M.find_as(Key);
if (Result != M.end()) {
for (const typename Map::mapped_type::value_type Mapped : Result->second)
TouchedInstructions.set(InstrToDFSNum(Mapped));
// ^^^^^^^ assumes that Mapped is an Intruction.
M.erase(Result);
}
}I'm not sure if this is the right fix (please correct me if it's wrong), but as
far as I can tell, the purpose of AdditionalUsers is to ensure that a def's
use(s) are touched and re-visited if the def itself is altered (simplified or
whatever). Touching requires that the use has an InstrDFS, which means that it
has to be a concrete instruction that actually exists in the function.
ValueOp fails this requirement by virtue of being a parent-less clone.