This is an archive of the discontinued LLVM Phabricator instance.

[WinCFG] Handle constant casts carefully in .gfids emission
ClosedPublic

Authored by rnk on Oct 31 2019, 11:39 AM.

Details

Summary

The general Function::hasAddressTaken has two issues that make it
inappropriate for our purposes:

  1. it is sensitive to dead constant users (PR43858 / crbug.com/1019970), leading to different codegen when debu info is enabled
  2. it considers direct calls via a function cast to be address escapes

The first is fixable, but the second is not, because IPO clients rely on
this behavior. They assume this function means that all call sites are
analyzable for IPO purposes.

So, implement our own analysis, which gets closer to finding functions
that may be indirect call targets.

Event Timeline

rnk created this revision.Oct 31 2019, 11:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2019, 11:39 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

I mentioned this on the bug, but do we care if a reference to a function is optimized away at the MIR level?

llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
71

Does stripPointerCasts do the right thing here? It can look through multiple levels of casts.

rnk marked an inline comment as done.Oct 31 2019, 1:26 PM

I mentioned this on the bug, but do we care if a reference to a function is optimized away at the MIR level?

I don't think it's worth the work of tracking these things at the MIR level. The new implementation will come with its own set of bugs and quirks, some of which I mentioned on the bug: large code model, target specificity, separate codepath for emission as a global initializer.

We promise that adding -g doesn't change the code we generate, but we don't promise that adding -guard:cf,nochecks won't change the generated code.

llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
71

Thanks, this should compare to Fn. I couldn't think of another simple way to identify pointer casts.

rnk updated this revision to Diff 227510.Nov 1 2019, 12:54 PM
  • fix cast detection
efriedma accepted this revision.Nov 1 2019, 1:04 PM

LGTM

llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
55

Even if we did decide CFG should protect indirectbr, the target would be the block in question, not the function. So this FIXME isn't really in the right place.

This revision is now accepted and ready to land.Nov 1 2019, 1:04 PM
This revision was automatically updated to reflect the committed changes.