This is an archive of the discontinued LLVM Phabricator instance.

[ModuleMap][CrashReproducer] Collect headers from inner frameworks
ClosedPublic

Authored by bruno on May 11 2016, 7:19 PM.

Details

Summary

(1) Collect headers under inner frameworks (frameworks inside other
other frameworks).
(2) Make sure we also collect the right header files inside them.

More info on (2):

Consider a dummy framework module B, with header Frameworks/B/B.h. Now
consider that another framework A, with header Frameworks/A/A.h, has a
layout with a inner framework Frameworks/A/Frameworks/B/B.h, where the
"B/B.h" part is a symlink for Frameworks/B/B.h. Also assume that
Frameworks/A/A.h includes <B/B.h>.

When parsing header Frameworks/A/A.h, framework module lookup is
performed in search for B, and it happens that
"Frameworks/A/Frameworks/B/B.h" path is registered in the module instead
of real "Frameworks/B/B.h". This occurs because
"Frameworks/A/Frameworks/B/B.h" is scanned first by the FileManager,
when looking for inner framework modules under Frameworks/A/Frameworks.
This makes Frameworks/A/Frameworks/B/B.h the default cached named inside
the FileManager for the B.h file UID.

This leads to modules being built without consistent paths to underlying
header files. This is usually not a problem in regular compilation flow,
but it's an issue when running the crash reproducer. The issue is that
clangs collect "Frameworks/A/Frameworks/B/B.h" but not
"Frameworks/B/B.h" into the VFS, leading to err_mmap_umbrella_clash. So
make sure we also collect the original header.

Diff Detail

Event Timeline

bruno updated this revision to Diff 56991.May 11 2016, 7:19 PM
bruno retitled this revision from to [ModuleMap][CrashReproducer] Collect headers from inner frameworks.
bruno updated this object.
bruno added a reviewer: benlangmuir.
bruno added a subscriber: cfe-commits.
benlangmuir edited edge metadata.May 13 2016, 8:59 AM

Let's move the code looks up the alternate name out of the ModuleMap parser, and into the dependency collector callbacks. This feels like an implementation detail of the dependency collector. We could add a new callback specifically for umbrella headers and handle this inside there.

bruno updated this revision to Diff 57263.May 13 2016, 3:20 PM
bruno edited edge metadata.

Updated after Ben's suggestions!

benlangmuir accepted this revision.May 13 2016, 3:22 PM
benlangmuir edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 13 2016, 3:22 PM
bruno closed this revision.May 13 2016, 3:29 PM

Thanks Ben!

Committed r269502