[llvm-symbolizer][llvm-nm] Fix AArch64 and ARM mapping symbols handling.

[yroux]

Summary

The goal of this patch is to exclude AArch64 mapping symbols ($x and $d) for symtab symbolization as it was done for ARM since D95916 to bring the bots back to green state.

This is implemented by setting SF_FormatSpecific such that llvm-symbolizer will ignore them, and use this flag to re-implement llvm-nm --special-syms option, by doing so it will work on AArch64 and ARM (it wasn't working on the later one since SF_FormatSpecific symbols were ignored in llvm-nm).

[MaskRay]

LGTM.

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

2

# REQUIRES: aarch64-registered-target
## Ignore AArch64 mapping symbols (with a prefix of $d or $x).

[MaskRay]

I suggest you split the llvm-nm part to a separate patch because that would affect RISC-V: D98669

[jhenderson]

You've got a set of test failures being reported by the pre-merge bots. They look like they could be coming from this patch. Please take a look at them.

I also agree with @MaskRay that you should split the llvm-nm aspect into its own patch.

[yroux]

Thanks for the reviews,

@jhenderson, yes I'm looking at these COFF regressions, the reason I've included llvm-nm changes into this patch is that marking these symbols as SF_FormatSpecific breaks llvm --special-syms, thus I'd prefer to restrict llvm-nm changes to ARM and AArch64 targets if that's fine for you, otherwise I'll just split the patch.

[jhenderson]

Could you clarify why adding the format specific flag breaks the --special-syms option?

[yroux]

Sure, because in dumpSymbolNamesFromObject line1815 these symbols are skipped if they are not debug ones, thus not printed even if --special-syms flag is used.

[yroux]

This new version should just change llvm-nm behavior for ELF ARM and AArch64 targets to handle SF_FormatSpecific symbols and doesn't change the processing of COFF or debug symbols

[jhenderson]

I assume there's already test coverage for --debug-syms with ARM and AARCH64 symbols?

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

9–12

It might be helpful to illustrate with an llvm-nm execution where the $d and $x appear, to help validate the test. At the moment, this test simply shows that foo is used for values 0 and 4.

llvm/test/tools/llvm-nm/ARM/special-syms.test

2

Add a comment to the top of this test explaining what symbols are considered special for ARM.

Do you need an AARCH64 version of this test?

llvm/tools/llvm-nm/llvm-nm.cpp

1808

Also, clang-format the new code below.

[yroux]

Hmm, there is tools/llvm-nm/debug-syms.test which tests ARM --special-syms and --debug-syms, notice that with this change --special-syms is no more needed to dump mapping-symbols when --debug-syms is used, which is now in sync with gnu binutils nm. I don't see an AArch64 test though

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

9–12

I agree, I'll also add one to the existing arm version of this test

llvm/test/tools/llvm-nm/ARM/special-syms.test

2

I'll add a comment which describes the test.

AArch64 version already exists under llvm-nm/AArch64 directory

[jhenderson]

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

4

I have this vague, and quite possibly incorrect, idea that you can just omit the "--" part of the triple completely (i.e. -triple=aarch64).

5

Add a comment explaining why the llvm-nm line is useful. Ditto for the ARM case.

llvm/test/tools/llvm-nm/ARM/special-syms.test

3

llvm/test/tools/llvm-nm/debug-syms.test

2–3

Does this behaviour (that mapping symbols are printed with --debug-syms and not --special-syms) match GNU nm behaviour?

llvm/tools/llvm-nm/llvm-nm.cpp

1808

Don't forget to run clang-format on this. I think there's a trailing space on the second line of the comment.

[yroux]

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

4

Ah yes that's correct, I think I'm use to write it that way because I saw that in other tests, but I'll change that

5

OK

llvm/test/tools/llvm-nm/debug-syms.test

2–3

Yes with that change llvm-nm will match nm behaviour w/r to --special-syms and --debug-syms

[jhenderson]

LGTM, with comment suggestion.

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

6

llvm/test/DebugInfo/Symbolize/ELF/arm-mapping-symbol.s

6

llvm/include/llvm/Object/ELFObjectFile.h

if (Expected<typename ELFT::SymRange> SymbolsOrErr =
           EF.symbols(DotDynSymSec)) {
     // Set the SF_FormatSpecific flag for the 0-index null symbol.
     if (ESym == SymbolsOrErr->begin())
       Result |= SymbolRef::SF_FormatSpecific;
   } else
     // TODO: Test this error.
     return SymbolsOrErr.takeError();
 
-  if (EF.getHeader().e_machine == ELF::EM_ARM) {
+  if (EF.getHeader().e_machine == ELF::EM_AARCH64) {
+    if (Expected<StringRef> NameOrErr = getSymbolName(Sym)) {
+      StringRef Name = *NameOrErr;
+      if (Name.startswith("$d") || Name.startswith("$x"))
+        Result |= SymbolRef::SF_FormatSpecific;
+    } else {
+      // TODO: Actually report errors helpfully.
+      consumeError(NameOrErr.takeError());
+    }
+  } else if (EF.getHeader().e_machine == ELF::EM_ARM) {
     if (Expected<StringRef> NameOrErr = getSymbolName(Sym)) {
       StringRef Name = *NameOrErr;
       if (Name.startswith("$d") || Name.startswith("$t") ||
           Name.startswith("$a"))
         Result |= SymbolRef::SF_FormatSpecific;
     } else {
       // TODO: Actually report errors helpfully.
       consumeError(NameOrErr.takeError());

llvm/test/DebugInfo/Symbolize/ELF/aarch64-mapping-symbol.s

+# REQUIRES: aarch64-registered-target
+## Ignore AArch64 mapping symbols (with a prefix of $d or $x).
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t
+
+## Verify that mapping symbols are actually present in the object. 
+# RUN: llvm-nm --special-syms %t | FileCheck %s -check-prefix MAPPING_SYM
+
+# MAPPING_SYM:      0000000000000000 t $d.0
+# MAPPING_SYM-NEXT: 000000000000000c t $d.2
+# MAPPING_SYM-NEXT: 0000000000000004 t $x.1
+# MAPPING_SYM-NEXT: 0000000000000000 T foo
+
+# RUN: llvm-symbolizer --obj=%t 0 4 0xc | FileCheck %s -check-prefix SYMBOL
+
+# SYMBOL:      foo
+# SYMBOL-NEXT: ??:0:0
+# SYMBOL-EMPTY:
+# SYMBOL:      foo
+# SYMBOL-NEXT: ??:0:0
+# SYMBOL-EMPTY:
+# SYMBOL:      foo
+# SYMBOL-NEXT: ??:0:0
+
+    .global foo
+foo:
+    .word 32
+    nop
+    nop
+    .word 42

llvm/test/DebugInfo/Symbolize/ELF/arm-mapping-symbol.s

 # REQUIRES: arm-registered-target
 ## Ignore ARM mapping symbols (with a prefix of $a, $d or $t).
 
 # RUN: llvm-mc -filetype=obj -triple=armv7-none-linux %s -o %t
-# RUN: llvm-symbolizer --obj=%t 4 8 | FileCheck %s
+
+## Verify that mapping symbols are actually present in the object.
+# RUN: llvm-nm --special-syms %t | FileCheck %s -check-prefix MAPPING_A
+
+# MAPPING_A:      00000004 t $a.1
+# MAPPING_A-NEXT: 00000000 t $d.0
+# MAPPING_A-NEXT: 00000008 t $d.2
+# MAPPING_A-NEXT: 00000000 T foo
+
 # RUN: llvm-mc -filetype=obj -triple=thumbv7-none-linux %s -o %tthumb
-# RUN: llvm-symbolizer --obj=%tthumb 4 8 | FileCheck %s
+# RUN: llvm-nm --special-syms %tthumb | FileCheck %s -check-prefix MAPPING_T
+
+# MAPPING_T:      00000000 t $d.0
+# MAPPING_T-NEXT: 00000006 t $d.2
+# MAPPING_T-NEXT: 00000004 t $t.1
+# MAPPING_T-NEXT: 00000000 T foo
+
+# RUN: llvm-symbolizer --obj=%t 4 8 | FileCheck %s -check-prefix SYMBOL
+# RUN: llvm-symbolizer --obj=%tthumb 4 8 | FileCheck %s -check-prefix SYMBOL
 
-# CHECK:       foo
-# CHECK-NEXT:  ??:0:0
-# CHECK-EMPTY:
-# CHECK-NEXT:  foo
-# CHECK-NEXT:  ??:0:0
+# SYMBOL:       foo
+# SYMBOL-NEXT:  ??:0:0
+# SYMBOL-EMPTY:
+# SYMBOL-NEXT:  foo
+# SYMBOL-NEXT:  ??:0:0
 
 .globl foo
 foo:
   .word 32
   nop
   .word 32

llvm/test/tools/llvm-nm/ARM/special-syms.test

+## Test --special-syms flag for ARM mapping symbols used to mark transitions
+## between ARM code, THUMB code and data ($a, $t, $t).
+#
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-nm %t | count 0
+# RUN: llvm-nm --special-syms %t | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x4
+Symbols:
+  - Name:            '$d.0'
+    Section:         .text
+  - Name:            '$a.1'
+    Section:         .text
+  - Name:            '$t.1'
+    Section:         .text
+
+# CHECK: 00000000 t $a.1
+# CHECK: 00000000 t $d.0
+# CHECK: 00000000 t $t.1

llvm/test/tools/llvm-nm/debug-syms.test

 # RUN: yaml2obj %s -o %t.o
-# RUN: llvm-nm --special-syms --debug-syms %t.o | FileCheck %s --implicit-check-not {{.}} --check-prefix SYMBOL
-# RUN: llvm-nm --special-syms -a %t.o | FileCheck %s --implicit-check-not {{.}} --check-prefix SYMBOL
+# RUN: llvm-nm --debug-syms %t.o | FileCheck %s --implicit-check-not {{.}} --check-prefix SYMBOL
+# RUN: llvm-nm -a %t.o | FileCheck %s --implicit-check-not {{.}} --check-prefix SYMBOL
 
 # SYMBOL:      0000000000000000 n $a
 # SYMBOL-NEXT: 0000000000000000 n $d
 # SYMBOL-NEXT: 0000000000000000 n $t
 # SYMBOL-NEXT: 0000000000000000 a file_sym
 # SYMBOL-NEXT: 0000000000000000 n section
 
 # RUN: llvm-nm --debug-syms --dynamic %t.o | FileCheck %s --implicit-check-not {{.}} --check-prefix DYNSYM

llvm/tools/llvm-nm/llvm-nm.cpp

if (OutputFormat == posix && !ArchiveName.empty())
     S << ArchiveName << "[" << CurrentFilename << "]: ";
   else {
     if (!ArchiveName.empty())
       S << ArchiveName << ":";
     S << CurrentFilename << ": ";
   }
 }
 
-static bool isSpecialSym(SymbolicFile &Obj, StringRef Name) {
-  auto *ELFObj = dyn_cast<ELFObjectFileBase>(&Obj);
-  if (!ELFObj)
-    return false;
-  uint16_t EMachine = ELFObj->getEMachine();
-  if (EMachine != ELF::EM_ARM && EMachine != ELF::EM_AARCH64)
-    return false;
-  return !Name.empty() && Name[0] == '$';
-}
-
 static void sortAndPrintSymbolList(SymbolicFile &Obj, bool printName,
                                    StringRef ArchiveName,
                                    StringRef ArchitectureName) {
   if (!NoSort) {
     using Comparator = bool (*)(const NMSymbol &, const NMSymbol &);
     Comparator Cmp;
     if (NumericSort)
       Cmp = &compareSymbolAddress;
if (S.Sym.getRawDataRefImpl().p) {
       }
       SymFlags = *SymFlagsOrErr;
     } else
       SymFlags = S.SymFlags;
 
     bool Undefined = SymFlags & SymbolRef::SF_Undefined;
     bool Global = SymFlags & SymbolRef::SF_Global;
     bool Weak = SymFlags & SymbolRef::SF_Weak;
+    bool FormatSpecific = SymFlags & SymbolRef::SF_FormatSpecific;
     if ((!Undefined && UndefinedOnly) || (Undefined && DefinedOnly) ||
         (!Global && ExternalOnly) || (Weak && NoWeakSymbols) ||
-        (!SpecialSyms && isSpecialSym(Obj, Name)))
+        (FormatSpecific && !(SpecialSyms || DebugSyms)))
       continue;
     if (PrintFileName)
       writeFileName(outs(), ArchiveName, ArchitectureName);
     if ((OutputFormat == just_symbols ||
          (UndefinedOnly && MachO && OutputFormat != darwin)) &&
         OutputFormat != posix) {
       outs() << Name << "\n";
       continue;
if (!(MachO && DyldInfoOnly)) {
     size_t I = -1;
     for (BasicSymbolRef Sym : Symbols) {
       ++I;
       Expected<uint32_t> SymFlagsOrErr = Sym.getFlags();
       if (!SymFlagsOrErr) {
         error(SymFlagsOrErr.takeError(), Obj.getFileName());
         return;
       }
-      if (!DebugSyms && (*SymFlagsOrErr & SymbolRef::SF_FormatSpecific))
+
+      // Don't drop format specifc symbols for ARM and AArch64 ELF targets, they
+      // are used to repesent mapping symbols and needed to honor the
+      // --special-syms option.
+      auto *ELFObj = dyn_cast<ELFObjectFileBase>(&Obj);
+      if ((!ELFObj || (ELFObj->getEMachine() != ELF::EM_ARM &&
+                       ELFObj->getEMachine() != ELF::EM_AARCH64)) &&
+          !DebugSyms && (*SymFlagsOrErr & SymbolRef::SF_FormatSpecific))
         continue;
       if (WithoutAliases && (*SymFlagsOrErr & SymbolRef::SF_Indirect))
         continue;
       // If a "-s segname sectname" option was specified and this is a Mach-O
       // file and this section appears in this file, Nsect will be non-zero then
       // see if this symbol is a symbol from that section and if not skip it.
       if (Nsect && Nsect != getNsectInMachO(*MachO, Sym))
         continue;