[dsymutil] Warn on timestmap mismatch between object file and debug map

[JDevlieghere]

Summary

Add a warning when the timestmap doesn't match between the object file and the debug map entry. We were already emitting such warnings for archive members and swift interface files. This patch also unifies the warning across all three.

[aprantl]

llvm/tools/dsymutil/BinaryHolder.cpp

103

If I'm reading this right, we are warning for archive members (where the timestamp is used to disambiguate between same-named objects in different slices) only in verbose mode and here always? I would have expected it to be the other way round or at least consistent between the two.

[JDevlieghere]

llvm/tools/dsymutil/BinaryHolder.cpp

103

An archive can contain multiple files with the same name but different timestamps. If we don't find a perfect match, we'll error out (see the continue on line 202), so this is actually a stronger requirement than for object and swift interface files, where we'll print the warning but use the file regardless.

[aprantl]

llvm/tools/dsymutil/BinaryHolder.cpp

103

Oh, this would print the warning even if we just haven't yet found the exact match because it comes later? That would be bad!

[thakis]

Looks like this breaks tests on windows even after all the fix attempts: http://45.33.8.238/win/31260/step_11.txt

[davezarzycki]

The test is just broken. Tests are not allowed to modify the source as they run. I'm sorry but I need to revert this. If you need help testing on a read-only source checkout, please let me know.

FAIL: LLVM :: tools/dsymutil/debug-map-parsing.test (66341 of 73748)
******************** TEST 'LLVM :: tools/dsymutil/debug-map-parsing.test' FAILED ********************
Script:
--
: 'RUN: at line 1';   touch /home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/basic.macho.x86_64.o
: 'RUN: at line 2';   /tmp/_update_lc/r/bin/dsymutil -dump-debug-map -oso-prepend-path=/home/dave/ro_s/lp/llvm/test/tools/dsymutil /home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/basic.macho.x86_64 | /tmp/_update_lc/r/bin/FileCheck /home/dave/ro_s/lp/llvm/test/tools/dsymutil/debug-map-parsing.test
: 'RUN: at line 3';   /tmp/_update_lc/r/bin/dsymutil -dump-debug-map -oso-prepend-path=/home/dave/ro_s/lp/llvm/test/tools/dsymutil /home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/basic-lto.macho.x86_64 | /tmp/_update_lc/r/bin/FileCheck /home/dave/ro_s/lp/llvm/test/tools/dsymutil/debug-map-parsing.test --check-prefix=CHECK-LTO
: 'RUN: at line 4';   /tmp/_update_lc/r/bin/dsymutil -verbose -dump-debug-map -oso-prepend-path=/home/dave/ro_s/lp/llvm/test/tools/dsymutil /home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/basic-archive.macho.x86_64 2>&1 | /tmp/_update_lc/r/bin/FileCheck /home/dave/ro_s/lp/llvm/test/tools/dsymutil/debug-map-parsing.test --check-prefix=CHECK-ARCHIVE
: 'RUN: at line 5';   /tmp/_update_lc/r/bin/dsymutil -dump-debug-map /home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/basic.macho.x86_64 2>&1 | /tmp/_update_lc/r/bin/FileCheck /home/dave/ro_s/lp/llvm/test/tools/dsymutil/debug-map-parsing.test --check-prefix=NOT-FOUND
: 'RUN: at line 6';   not /tmp/_update_lc/r/bin/dsymutil -dump-debug-map /home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/inexistant 2>&1 | /tmp/_update_lc/r/bin/FileCheck /home/dave/ro_s/lp/llvm/test/tools/dsymutil/debug-map-parsing.test --check-prefix=NO-EXECUTABLE
--
Exit Code: 1

Command Output (stderr):
--
touch: cannot touch '/home/dave/ro_s/lp/llvm/test/tools/dsymutil/Inputs/basic.macho.x86_64.o': Read-only file system

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (1):
  LLVM :: tools/dsymutil/debug-map-parsing.test

[sberg]

As also reported at https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20210118/872711.html "Re: [llvm] 48d2068 - [dsymutil] Warn on timestmap mismatch between object file and debug map" about the re-revert https://github.com/llvm/llvm-project/commit/48d2068fb738302b9fba91c9bf8e4b051c3909eb "[dsymutil] Warn on timestmap mismatch between object file and debug map" (but for which there appears to be no Phabricator review of its own):

I have no idea what this warning is all about, and what exactly triggers it, but it looks like it breaks the

# Check if the C compiler supports the "visibility" function attribute
# If supported, defines HAVE_FUNC_ATTRIBUTE_VISIBILITY
AX_GCC_FUNC_ATTRIBUTE(visibility)

configure check used by Little-CMS (https://github.com/mm2/Little-CMS/blob/a0fc38ddd55a0341dafa7414ac624c1f09271401/configure.ac#L70; e.g. built as part of building LibreOffice), when building on macOS:

configure:15908: checking for __attribute__((visibility))
configure:15930: /Users/stephan/Software/llvm/inst/bin/clang -m64 -mmacosx-version-min=10.15 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk -fno-sanitize-recover=all -ferror-limit=0 -o conftest  -ggdb2 -Xclang -debug-info-kind=constructor -O0 -fstrict-aliasing -fstrict-overflow  -I/Users/stephan/Software/lo/core/include  -I/Users/stephan/Software/adoptopenjdk.net/OpenJDK15U-jdk_x64_mac_hotspot_15.0.1_9/jdk-15.0.1+9/Contents/Home/include  -I/Users/stephan/Software/adoptopenjdk.net/OpenJDK15U-jdk_x64_mac_hotspot_15.0.1_9/jdk-15.0.1+9/Contents/Home/include/darwin  conftest.c  >&5
warning: /Users/stephan/Software/lo/TMPDIR/conftest-01e065.o: timestamp mismatch between object file (2021-01-18 19:22:51.045749227) and debug map (2021-01-18 19:22:51.000000000)
warning: /Users/stephan/Software/lo/TMPDIR/conftest-01e065.o: timestamp mismatch between object file (2021-01-18 19:22:51.045749227) and debug map (2021-01-18 19:22:51.000000000)
configure:15930: $? = 0
configure:15943: result: no

That is, the warning output apparently confuses the AX_GCC_FUNC_ATTRIBUTE logic and makes it erroneously assume that Clang does not support attribute((visibility)).

[JDevlieghere]

In D94536#2507008, @sberg wrote:

As also reported at https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20210118/872711.html "Re: [llvm] 48d2068 - [dsymutil] Warn on timestmap mismatch between object file and debug map" about the re-revert https://github.com/llvm/llvm-project/commit/48d2068fb738302b9fba91c9bf8e4b051c3909eb "[dsymutil] Warn on timestmap mismatch between object file and debug map" (but for which there appears to be no Phabricator review of its own):

I have no idea what this warning is all about, and what exactly triggers it, but it looks like it breaks the

# Check if the C compiler supports the "visibility" function attribute
# If supported, defines HAVE_FUNC_ATTRIBUTE_VISIBILITY
AX_GCC_FUNC_ATTRIBUTE(visibility)

configure check used by Little-CMS (https://github.com/mm2/Little-CMS/blob/a0fc38ddd55a0341dafa7414ac624c1f09271401/configure.ac#L70; e.g. built as part of building LibreOffice), when building on macOS:

configure:15908: checking for __attribute__((visibility))
configure:15930: /Users/stephan/Software/llvm/inst/bin/clang -m64 -mmacosx-version-min=10.15 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk -fno-sanitize-recover=all -ferror-limit=0 -o conftest  -ggdb2 -Xclang -debug-info-kind=constructor -O0 -fstrict-aliasing -fstrict-overflow  -I/Users/stephan/Software/lo/core/include  -I/Users/stephan/Software/adoptopenjdk.net/OpenJDK15U-jdk_x64_mac_hotspot_15.0.1_9/jdk-15.0.1+9/Contents/Home/include  -I/Users/stephan/Software/adoptopenjdk.net/OpenJDK15U-jdk_x64_mac_hotspot_15.0.1_9/jdk-15.0.1+9/Contents/Home/include/darwin  conftest.c  >&5
warning: /Users/stephan/Software/lo/TMPDIR/conftest-01e065.o: timestamp mismatch between object file (2021-01-18 19:22:51.045749227) and debug map (2021-01-18 19:22:51.000000000)
warning: /Users/stephan/Software/lo/TMPDIR/conftest-01e065.o: timestamp mismatch between object file (2021-01-18 19:22:51.045749227) and debug map (2021-01-18 19:22:51.000000000)
configure:15930: $? = 0
configure:15943: result: no

That is, the warning output apparently confuses the AX_GCC_FUNC_ATTRIBUTE logic and makes it erroneously assume that Clang does not support attribute((visibility)).

Seems like a mismatch in precision between the timestamp in the debug map and the one return from the filesystem. I don't think we need that much precision so i'll truncate it to seconds.

[JDevlieghere]

Fixed in f354b87df23799ee0b6c718894140c846eafc82d

llvm/test/tools/dsymutil/X86/swift-ast-x86_64.test

 
 READOBJ: Name:{{.*}}__swift_ast
 READOBJ: |SWIFTMODULE DATA|
 READOBJ-NEXT: |.|
 
 DWARFDUMP: __swift_ast
 
 RUN: dsymutil -oso-prepend-path %p/.. %p/../Inputs/swift-ast.macho.x86_64 -no-output -verbose 2>&1 | FileCheck %s --check-prefix=TIMESTAMP
-TIMESTAMP: warning: Timestamp mismatch
+TIMESTAMP: warning: {{.*}}/swift-ast.swiftmodule: timestamp mismatch between swift interface file ({{.*}}) and debug map ({{.*}})
 
 RUN: dsymutil -s %T/swift-ast.dSYM/Contents/Resources/DWARF/swift-ast.macho.x86_64 | FileCheck %s --check-prefix=NAST
 NAST-NOT: N_AST

llvm/test/tools/dsymutil/debug-map-parsing.test

+RUN: touch %p/Inputs/basic.macho.x86_64.o
 RUN: dsymutil -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic.macho.x86_64 | FileCheck %s
 RUN: dsymutil -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic-lto.macho.x86_64 | FileCheck %s --check-prefix=CHECK-LTO
 RUN: dsymutil -verbose -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic-archive.macho.x86_64 2>&1 | FileCheck %s --check-prefix=CHECK-ARCHIVE
 RUN: dsymutil -dump-debug-map %p/Inputs/basic.macho.x86_64 2>&1 | FileCheck %s --check-prefix=NOT-FOUND
 RUN: not dsymutil -dump-debug-map %p/Inputs/inexistant 2>&1 | FileCheck %s --check-prefix=NO-EXECUTABLE
 
 
 Check that We can parse the debug map of the basic executable.

 CHECK-LTO: ...
 
 Check thet we correctly handle debug maps with archive members (including only
 opening the archive once if mulitple of its members are used).
 
 CHECK-ARCHIVE:      trying to open {{.*}}basic-archive.macho.x86_64'
 CHECK-ARCHIVE-NEXT:   loaded object.
 CHECK-ARCHIVE-NEXT: trying to open {{.*}}/Inputs/basic1.macho.x86_64.o'
+CHECK-ARCHIVE-NEXT: warning: {{.*}}/Inputs/basic1.macho.x86_64.o: timestamp mismatch between object file ({{.*}}) and debug map ({{.*}})
 CHECK-ARCHIVE-NEXT:   loaded object.
 CHECK-ARCHIVE-NEXT: trying to open {{.*}}/libbasic.a(basic2.macho.x86_64.o)'
 CHECK-ARCHIVE-NEXT:   loaded archive {{.*}}/libbasic.a'
 CHECK-ARCHIVE-NEXT:   found member in archive.
 CHECK-ARCHIVE-NEXT: trying to open {{.*}}/libbasic.a(basic3.macho.x86_64.o)'
 CHECK-ARCHIVE-NEXT:   found member in archive.
 CHECK-ARCHIVE: ---
 CHECK-ARCHIVE: triple: 'x86_64-apple-darwin'

llvm/tools/dsymutil/BinaryHolder.h

protected:
     std::string FatBinaryName;
   };
 
   /// Cached entry holding one or more (in case of a fat binary) object files.
   class ObjectEntry : public EntryBase {
   public:
     /// Load the given object binary in memory.
     Error load(IntrusiveRefCntPtr<vfs::FileSystem> VFS, StringRef Filename,
-               bool Verbose = false);
+               TimestampTy Timestamp, bool Verbose = false);
 
     /// Access all owned ObjectFiles.
     std::vector<const object::ObjectFile *> getObjects() const;
 
     /// Access to a derived version of all the currently owned ObjectFiles. The
     /// conversion might be invalid, in which case an Error is returned.
     template <typename ObjectFileType>
     Expected<std::vector<const ObjectFileType *>> getObjectsAs() const {

llvm/tools/dsymutil/BinaryHolder.cpp

if (!ErrOrArchive)
       return ErrOrArchive.takeError();
     Archives.push_back(std::move(*ErrOrArchive));
   }
 
   return Error::success();
 }
 
 Error BinaryHolder::ObjectEntry::load(IntrusiveRefCntPtr<vfs::FileSystem> VFS,
-                                      StringRef Filename, bool Verbose) {
+                                      StringRef Filename, TimestampTy Timestamp,
+                                      bool Verbose) {
   // Try to load regular binary and force it to be memory mapped.
   auto ErrOrBuff = (Filename == "-")
                        ? MemoryBuffer::getSTDIN()
                        : VFS->getBufferForFile(Filename, -1, false);
   if (auto Err = ErrOrBuff.getError())
     return errorCodeToError(Err);
 
+  if (Filename != "-" && Timestamp != sys::TimePoint<>()) {
+    llvm::ErrorOr<vfs::Status> Stat = VFS->status(Filename);
+    if (!Stat)
+      return errorCodeToError(Stat.getError());
+    if (Timestamp != Stat->getLastModificationTime())
+      WithColor::warning() << Filename
+                           << ": timestamp mismatch between object file ("
+                           << Stat->getLastModificationTime()
+                           << ") and debug map (" << Timestamp << ")\n";
+  }
+
   MemBuffer = std::move(*ErrOrBuff);
 
   if (Verbose)
     WithColor::note() << "loaded object.\n";
 
   // Load one or more object buffers, depending on whether we're dealing with a
   // fat binary.
   std::vector<MemoryBufferRef> ObjectBuffers;
for (auto Child : Archive->children(Err)) {
         if (*NameOrErr == ObjectFilename) {
           auto ModTimeOrErr = Child.getLastModified();
           if (!ModTimeOrErr)
             return ModTimeOrErr.takeError();
 
           if (Timestamp != sys::TimePoint<>() &&
               Timestamp != ModTimeOrErr.get()) {
             if (Verbose)
-              WithColor::warning() << "member has timestamp mismatch.\n";
+              WithColor::warning()
+                  << *NameOrErr
+                  << ": timestamp mismatch between archive member ("
+                  << ModTimeOrErr.get() << ") and debug map (" << Timestamp
+                  << ")\n";
             continue;
           }
 
           if (Verbose)
             WithColor::note() << "found member in archive.\n";
 
           auto ErrOrMem = Child.getMemoryBufferRef();
           if (!ErrOrMem)
if (isArchive(Filename)) {
     }
   }
 
   // If this is an object, we might have it cached. If not we'll have to load
   // it from the file system and cache it now.
   std::lock_guard<std::mutex> Lock(ObjectCacheMutex);
   if (!ObjectCache.count(Filename)) {
     ObjectEntry &OE = ObjectCache[Filename];
-    auto Err = OE.load(VFS, Filename, Verbose);
+    auto Err = OE.load(VFS, Filename, Timestamp, Verbose);
     if (Err) {
       ObjectCache.erase(Filename);
       return std::move(Err);
     }
   }
 
   return ObjectCache[Filename];
 }

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

if (Obj->getType() == MachO::N_AST) {
       if (!Options.NoTimestamp) {
         // The modification can have sub-second precision so we need to cast
         // away the extra precision that's not present in the debug map.
         auto ModificationTime =
             std::chrono::time_point_cast<std::chrono::seconds>(
                 Stat.getLastModificationTime());
         if (ModificationTime != Obj->getTimestamp()) {
           // Not using the helper here as we can easily stream TimePoint<>.
-          WithColor::warning() << "Timestamp mismatch for " << File << ": "
-                               << Stat.getLastModificationTime() << " and "
-                               << sys::TimePoint<>(Obj->getTimestamp()) << "\n";
+          WithColor::warning()
+              << File << ": timestamp mismatch between swift interface file ("
+              << sys::TimePoint<>(Obj->getTimestamp()) << ") and debug map ("
+              << sys::TimePoint<>(Obj->getTimestamp()) << ")\n";
           continue;
         }
       }
 
       // Copy the module into the .swift_ast section.
       if (!Options.NoOutput)
         Streamer->emitSwiftAST((*ErrorOrMem)->getBuffer());