[PATCH] D15798: Fix for Bug 24852 (crash with -debug -instcombine)

Than McIntosh via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 6 15:38:33 PST 2016


thanm added a comment.

> However, as Duncan said, I don't think this is the test we want and a simple C++ unitest seems less fragile to assert that "we can print a value that has no parent but has a metadata ".


You and Duncan are right -- a C++ unit test would be smaller/better. I'll see about writing one.

Thanks for the comments.


================
Comment at: lib/IR/AsmWriter.cpp:3157
@@ -3156,3 +3156,3 @@
     Out << Separator;
-    if (Kind < MDNames.size()) {
+    if (TheModule && Kind < MDNames.size()) {
       Out << "!";
----------------
joker.eph wrote:
> thanm wrote:
> > joker.eph wrote:
> > > Why do you need `TheModule` here?
> > TheModule is a null pointer in the case where the instruction has no parent BB.
> This does not answer my question. Why do you need this check *here*? A few lines above, `TheModule` was dereferenced, it makes sense. Here it does not seem to be the case.
You are right -- not sure what my thinking was there. I'll take it out in a subsequent patch. Thanks.


http://reviews.llvm.org/D15798





More information about the llvm-commits mailing list