[ORC] fix use-after-move. NFC

[nickdesaulniers]

Summary

scan-build flagged a potential use-after-move in debug builds. It's not
safe that a moved from value contains anything but garbage. Manually
DRY up these repeated expressions.

[nickdesaulniers]

• git-clang-format HEAD~

[lhames]

LGTM! Thanks Nick!

[dblaikie]

Hey @lhames - is this code tested? I guess they're only debug output, so might not be tested. (was the use-after-move likely to be UB (null deref, etc) or just likely to log not what was intended?)

Reckon it'd be suitable to not use "auto" for the N and/or ES variables? The "N" variable perhaps makes me more concerned than ES - because is it a string reference? (that might be invalidated by the move), a return by value (that then means it's reference extending, which is best avoided unless really needed), etc?

[nickdesaulniers]

because is it a string reference? (that might be invalidated by the move), a return by value (that then means it's reference extending, which is best avoided unless really needed),

Do you have links or more info to these problems? I've never heard the term "reference extending" before.

[dblaikie]

In D62112#1511074, @nickdesaulniers wrote:

because is it a string reference? (that might be invalidated by the move), a return by value (that then means it's reference extending, which is best avoided unless really needed),

Do you have links or more info to these problems? I've never heard the term "reference extending" before.

Here's some info on reference lifetime extension: https://abseil.io/tips/107

As for string reference invalidation, I was picturing something like this:

struct foo { std::string s; const std::string &get_foo() { return s; } }; //struct isn't necessary, but to make it closer to the example
void func(foo f) {
  const std::string &ref = f.get_foo();
  foo g = std::move(f);
  /* do things with 'ref' here */
}

but at the last line there, the string inside 'f' has been moved into 'g', so the reference refers to the std::string in 'f' which is now in a valid but unspecified state (could contain the original string data (probably does if it's short enougH), or could point to a zero-length/empty string (if the data was too long and was dynamically allocated, and thus the allocation was moved over to the string in 'g' instead))

llvm/lib/ExecutionEngine/Orc/Layer.cpp

void BasicIRLayerMaterializationUnit::materialize(
   SymbolToDefinition.clear();
 
   // If cloneToNewContextOnEmit is set, clone the module now.
   if (L.getCloneToNewContextOnEmit())
     TSM = cloneToNewContext(TSM);
 
 #ifndef NDEBUG
   auto &ES = R.getTargetJITDylib().getExecutionSession();
+  auto &N = R.getTargetJITDylib().getName();
 #endif // NDEBUG
 
   auto Lock = TSM.getContextLock();
   LLVM_DEBUG(ES.runSessionLocked([&]() {
-    dbgs() << "Emitting, for " << R.getTargetJITDylib().getName() << ", "
-           << *this << "\n";
+    dbgs() << "Emitting, for " << N << ", " << *this << "\n";
   }););
   L.emit(std::move(R), std::move(TSM));
   LLVM_DEBUG(ES.runSessionLocked([&]() {
-    dbgs() << "Finished emitting, for " << R.getTargetJITDylib().getName()
-           << ", " << *this << "\n";
+    dbgs() << "Finished emitting, for " << N << ", " << *this << "\n";
   }););
 }
 
 ObjectLayer::ObjectLayer(ExecutionSession &ES) : ES(ES) {}
 
 ObjectLayer::~ObjectLayer() {}
 
 Error ObjectLayer::add(JITDylib &JD, std::unique_ptr<MemoryBuffer> O,