This is an archive of the discontinued LLVM Phabricator instance.

[demangler] Add ItaniumPartialDemangler::isCtorOrDtor
ClosedPublic

Authored by MaskRay on May 22 2018, 11:22 PM.

Details

Summary

This feature can be used to implement gold compatible --icf=safe in LLD, which is used in some Android and Chrome builds.

In C++, ctors and dtors are not allowed to be taken addresses. This
property is exploited by gold --icf=safe to fold only ctors and dtors.
We don't need to be very precise here to catch all the cases,
false negatives would not hurt.

Diff Detail

Repository
rL LLVM

Event Timeline

MaskRay created this revision.May 22 2018, 11:22 PM

Hi, thanks for working on this! This looks pretty good, I just have a few minor comments...

lib/Demangle/ItaniumDemangle.cpp
5268–5269 ↗(On Diff #148161)

Seems kinda strange to talk about LLD here, this isn't necessarily a LLD-specific function. Maybe just say that false negatives are possible?

We don't even need to conservatively allow false-negatives here though, it would be straightforward to exactly match all constructor/destructors, right? You just need to add AbiTagAttr and StdQualifiedName to your loop below.

5271 ↗(On Diff #148161)

No reason to include DtorName or QualifiedName, those can only appear in expressions (via an <unresolved-name>) so they should be unreachable.

MaskRay updated this revision to Diff 148315.May 23 2018, 4:33 PM

Catch StdQualifiedName and AbiTagAttr

MaskRay marked 2 inline comments as done.May 23 2018, 4:35 PM
MaskRay added inline comments.
lib/Demangle/ItaniumDemangle.cpp
5271 ↗(On Diff #148161)

Done.

Thanks. I see, these cases are covered by getFunctionBaseName above.

MaskRay marked an inline comment as done.May 23 2018, 4:35 PM
MaskRay updated this revision to Diff 148319.May 23 2018, 4:38 PM

don't change QualifiedName

erik.pilkington accepted this revision.May 23 2018, 4:45 PM

LGTM, thanks!

This revision is now accepted and ready to land.May 23 2018, 4:45 PM
This revision was automatically updated to reflect the committed changes.

Are you planning to use this for anything aside from D47249 which was
abandoned?

Peter