[PATCH] D79698: Run Coverage pass before other *San passes under new pass manager

Thu May 14 15:47:37 PDT 2020

leonardchan added a comment.

In D79698#2037425 <https://reviews.llvm.org/D79698#2037425>, @aeubanks wrote:

> In D79698#2037305 <https://reviews.llvm.org/D79698#2037305>, @leonardchan wrote:
>
> > Just to followup on this: We believe the root cause of the error we're running into is that some sancov guards in one comdat are being referenced by symbols in another comdat, so due to linking order, the other comdat in question is referencing a sancov guard that was in a discarded comdat group. I posted full details of this on llvm-dev <http://lists.llvm.org/pipermail/llvm-dev/2020-May/141583.html>. We believe we're running into this error now because with this patch, inlining occurs after the sanitizers are run.
> >
> > Based on some discussion in http://lists.llvm.org/pipermail/llvm-dev/2020-May/141558.html, it seems appropriate that the solution for this is to just run sanitizers after inlining, so we would avoid this error that's brought about by inlining after sanitizer runs. Since this has been breaking us for a few days, and unless other folks don't mind, I'm thinking it might be appropriate to temporarily revert this until there's a fix for either
>
>
> Maybe the fix is to use `registerScalarOptimizerLateEPCallback()` instead of `registerPipelineStartEPCallback()`. But I'm not sure how to test that, could you try that and lmk if that fixes the issue?

Thanks. Will try this out later.

>> (1) running sanitizers after inlining or
>>  (2) changing which comdat groups the sancov guards get placed in (such that guards in one group aren't referenced by functions defined in another group)
>> 
>> I'm not sure how long it would take to implement either solution, so I think temporarily reverting might be ok here.
> 
> Sure, feel free to revert.

Reverted in e9802aa4221ba3857041c2328639ce2aac0ace67 <https://reviews.llvm.org/rGe9802aa4221ba3857041c2328639ce2aac0ace67>.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79698