Page MenuHomePhabricator

LTO: Export functions referenced by non-canonical CFI jump tables
ClosedPublic

Authored by samitolvanen on May 25 2021, 3:21 PM.

Details

Summary

LowerTypeTests pass adds functions with a non-canonical jump table
to cfiFunctionDecls instead of cfiFunctionDefs. As the jump table
is in the regular LTO object, these functions will also need to be
exported. This change fixes the non-canonical jump table case and
adds a test similar to the existing one for canonical jump tables.

Diff Detail

Event Timeline

samitolvanen created this revision.May 25 2021, 3:21 PM
samitolvanen requested review of this revision.May 25 2021, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2021, 3:22 PM
kees added a subscriber: kees.May 26 2021, 12:14 PM

PoC from Sami:
https://godbolt.org/z/xfsWjhGhq

This change looks sane to me.

Is this related to b/158034873 or another reported bug?

Adding another possible reviewer since @eugenis added these data structures (D34168). With the caveat that I'm not very familiar at all with this CFI transformation, the patch looks ok to me after looking at how cfiFunctionDecls is used. However it would be great to get a review from either @pcc or @eugenis.

llvm/test/LTO/Resolution/X86/export-jumptable-noncanonical.ll
14

I assume this is part of a non-canonical jump table, which is why we don't internalize with this patch. How can I tell that it is part of a non-canonical rather than canonical jump table?

Is this related to b/158034873 or another reported bug?

No, this patch just makes it possible to compile stand-alone programs with non-canonical CFI jump tables.

samitolvanen added inline comments.Jun 2 2021, 8:14 AM
llvm/test/LTO/Resolution/X86/export-jumptable-noncanonical.ll
14

I assume this is part of a non-canonical jump table, which is why we don't internalize with this patch. How can I tell that it is part of a non-canonical rather than canonical jump table?

Correct. The "CFI Canonical Jump Tables" module flag is set to 0 at the bottom of the file to use non-canonical jump tables.

ormris added a subscriber: ormris.Jun 2 2021, 10:01 AM
pcc accepted this revision.Jun 8 2021, 2:28 PM

LGTM

This revision is now accepted and ready to land.Jun 8 2021, 2:28 PM

Thanks, Peter. I don't have commit access, so Nick, would you mind committing this for me? The Windows build failure looks unrelated.

This revision was landed with ongoing or failed builds.Jun 8 2021, 3:07 PM
This revision was automatically updated to reflect the committed changes.