Skip to content

Commit 0d1129d

Browse files
sisshiki1969claude
andcommitted
AArch64 dump_code: locate gobjdump including keg-only binutils
dump_code() shelled out to a hardcoded `gobjdump` and left a stray `dbg!`. Replace this with a `find_objdump` helper that honours an $OBJDUMP override, then searches PATH and the well-known Homebrew prefixes. Crucially this includes the keg-only `opt/binutils/bin` location, which Homebrew does not symlink onto PATH, so `gobjdump` was previously never found and dump_code fell back to the LLVM `objdump` (which rejects `-b binary -m aarch64`) and produced no output. GNU `gobjdump` is now preferred across all locations before plain `objdump`, so a later-in-PATH GNU tool wins over an earlier LLVM one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c7e95d8 commit 0d1129d

1 file changed

Lines changed: 77 additions & 10 deletions

File tree

monoasm/src/arm64.rs

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,11 @@ pub(crate) enum TargetType {
482482
/// the bitfields of the instruction word already emitted at `pos`,
483483
/// rather than into a separate displacement slot. `kind` selects the
484484
/// immediate layout.
485-
Rel { page: Page, pos: Pos, kind: Arm64Reloc },
485+
Rel {
486+
page: Page,
487+
pos: Pos,
488+
kind: Arm64Reloc,
489+
},
486490
}
487491

488492
impl JitMemory {
@@ -501,11 +505,12 @@ impl JitMemory {
501505
/// Dump the generated machine code as an objdump-style disassembly
502506
/// listing (the AArch64 counterpart of the x86-64 `dump_code`).
503507
///
504-
/// The disassembler binary defaults to `objdump`, which is the native
505-
/// tool on an aarch64 host. When running the emulated tests on a
506-
/// non-aarch64 host, set the `OBJDUMP` environment variable to a
507-
/// cross-capable binutils (e.g. `aarch64-linux-gnu-objdump`) so the
508-
/// A64 stream is decoded correctly.
508+
/// The disassembler binary is located by [`find_objdump`]: an explicit
509+
/// `OBJDUMP` override wins, otherwise `gobjdump` (the GNU binutils name
510+
/// on macOS) then `objdump` are searched on `PATH` and the usual
511+
/// Homebrew prefixes. `gobjdump` is preferred because the GNU CLI flags
512+
/// used here (`-b binary -m aarch64`) differ from the LLVM `objdump`
513+
/// shipped as the system tool on macOS.
509514
pub fn dump_code(&self) -> Result<String, std::io::Error> {
510515
use std::io::Write;
511516
use std::process::Command;
@@ -514,7 +519,7 @@ impl JitMemory {
514519
let (start_pos, code_end, _end_pos) = self.code_block.last().unwrap();
515520
file.write_all(&asm[start_pos.0..code_end.0]).unwrap();
516521

517-
let objdump = std::env::var("OBJDUMP").unwrap_or_else(|_| "objdump".to_string());
522+
let objdump = find_objdump()?;
518523
Command::new(objdump)
519524
.args([
520525
"-D",
@@ -528,7 +533,6 @@ impl JitMemory {
528533
.map(|o| {
529534
std::str::from_utf8(&o.stdout)
530535
.unwrap()
531-
.to_string()
532536
.split_inclusive('\n')
533537
.filter(|s| {
534538
s.len() > 1
@@ -568,6 +572,65 @@ impl JitMemory {
568572
}
569573
}
570574

575+
/// Locate a GNU `objdump` capable of disassembling the AArch64 byte stream
576+
/// emitted by [`JitMemory::dump_code`].
577+
///
578+
/// Resolution order:
579+
/// 1. the `OBJDUMP` environment variable, if set (e.g. a cross binutils
580+
/// such as `aarch64-linux-gnu-objdump`);
581+
/// 2. `gobjdump` then `objdump` on each `PATH` entry;
582+
/// 3. `gobjdump` then `objdump` under the common Homebrew prefixes. This
583+
/// includes the keg-only `binutils` location
584+
/// (`/opt/homebrew/opt/binutils/bin`), which Homebrew does *not* symlink
585+
/// onto `PATH`, plus the regular `bin` dirs — `cargo test` and the qemu
586+
/// runner may also launch with a minimal `PATH`.
587+
///
588+
/// `gobjdump` is tried first because on macOS the system `objdump` is the
589+
/// LLVM tool, whose CLI does not accept the GNU `-b binary -m aarch64`
590+
/// flags used by `dump_code`.
591+
fn find_objdump() -> Result<std::path::PathBuf, std::io::Error> {
592+
use std::path::{Path, PathBuf};
593+
594+
if let Some(p) = std::env::var_os("OBJDUMP") {
595+
return Ok(PathBuf::from(p));
596+
}
597+
598+
// Directories to scan: `PATH` first, then well-known Homebrew prefixes.
599+
// Homebrew's `binutils` is keg-only, so `gobjdump` lives under
600+
// `opt/binutils/bin` and is absent from a default `PATH`.
601+
let mut dirs: Vec<PathBuf> = Vec::new();
602+
if let Some(paths) = std::env::var_os("PATH") {
603+
dirs.extend(std::env::split_paths(&paths));
604+
}
605+
for prefix in [
606+
"/opt/homebrew/opt/binutils/bin",
607+
"/usr/local/opt/binutils/bin",
608+
"/opt/homebrew/bin",
609+
"/usr/local/bin",
610+
] {
611+
dirs.push(Path::new(prefix).to_path_buf());
612+
}
613+
614+
// Prefer GNU `gobjdump` over plain `objdump` *everywhere*: on macOS the
615+
// `objdump` found on `PATH` is the LLVM tool, which rejects the GNU
616+
// `-b binary -m aarch64` flags, so a later-in-PATH `gobjdump` must win.
617+
for cand in ["gobjdump", "objdump"] {
618+
for dir in &dirs {
619+
let full = dir.join(cand);
620+
if full.is_file() {
621+
return Ok(full);
622+
}
623+
}
624+
}
625+
626+
Err(std::io::Error::new(
627+
std::io::ErrorKind::NotFound,
628+
"could not find `gobjdump` or `objdump`; install GNU binutils \
629+
(`brew install binutils` provides `gobjdump`) or set $OBJDUMP to a \
630+
GNU objdump that understands aarch64",
631+
))
632+
}
633+
571634
// ===========================================================================
572635
// JIT page protection (W^X) and I-cache maintenance
573636
// ===========================================================================
@@ -648,8 +711,12 @@ impl JitProtect {
648711
unsafe {
649712
protect(contents, PAGE_SIZE * 2, Protection::READ_WRITE_EXECUTE)
650713
.expect("Mprotect failed.");
651-
protect(contents.add(PAGE_SIZE * 2), PAGE_SIZE, Protection::READ_WRITE)
652-
.expect("Mprotect failed.");
714+
protect(
715+
contents.add(PAGE_SIZE * 2),
716+
PAGE_SIZE,
717+
Protection::READ_WRITE,
718+
)
719+
.expect("Mprotect failed.");
653720
}
654721
(contents, unsafe { contents.add(PAGE_SIZE * 2) })
655722
}

0 commit comments

Comments
 (0)