Author: Jan Svoboda Date: 2023-08-24T14:43:23-07:00 New Revision: b6ba804f7775f89f230ee1e62526a2f8225c7966
URL: https://github.com/llvm/llvm-project/commit/b6ba804f7775f89f230ee1e62526a2f8225c7966 DIFF: https://github.com/llvm/llvm-project/commit/b6ba804f7775f89f230ee1e62526a2f8225c7966.diff LOG: Revert "[clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file" This reverts commit 7d1565727dad3acb54fe76a908630843835d7bc8. Added: Modified: clang/include/clang/Basic/Module.h clang/include/clang/Serialization/ASTBitCodes.h clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/GlobalModuleIndex.cpp clang/test/Modules/ASTSignature.c Removed: ################################################################################ diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 7f1eb6faecf4e7..a4ad8ad2f768fe 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -81,12 +81,6 @@ struct ASTFileSignature : std::array<uint8_t, 20> { return Sentinel; } - static ASTFileSignature createDummy() { - ASTFileSignature Dummy; - Dummy.fill(0x00); - return Dummy; - } - template <typename InputIt> static ASTFileSignature create(InputIt First, InputIt Last) { assert(std::distance(First, Last) == size && diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index e759b21474c841..3ad22bbcf5d12d 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -41,7 +41,7 @@ namespace serialization { /// Version 4 of AST files also requires that the version control branch and /// revision match exactly, since there is no backward compatibility of /// AST files at this time. -const unsigned VERSION_MAJOR = 29; +const unsigned VERSION_MAJOR = 28; /// AST file minor version number supported by this version of /// Clang. diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index f2c7c03ff09360..b9d197c15c1344 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -128,17 +128,10 @@ class ASTWriter : public ASTDeserializationListener, /// The module we're currently writing, if any. Module *WritingModule = nullptr; - /// The byte range representing all the UNHASHED_CONTROL_BLOCK. - std::pair<uint64_t, uint64_t> UnhashedControlBlockRange; - /// The bit offset of the AST block hash blob. - uint64_t ASTBlockHashOffset = 0; - /// The bit offset of the signature blob. - uint64_t SignatureOffset = 0; - - /// The bit offset of the first bit inside the AST_BLOCK. + /// The offset of the first bit inside the AST_BLOCK. uint64_t ASTBlockStartOffset = 0; - /// The byte range representing all the AST_BLOCK. + /// The range representing all the AST_BLOCK. std::pair<uint64_t, uint64_t> ASTBlockRange; /// The base directory for any relative paths we emit. @@ -502,11 +495,12 @@ class ASTWriter : public ASTDeserializationListener, StringRef isysroot); /// Write out the signature and diagnostic options, and return the signature. - void writeUnhashedControlBlock(Preprocessor &PP, ASTContext &Context); - ASTFileSignature backpatchSignature(); + ASTFileSignature writeUnhashedControlBlock(Preprocessor &PP, + ASTContext &Context); /// Calculate hash of the pcm content. - std::pair<ASTFileSignature, ASTFileSignature> createSignature() const; + static std::pair<ASTFileSignature, ASTFileSignature> + createSignature(StringRef AllBytes, StringRef ASTBlockBytes); void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts); void WriteSourceManagerBlock(SourceManager &SourceMgr, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index e707f346a2c652..82dac5fbfc888d 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -4734,6 +4734,12 @@ ASTReader::ReadASTCore(StringRef FileName, ShouldFinalizePCM = true; return Success; + case UNHASHED_CONTROL_BLOCK_ID: + // This block is handled using look-ahead during ReadControlBlock. We + // shouldn't get here! + Error("malformed block record in AST file"); + return Failure; + default: if (llvm::Error Err = Stream.SkipBlock()) { Error(std::move(Err)); @@ -4853,18 +4859,13 @@ ASTReader::ASTReadResult ASTReader::readUnhashedControlBlockImpl( } switch ((UnhashedControlBlockRecordTypes)MaybeRecordType.get()) { case SIGNATURE: - if (F) { - F->Signature = ASTFileSignature::create(Blob.begin(), Blob.end()); - assert(F->Signature != ASTFileSignature::createDummy() && - "Dummy AST file signature not backpatched in ASTWriter."); - } + if (F) + F->Signature = ASTFileSignature::create(Record.begin(), Record.end()); break; case AST_BLOCK_HASH: - if (F) { - F->ASTBlockHash = ASTFileSignature::create(Blob.begin(), Blob.end()); - assert(F->ASTBlockHash != ASTFileSignature::createDummy() && - "Dummy AST block hash not backpatched in ASTWriter."); - } + if (F) + F->ASTBlockHash = + ASTFileSignature::create(Record.begin(), Record.end()); break; case DIAGNOSTIC_OPTIONS: { bool Complain = (ClientLoadCapabilities & ARR_OutOfDate) == 0; @@ -5166,12 +5167,9 @@ static ASTFileSignature readASTFileSignature(StringRef PCH) { consumeError(MaybeRecord.takeError()); return ASTFileSignature(); } - if (SIGNATURE == MaybeRecord.get()) { - auto Signature = ASTFileSignature::create(Blob.begin(), Blob.end()); - assert(Signature != ASTFileSignature::createDummy() && - "Dummy AST file signature not backpatched in ASTWriter."); - return Signature; - } + if (SIGNATURE == MaybeRecord.get()) + return ASTFileSignature::create(Record.begin(), + Record.begin() + ASTFileSignature::size); } } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index bad6723840f37d..2a12779b0d7d12 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1120,97 +1120,50 @@ adjustFilenameForRelocatableAST(const char *Filename, StringRef BaseDir) { } std::pair<ASTFileSignature, ASTFileSignature> -ASTWriter::createSignature() const { - StringRef AllBytes(Buffer.data(), Buffer.size()); - +ASTWriter::createSignature(StringRef AllBytes, StringRef ASTBlockBytes) { llvm::SHA1 Hasher; - Hasher.update(AllBytes.slice(ASTBlockRange.first, ASTBlockRange.second)); + Hasher.update(ASTBlockBytes); ASTFileSignature ASTBlockHash = ASTFileSignature::create(Hasher.result()); - // Add the remaining bytes: - // 1. Before the unhashed control block. - Hasher.update(AllBytes.slice(0, UnhashedControlBlockRange.first)); - // 2. Between the unhashed control block and the AST block. + // Add the remaining bytes (i.e. bytes before the unhashed control block that + // are not part of the AST block). + Hasher.update( + AllBytes.take_front(ASTBlockBytes.bytes_end() - AllBytes.bytes_begin())); Hasher.update( - AllBytes.slice(UnhashedControlBlockRange.second, ASTBlockRange.first)); - // 3. After the AST block. - Hasher.update(AllBytes.slice(ASTBlockRange.second, StringRef::npos)); + AllBytes.take_back(AllBytes.bytes_end() - ASTBlockBytes.bytes_end())); ASTFileSignature Signature = ASTFileSignature::create(Hasher.result()); return std::make_pair(ASTBlockHash, Signature); } -ASTFileSignature ASTWriter::backpatchSignature() { - if (!WritingModule || - !PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) - return {}; - - // For implicit modules, write the hash of the PCM as its signature. - - auto BackpatchSignatureAt = [&](const ASTFileSignature &S, uint64_t BitNo) { - using WordT = unsigned; - std::array<WordT, sizeof(ASTFileSignature) / sizeof(WordT)> Words; - static_assert(sizeof(Words) == sizeof(S)); - std::memcpy(Words.data(), S.data(), sizeof(ASTFileSignature)); - for (WordT Word : Words) { - Stream.BackpatchWord(BitNo, Word); - BitNo += sizeof(WordT) * 8; - } - }; - - ASTFileSignature ASTBlockHash; - ASTFileSignature Signature; - std::tie(ASTBlockHash, Signature) = createSignature(); - - BackpatchSignatureAt(ASTBlockHash, ASTBlockHashOffset); - BackpatchSignatureAt(Signature, SignatureOffset); - - return Signature; -} - -void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, - ASTContext &Context) { +ASTFileSignature ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, + ASTContext &Context) { using namespace llvm; // Flush first to prepare the PCM hash (signature). Stream.FlushToWord(); - UnhashedControlBlockRange.first = Stream.GetCurrentBitNo() >> 3; + auto StartOfUnhashedControl = Stream.GetCurrentBitNo() >> 3; // Enter the block and prepare to write records. RecordData Record; Stream.EnterSubblock(UNHASHED_CONTROL_BLOCK_ID, 5); // For implicit modules, write the hash of the PCM as its signature. + ASTFileSignature Signature; if (WritingModule && PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) { - // At this point, we don't know the actual signature of the file or the AST - // block - we're only able to compute those at the end of the serialization - // process. Let's store dummy signatures for now, and replace them with the - // real ones later on. - // The bitstream VBR-encodes record elements, which makes backpatching them - // really diff icult. Let's store the signatures as blobs instead - they are - // guaranteed to be word-aligned, and we control their format/encoding. - auto Dummy = ASTFileSignature::createDummy(); - SmallString<128> Blob{Dummy.begin(), Dummy.end()}; - - auto Abbrev = std::make_shared<BitCodeAbbrev>(); - Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH)); - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); - unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); - - Abbrev = std::make_shared<BitCodeAbbrev>(); - Abbrev->Add(BitCodeAbbrevOp(SIGNATURE)); - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); - unsigned SignatureAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); - - Record.push_back(AST_BLOCK_HASH); - Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob); - ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8; + ASTFileSignature ASTBlockHash; + auto ASTBlockStartByte = ASTBlockRange.first >> 3; + auto ASTBlockByteLength = (ASTBlockRange.second >> 3) - ASTBlockStartByte; + std::tie(ASTBlockHash, Signature) = createSignature( + StringRef(Buffer.begin(), StartOfUnhashedControl), + StringRef(Buffer.begin() + ASTBlockStartByte, ASTBlockByteLength)); + + Record.append(ASTBlockHash.begin(), ASTBlockHash.end()); + Stream.EmitRecord(AST_BLOCK_HASH, Record); Record.clear(); - - Record.push_back(SIGNATURE); - Stream.EmitRecordWithBlob(SignatureAbbrev, Record, Blob); - SignatureOffset = Stream.GetCurrentBitNo() - Blob.size() * 8; + Record.append(Signature.begin(), Signature.end()); + Stream.EmitRecord(SIGNATURE, Record); Record.clear(); } @@ -1279,7 +1232,7 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // Leave the options block. Stream.ExitBlock(); - UnhashedControlBlockRange.second = Stream.GetCurrentBitNo() >> 3; + return Signature; } /// Write the control block. @@ -4737,7 +4690,6 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, ASTContext &Context = SemaRef.Context; Preprocessor &PP = SemaRef.PP; - writeUnhashedControlBlock(PP, Context); collectNonAffectingInputFiles(); @@ -4886,7 +4838,7 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, // Write the remaining AST contents. Stream.FlushToWord(); - ASTBlockRange.first = Stream.GetCurrentBitNo() >> 3; + ASTBlockRange.first = Stream.GetCurrentBitNo(); Stream.EnterSubblock(AST_BLOCK_ID, 5); ASTBlockStartOffset = Stream.GetCurrentBitNo(); @@ -5239,13 +5191,13 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, Stream.EmitRecord(STATISTICS, Record); Stream.ExitBlock(); Stream.FlushToWord(); - ASTBlockRange.second = Stream.GetCurrentBitNo() >> 3; + ASTBlockRange.second = Stream.GetCurrentBitNo(); // Write the module file extension blocks. for (const auto &ExtWriter : ModuleFileExtensionWriters) WriteModuleFileExtension(SemaRef, *ExtWriter); - return backpatchSignature(); + return writeUnhashedControlBlock(PP, Context); } void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) { diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index 92417c7bf1d524..d57f4cec2eab6f 100644 --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -697,12 +697,9 @@ llvm::Error GlobalModuleIndexBuilder::loadModuleFile(const FileEntry *File) { } // Get Signature. - if (State == DiagnosticOptionsBlock && Code == SIGNATURE) { - auto Signature = ASTFileSignature::create(Blob.begin(), Blob.end()); - assert(Signature != ASTFileSignature::createDummy() && - "Dummy AST file signature not backpatched in ASTWriter."); - getModuleFileInfo(File).Signature = Signature; - } + if (State == DiagnosticOptionsBlock && Code == SIGNATURE) + getModuleFileInfo(File).Signature = ASTFileSignature::create( + Record.begin(), Record.begin() + ASTFileSignature::size); // We don't care about this record. } diff --git a/clang/test/Modules/ASTSignature.c b/clang/test/Modules/ASTSignature.c index 11d7c5c9d43d53..f0df1e4e78236f 100644 --- a/clang/test/Modules/ASTSignature.c +++ b/clang/test/Modules/ASTSignature.c @@ -1,6 +1,6 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only \ -// RUN: -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \ +// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \ +// RUN: -fimplicit-module-maps -fmodules-strict-context-hash \ // RUN: -fmodules-cache-path=%t -fdisable-module-hash %s // RUN: cp %t/MyHeader2.pcm %t1.pcm // RUN: rm -rf %t @@ -8,18 +8,17 @@ // RUN: -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \ // RUN: -fmodules-cache-path=%t -fdisable-module-hash %s // RUN: cp %t/MyHeader2.pcm %t2.pcm -// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t1.pcm > %t1.dump -// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t2.pcm > %t2.dump +// RUN: llvm-bcanalyzer --dump --disable-histogram %t1.pcm > %t1.dump +// RUN: llvm-bcanalyzer --dump --disable-histogram %t2.pcm > %t2.dump // RUN: cat %t1.dump %t2.dump | FileCheck %s #include "my_header_2.h" my_int var = 42; -// CHECK: <AST_BLOCK_HASH abbrevid={{[0-9]+}}/> blob data = '[[AST_BLOCK_HASH:.*]]' -// CHECK: <SIGNATURE abbrevid={{[0-9]+}}/> blob data = '[[SIGNATURE:.*]]' -// CHECK: <AST_BLOCK_HASH abbrevid={{[0-9]+}}/> blob data = '[[AST_BLOCK_HASH]]' -// CHECK-NOT: <SIGNATURE abbrevid={{[0-9]+}}/> blob data = '[[SIGNATURE]]' -// The modules built by this test are designed to yield the same AST but distinct AST files. -// If this test fails, it means that either the AST block has become non-relocatable, -// or the file signature stopped hashing some parts of the AST file. +// CHECK: [[AST_BLOCK_HASH:<AST_BLOCK_HASH .*>]] +// CHECK: [[SIGNATURE:<SIGNATURE .*>]] +// CHECK: [[AST_BLOCK_HASH]] +// CHECK-NOT: [[SIGNATURE]] +// The modules built by this test are designed to yield the same AST. If this +// test fails, it means that the AST block is has become non-relocatable. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
