aprantl added a comment. In https://reviews.llvm.org/D38042#875334, @chandlerc wrote:
> In https://reviews.llvm.org/D38042#875328, @aprantl wrote: > > > In https://reviews.llvm.org/D38042#875303, @chandlerc wrote: > > > > > But, the verifier itself will just "crash". It won't print a stack trace, > > > but I don't see why that's much better? And this flag is supposed to be a > > > developer option and not a user facing one, so I'm somewhat confused at > > > what the intent is here... > > > > > > No, it will report_fatal_error() instead of crashing the compiler later on. > > In any case, that is not my primary motivation: The intent is exactly what > > is illustrated by the testcase, stripping malformed debug info metadata > > produced by older, buggy versions of clang. The backstory to this is that > > historically the Verifier was very weak when it came to verifying debug > > info. To allow us to make the Verifier better (stricter), while still > > supporting importing of older .bc files produced by older compilers, we > > added a mechanism that strips all debug info metadata if the verification > > of the debug info failed, but the bitcode is otherwise correct. > > > Ok, that use case makes way more sense. I'd replace the change description > with some discussion of this use case. > > My next question is -- why is this done by the verifier? It seems *really* > bad that the verifier *changes the IR*. Don't get me wrong, what you're > trying to do (strip malformed debug info) makes perfect sense. But I think > that the thing which does that shouldn't be called a verifier. It might *use* > the verifier of course. That was a purely pragmatic decision: Most (but not all) LLVM-based tools are running the Verifier as an LLVM pass so adding the stripping into the pass was the least invasive way of implementing this feature. If we are truly bothered by this, I think what could work is to separate out a second StripBrokenDebugInfo pass that depends on the Verifier and is guaranteed to run immediately after it. I don't see this adding much value though, and we would have to modify all tools to schedule the new pass explicitly. Do you think that would be worth pursuing? > Last but not least, I still suspect that this shouldn't be run here. If the > user wants to disable LLVM passes *and emits LLVM IR*, they should get it > unperturbed. The stripping of malformed debug info seems like it should > happen later as part of the passes to emit code, and I'd actually expect the > LLVM code generator to add the necessary pass rather than relying on every > frontend remembering to do so? The user wants to disable LLVM optimizations (`-disable-llvm-optzns`) not LLVM passes. Also, I believe the Verifier is special. The Verifier protects LLVM from crashing and as a user I think I would prefer a Verifier error over clang crashing while emitting bitcode. Because of auto-upgrades users already don't get the IR unperturbed, and I view stripping of broken debug info as a (in all fairness: very brutal :-) auto-upgrade. https://reviews.llvm.org/D38042 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
