[PATCH] D62888: [NewPM] Port Sancov

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1231-1232
+      MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
+      MPM.addPass(
+          createModuleToFunctionPassAdaptor(SanitizerCoveragePass(SancovOpts)));
+    }
----------------
chandlerc wrote:
> leonardchan wrote:
> > chandlerc wrote:
> > > Is there an existing function pass pipeline we can put this in, maybe by using an extension point? That would make it much faster to run I suspect.
> > I tried this earlier, but when I use `registerOptimizerLastEPCallback()` before the default module pipeline is made, I end up getting this linker error for `-O2` runs:
> > 
> > ```
> > fuzzer.c:(.text.sancov.module_ctor_8bit_counters[sancov.module_ctor_8bit_counters]+0x11): undefined reference to `__start___sancov_pcs'
> > fuzzer.c:(.text.sancov.module_ctor_8bit_counters[sancov.module_ctor_8bit_counters]+0x16): undefined reference to `__stop___sancov_pcs'
> > ```
> > 
> > I don't understand how I get this because the symbols are declared in the IR dump as:
> > 
> > ```
> > @__start___sancov_pcs = external hidden global i64*
> > @__stop___sancov_pcs = external hidden global i64*
> > ```
> > 
> > for both the optimized cases if I use the adaptor or EP. I'm still linking with the same compiler-rt in each case.
> > 
> > The only difference I think of using the adaptor after `buildPerModuleDefaultPipeline()` vs using the extension point callback before is that the order in which the pass will be called changes, although I still don't see how it could lead to this undefined reference if I still link against compiler-rt.
> > 
> > Additionally, the only difference between the IR between using the adaptor vs the EP is a few extra globals/declarations seem to be missing when using the EP, which makes me think another pass that ran after this one removed them, although these don't seem to be related to the error.
> > 
> Hmm...
> 
> Maybe those missing globals / declarations are relevant. Maybe they are necessary to get the linking against the sancov runtime to work correctly?
> 
> You could try adding these globals / declarations to the 'used' set so they don't get removed by later passes:
> https://llvm.org/docs/LangRef.html#the-llvm-compiler-used-global-variable
> 
> Otherwise, I have no idea, maybe makes sense to loop in the sanitizer folks explicitly for help debugging this...
Ah ok. I did not know about this global before. So it seems that the problem was related to the `__sancov_pcs` section being optimized out. Adding `__sancov_gen_` to `llvm.compiler.used` fixes this.

I did a little extra digging and I think it's `ConstantMergePass` or some other pass that works with this that seems to cause this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62888/new/

https://reviews.llvm.org/D62888