This is an archive of the discontinued LLVM Phabricator instance.

Fix crash when linking metadata with ODR type uniquing
ClosedPublic

Authored by tejohnson on Dec 28 2017, 9:50 PM.

Details

Summary

With DebugTypeODRUniquing enabled, during IR linking debug metadata
in the destination module may be reached from the source module.
This means that ConstantAsMetadata nodes (e.g. on DITemplateValueParameter)
may contain a value the destination module. When trying to map such
metadata nodes, we will attempt to map a GV already in the dest module.
linkGlobalValueProto will end up with a source GV that is the same as
the dest GV as well as the new GV. Trying to access the TypeMap for the
source GV type, which is actually a dest GV type, hits an assertion
since it appears that we have mapped into the source module (because the
type is the value not a key into the map).

Detect that we don't need to access the TypeMap in this case, since
there is no need to create a bitcast from the new GV to the source GV
type as they GV are the same.

Fixes PR35722.

Event Timeline

tejohnson created this revision.Dec 28 2017, 9:50 PM
mehdi_amini added inline comments.Dec 29 2017, 1:00 AM
lib/Linker/IRMover.cpp
957–962

Might be worth a comment?

test/ThinLTO/X86/Inputs/dicompositetype-unique2.ll
57

Can all the metadata input be minimized?

tejohnson marked 2 inline comments as done.Dec 29 2017, 7:54 AM
tejohnson added inline comments.
test/ThinLTO/X86/Inputs/dicompositetype-unique2.ll
57

The new version I'm going to upload minimizes it as much as I could while reproducing the assert.

tejohnson updated this revision to Diff 128337.Dec 29 2017, 7:55 AM
tejohnson marked an inline comment as done.

Address comments.

mehdi_amini accepted this revision.Jan 9 2018, 9:53 AM

The test still seems complex to me (do you need all the IR types? The code in the functions? And all these metadatas and metadata fields?

Otherwise it seems fine, can you backport to the LLVM 6 release branch?

This revision is now accepted and ready to land.Jan 9 2018, 9:53 AM

The test still seems complex to me (do you need all the IR types? The code in the functions? And all these metadatas and metadata fields?

Yes unfortunately. I tried various other ways to reduce it, without luck.

Otherwise it seems fine, can you backport to the LLVM 6 release branch?

Will do.

This revision was automatically updated to reflect the committed changes.