llvm-objdump: ensure a MachO symbol isn't STAB before looking up secion

[TNorthover]

The section field has been repurposed for some STAB symbol types, and if we blindly look it up we'll produce an error and terminate. Logic already existed

Existing stabs test had a section that was in range. Unfortunately I don't know of an easy way to produce stabs entries in LLVM (I thought they died in the 90s until this came up) so I just binary-edited it to cause a failure on existing llvm-objdump.

[llvmbot]

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

Author: Tim Northover (TNorthover)

Changes

The section field has been repurposed for some STAB symbol types, and if we blindly look it up we'll produce an error and terminate. Logic already existed

Existing stabs test had a section that was in range. Unfortunately I don't know of an easy way to produce stabs entries in LLVM (I thought they died in the 90s until this came up) so I just binary-edited it to cause a failure on existing llvm-objdump.

---

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

2 Files Affected:

• (modified) llvm/test/tools/llvm-objdump/MachO/Inputs/macho-stabs-x86_64 ()
• (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+11-10)

diff --git a/llvm/test/tools/llvm-objdump/MachO/Inputs/macho-stabs-x86_64 b/llvm/test/tools/llvm-objdump/MachO/Inputs/macho-stabs-x86_64
index e95c8204c710e0..c41573aa4bf3b0 100755
Binary files a/llvm/test/tools/llvm-objdump/MachO/Inputs/macho-stabs-x86_64 and b/llvm/test/tools/llvm-objdump/MachO/Inputs/macho-stabs-x86_64 differ
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 78cf67b1e630bb..f9f073e586913f 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -2824,16 +2824,6 @@ void Dumper::printSymbol(const SymbolRef &Symbol,
     reportUniqueWarning(AddrOrErr.takeError());
     return;
   }
-  uint64_t Address = *AddrOrErr;
-  section_iterator SecI = unwrapOrError(Symbol.getSection(), FileName);
-  if (SecI != O.section_end() && shouldAdjustVA(*SecI))
-    Address += AdjustVMA;
-  if ((Address < StartAddress) || (Address > StopAddress))
-    return;
-  SymbolRef::Type Type =
-      unwrapOrError(Symbol.getType(), FileName, ArchiveName, ArchitectureName);
-  uint32_t Flags =
-      unwrapOrError(Symbol.getFlags(), FileName, ArchiveName, ArchitectureName);
 
   // Don't ask a Mach-O STAB symbol for its section unless you know that
   // STAB symbol's section field refers to a valid section index. Otherwise
@@ -2852,6 +2842,17 @@ void Dumper::printSymbol(const SymbolRef &Symbol,
                                  : unwrapOrError(Symbol.getSection(), FileName,
                                                  ArchiveName, ArchitectureName);
 
+  uint64_t Address = *AddrOrErr;
+  if (Section != O.section_end() && shouldAdjustVA(*Section))
+    Address += AdjustVMA;
+  if ((Address < StartAddress) || (Address > StopAddress))
+    return;
+  SymbolRef::Type Type =
+      unwrapOrError(Symbol.getType(), FileName, ArchiveName, ArchitectureName);
+  uint32_t Flags =
+      unwrapOrError(Symbol.getFlags(), FileName, ArchiveName, ArchitectureName);
+
+
   StringRef Name;
   if (Type == SymbolRef::ST_Debug && Section != O.section_end()) {
     if (Expected<StringRef> NameOrErr = Section->getName())

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[dwblaikie]

@adrian-prantl @JDevlieghere - perhaps one of you/your cohort might have further insight on stabs? (I believe they're used in the macho debug info model - but perhaps they're only generated by the linker? So maybe we don't have code in the llvm subproject that can generate them for testing? - but could at least include instructions on how to generate them with lld or something to include in the test case for future maintenance?)

[adrian-prantl]

You can get STABS by compiling a program with debug info:

echo 'int main() { return 0; }'>/tmp/t.c
clang -g /tmp/t.c -o /tmp/t.o -c
xcrun ld /tmp/t.o -o /tmp/a.out

if we run a.out through obj2yaml

 dsymutil -s /tmp/a.out | grep OSO
[     3] 00000026 66 (N_OSO        ) 00     0001   000000006605f353 '/private/tmp/t.o'
obj2yaml /tmp/a.out | grep '/private/tmp/t.o'
    - '/private/tmp/t.o'

the STABS seem to survive the conversion to yaml, so we should be able to use it in a test case.

[adrian-prantl]

If anyone is actually curious, here's the documentation of why Darwin still use STABS in the linker: https://wiki.dwarfstd.org/Apple's_%22Lazy%22_DWARF_Scheme.md

[TNorthover]

Interesting history, thanks.

I've updated the test to use a YAML input instead. I've not tried reducing it beyond what came out of compiling int main() {} though, there are so many brute offsets and sizes I'd almost certainly break something if I tried (and it may not even be detected until later).

[adrian-prantl]

Looks like the discussion fizzled out. The tests pass, so I'll go ahead and merge this patch for now if no-one objects.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/3845

Here is the relevant piece of the build log for the reference:

Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug53727.cpp (848 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49779.cpp (849 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug47654.cpp (850 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug50022.cpp (851 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/wtime.c (852 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/std_complex_arithmetic.cpp (853 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu :: offloading/bug49021.cpp (854 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/complex_reduction.cpp (855 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/bug49021.cpp (856 of 858)
PASS: libomptarget :: x86_64-pc-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (857 of 858)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1232.297903