Skip to content

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

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.

Sign up for GitHub

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 2 commits into from
Oct 22, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d02ad93
[llvm-objdump] Fix --source with --macho flag
rjmansfield Oct 16, 2025
3ac6dbf
Address reviewer feedback.
rjmansfield Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions llvm/test/tools/llvm-objdump/MachO/disassemble-source-dsym.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
32 changes: 25 additions & 7 deletions llvm/tools/llvm-objdump/MachODump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch.

// Setup the DIContext
diContext = DWARFContext::create(*DbgObj);
} else {
return;
}
}

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

if (FilterSections.empty())
outs() << "(" << DisSegName << "," << DisSectName << ") section\n";

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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;

Expand Down