[PATCH] D33224: [GISel]: Fix more Undefined behavior in GlobalISel::IRTranslator

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

Is it possible to add a test case for this? It seems that a test where the last instruction has a DILocation should trigger it.



================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:1132-1140
+  // MachineIRBuilder holds a DebugLoc which can hold on to a TrackingMDRef
+  // that can outlive the context.
+  // When the LLVMContext is destroyed, it first releases all MDNode references
+  // and then destroys the passes. If we don't clear the Builders(which may
+  // hold some ref), the DebugLoc will again get destroyed leading to
+  // undefined behavior.
+  // We do it here - but we could probably also do it in
----------------
Based on our discussion off-list it was my understanding that ~IRTranslator() wasn't the only trigger for this. Wasn't the call to MachineIRBuilder::setMF() in IRTranslator::runOnMachineFunction() also triggering it?

If so, I'd suggest adapting this comment to talk about the MachineIRBuilder/DebugLoc outliving the DILocation it holds rather than going into detail about the LLVMContext case. That way it will cover both.


Repository:
  rL LLVM

https://reviews.llvm.org/D33224