Page MenuHomePhabricator

Add ehcont section support
ClosedPublic

Authored by arlosi on Jan 15 2021, 3:10 PM.

Details

Summary

In the future Windows will enable Control-flow Enforcement Technology (CET aka shadow stacks). To protect the path where the context is updated during exception handling, the binary is required to enumerate valid unwind entrypoints in a dedicated section which is validated when the context is being set during exception handling.

This change allows llvm to generate the section that contains the appropriate symbol references in the form expected by the msvc linker.

This feature is enabled through a new module flag, ehcontguard, which was modelled on the cfguard flag.

The change includes a test that when the module flag is enabled the section is correctly generated.

The set of exception continuation information includes returns from exceptional control flow (catchret in llvm).

In order to collect catchret we:

  1. Includes an additional flag on machine basic blocks to indicate that the given block is the target of a catchret operation,
  2. Introduces a new machine function pass to insert and collect symbols at the start of each of each block, and
  3. Combines these targets with the other EHCont targets that were already being collected.

Change originally authored by Daniel Frampton <dframpto@microsoft.com>

For more details, see MSVC documentation for /guard:ehcont

https://docs.microsoft.com/en-us/cpp/build/reference/guard-enable-eh-continuation-metadata

Diff Detail

Event Timeline

arlosi created this revision.Jan 15 2021, 3:10 PM
arlosi requested review of this revision.Jan 15 2021, 3:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 15 2021, 3:10 PM
rnk added inline comments.Jan 19 2021, 2:54 PM
llvm/lib/CodeGen/EHContGuardCatchret.cpp
81

If we do use this name, I would prefer to use as short a name as possible. C++ names are long enough that they significantly contribute to object file size. Let's use getFunctionNumber() here instead, so we get names like $ehgcr_2_42. You'll need a non-digit separator to ensure the names are unique (consider function 5 bb 55 and function 55 bb 5).

This wasn't as much of a concern for longjmp tables, since setjmp is less common and mostly appears in C code.

83

This could be unreliable if some later pass reorders instructions or deletes the first instruction. I think it would be more reliable to take the address of the basic block. You can do this with MBB->setHasAddressTaken and MachineModuleInfo::getAddrLabelSymbol.

arlosi added inline comments.Jan 22 2021, 6:17 PM
llvm/lib/CodeGen/EHContGuardCatchret.cpp
83

MachineModuleInfo::getAddrLabelSymbol works on BasicBlocks rather than MachineBasicBlocks. So even if I setHasAddressTaken on the MBB, the associated BB might still not have it's address taken, which causes an assert failure in getAddrLabelSymbolToEmit.

I also tried using MBB.setLabelMustBeEmitted() and MBB.getSymbol(), but that produces a symbol name that doesn't end up in the symbol table.

We could add an additional field to MachineBasicBlock to store this special symbol, then emit it in the AsmPrinter in emitBasicBlockStart.

Do you have a preference, or another better solution?

arlosi updated this revision to Diff 319736.Jan 27 2021, 7:12 PM

Use a new field in MachineBasicBlock to store the symbol for ehcont; then emit it in the AsmPrinter.

arlosi updated this revision to Diff 319737.Jan 27 2021, 7:13 PM

Run clang-format

arlosi updated this revision to Diff 319865.Jan 28 2021, 7:38 AM

Include both commits.

arlosi edited the summary of this revision. (Show Details)Feb 4 2021, 11:29 AM
pengfei accepted this revision.Feb 4 2021, 6:12 PM

The changes LGTM, but I'd let @rnk sign off as well.

This revision is now accepted and ready to land.Feb 4 2021, 6:12 PM

@rnk if you could take a look that would be great.

Also, I do not have commit access.

@rnk if you could take a look that would be great.

Also, I do not have commit access.

I can commit it, but let's wait one or two days to see if there're objections from other people.

I'm looking through this again today, and I don't think we need both IsEHCatchRetTarget and isEHFuncletEntry to be added to MBB. I'll simplify this and post an update later today.

arlosi updated this revision to Diff 322907.Wed, Feb 10, 10:35 PM

Remove redundant field IsEHContTarget on MachineBasicBlock

This revision was landed with ongoing or failed builds.Sun, Feb 14, 10:27 PM
Closed by commit rG080866470d3e: Add ehcont section support (authored by arlosi, committed by pengfei). · Explain Why
This revision was automatically updated to reflect the committed changes.
rnk added a comment.Mon, Mar 1, 11:52 AM

Sorry for the delay, thanks for implementing this.