r220493 - Add a "signature" to AST files to verify that they haven't changed
Richard Smith
richard at metafoo.co.uk
Fri Oct 24 13:13:04 PDT 2014
On Fri, Oct 24, 2014 at 8:47 AM, Ben Langmuir <blangmuir at apple.com> wrote:
>
> On Oct 24, 2014, at 8:19 AM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Fri, Oct 24, 2014 at 7:18 AM, Ben Langmuir <blangmuir at apple.com> wrote:
>
>>
>> On Oct 23, 2014, at 6:10 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> On Thu, Oct 23, 2014 at 5:36 PM, Ben Langmuir <blangmuir at apple.com>
>> wrote:
>>
>>>
>>> On Oct 23, 2014, at 4:52 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>>
>>> On Thu, Oct 23, 2014 at 3:54 PM, Ben Langmuir <blangmuir at apple.com>
>>> wrote:
>>>
>>>>
>>>> On Oct 23, 2014, at 3:34 PM, Richard Smith <richard at metafoo.co.uk>
>>>> wrote:
>>>>
>>>> On Thu, Oct 23, 2014 at 11:41 AM, Ben Langmuir <blangmuir at apple.com>
>>>> wrote:
>>>>
>>>>>
>>>>> On Oct 23, 2014, at 11:24 AM, David Blaikie <dblaikie at gmail.com>
>>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Oct 23, 2014 at 11:05 AM, Ben Langmuir <blangmuir at apple.com>
>>>>> wrote:
>>>>>
>>>>>> Author: benlangmuir
>>>>>> Date: Thu Oct 23 13:05:36 2014
>>>>>> New Revision: 220493
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=220493&view=rev
>>>>>> Log:
>>>>>> Add a "signature" to AST files to verify that they haven't changed
>>>>>>
>>>>>> Since the order of the IDs in the AST file (e.g. DeclIDs, SelectorIDs)
>>>>>> is not stable,
>>>>>
>>>>>
>>>>> Um... when you say "not stable" what do you mean? (& when you say
>>>>> "nothing changed" what do you mean)
>>>>>
>>>>>
>>>>> Identifiers, decls, macros, etc. are referred to by ID numbers, which
>>>>> depend on the order the entity is looked at during serialization. This in
>>>>> turn often depends on hash-table iteration order and is not stable across
>>>>> runs. When a module refers to a name in one of its imports, it will find
>>>>> the wrong name if the order has changed since it was first built.
>>>>>
>>>>> By “not changed” I mean that the sources have not changed.
>>>>>
>>>>>
>>>>> It's pretty important that the compiler produces identical bytes given
>>>>> identical input. We're pretty clear about fixing any cases where it doesn't
>>>>> (but maybe we haven't been as rigorous about that with AST serialization? I
>>>>> don't know - I'b be a bit scared if we haven’t)
>>>>>
>>>>>
>>>>> Maybe someone more familiar with the history of the ASTWriter can
>>>>> comment? +rsmith, +dgregor, +akyrtzidis
>>>>>
>>>>
>>>> Non-deterministic output is a bug, but it's been lower priority than
>>>> other stuff I've been looking at up until now. It's likely to get near the
>>>> top of my priority stack fairly soon. So... this patch is /technically/
>>>> making that existing problem worse by adding more nondeterminism to the
>>>> output, but since it's also fixing an immediate problem, I suggest we keep
>>>> this change in place until we've fixed the other nondeterminism, then
>>>> revert it.
>>>>
>>>> Does that seem reasonable?
>>>>
>>>>
>>>> How do you intend to maintain the deterministic output? My first
>>>> concern is that if it later regresses, the kind of bug report we will get
>>>> is ‘I used modules and this name lookup failed’.
>>>>
>>>
>>> I was intending on adding a test that builds some module twice and
>>> checks that it gives the same output.
>>>
>>>
>>>
>>>
>>> Also, we can rebuild a module and it legitimately changes (you rename
>>>> ‘A’ to ‘B’ inside one of the headers), but still the resulting pcm doesn’t
>>>> have a different size or mod-time from the previous version.
>>>>
>>>
>>> Hmm, aren't we already checking that the mtimes of the input files are
>>> the same as when we built the module before we use a module from the cache?
>>>
>>>
>>> Yes, but a pcm may be built apparently "at the same time” as its header
>>> file is modified, if the precision of mtime is too low. So we can still
>>> write a pcm “at the same time” as we last wrote it, leading to this problem
>>> of loading an out-of-date module.
>>>
>>
>> Only if the header file is written twice, both writes have the same
>> mtime, and we pick up the file between those two writes. All build systems
>> that use mtimes to determine whether they should rebuild are vulnerable to
>> this problem. (Most are also vulnerable to the input changing between being
>> read and the output changing, too, since they simply check mtime(output) <
>> mtime(inputs) to determine if a rebuild is necessary.)
>>
>>
>> I’m not really concerned by a header being modified more than once in a
>>> time period less than the mtime precision, but having the pcm built
>>> immediately before and after the header changes is more worrying.
>>>
>>
>> If we are checking that the mtimes of the input files are the same as
>> they were when we built the module, the only problems seem to be in your
>> "not really concerned" case, right?
>>
>>
>> I don’t think so. A.pcm imports C.pcm, and B.pcm imports C.pcm.
>>
>> * At the beginning C.h has mtime T1.
>> * We build A and C: mtime for A.pcm, C.pcm is T2; A.pcm expects the mtime
>> of C.pcm to be T2 and C.pcm expects the mtime of C.h to be T1.
>> * immediately change C.h, it’s mtime is T2
>> * Now we build B, and because C.h has changed we rebuild C.pcm. This can
>> happen quickly enough that the mtime of B.pcm and C.pcm are both T2.
>> * If we later import A, we will not know to rebuild it, because C.pcm and
>> C.h both have the expected mtime T2.
>>
>
> Thanks, I understand the disconnect now. I was imagining we'd also store
> the mtimes of C.pcm's inputs in A.pcm. I agree that storing them in C.pcm
> doesn't let A.pcm tell if it's got the right C.pcm. Could we address this
> by using the hash of that mtime data plus the signatures of C's imports as
> the signature of C?
>
>
> I think that would be sufficient if we can guarantee that the pcm is
> deterministically created. I admit that I’m still worried about that:
>
> I was intending on adding a test that builds some module twice and checks
>>> that it gives the same output.
>>>
>>>
> Can we exhaustively check that the output is deterministic? We can improve
> the testing as we find more issues, but I’m concerned at how mysterious the
> failure mode is. When I discovered that this was a problem it was after
> several days trying to debug a name lookup failure. I was lucky that it
> showed up in the method pool, which I eventually dumped out and saw that
> there were selectors that had mismatched identifiers in them. I don’t know
> what other crazy things can happen or if I would recognize them as symptoms
> of non-deterministic output.
>
We should be able to set up some larger determinism tests (building, say,
all of Clang twice and checking the modules come out the same); I expect
you could do something similar across Objective-C code. That should at
least give us confidence that we've found the majority of the issues.
> Are you suggesting we instead just check whether the mtimes of the input
>> files are no later than the mtime of the pcm file?
>>
>>
>> Nope.
>>
>>
>> Right now, we get at best 1 second mtime precision. We could get more
>>> precision on many (most?) filesystems for free, but I don’t know how
>>> universal high precision mtime is.
>>>
>>
>> I think everything other than FAT gives you sub-second precision these
>> days. That said, it doesn't save you from skew across NFS mounts and the
>> like, but people who share files across NFS, change them on one machine and
>> rebuild on another are probably used to timestamp-related issues :)
>>
>>
>> It’s unfortunate that “timestamp-related issues” here means mysterious
>> build failure with no obvious connection to the timestamp problem. Maybe
>> that’s the status quo on NFS though?
>>
>
> That's been my experience. GNU make sometimes spots it and emits a warning
> (I think this happens if it finds an mtime in the future), I'm not sure
> what other build tools do exactly.
>
>> This is all solved by hashing the pcm, but I don’t have any better
>>> deterministic ideas at the moment.
>>>
>>> Ben
>>>
>>> We need to get the "files changed after I read them but before I built
>>> the pcm file" case right, whether it be by checking mtimes or by computing
>>> a hash or whatever else.
>>>
>>>
>>> This has the same problem as the non-deterministic output, although it
>>>> is much less likely to happen in practice. I suggest we continue to have a
>>>> ‘signature’ but change it to be a hash of the file contents so that it
>>>> doesn’t introduce any additional non-determinism. We only need to hash the
>>>> file when it is written, so module loading won’t regress.
>>>>
>>>
>>> We can't completely discount the time cost of building a module; that
>>> will still be on the critical path for slower builds. If this hashing is
>>> low-overhead, it seems fine to me.
>>>
>>>
>>> In general, I think having a stable output would be great and I’m happy
>>>> to pitch in with testing and to help with the Objective-C bits :-)
>>>>
>>>
>>> Great, thanks!
>>>
>>> Ben
>>>>
>>>>
>>>> I briefly looked at making the IDs deterministic, but I saw several
>>>>> hash tables being iterated over with no apparently guarantee of order, and
>>>>> figured we were not trying to produce identical bytes. We certainly
>>>>> *aren’t* producing identical bytes, and when this happens it causes utter
>>>>> insanity.
>>>>>
>>>>> Ben
>>>>>
>>>>>
>>>>>
>>>>>> it is not safe to load an AST file that depends on
>>>>>> another AST file that has been rebuilt since the importer was built,
>>>>>> even if "nothing changed". We previously used size and modtime to
>>>>>> check
>>>>>> this, but I've seen cases where a module rebuilt quickly enough to
>>>>>> foil
>>>>>> this check and caused very hard to debug build errors.
>>>>>>
>>>>>> To save cycles when we're loading the AST, we just generate a random
>>>>>> nonce value and check that it hasn't changed when we load an imported
>>>>>> module, rather than actually hash the whole file.
>>>>>>
>>>>>> This is slightly complicated by the fact that we need to verify the
>>>>>> signature inside addModule, since we might otherwise consider that a
>>>>>> mdoule is "OutOfDate" when really it is the importer that is out of
>>>>>> date. I didn't see any regressions in module load time after this
>>>>>> change.
>>>>>>
>>>>>> Added:
>>>>>> cfe/trunk/test/Modules/rebuild.m
>>>>>> Modified:
>>>>>> cfe/trunk/include/clang/Serialization/ASTBitCodes.h
>>>>>> cfe/trunk/include/clang/Serialization/ASTReader.h
>>>>>> cfe/trunk/include/clang/Serialization/Module.h
>>>>>> cfe/trunk/include/clang/Serialization/ModuleManager.h
>>>>>> cfe/trunk/lib/Serialization/ASTReader.cpp
>>>>>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>>>>>> cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>>>>>> cfe/trunk/lib/Serialization/Module.cpp
>>>>>> cfe/trunk/lib/Serialization/ModuleManager.cpp
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
>>>>>> +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Thu Oct 23
>>>>>> 13:05:36 2014
>>>>>> @@ -288,7 +288,10 @@ namespace clang {
>>>>>>
>>>>>> /// \brief Record code for the module map file that was used
>>>>>> to build this
>>>>>> /// AST file.
>>>>>> - MODULE_MAP_FILE = 14
>>>>>> + MODULE_MAP_FILE = 14,
>>>>>> +
>>>>>> + /// \brief Record code for the signature that identifiers this
>>>>>> AST file.
>>>>>> + SIGNATURE = 15
>>>>>> };
>>>>>>
>>>>>> /// \brief Record types that occur within the input-files block
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
>>>>>> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Thu Oct 23
>>>>>> 13:05:36 2014
>>>>>> @@ -1125,6 +1125,7 @@ private:
>>>>>> SourceLocation ImportLoc, ModuleFile
>>>>>> *ImportedBy,
>>>>>> SmallVectorImpl<ImportedModule> &Loaded,
>>>>>> off_t ExpectedSize, time_t
>>>>>> ExpectedModTime,
>>>>>> + serialization::ASTFileSignature
>>>>>> ExpectedSignature,
>>>>>> unsigned ClientLoadCapabilities);
>>>>>> ASTReadResult ReadControlBlock(ModuleFile &F,
>>>>>> SmallVectorImpl<ImportedModule>
>>>>>> &Loaded,
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Serialization/Module.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/Module.h?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Serialization/Module.h (original)
>>>>>> +++ cfe/trunk/include/clang/Serialization/Module.h Thu Oct 23
>>>>>> 13:05:36 2014
>>>>>> @@ -97,6 +97,8 @@ public:
>>>>>> bool isNotFound() const { return Val.getInt() == NotFound; }
>>>>>> };
>>>>>>
>>>>>> +typedef unsigned ASTFileSignature;
>>>>>> +
>>>>>> /// \brief Information about a module that has been loaded by the
>>>>>> ASTReader.
>>>>>> ///
>>>>>> /// Each instance of the Module class corresponds to a single AST
>>>>>> file, which
>>>>>> @@ -152,6 +154,10 @@ public:
>>>>>> /// \brief The file entry for the module file.
>>>>>> const FileEntry *File;
>>>>>>
>>>>>> + /// \brief The signature of the module file, which may be used
>>>>>> along with size
>>>>>> + /// and modification time to identify this particular file.
>>>>>> + ASTFileSignature Signature;
>>>>>> +
>>>>>> /// \brief Whether this module has been directly imported by the
>>>>>> /// user.
>>>>>> bool DirectlyImported;
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Serialization/ModuleManager.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ModuleManager.h?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Serialization/ModuleManager.h (original)
>>>>>> +++ cfe/trunk/include/clang/Serialization/ModuleManager.h Thu Oct 23
>>>>>> 13:05:36 2014
>>>>>> @@ -179,6 +179,12 @@ public:
>>>>>> /// \param ExpectedModTime The expected modification time of the
>>>>>> module
>>>>>> /// file, used for validation. This will be zero if unknown.
>>>>>> ///
>>>>>> + /// \param ExpectedSignature The expected signature of the module
>>>>>> file, used
>>>>>> + /// for validation. This will be zero if unknown.
>>>>>> + ///
>>>>>> + /// \param ReadSignature Reads the signature from an AST file
>>>>>> without actually
>>>>>> + /// loading it.
>>>>>> + ///
>>>>>> /// \param Module A pointer to the module file if the module was
>>>>>> successfully
>>>>>> /// loaded.
>>>>>> ///
>>>>>> @@ -191,6 +197,9 @@ public:
>>>>>> SourceLocation ImportLoc,
>>>>>> ModuleFile *ImportedBy, unsigned
>>>>>> Generation,
>>>>>> off_t ExpectedSize, time_t
>>>>>> ExpectedModTime,
>>>>>> + ASTFileSignature ExpectedSignature,
>>>>>> +
>>>>>> std::function<ASTFileSignature(llvm::BitstreamReader &)>
>>>>>> + ReadSignature,
>>>>>> ModuleFile *&Module,
>>>>>> std::string &ErrorStr);
>>>>>>
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
>>>>>> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Oct 23 13:05:36 2014
>>>>>> @@ -2363,6 +2363,11 @@ ASTReader::ReadControlBlock(ModuleFile &
>>>>>> break;
>>>>>> }
>>>>>>
>>>>>> + case SIGNATURE:
>>>>>> + assert((!F.Signature || F.Signature == Record[0]) &&
>>>>>> "signature changed");
>>>>>> + F.Signature = Record[0];
>>>>>> + break;
>>>>>> +
>>>>>> case IMPORTS: {
>>>>>> // Load each of the imported PCH files.
>>>>>> unsigned Idx = 0, N = Record.size();
>>>>>> @@ -2376,6 +2381,7 @@ ASTReader::ReadControlBlock(ModuleFile &
>>>>>> SourceLocation::getFromRawEncoding(Record[Idx++]);
>>>>>> off_t StoredSize = (off_t)Record[Idx++];
>>>>>> time_t StoredModTime = (time_t)Record[Idx++];
>>>>>> + ASTFileSignature StoredSignature = Record[Idx++];
>>>>>> unsigned Length = Record[Idx++];
>>>>>> SmallString<128> ImportedFile(Record.begin() + Idx,
>>>>>> Record.begin() + Idx + Length);
>>>>>> @@ -2383,7 +2389,7 @@ ASTReader::ReadControlBlock(ModuleFile &
>>>>>>
>>>>>> // Load the AST file.
>>>>>> switch(ReadASTCore(ImportedFile, ImportedKind, ImportLoc,
>>>>>> &F, Loaded,
>>>>>> - StoredSize, StoredModTime,
>>>>>> + StoredSize, StoredModTime,
>>>>>> StoredSignature,
>>>>>> ClientLoadCapabilities)) {
>>>>>> case Failure: return Failure;
>>>>>> // If we have to ignore the dependency, we'll have to
>>>>>> ignore this too.
>>>>>> @@ -3552,7 +3558,7 @@ ASTReader::ASTReadResult ASTReader::Read
>>>>>> SmallVector<ImportedModule, 4> Loaded;
>>>>>> switch(ASTReadResult ReadResult = ReadASTCore(FileName, Type,
>>>>>> ImportLoc,
>>>>>>
>>>>>> /*ImportedBy=*/nullptr, Loaded,
>>>>>> - 0, 0,
>>>>>> + 0, 0, 0,
>>>>>>
>>>>>> ClientLoadCapabilities)) {
>>>>>> case Failure:
>>>>>> case Missing:
>>>>>> @@ -3719,6 +3725,8 @@ ASTReader::ASTReadResult ASTReader::Read
>>>>>> return Success;
>>>>>> }
>>>>>>
>>>>>> +static ASTFileSignature readASTFileSignature(llvm::BitstreamReader
>>>>>> &StreamFile);
>>>>>> +
>>>>>> ASTReader::ASTReadResult
>>>>>> ASTReader::ReadASTCore(StringRef FileName,
>>>>>> ModuleKind Type,
>>>>>> @@ -3726,12 +3734,14 @@ ASTReader::ReadASTCore(StringRef FileNam
>>>>>> ModuleFile *ImportedBy,
>>>>>> SmallVectorImpl<ImportedModule> &Loaded,
>>>>>> off_t ExpectedSize, time_t ExpectedModTime,
>>>>>> + ASTFileSignature ExpectedSignature,
>>>>>> unsigned ClientLoadCapabilities) {
>>>>>> ModuleFile *M;
>>>>>> std::string ErrorStr;
>>>>>> ModuleManager::AddModuleResult AddResult
>>>>>> = ModuleMgr.addModule(FileName, Type, ImportLoc, ImportedBy,
>>>>>> getGeneration(), ExpectedSize,
>>>>>> ExpectedModTime,
>>>>>> + ExpectedSignature, readASTFileSignature,
>>>>>> M, ErrorStr);
>>>>>>
>>>>>> switch (AddResult) {
>>>>>> @@ -4029,6 +4039,34 @@ static bool SkipCursorToBlock(BitstreamC
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> +static ASTFileSignature readASTFileSignature(llvm::BitstreamReader
>>>>>> &StreamFile){
>>>>>> + BitstreamCursor Stream(StreamFile);
>>>>>> + if (Stream.Read(8) != 'C' ||
>>>>>> + Stream.Read(8) != 'P' ||
>>>>>> + Stream.Read(8) != 'C' ||
>>>>>> + Stream.Read(8) != 'H') {
>>>>>> + return 0;
>>>>>> + }
>>>>>> +
>>>>>> + // Scan for the CONTROL_BLOCK_ID block.
>>>>>> + if (SkipCursorToBlock(Stream, CONTROL_BLOCK_ID))
>>>>>> + return 0;
>>>>>> +
>>>>>> + // Scan for SIGNATURE inside the control block.
>>>>>> + ASTReader::RecordData Record;
>>>>>> + while (1) {
>>>>>> + llvm::BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
>>>>>> + if (Entry.Kind == llvm::BitstreamEntry::EndBlock ||
>>>>>> + Entry.Kind != llvm::BitstreamEntry::Record)
>>>>>> + return 0;
>>>>>> +
>>>>>> + Record.clear();
>>>>>> + StringRef Blob;
>>>>>> + if (SIGNATURE == Stream.readRecord(Entry.ID, Record, &Blob))
>>>>>> + return Record[0];
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> /// \brief Retrieve the name of the original source file name
>>>>>> /// directly from the AST file, without actually loading the AST
>>>>>> /// file.
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
>>>>>> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Oct 23 13:05:36 2014
>>>>>> @@ -51,6 +51,7 @@
>>>>>> #include "llvm/Support/MemoryBuffer.h"
>>>>>> #include "llvm/Support/OnDiskHashTable.h"
>>>>>> #include "llvm/Support/Path.h"
>>>>>> +#include "llvm/Support/Process.h"
>>>>>> #include <algorithm>
>>>>>> #include <cstdio>
>>>>>> #include <string.h>
>>>>>> @@ -862,6 +863,7 @@ void ASTWriter::WriteBlockInfoBlock() {
>>>>>> // Control Block.
>>>>>> BLOCK(CONTROL_BLOCK);
>>>>>> RECORD(METADATA);
>>>>>> + RECORD(SIGNATURE);
>>>>>> RECORD(MODULE_NAME);
>>>>>> RECORD(MODULE_MAP_FILE);
>>>>>> RECORD(IMPORTS);
>>>>>> @@ -1092,6 +1094,14 @@ adjustFilenameForRelocatablePCH(const ch
>>>>>> return Filename + Pos;
>>>>>> }
>>>>>>
>>>>>> +static ASTFileSignature getSignature() {
>>>>>> + while (1) {
>>>>>> + if (ASTFileSignature S = llvm::sys::Process::GetRandomNumber())
>>>>>> + return S;
>>>>>> + // Rely on GetRandomNumber to eventually return non-zero...
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> /// \brief Write the control block.
>>>>>> void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext
>>>>>> &Context,
>>>>>> StringRef isysroot,
>>>>>> @@ -1121,6 +1131,11 @@ void ASTWriter::WriteControlBlock(Prepro
>>>>>> Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
>>>>>> getClangFullRepositoryVersion());
>>>>>>
>>>>>> + // Signature
>>>>>> + Record.clear();
>>>>>> + Record.push_back(getSignature());
>>>>>> + Stream.EmitRecord(SIGNATURE, Record);
>>>>>> +
>>>>>> // Module name
>>>>>> if (WritingModule) {
>>>>>> BitCodeAbbrev *Abbrev = new BitCodeAbbrev();
>>>>>> @@ -1173,6 +1188,7 @@ void ASTWriter::WriteControlBlock(Prepro
>>>>>> AddSourceLocation((*M)->ImportLoc, Record);
>>>>>> Record.push_back((*M)->File->getSize());
>>>>>> Record.push_back((*M)->File->getModificationTime());
>>>>>> + Record.push_back((*M)->Signature);
>>>>>> const std::string &FileName = (*M)->FileName;
>>>>>> Record.push_back(FileName.size());
>>>>>> Record.append(FileName.begin(), FileName.end());
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp (original)
>>>>>> +++ cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp Thu Oct 23
>>>>>> 13:05:36 2014
>>>>>> @@ -591,6 +591,10 @@ bool GlobalModuleIndexBuilder::loadModul
>>>>>> off_t StoredSize = (off_t)Record[Idx++];
>>>>>> time_t StoredModTime = (time_t)Record[Idx++];
>>>>>>
>>>>>> + // Skip the stored signature.
>>>>>> + // FIXME: we could read the signature out of the import and
>>>>>> validate it.
>>>>>> + Idx++;
>>>>>> +
>>>>>> // Retrieve the imported file name.
>>>>>> unsigned Length = Record[Idx++];
>>>>>> SmallString<128> ImportedFile(Record.begin() + Idx,
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Serialization/Module.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/Module.cpp?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Serialization/Module.cpp (original)
>>>>>> +++ cfe/trunk/lib/Serialization/Module.cpp Thu Oct 23 13:05:36 2014
>>>>>> @@ -21,7 +21,7 @@ using namespace serialization;
>>>>>> using namespace reader;
>>>>>>
>>>>>> ModuleFile::ModuleFile(ModuleKind Kind, unsigned Generation)
>>>>>> - : Kind(Kind), File(nullptr), DirectlyImported(false),
>>>>>> + : Kind(Kind), File(nullptr), Signature(0), DirectlyImported(false),
>>>>>> Generation(Generation), SizeInBits(0),
>>>>>> LocalNumSLocEntries(0), SLocEntryBaseID(0),
>>>>>> SLocEntryBaseOffset(0), SLocEntryOffsets(nullptr),
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=220493&r1=220492&r2=220493&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
>>>>>> +++ cfe/trunk/lib/Serialization/ModuleManager.cpp Thu Oct 23 13:05:36
>>>>>> 2014
>>>>>> @@ -57,6 +57,9 @@ ModuleManager::addModule(StringRef FileN
>>>>>> SourceLocation ImportLoc, ModuleFile
>>>>>> *ImportedBy,
>>>>>> unsigned Generation,
>>>>>> off_t ExpectedSize, time_t ExpectedModTime,
>>>>>> + ASTFileSignature ExpectedSignature,
>>>>>> +
>>>>>> std::function<ASTFileSignature(llvm::BitstreamReader &)>
>>>>>> + ReadSignature,
>>>>>> ModuleFile *&Module,
>>>>>> std::string &ErrorStr) {
>>>>>> Module = nullptr;
>>>>>> @@ -130,6 +133,21 @@ ModuleManager::addModule(StringRef FileN
>>>>>> // Initialize the stream
>>>>>> New->StreamFile.init((const unsigned char
>>>>>> *)New->Buffer->getBufferStart(),
>>>>>> (const unsigned char
>>>>>> *)New->Buffer->getBufferEnd());
>>>>>> +
>>>>>> + if (ExpectedSignature) {
>>>>>> + New->Signature = ReadSignature(New->StreamFile);
>>>>>> + if (New->Signature != ExpectedSignature) {
>>>>>> + ErrorStr = New->Signature ? "signature mismatch"
>>>>>> + : "could not read module
>>>>>> signature";
>>>>>> +
>>>>>> + // Remove the module file immediately, since removeModules
>>>>>> might try to
>>>>>> + // invalidate the file cache for Entry, and that is not safe
>>>>>> if this
>>>>>> + // module is *itself* up to date, but has an out-of-date
>>>>>> importer.
>>>>>> + Modules.erase(Entry);
>>>>>> + Chain.pop_back();
>>>>>> + return OutOfDate;
>>>>>> + }
>>>>>> + }
>>>>>> }
>>>>>>
>>>>>> if (ImportedBy) {
>>>>>>
>>>>>> Added: cfe/trunk/test/Modules/rebuild.m
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/rebuild.m?rev=220493&view=auto
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/test/Modules/rebuild.m (added)
>>>>>> +++ cfe/trunk/test/Modules/rebuild.m Thu Oct 23 13:05:36 2014
>>>>>> @@ -0,0 +1,29 @@
>>>>>> +// REQUIRES: shell
>>>>>> +// RUN: rm -rf %t
>>>>>> +
>>>>>> +// Build Module and set its timestamp
>>>>>> +// RUN: echo '@import Module;' | %clang_cc1 -fmodules
>>>>>> -fmodules-cache-path=%t -fdisable-module-hash -fsyntax-only -F %S/Inputs -x
>>>>>> objective-c -
>>>>>> +// RUN: touch -m -a -t 201101010000 %t/Module.pcm
>>>>>> +// RUN: cp %t/Module.pcm %t/Module.pcm.saved
>>>>>> +// RUN: wc -c %t/Module.pcm > %t/Module.size.saved
>>>>>> +
>>>>>> +// Build DependsOnModule
>>>>>> +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t
>>>>>> -fdisable-module-hash -fsyntax-only -F %S/Inputs %s
>>>>>> +// RUN: diff %t/Module.pcm %t/Module.pcm.saved
>>>>>> +// RUN: cp %t/DependsOnModule.pcm %t/DependsOnModule.pcm.saved
>>>>>> +
>>>>>> +// Rebuild Module, reset its timestamp, and verify its size hasn't
>>>>>> changed
>>>>>> +// RUN: rm %t/Module.pcm
>>>>>> +// RUN: echo '@import Module;' | %clang_cc1 -fmodules
>>>>>> -fmodules-cache-path=%t -fdisable-module-hash -fsyntax-only -F %S/Inputs -x
>>>>>> objective-c -
>>>>>> +// RUN: touch -m -a -t 201101010000 %t/Module.pcm
>>>>>> +// RUN: wc -c %t/Module.pcm > %t/Module.size
>>>>>> +// RUN: diff %t/Module.size %t/Module.size.saved
>>>>>> +// RUN: cp %t/Module.pcm %t/Module.pcm.saved.2
>>>>>> +
>>>>>> +// But the signature at least is expected to change, so we rebuild
>>>>>> DependsOnModule.
>>>>>> +// NOTE: if we change how the signature is created, this test may
>>>>>> need updating.
>>>>>> +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t
>>>>>> -fdisable-module-hash -fsyntax-only -F %S/Inputs %s
>>>>>> +// RUN: diff %t/Module.pcm %t/Module.pcm.saved.2
>>>>>> +// RUN: not diff %t/DependsOnModule.pcm %t/DependsOnModule.pcm.saved
>>>>>> +
>>>>>> + at import DependsOnModule;
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>
>>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141024/e472c3f7/attachment.html>
More information about the cfe-commits
mailing list