This is an archive of the discontinued LLVM Phabricator instance.

[clang][modules] Respect "-fmodule-name=" when serializing included files into a PCH
ClosedPublic

Authored by jansvoboda11 on Aug 9 2023, 4:05 PM.

Details

Summary

Clang writes the set of textually included files into AST files, so that importers know to avoid including those files again and instead deserialize their contents from the AST on-demand.

Logic for determining the set of included files files only considers headers that are either non-modular or that are modular but with HeaderFileInfo::isCompilingModuleHeader set. Logic for computing that bit is different than the one that determines whether to include a header textually with the "-fmodule-name=Mod" option. That can lead to header from module "Mod" being included textually in a PCH, but be omitted in the serialized set of included files. This can then allow such header to be textually included from importer of the PCH, wreaking havoc.

This patch fixes that by aligning the logic for computing HeaderFileInfo::isCompilingModuleHeader with the logic for deciding whether to include modular header textually.

As far as I can tell, this bug has been in Clang for forever. It got accidentally "fixed" by D114095 (that changed the logic for determining the set of included files) and got broken again in D155131 (which is essentially a revert of the former).

rdar://113520515

Diff Detail

Event Timeline

jansvoboda11 created this revision.Aug 9 2023, 4:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 4:05 PM
Herald added a subscriber: ributzka. · View Herald Transcript
jansvoboda11 requested review of this revision.Aug 9 2023, 4:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 4:05 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jansvoboda11 edited the summary of this revision. (Show Details)Aug 9 2023, 4:14 PM
jansvoboda11 edited the summary of this revision. (Show Details)Aug 10 2023, 9:22 AM
benlangmuir accepted this revision.Aug 10 2023, 9:31 AM
This revision is now accepted and ready to land.Aug 10 2023, 9:31 AM