This is an archive of the discontinued LLVM Phabricator instance.

[COFF] Improve synthetic symbol handling
ClosedPublic

Authored by rnk on Jun 23 2017, 3:17 PM.

Details

Summary

The main change is that we can have SECREL and SECTION relocations
against ___safe_se_handler_table, which is important for handling the
debug info in the MSVCRT.

Previously we were using DefinedRelative for safe_se_handler_table and
ImageBase, and after we implement CFGuard, we plan to extend it to
handle guard_fids_table, guard_longjmp_table, and more. However,
DefinedRelative is really only suitable for implementing __ImageBase,
because it lacks a Chunk, which you need in order to figure out the
output section index and output section offset when resolving SECREl and
SECTION relocations.

This change renames DefinedRelative to DefinedSynthetic and gives it a
Chunk. One wart is that __ImageBase doesn't have a chunk. It points to
the PE header, effectively. We could split DefinedRelative and
DefinedSynthetic if we think that's cleaner and creates fewer special
cases.

I also added safeseh.s, which checks that we don't emit a safe seh table
entries pointing to garbage collected handlers and that we don't emit a
table at all when there are no handlers.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk created this revision.Jun 23 2017, 3:17 PM
rnk added a reviewer: ruiu.Jun 23 2017, 3:19 PM
rnk added subscribers: llvm-commits, pcc, inglorion.

ptal

ruiu edited edge metadata.Jun 23 2017, 3:39 PM

Overall looking good, a few minor comments.

lld/COFF/Symbols.h
170 ↗(On Diff #103787)

Why is it 64 bit?

lld/COFF/Writer.cpp
785–786 ↗(On Diff #103787)

Since we always add "___safe_se_handler_table" and "___safe_se_handler_count" (three underscores), I think you can use Symtab->find instead of Symtab->findUnderscore.

Please add comment why you want to change the type of the symbol from Absolute to Synthetic.

rnk marked an inline comment as done.Jun 23 2017, 3:47 PM
rnk added inline comments.
lld/COFF/Symbols.h
170 ↗(On Diff #103787)

I'll fix it. I was worried about truncation warnings.

rnk updated this revision to Diff 103789.Jun 23 2017, 3:48 PM
  • comments
ruiu accepted this revision.Jun 23 2017, 3:50 PM

LGTM

lld/COFF/Writer.cpp
785–786 ↗(On Diff #103787)

... so that we can apply secrel relocations against these options?

This revision is now accepted and ready to land.Jun 23 2017, 3:50 PM
This revision was automatically updated to reflect the committed changes.
lld/trunk/test/COFF/safeseh.s