This is an archive of the discontinued LLVM Phabricator instance.

allow COFF .def directive in module assembly when using ThinLTO
ClosedPublic

Authored by inglorion on Jan 22 2019, 5:17 PM.

Details

Summary

Using COFF's .def directive in module assembly used to crash ThinLTO
with "this directive only supported on COFF targets" when getting
symbol information in ModuleSymbolTable. This change allows
ModuleSymbolTable to process such code and adds a test to verify that
the .def directive has the desired effect on the native object file,
with and without ThinLTO.

Fixes https://bugs.llvm.org/show_bug.cgi?id=36789

Event Timeline

inglorion created this revision.Jan 22 2019, 5:17 PM
rnk added inline comments.Jan 23 2019, 11:09 AM
clang/test/CodeGen/inline-asm-coff.c
3–4 ↗(On Diff #183002)

This test isn't going to do the right thing on Windows ARM, and I don't think we need this high-level of an integration test.

I'd recommend writing a .ll test similar to the llvm/test/Transforms/ThinLTOBitcodeWriter/symver.ll one. You should be able to use the same assembly as module level asm in LLVM IR, and exercise the same behavior.

inglorion updated this revision to Diff 183165.Jan 23 2019, 1:19 PM
inglorion retitled this revision from allow COFF .def directive in inline assembly when using ThinLTO to allow COFF .def directive in module assembly when using ThinLTO.
inglorion edited the summary of this revision. (Show Details)

replaced .c test with an .ll test

rnk accepted this revision.Jan 23 2019, 1:32 PM

lgtm

This revision is now accepted and ready to land.Jan 23 2019, 1:32 PM
inglorion updated this revision to Diff 183209.Jan 23 2019, 4:48 PM

add missing .1

pcc added inline comments.Jan 23 2019, 8:26 PM
llvm/test/MC/COFF/module-asm-coff.ll
2

This isn't really a test of the MC layer. Can it be simplified to use llvm-nm and moved under test/Object?

This revision was automatically updated to reflect the committed changes.

Just saw your comment, pcc. I'll look into that.

pcc, does D57192 implement your suggestion?