Page MenuHomePhabricator

[ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file
ClosedPublic

Authored by akyrtzi on Jan 21 2021, 11:46 AM.

Details

Summary

This addresses an issue with how the PCH preable works, specifically:

  1. When using a PCH/preamble the module hash changes and a different cache directory is used
  2. When the preamble is used, PCH & PCM validation is disabled.

Due to combination of #1 and #2, reparsing with preamble enabled can end up loading a stale module file before a header change and using it without updating it because validation is disabled and it doesn’t check that the header has changed and the module file is out-of-date.

rdar://72611253

Diff Detail

Event Timeline

akyrtzi created this revision.Jan 21 2021, 11:46 AM
akyrtzi requested review of this revision.Jan 21 2021, 11:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2021, 11:46 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
akyrtzi updated this revision to Diff 318314.Jan 21 2021, 2:06 PM

clang-format changes

bnbarham accepted this revision.Jan 21 2021, 2:08 PM

LGTM, just the one minor comment

clang/lib/Serialization/ASTReader.cpp
2221–2222

There's a getValueOr method that you could use instead (ie. CurrentDeserializingModuleKind.getValueOr(M.Kind)).

clang/test/Index/preamble-reparse-changed-module.m
8

For some reason I was expecting the trial's to be 1 based, not 0. I'm not sure why though.

This revision is now accepted and ready to land.Jan 21 2021, 2:08 PM
akyrtzi updated this revision to Diff 318333.Jan 21 2021, 3:36 PM

Use getValueOr

akyrtzi updated this revision to Diff 318335.Jan 21 2021, 3:37 PM
akyrtzi edited the summary of this revision. (Show Details)

Fix typo in commit message, 'state' -> 'stale'