This is an archive of the discontinued LLVM Phabricator instance.

simplify COFF module assembly test and move it to Object
ClosedPublic

Authored by inglorion on Jan 24 2019, 2:08 PM.

Event Timeline

inglorion created this revision.Jan 24 2019, 2:08 PM
pcc added inline comments.Jan 24 2019, 2:29 PM
llvm/test/Object/X86/coff-asm.ll
3–4

Don't you just need these two lines to test your change? The change makes it so that llvm-nm doesn't crash on this input, right?

inglorion marked an inline comment as done.Jan 24 2019, 3:21 PM
inglorion added inline comments.
llvm/test/Object/X86/coff-asm.ll
3–4

llvm-nm doesn't crash here (or in the next invocation, for that matter) even without D57073. Does it use RecordStreamer? My impression was the crashing code path is only invoked when reading in module assembly in ModuleSymbolTable.

pcc added inline comments.Jan 24 2019, 3:39 PM
llvm/test/Object/X86/coff-asm.ll
3–4

It should be using RecordStreamer via IRObjectFile.

My mistake, I didn't notice that you were calling llc here. You should be able to reproduce the problem by running llvm-nm on the file that you're calling %t.thinlto.bc here.

inglorion updated this revision to Diff 183428.Jan 24 2019, 4:01 PM

Simpler test. This crashes without D57073 and passes with D57073.

inglorion updated this revision to Diff 183429.Jan 24 2019, 4:02 PM

Re-upload only the test change.

Harbormaster completed remote builds in B27278: Diff 183429.
pcc accepted this revision.Jan 24 2019, 4:13 PM

LGTM, thanks!

This revision is now accepted and ready to land.Jan 24 2019, 4:13 PM
This revision was automatically updated to reflect the committed changes.