[clang] e0589d7 - Switch to using LEB encoding for key and data lengths in on-disk hash
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 18 17:19:16 PST 2021
Author: Richard Smith
Date: 2021-02-18T17:19:01-08:00
New Revision: e0589d70fb8e29842fab228df716f73b378710a6
URL: https://github.com/llvm/llvm-project/commit/e0589d70fb8e29842fab228df716f73b378710a6
DIFF: https://github.com/llvm/llvm-project/commit/e0589d70fb8e29842fab228df716f73b378710a6.diff
LOG: Switch to using LEB encoding for key and data lengths in on-disk hash
tables.
This gives a modest AST file size reduction, while also fixing crashes
in cases where the key or data length doesn't fit into 16 bits.
Unfortunately, such situations tend to require huge test cases (such as
more than 16K modules or an overload set with 16K entries), and I
couldn't get a testcase to finish in a reasonable amount of time, so no
test is included for that bugfix.
No functionality change intended (other than the bugfix).
Added:
Modified:
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h
index a641a26661ae..51d03fcca344 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -298,7 +298,7 @@ class ModuleFile {
///
/// This pointer points into a memory buffer, where the on-disk hash
/// table for identifiers actually lives.
- const char *IdentifierTableData = nullptr;
+ const unsigned char *IdentifierTableData = nullptr;
/// A pointer to an on-disk hash table of opaque type
/// IdentifierHashTable.
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index a47bf51b63f2..ccd3e8d69d3a 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -114,6 +114,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/LEB128.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SaveAndRestore.h"
@@ -812,6 +813,31 @@ void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
// AST reader implementation
//===----------------------------------------------------------------------===//
+static uint64_t readULEB(const unsigned char *&P) {
+ unsigned Length = 0;
+ const char *Error = nullptr;
+
+ uint64_t Val = llvm::decodeULEB128(P, &Length, nullptr, &Error);
+ if (Error)
+ llvm::report_fatal_error(Error);
+ P += Length;
+ return Val;
+}
+
+/// Read ULEB-encoded key length and data length.
+static std::pair<unsigned, unsigned>
+readULEBKeyDataLength(const unsigned char *&P) {
+ unsigned KeyLen = readULEB(P);
+ if ((unsigned)KeyLen != KeyLen)
+ llvm::report_fatal_error("key too large");
+
+ unsigned DataLen = readULEB(P);
+ if ((unsigned)DataLen != DataLen)
+ llvm::report_fatal_error("data too large");
+
+ return std::make_pair(KeyLen, DataLen);
+}
+
void ASTReader::setDeserializationListener(ASTDeserializationListener *Listener,
bool TakeOwnership) {
DeserializationListener = Listener;
@@ -824,11 +850,7 @@ unsigned ASTSelectorLookupTrait::ComputeHash(Selector Sel) {
std::pair<unsigned, unsigned>
ASTSelectorLookupTrait::ReadKeyDataLength(const unsigned char*& d) {
- using namespace llvm::support;
-
- unsigned KeyLen = endian::readNext<uint16_t, little, unaligned>(d);
- unsigned DataLen = endian::readNext<uint16_t, little, unaligned>(d);
- return std::make_pair(KeyLen, DataLen);
+ return readULEBKeyDataLength(d);
}
ASTSelectorLookupTrait::internal_key_type
@@ -894,11 +916,7 @@ unsigned ASTIdentifierLookupTraitBase::ComputeHash(const internal_key_type& a) {
std::pair<unsigned, unsigned>
ASTIdentifierLookupTraitBase::ReadKeyDataLength(const unsigned char*& d) {
- using namespace llvm::support;
-
- unsigned DataLen = endian::readNext<uint16_t, little, unaligned>(d);
- unsigned KeyLen = endian::readNext<uint16_t, little, unaligned>(d);
- return std::make_pair(KeyLen, DataLen);
+ return readULEBKeyDataLength(d);
}
ASTIdentifierLookupTraitBase::internal_key_type
@@ -1086,11 +1104,7 @@ ASTDeclContextNameLookupTrait::ReadFileRef(const unsigned char *&d) {
std::pair<unsigned, unsigned>
ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char *&d) {
- using namespace llvm::support;
-
- unsigned KeyLen = endian::readNext<uint16_t, little, unaligned>(d);
- unsigned DataLen = endian::readNext<uint16_t, little, unaligned>(d);
- return std::make_pair(KeyLen, DataLen);
+ return readULEBKeyDataLength(d);
}
ASTDeclContextNameLookupTrait::internal_key_type
@@ -1847,11 +1861,7 @@ bool HeaderFileInfoTrait::EqualKey(internal_key_ref a, internal_key_ref b) {
std::pair<unsigned, unsigned>
HeaderFileInfoTrait::ReadKeyDataLength(const unsigned char*& d) {
- using namespace llvm::support;
-
- unsigned KeyLen = (unsigned) endian::readNext<uint16_t, little, unaligned>(d);
- unsigned DataLen = (unsigned) *d++;
- return std::make_pair(KeyLen, DataLen);
+ return readULEBKeyDataLength(d);
}
HeaderFileInfoTrait::internal_key_type
@@ -3188,12 +3198,13 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
}
case IDENTIFIER_TABLE:
- F.IdentifierTableData = Blob.data();
+ F.IdentifierTableData =
+ reinterpret_cast<const unsigned char *>(Blob.data());
if (Record[0]) {
F.IdentifierLookupTable = ASTIdentifierLookupTable::Create(
- (const unsigned char *)F.IdentifierTableData + Record[0],
- (const unsigned char *)F.IdentifierTableData + sizeof(uint32_t),
- (const unsigned char *)F.IdentifierTableData,
+ F.IdentifierTableData + Record[0],
+ F.IdentifierTableData + sizeof(uint32_t),
+ F.IdentifierTableData,
ASTIdentifierLookupTrait(*this, F));
PP.getIdentifierTable().setExternalIdentifierLookup(this);
@@ -4320,8 +4331,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
// Preload all the pending interesting identifiers by marking them out of
// date.
for (auto Offset : F.PreloadIdentifierOffsets) {
- const unsigned char *Data = reinterpret_cast<const unsigned char *>(
- F.IdentifierTableData + Offset);
+ const unsigned char *Data = F.IdentifierTableData + Offset;
ASTIdentifierLookupTrait Trait(*this, F);
auto KeyDataLen = Trait.ReadKeyDataLength(Data);
@@ -8524,17 +8534,13 @@ IdentifierInfo *ASTReader::DecodeIdentifierInfo(IdentifierID ID) {
assert(I != GlobalIdentifierMap.end() && "Corrupted global identifier map");
ModuleFile *M = I->second;
unsigned Index = ID - M->BaseIdentifierID;
- const char *Str = M->IdentifierTableData + M->IdentifierOffsets[Index];
-
- // All of the strings in the AST file are preceded by a 16-bit length.
- // Extract that 16-bit length to avoid having to execute strlen().
- // NOTE: 'StrLenPtr' is an 'unsigned char*' so that we load bytes as
- // unsigned integers. This is important to avoid integer overflow when
- // we cast them to 'unsigned'.
- const unsigned char *StrLenPtr = (const unsigned char*) Str - 2;
- unsigned StrLen = (((unsigned) StrLenPtr[0])
- | (((unsigned) StrLenPtr[1]) << 8)) - 1;
- auto &II = PP.getIdentifierTable().get(StringRef(Str, StrLen));
+ const unsigned char *Data =
+ M->IdentifierTableData + M->IdentifierOffsets[Index];
+
+ ASTIdentifierLookupTrait Trait(*this, *M);
+ auto KeyDataLen = Trait.ReadKeyDataLength(Data);
+ auto Key = Trait.ReadKey(Data, KeyDataLen.first);
+ auto &II = PP.getIdentifierTable().get(Key);
IdentifiersLoaded[ID] = &II;
markIdentifierFromAST(*this, II);
if (DeserializationListener)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 07787475de36..70713976d692 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -95,6 +95,7 @@
#include "llvm/Support/EndianStream.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/LEB128.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/OnDiskHashTable.h"
#include "llvm/Support/Path.h"
@@ -1615,6 +1616,15 @@ static unsigned CreateSLocExpansionAbbrev(llvm::BitstreamWriter &Stream) {
return Stream.EmitAbbrev(std::move(Abbrev));
}
+/// Emit key length and data length as ULEB-encoded data, and return them as a
+/// pair.
+static std::pair<unsigned, unsigned>
+emitULEBKeyDataLength(unsigned KeyLen, unsigned DataLen, raw_ostream &Out) {
+ llvm::encodeULEB128(KeyLen, Out);
+ llvm::encodeULEB128(DataLen, Out);
+ return std::make_pair(KeyLen, DataLen);
+}
+
namespace {
// Trait used for the on-disk hash table of header search information.
@@ -1657,19 +1667,14 @@ namespace {
std::pair<unsigned, unsigned>
EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
- using namespace llvm::support;
-
- endian::Writer LE(Out, little);
unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
- LE.write<uint16_t>(KeyLen);
unsigned DataLen = 1 + 2 + 4 + 4;
for (auto ModInfo : Data.KnownHeaders)
if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
DataLen += 4;
if (Data.Unresolved.getPointer())
DataLen += 4;
- LE.write<uint8_t>(DataLen);
- return std::make_pair(KeyLen, DataLen);
+ return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
void EmitKey(raw_ostream& Out, key_type_ref key, unsigned KeyLen) {
@@ -3008,11 +3013,7 @@ class ASTMethodPoolTrait {
std::pair<unsigned, unsigned>
EmitKeyDataLength(raw_ostream& Out, Selector Sel,
data_type_ref Methods) {
- using namespace llvm::support;
-
- endian::Writer LE(Out, little);
unsigned KeyLen = 2 + (Sel.getNumArgs()? Sel.getNumArgs() * 4 : 4);
- LE.write<uint16_t>(KeyLen);
unsigned DataLen = 4 + 2 + 2; // 2 bytes for each of the method counts
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
@@ -3022,8 +3023,7 @@ class ASTMethodPoolTrait {
Method = Method->getNext())
if (Method->getMethod())
DataLen += 4;
- LE.write<uint16_t>(DataLen);
- return std::make_pair(KeyLen, DataLen);
+ return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
void EmitKey(raw_ostream& Out, Selector Sel, unsigned) {
@@ -3320,6 +3320,15 @@ class ASTIdentifierTableTrait {
std::pair<unsigned, unsigned>
EmitKeyDataLength(raw_ostream& Out, IdentifierInfo* II, IdentID ID) {
+ // Record the location of the identifier data. This is used when generating
+ // the mapping from persistent IDs to strings.
+ Writer.SetIdentifierOffset(II, Out.tell());
+
+ // Emit the offset of the key/data length information to the interesting
+ // identifiers table if necessary.
+ if (InterestingIdentifierOffsets && isInterestingIdentifier(II))
+ InterestingIdentifierOffsets->push_back(Out.tell());
+
unsigned KeyLen = II->getLength() + 1;
unsigned DataLen = 4; // 4 bytes for the persistent ID << 1
auto MacroOffset = Writer.getMacroDirectivesOffset(II);
@@ -3336,31 +3345,11 @@ class ASTIdentifierTableTrait {
DataLen += 4;
}
}
-
- using namespace llvm::support;
-
- endian::Writer LE(Out, little);
-
- assert((uint16_t)DataLen == DataLen && (uint16_t)KeyLen == KeyLen);
- LE.write<uint16_t>(DataLen);
- // We emit the key length after the data length so that every
- // string is preceded by a 16-bit length. This matches the PTH
- // format for storing identifiers.
- LE.write<uint16_t>(KeyLen);
- return std::make_pair(KeyLen, DataLen);
+ return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
void EmitKey(raw_ostream& Out, const IdentifierInfo* II,
unsigned KeyLen) {
- // Record the location of the key data. This is used when generating
- // the mapping from persistent IDs to strings.
- Writer.SetIdentifierOffset(II, Out.tell());
-
- // Emit the offset of the key/data length information to the interesting
- // identifiers table if necessary.
- if (InterestingIdentifierOffsets && isInterestingIdentifier(II))
- InterestingIdentifierOffsets->push_back(Out.tell() - 4);
-
Out.write(II->getNameStart(), KeyLen);
}
@@ -3573,9 +3562,6 @@ class ASTDeclContextNameLookupTrait {
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &Out,
DeclarationNameKey Name,
data_type_ref Lookup) {
- using namespace llvm::support;
-
- endian::Writer LE(Out, little);
unsigned KeyLen = 1;
switch (Name.getKind()) {
case DeclarationName::Identifier:
@@ -3595,15 +3581,11 @@ class ASTDeclContextNameLookupTrait {
case DeclarationName::CXXUsingDirective:
break;
}
- LE.write<uint16_t>(KeyLen);
// 4 bytes for each DeclID.
unsigned DataLen = 4 * (Lookup.second - Lookup.first);
- assert(uint16_t(DataLen) == DataLen &&
- "too many decls for serialized lookup result");
- LE.write<uint16_t>(DataLen);
- return std::make_pair(KeyLen, DataLen);
+ return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
void EmitKey(raw_ostream &Out, DeclarationNameKey Name, unsigned) {
More information about the cfe-commits
mailing list