[llvm-objdump] Fix --source with --macho flag #163810

rjmansfield · 2025-10-16T16:03:17Z

The --source option was broken when using the --macho flag because DisassembleMachO() only initialized debug info when UseDbg was true, and would return early if no dSYM was found.

llvmbot · 2025-10-16T16:03:55Z

@llvm/pr-subscribers-llvm-binary-utilities

Author: Ryan Mansfield (rjmansfield)

Changes

The --source option was broken when using the --macho flag because DisassembleMachO() only initialized debug info when UseDbg was true, and would return early if no dSYM was found.

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

2 Files Affected:

(modified) llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test (+12)
(modified) llvm/tools/llvm-objdump/MachODump.cpp (+25-7)

diff --git a/llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test b/llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test
index aaaf6bffe4a33..f7fb621244338 100644
--- a/llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test
+++ b/llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test
@@ -13,4 +13,16 @@
 # RUN: dsymutil -f  -oso-prepend-path=%p/../../dsymutil/ %t3 -o %t3.dSYM
 # RUN: llvm-objdump --source --prefix=%p/../../dsymutil  %t3 | FileCheck --check-prefix=SOURCE %s
 
+## Test that --source works with --macho flag
+
+## --macho w/ explicit .dSYM
+# RUN: llvm-objdump < %p/../../dsymutil/Inputs/basic.macho.x86_64 - --source --macho --dsym=%t1.dSYM --prefix=%p/../../dsymutil | \
+# RUN:   FileCheck --check-prefix=SOURCE %s
+
+## --macho w/ auto-detected .dSYM (dir)
+# RUN: llvm-objdump --source --macho --prefix=%p/../../dsymutil %t2 | FileCheck --check-prefix=SOURCE %s
+
+## --macho w/ auto-detected .dSYM (file)
+# RUN: llvm-objdump --source --macho --prefix=%p/../../dsymutil %t3 | FileCheck --check-prefix=SOURCE %s
+
 # SOURCE: ; int bar(int arg) {
diff --git a/llvm/tools/llvm-objdump/MachODump.cpp b/llvm/tools/llvm-objdump/MachODump.cpp
index 8e9c91fde544d..2d1259191c580 100644
--- a/llvm/tools/llvm-objdump/MachODump.cpp
+++ b/llvm/tools/llvm-objdump/MachODump.cpp
@@ -13,6 +13,7 @@
 #include "MachODump.h"
 
 #include "ObjdumpOptID.h"
+#include "SourcePrinter.h"
 #include "llvm-objdump.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
@@ -7415,18 +7416,24 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
   std::unique_ptr<DIContext> diContext;
   std::unique_ptr<Binary> DSYMBinary;
   std::unique_ptr<MemoryBuffer> DSYMBuf;
-  if (UseDbg) {
-    // If separate DSym file path was specified, parse it as a macho file,
-    // get the sections and supply it to the section name parsing machinery.
-    if (const ObjectFile *DbgObj =
-            getMachODSymObject(MachOOF, Filename, DSYMBinary, DSYMBuf)) {
+  const ObjectFile *DbgObj = MachOOF;
+  if (UseDbg || PrintSource || PrintLines) {
+    // Look for debug info in external dSYM file or embedded in the object.
+    // getMachODSymObject returns MachOOF by default if no external dSYM found.
+    const ObjectFile *DSym =
+        getMachODSymObject(MachOOF, Filename, DSYMBinary, DSYMBuf);
+    if (!DSym)
+      return;
+    DbgObj = DSym;
+    if (UseDbg) {
       // Setup the DIContext
       diContext = DWARFContext::create(*DbgObj);
-    } else {
-      return;
     }
   }
 
+  SourcePrinter SP(DbgObj, TheTarget->getName());
+  LiveElementPrinter LEP(*MRI, *STI);
+
   if (FilterSections.empty())
     outs() << "(" << DisSegName << "," << DisSectName << ") section\n";
 
@@ -7605,6 +7612,12 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
           outs() << SymName << ":\n";
 
         uint64_t PC = SectAddress + Index;
+
+        if (PrintSource || PrintLines) {
+          formatted_raw_ostream FOS(outs());
+          SP.printSourceLine(FOS, {PC, SectIdx}, Filename, LEP);
+        }
+
         if (LeadingAddr) {
           if (FullLeadingAddr) {
             if (MachOOF->is64Bit())
@@ -7696,6 +7709,11 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
 
         uint64_t PC = SectAddress + Index;
 
+        if (PrintSource || PrintLines) {
+          formatted_raw_ostream FOS(outs());
+          SP.printSourceLine(FOS, {PC, SectIdx}, Filename, LEP);
+        }
+
         if (DumpAndSkipDataInCode(PC, Bytes.data() + Index, Dices, InstSize))
           continue;

jh7370

I've added a couple of people who are more familiar with the details of Mach-O format. Hopefully one of them can review this for you.

llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test

drodriguez

Not sure if for llvm-objdump the options that are not explicitly Mach-O should work with Mach-O. Or should work the same than when --macho is not provided. It seems that --macho might have been introduced at a time that a lot of functionality was not existing.

I have checked, to the best of my ability, if this would change anything for llvm-otool, which output probably should be compatible with Apple's otool, and I don't think otool options give access to the same functionality that --source, --line-numbers, or -g, so the changes in the code should not be hit by otool.

drodriguez · 2025-10-17T23:13:01Z

llvm/tools/llvm-objdump/MachODump.cpp

+    if (!DSym)
+      return;
+    DbgObj = DSym;
+    if (UseDbg) {


Should PrintLines also be checked here?

The docs for --line-numbers say "When disassembling, display source line numbers.", while the docs for -g say "Print line information from debug info if available". It seems to be the same thing, except that -g is exclusive for the Mach-O parser. Probably they were both introduced without knowledge of each other.

Yes, good catch.

drodriguez · 2025-10-17T23:28:39Z

llvm/tools/llvm-objdump/MachODump.cpp

+  SourcePrinter SP(DbgObj, TheTarget->getName());
+  LiveElementPrinter LEP(*MRI, *STI);
+


Not very important, and I haven't double checked the code compiles/works as I think it should.

This would happen even if PrintSource or PrintLines is not used. Also FOS below is created for each line to be printed, and later destroyed (probably cheap, but…)

With a lambda, it is possible to avoid some of these problems:

std::function<void(object::SectionedAddress Address, StringRef ObjectFilename)> printSourceLineIfNeeded; if (PrintSource || PrintLines) { printSourceLineIfNeeded = [DbgObj, TheTarget, &MRI, &STI](object::SectionedAddress Address, StringRef ObjectFilename) { static SourcePrinter SP(DbgObj, TheTarget->getName()); static LiveElementPrinter LEP(*MRI, *STI); static formatted_raw_ostream FOS(outs()); SP.printSourceLine(FOS, Address, ObjectFilename, LEP); } } else { printSourceLineIfNeeded = [](object::SectionedAddress Address, StringRef ObjectFilename) {}; } ... // Below, use the lambda printSourceLineIfNeeded({PC, SectIdx}, Filename);

DisassembleMachO can be called here multiple times e.g. different slices of a universal binary. The statics in the lambda might be an issue. I can use a std::optional here to avoid the unnecessary initialization.

That's right. I thought about static being "local" to each of the outside function invocation, but it might not be like that. A small functor might work as I thought, but this should be cheap enough. No need to over-optimize if it is not a problem.

rjmansfield · 2025-10-20T12:57:22Z

Not sure if for llvm-objdump the options that are not explicitly Mach-O should work with Mach-O. Or should work the same than when --macho is not provided. It seems that --macho might have been introduced at a time that a lot of functionality was not existing.

Yeah, I couldn't think of why --source shouldn't work with --macho. Most of the other generic objdump options are supported. There's indication what's not supported with --macho by design, or what's just been overlooked. Additionally, even if a user specifies -S without --macho and the binary is universal, objdump will use macho object parser silently and then line/source information will not be shown, which may be confusing to the user as it will work for single arch binaries.

Fix --line-numbers with --macho and add test. Use std::optional to defer initialization of SourcePrinter/LiveElementPrinter.

rjmansfield · 2025-10-22T15:16:33Z

If this change is OK, could someone merge on my behalf? Thanks.

drodriguez · 2025-10-22T16:57:04Z

Restarted that failed job. There was no error. It might have been infra failure. Let's wait to see if we get green.

[llvm-objdump] Fix --source with --macho flag

d02ad93

The --source option was broken when using the --macho flag because DisassembleMachO() only initialized debug info when UseDbg was true, and would return early if no dSYM was found.

llvmbot added the llvm:binary-utilities label Oct 16, 2025

jh7370 requested review from alexander-shaposhnikov and drodriguez October 17, 2025 08:53

jh7370 reviewed Oct 17, 2025

View reviewed changes

llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test Outdated Show resolved Hide resolved

drodriguez reviewed Oct 17, 2025

View reviewed changes

Address reviewer feedback.

3ac6dbf

Fix --line-numbers with --macho and add test. Use std::optional to defer initialization of SourcePrinter/LiveElementPrinter.

drodriguez approved these changes Oct 20, 2025

View reviewed changes

drodriguez mentioned this pull request Oct 20, 2025

[llvm-objdump] Fix --disassembler-color with --macho flag #163815

Open

alexander-shaposhnikov approved these changes Oct 20, 2025

View reviewed changes

drodriguez merged commit a746fe5 into llvm:main Oct 22, 2025
13 of 14 checks passed

rjmansfield deleted the fix_objdump_src_macho branch October 22, 2025 19:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[llvm-objdump] Fix --source with --macho flag #163810

[llvm-objdump] Fix --source with --macho flag #163810

rjmansfield commented Oct 16, 2025

Uh oh!

llvmbot commented Oct 16, 2025

Uh oh!

jh7370 left a comment

Uh oh!

Uh oh!

drodriguez left a comment

Uh oh!

drodriguez Oct 17, 2025

Uh oh!

rjmansfield Oct 20, 2025

Uh oh!

drodriguez Oct 17, 2025

Uh oh!

rjmansfield Oct 20, 2025

Uh oh!

drodriguez Oct 20, 2025

Uh oh!

rjmansfield commented Oct 20, 2025

Uh oh!

rjmansfield commented Oct 22, 2025

Uh oh!

drodriguez commented Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		SourcePrinter SP(DbgObj, TheTarget->getName());
		LiveElementPrinter LEP(MRI, STI);

[llvm-objdump] Fix --source with --macho flag #163810

[llvm-objdump] Fix --source with --macho flag #163810

Conversation

rjmansfield commented Oct 16, 2025

Uh oh!

llvmbot commented Oct 16, 2025

Uh oh!

jh7370 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

drodriguez left a comment

Choose a reason for hiding this comment

Uh oh!

drodriguez Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

rjmansfield Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

drodriguez Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

rjmansfield Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

drodriguez Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

rjmansfield commented Oct 20, 2025

Uh oh!

rjmansfield commented Oct 22, 2025

Uh oh!

drodriguez commented Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants