Skip to content

[dsymutil] Fix spurious warnings in MachODebugMapParser #78794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

JDevlieghere
Copy link
Member

When the MachODebugMapParser encounters an object file that cannot be found on disk, it currently leaves the parser in an incoherent state, resulting in spurious warnings that can in turn slow down dsymutil.

rdar://117515153

@JDevlieghere
Copy link
Member Author

This addresses #78411

@llvmbot
Copy link
Member

llvmbot commented Jan 19, 2024

@llvm/pr-subscribers-debuginfo

Author: Jonas Devlieghere (JDevlieghere)

Changes

When the MachODebugMapParser encounters an object file that cannot be found on disk, it currently leaves the parser in an incoherent state, resulting in spurious warnings that can in turn slow down dsymutil.

rdar://117515153


Full diff: https://github.com/llvm/llvm-project/pull/78794.diff

4 Files Affected:

  • (added) llvm/test/tools/dsymutil/ARM/missing-object-warning.test (+5)
  • (added) llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foo.o ()
  • (added) llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foobar.out ()
  • (modified) llvm/tools/dsymutil/MachODebugMapParser.cpp (+2-3)
diff --git a/llvm/test/tools/dsymutil/ARM/missing-object-warning.test b/llvm/test/tools/dsymutil/ARM/missing-object-warning.test
new file mode 100644
index 00000000000000..62b3ecb31888b1
--- /dev/null
+++ b/llvm/test/tools/dsymutil/ARM/missing-object-warning.test
@@ -0,0 +1,5 @@
+RUN: dsymutil -oso-prepend-path %p/../Inputs --dump-debug-map %p/../Inputs/private/tmp/missing/foobar.out 2>&1 | FileCheck %s
+
+CHECK: bar.o unable to open object file
+CHECK-NOT: could not find object file symbol for symbol _bar
+CHECK-NOT: could not find object file symbol for symbol _main
diff --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foo.o b/llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foo.o
new file mode 100644
index 00000000000000..333b39c45b1452
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foo.o differ
diff --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foobar.out b/llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foobar.out
new file mode 100755
index 00000000000000..2574c85b992924
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/missing/foobar.out differ
diff --git a/llvm/tools/dsymutil/MachODebugMapParser.cpp b/llvm/tools/dsymutil/MachODebugMapParser.cpp
index 524a6795c360e9..6a9f25681cdd1c 100644
--- a/llvm/tools/dsymutil/MachODebugMapParser.cpp
+++ b/llvm/tools/dsymutil/MachODebugMapParser.cpp
@@ -186,6 +186,8 @@ void MachODebugMapParser::addCommonSymbols() {
 /// everything up to add symbols to the new one.
 void MachODebugMapParser::switchToNewDebugMapObject(
     StringRef Filename, sys::TimePoint<std::chrono::seconds> Timestamp) {
+  addCommonSymbols();
+  resetParserState();
 
   SmallString<80> Path(PathPrefix);
   sys::path::append(Path, Filename);
@@ -206,9 +208,6 @@ void MachODebugMapParser::switchToNewDebugMapObject(
     return;
   }
 
-  addCommonSymbols();
-  resetParserState();
-
   CurrentDebugMapObject =
       &Result->addDebugMapObject(Path, Timestamp, MachO::N_OSO);
 

When the MachODebugMapParser encounters an object file that cannot be
found on disk, it currently leaves the parser in an incoherent state,
resulting in spurious warnings that can in turn slow down dsymutil.

This fixes llvm#78411.

rdar://117515153
@JDevlieghere
Copy link
Member Author

@Alpha-10000

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the test this SGTM!

@Alpha-10000
Copy link
Contributor

LGTM

@JDevlieghere JDevlieghere merged commit 593395f into llvm:main Jan 19, 2024
@JDevlieghere JDevlieghere deleted the dsymutil-117515153 branch January 19, 2024 23:11
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Jan 19, 2024
When the MachODebugMapParser encounters an object file that cannot be
found on disk, it currently leaves the parser in an incoherent state,
resulting in spurious warnings that can in turn slow down dsymutil.

This fixes llvm#78411.

rdar://117515153
(cherry picked from commit 593395f)
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Jan 19, 2024
When the MachODebugMapParser encounters an object file that cannot be
found on disk, it currently leaves the parser in an incoherent state,
resulting in spurious warnings that can in turn slow down dsymutil.

This fixes llvm#78411.

rdar://117515153
(cherry picked from commit 593395f)
JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Jan 21, 2024
[dsymutil] Fix spurious warnings in MachODebugMapParser (llvm#78794)
@yulingtianxia
Copy link

L308-L309 of MachODebugMapParser::switchToNewLibDebugMapObject may have the same issue @JDevlieghere

@Alpha-10000
Copy link
Contributor

Alpha-10000 commented Jan 22, 2024

We want to keep the DMO as long as we're not encountering a new mergeable library.
A longer term change here would probably be to have/reuse a Duplicates set similarly to what's being done for OSOs (L642), which might get rid of spurious warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants