[clang][modules] Use relative offsets for input files

[jansvoboda11]

Summary

This patch replaces absolute offsets into the input files block with offsets relative to the block start. This makes the whole section "relocatable". I confirmed all other uses of GetCurrentBitNo() are turned into relative offsets before being serialized into the AST file.

[benlangmuir]

clang/include/clang/Serialization/ModuleFile.h

249

Doesn't InputFilesCursor already know where the input files block starts?

clang/lib/Serialization/ASTReader.cpp

5334

We should initialize this to something - either 0 or maybe ~0 so it will be invalid?

[jansvoboda11]

clang/include/clang/Serialization/ModuleFile.h

249

I think it does. We should be able to remove this and always call InputFilesCursor::GetCurrentBitNo() at the start. I implemented it this way because it's consistent with the existing pattern: SLocEntryCursor/SourceManagerBlockStartOffset, MacroCursor/MacroOffsetsBase, DeclsCursor/DeclsBlockStartOffset.

LMK if you feel strongly about it and I can look into getting rid of the extra offset base members.

clang/lib/Serialization/ASTReader.cpp

5334

Good point, I'll do that along with initializing the member variable.

[benlangmuir]

clang/include/clang/Serialization/ModuleFile.h

249

Since it's consistent with other uses I'm fine with keeping it as-is for this patch. Would be nice to clean them all up in a follow up, but not required.

[jansvoboda11]

Initialize absolute offsets in ASTReader/ModuleFile.

[phosek]

After this change, all libc++ clang_modules_include.gen.py tests started failing on our Linux builders:

Script:
--
: 'COMPILED WITH';  /b/s/w/ir/x/w/llvm_build/./bin/clang++ /b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp -pthread --target=aarch64-unknown-linux-gnu -nostdinc++ -I /b/s/w/ir/x/w/llvm_build/include/c++/v1 -I /b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1 -I /b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  -fsyntax-only
: 'RUN: at line 2';   /b/s/w/ir/x/w/llvm_build/./bin/clang++ /b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp -pthread --target=aarch64-unknown-linux-gnu -nostdinc++ -I /b/s/w/ir/x/w/llvm_build/include/c++/v1 -I /b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1 -I /b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -Wno-unknown-pragmas -Wno-pass-failed -Wno-mismatched-new-delete -Wno-redundant-move -Wno-self-move -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  -fmodules -fcxx-modules -fmodules-cache-path=/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/Output/algorithm.compile.pass.cpp.dir/t.tmp -fsyntax-only
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "COMPILED WITH"
$ "/b/s/w/ir/x/w/llvm_build/./bin/clang++" "/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp" "-pthread" "--target=aarch64-unknown-linux-gnu" "-nostdinc++" "-I" "/b/s/w/ir/x/w/llvm_build/include/c++/v1" "-I" "/b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1" "-I" "/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support" "-std=c++26" "-Werror" "-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" "-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wno-reserved-module-identifier" "-Wno-user-defined-literals" "-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-Wno-local-type-template-args" "-Wno-c++11-extensions" "-Wno-unknown-pragmas" "-Wno-pass-failed" "-Wno-mismatched-new-delete" "-Wno-redundant-move" "-Wno-self-move" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" "-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" "-Wuser-defined-warnings" "-fsyntax-only"
$ ":" "RUN: at line 2"
$ "/b/s/w/ir/x/w/llvm_build/./bin/clang++" "/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp" "-pthread" "--target=aarch64-unknown-linux-gnu" "-nostdinc++" "-I" "/b/s/w/ir/x/w/llvm_build/include/c++/v1" "-I" "/b/s/w/ir/x/w/llvm_build/include/aarch64-unknown-linux-gnu/c++/v1" "-I" "/b/s/w/ir/x/w/llvm-llvm-project/libcxx/test/support" "-std=c++26" "-Werror" "-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" "-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wno-reserved-module-identifier" "-Wno-user-defined-literals" "-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-Wno-local-type-template-args" "-Wno-c++11-extensions" "-Wno-unknown-pragmas" "-Wno-pass-failed" "-Wno-mismatched-new-delete" "-Wno-redundant-move" "-Wno-self-move" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" "-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" "-Wuser-defined-warnings" "-fmodules" "-fcxx-modules" "-fmodules-cache-path=/b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/Output/algorithm.compile.pass.cpp.dir/t.tmp" "-fsyntax-only"
# command stderr:
clang++: clang/lib/Serialization/ASTReader.cpp:6515: void clang::ASTReader::ReadPragmaDiagnosticMappings(DiagnosticsEngine &): Assertion `IDAndOffset.second == 0 && "not a start location for a FileID"' failed.
While building module 'std_algorithm' imported from /b/s/w/ir/x/w/llvm_build/runtimes/runtimes-aarch64-unknown-linux-gnu-bins/test/libcxx/clang_modules_include.gen.py/algorithm.compile.pass.cpp:22:
While building module 'std_private_algorithm_adjacent_find' imported from /b/s/w/ir/x/w/llvm_build/include/c++/v1/algorithm:1744:
While building module 'std_private_algorithm_iterator_operations' imported from /b/s/w/ir/x/w/llvm_build/include/c++/v1/__algorithm/adjacent_find.h:14:
While building module 'std_private_algorithm_ranges_iterator_concept' imported from /b/s/w/ir/x/w/llvm_build/include/c++/v1/__algorithm/iterator_operations.h:13:
In file included from <module-includes>:1:
/b/s/w/ir/x/w/llvm_build/include/c++/v1/__algorithm/ranges_iterator_concept.h:13:10: fatal error: could not build module 'std_private_iterator_concepts'
   13 | #include <__iterator/concepts.h>
      |  ~~~~~~~~^
...

You can see the full output at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8771862804321535569/test-results. Would it be possible to revert this change?

[jansvoboda11]

In D158572#4615198, @phosek wrote:

After this change, all libc++ clang_modules_include.gen.py tests started failing on our Linux builders:

...

You can see the full output at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-arm64/b8771862804321535569/test-results. Would it be possible to revert this change?

Thanks for notifying me. Reverting now...

[jansvoboda11]

@phosek BTW can you confirm whether these started failing with this patch or with D158573?

[phosek]

In D158572#4615231, @jansvoboda11 wrote:

@phosek BTW can you confirm whether these started failing with this patch or with D158573?

I'm trying to do a local build now to find out.

[phosek]

In D158572#4615234, @phosek wrote:

In D158572#4615231, @jansvoboda11 wrote:

@phosek BTW can you confirm whether these started failing with this patch or with D158573?

I'm trying to do a local build now to find out.

Looks like it was D158573 so it should be safe to reland this, sorry about the confusion.

clang/include/clang/Serialization/ModuleFile.h

public:
   /// ContinuousRangeMaps described below have not yet been populated.
   StringRef ModuleOffsetMap;
 
   // === Input Files ===
 
   /// The cursor to the start of the input-files block.
   llvm::BitstreamCursor InputFilesCursor;
 
-  /// Offsets for all of the input file entries in the AST file.
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase = 0;
+
+  /// Relative offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.
   std::vector<InputFile> InputFilesLoaded;
 
   /// The input file infos that have been loaded from this AST file.
   std::vector<InputFileInfo> InputFileInfosLoaded;
 

clang/lib/Serialization/ASTReader.cpp

InputFileInfo ASTReader::getInputFileInfo(ModuleFile &F, unsigned ID) {
 
   // If we've already loaded this input file, return it.
   if (!F.InputFileInfosLoaded[ID - 1].Filename.empty())
     return F.InputFileInfosLoaded[ID - 1];
 
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+                                         F.InputFileOffsets[ID - 1])) {
     // FIXME this drops errors on the floor.
     consumeError(std::move(Err));
   }
 
   Expected<unsigned> MaybeCode = Cursor.ReadCode();
   if (!MaybeCode) {
     // FIXME this drops errors on the floor.
     consumeError(MaybeCode.takeError());
if (F.InputFilesLoaded[ID-1].getFile())
     return F.InputFilesLoaded[ID-1];
 
   if (F.InputFilesLoaded[ID-1].isNotFound())
     return InputFile();
 
   // Go find this input file.
   BitstreamCursor &Cursor = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+                                         F.InputFileOffsets[ID - 1])) {
     // FIXME this drops errors on the floor.
     consumeError(std::move(Err));
   }
 
   InputFileInfo FI = getInputFileInfo(F, ID);
   off_t StoredSize = FI.StoredSize;
   time_t StoredTime = FI.StoredTime;
   bool Overridden = FI.Overridden;
case llvm::BitstreamEntry::SubBlock:
         if (llvm::Error Err = Stream.SkipBlock()) {
           Error(std::move(Err));
           return Failure;
         }
         if (ReadBlockAbbrevs(F.InputFilesCursor, INPUT_FILES_BLOCK_ID)) {
           Error("malformed block record in AST file");
           return Failure;
         }
+        F.InputFilesOffsetBase = F.InputFilesCursor.GetCurrentBitNo();
         continue;
 
       case OPTIONS_BLOCK_ID:
         // If we're reading the first module for this group, check its options
         // are compatible with ours. For modules it imports, no further checking
         // is required, because we checked them when we built it.
         if (Listener && !ImportedBy) {
           // Should we allow the configuration of the module file to differ from
bool ASTReader::readASTFileControlBlock(
   // Scan for the CONTROL_BLOCK_ID block.
   if (SkipCursorToBlock(Stream, CONTROL_BLOCK_ID))
     return true;
 
   bool NeedsInputFiles = Listener.needsInputFileVisitation();
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase = 0;
 
   RecordData Record;
   std::string ModuleDir;
   bool DoneWithControlBlock = false;
   while (!DoneWithControlBlock) {
     Expected<llvm::BitstreamEntry> MaybeEntry = Stream.advance();
     if (!MaybeEntry) {
       // FIXME this drops the error on the floor.
case llvm::BitstreamEntry::SubBlock: {
         if (llvm::Error Err = Stream.SkipBlock()) {
           // FIXME this drops the error on the floor.
           consumeError(std::move(Err));
           return true;
         }
         if (NeedsInputFiles &&
             ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
           return true;
+        InputFilesOffsetBase = InputFilesCursor.GetCurrentBitNo();
         break;
 
       default:
         if (llvm::Error Err = Stream.SkipBlock()) {
           // FIXME this drops the error on the floor.
           consumeError(std::move(Err));
           return true;
         }
case INPUT_FILE_OFFSETS: {
         // Go find this input file.
         bool isSystemFile = I >= NumUserFiles;
 
         if (isSystemFile && !NeedsSystemInputFiles)
           break; // the rest are system input files
 
         BitstreamCursor &Cursor = InputFilesCursor;
         SavedStreamPosition SavedPosition(Cursor);
-        if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+        if (llvm::Error Err =
+                Cursor.JumpToBit(InputFilesOffsetBase + InputFileOffs[I])) {
           // FIXME this drops errors on the floor.
           consumeError(std::move(Err));
         }
 
         Expected<unsigned> MaybeCode = Cursor.ReadCode();
         if (!MaybeCode) {
           // FIXME this drops errors on the floor.
           consumeError(MaybeCode.takeError());

clang/lib/Serialization/ASTWriter.cpp

void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
 
   // Create input file hash abbreviation.
   auto IFHAbbrev = std::make_shared<BitCodeAbbrev>();
   IFHAbbrev->Add(BitCodeAbbrevOp(INPUT_FILE_HASH));
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector<InputFileEntry> UserFiles;
   std::vector<InputFileEntry> SystemFiles;
   for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size(); I != N; ++I) {
     // Get this source location entry.
     const SrcMgr::SLocEntry *SLoc = &SourceMgr.getLocalSLocEntry(I);
     assert(&SourceMgr.getSLocEntry(FileID::get(I)) == SLoc);
 
void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
   // Write out all of the input files.
   std::vector<uint64_t> InputFileOffsets;
   for (const auto &Entry : SortedFiles) {
     uint32_t &InputFileID = InputFileIDs[Entry.File];
     if (InputFileID != 0)
       continue; // already recorded this file.
 
     // Record this entry's offset.
-    InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+    InputFileOffsets.push_back(Stream.GetCurrentBitNo() - InputFilesOffsetBase);
 
     InputFileID = InputFileOffsets.size();
 
     if (!Entry.IsSystemFile)
       ++UserFilesNum;
 
     // Emit size/modification time for this file.
     // And whether this file was overridden.