[PATCH] D33370: Don't verify cross-imported bitcode in FunctionImporter

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 19 15:45:50 PDT 2017


aprantl added a comment.

In https://reviews.llvm.org/D33370#759915, @mehdi_amini wrote:

> > It turns out that the SrcModule in FunctionImporter is in a really inconsistent intermediate state at the point where I ran the Verifier.
>
> Oh right I forgot about this: lazy-loaded modules, even after materializing metadata, aren't passing the verified. This is super annoying.
>
> > After poking around a bit I came to the conclusion that it probably isn't even desirable to run the full-module verifier on each cross-imported module: Over the course of building the entire project the same module will be verified over and over again, which is a waste of resources. When we want to lazy-load metadata running a whole-module Verifier is also problematic.
>
> There is something I'm missing: I thought the important part of the verifier was to be able to drop stalled debug information which is important for backward compatibility? This is the only reason I suggested doing it at this point of the process (before importing)...


That is correct*. This why I mentioned that doing it like in this patch will not catch any metadata so egregiously wrong that it would crash the FunctionImporter. If the debug info is structurally sound and can be handled by the FunctionImporter then doing the full verification after importing will still allow us to strip malformed imported debug info.

*) note that it isn't really about backwards-compatibility (we have auto-upgrades for that), but about metadata generated by non-clang frontends. If we have to support every metadata every produced by any frontend and at one point accepted by the verifier, we could effectively never improve the verifier by making it stricter — and historically the verifier was very lax.


Repository:
  rL LLVM

https://reviews.llvm.org/D33370





More information about the llvm-commits mailing list