[llvm-objcopy] Preserve ARM and AArch64 mapping symbols

[ikudrin]

Summary

Mapping symbols are required by ARM/AArch64 ELF ABI. They help to disassemble files correctly and are also used in linkers. Nonetheless, for executable files, the symbols can be stripped to better resemble the behavior of GNU's objcopy.

[MaskRay]

(@DavidSpickett @peter.smith)

I have played with aarch64-linux-gnu-strip a bit. It does seem that whether mapping symbols are removed is related to ET_REL or not.
Does the patch improve compatibility? Seems so to me.

In --strip-unneeded mode, GNU strip retains mapping symbols for ET_REL but not for non-ET_REL.

They help to disassemble files correctly and are also used in linkers.

ld.lld doesn't have special rules for mapping symbols.

Disassembling is an interesting angle. But removing unneeded local symbols (e.g. internal linkage functions) can make the disassembly results worse, too.
It is unclear to me how to want to preserve some local symbols but not others.
That said, the documentation for --discard-locals (in llvm-objcopy and GNU objcopy) mentions compiler-generated local symbols, and I may put mapping symbols into this category as well, but it seems to need some clarification.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

231

aaelf64 says All mapping symbols have type STT_NOTYPE and binding STB_LOCAL.

The Sym.getShndx() == SHN_UNDEF condition shouldn't appear in practice but seems fine to keep (an undefined local is assuredly an error).

[peter.smith]

From https://sourceware.org/binutils/docs/binutils/objcopy.html

-x
--discard-all
Do not copy non-global symbols from the source file.

-X
--discard-locals
Do not copy compiler-generated local symbols. (These usually start with ‘L’ or ‘.’.)
--strip-unneeded
Remove all symbols that are not needed for relocation processing in addition to debugging symbols and sections stripped by --strip-debug.

Empirically with arm-none-eabi-objcopy from binutils 2.35 and aarch64-linux-gnu-objcopy from binutils 2.34 I get the same results:

filetype/option	--discard-locals	--discard-all	--strip-unneeded
object	mapping symbols preserved	mapping symbols preserved	mapping symbols preserved
executable	mapping symbols preserved	mapping symbols removed	all non dynamic symbols removed

I would expect mapping symbols to be preserved in local objects when doing these transformations. They are required in AArch32 for the following things:

• Denoting Arm/Thumb changes mid-function (assembly) and constant pools (needed for disassembly)
• Linker patching (effectively linker needs to disassemble the code accurately)
• big-endian output (linker needs to endian revers instructions but not constant pools). This is not supported in LLD right now.

LLD uses mapping symbols in https://github.com/llvm/llvm-project/blob/main/lld/ELF/ARMErrataFix.cpp#L316 for linker patching.

In AArch64 in practice mapping symbols are only needed if someone has used constant pools in assembly language. In LLD we account for that case in linker patching for https://github.com/llvm/llvm-project/blob/main/lld/ELF/AArch64ErrataFix.cpp#L430

For executables they are only used for disassembly. I guess --discard-locals could still keep them based on the GNU documentation entry.

Not had a chance to look at the implementation here. I'm broadly in favour with matching the GNU tools behaviour.

[ikudrin]

The patch aims to improve the compatibility, right.

So, do we have a consensus? Or the patch has to be improved somehow?

[peter.smith]

FWIW I'm happy with the proposed change.

[MaskRay]

llvm/test/tools/llvm-objcopy/ELF/strip-unneeded-arm.test

5

llvm-nm -j --special-syms %t1.o | FileCheck %s --match-full-lines

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

256

[ikudrin]

Thanks for the suggestions!

llvm/test/tools/llvm-objcopy/ELF/strip-unneeded-aarch64.test

+## Check that llvm-objcopy preserves AArch64 mapping symbols for relocatable files.
+# RUN: yaml2obj %s -o %t.o -DTYPE=REL
+# RUN: llvm-objcopy --strip-unneeded %t.o %t1.o
+# RUN: llvm-nm -j --special-syms %t1.o | FileCheck %s --match-full-lines
+# RUN: llvm-objcopy --discard-all %t.o %t2.o
+# RUN: llvm-nm -j --special-syms %t2.o | FileCheck %s --match-full-lines
+
+# CHECK:      $d
+# CHECK-NEXT: $d.d
+# CHECK-NEXT: $x
+# CHECK-NEXT: $x.x
+
+## A mapping symbol can be deleted if specified explicitly.
+# RUN: llvm-objcopy -w -N '$d*' %t.o %t3.o
+# RUN: llvm-nm --special-syms %t3.o | FileCheck /dev/null --implicit-check-not='$d'
+
+## Mapping symbols are not preserved for executable files
+# RUN: yaml2obj %s -o %t.exec -DTYPE=EXEC
+# RUN: llvm-objcopy --strip-unneeded %t.exec %t1.exec
+# RUN: llvm-nm --special-syms %t1.exec | count 0
+
+# RUN: yaml2obj %s -o %t.so -DTYPE=DYN
+# RUN: llvm-objcopy --strip-unneeded %t.so %t1.so
+# RUN: llvm-nm --special-syms %t1.so | count 0
+
+!ELF
+FileHeader:
+  Class:    ELFCLASS64
+  Data:     ELFDATA2LSB
+  Type:     ET_[[TYPE]]
+  Machine:  EM_AARCH64
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+Symbols:
+  - Name:     $a
+    Section:  .text
+  - Name:     $d
+    Section:  .text
+  - Name:     $dd
+    Section:  .text
+  - Name:     $d.d
+    Section:  .text
+  - Name:     $m
+    Section:  .text
+  - Name:     $t.t
+    Section:  .text
+  - Name:     $x
+    Section:  .text
+  - Name:     $xx
+    Section:  .text
+  - Name:     $x.x
+    Section:  .text

llvm/test/tools/llvm-objcopy/ELF/strip-unneeded-arm.test

+## Check that llvm-objcopy preserves ARM mapping symbols for relocatable files.
+# RUN: yaml2obj %s -o %t.o -DTYPE=REL
+# RUN: llvm-objcopy --strip-unneeded %t.o %t1.o
+# RUN: llvm-nm -j --special-syms %t1.o | FileCheck %s --match-full-lines
+# RUN: llvm-objcopy --discard-all %t.o %t2.o
+# RUN: llvm-nm -j --special-syms %t2.o | FileCheck %s --match-full-lines
+
+# CHECK:      $a
+# CHECK-NEXT: $a.a
+# CHECK-NEXT: $d
+# CHECK-NEXT: $d.d
+# CHECK-NEXT: $t
+# CHECK-NEXT: $t.t
+# CHECK-NOT:  $x
+
+## A mapping symbol can be deleted if specified explicitly.
+# RUN: llvm-objcopy -w -N '$d*' %t.o %t3.o
+# RUN: llvm-nm --special-syms %t3.o | FileCheck /dev/null --implicit-check-not='$d'
+
+## Mapping symbols are not preserved for executable files
+# RUN: yaml2obj %s -o %t.exec -DTYPE=EXEC
+# RUN: llvm-objcopy --strip-unneeded %t.exec %t1.exec
+# RUN: llvm-nm --special-syms %t1.exec | count 0
+
+# RUN: yaml2obj %s -o %t.so -DTYPE=DYN
+# RUN: llvm-objcopy --strip-unneeded %t.so %t1.so
+# RUN: llvm-nm --special-syms %t1.so | count 0
+
+!ELF
+FileHeader:
+  Class:    ELFCLASS32
+  Data:     ELFDATA2LSB
+  Type:     ET_[[TYPE]]
+  Machine:  EM_ARM
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+Symbols:
+  - Name:     $a
+    Section:  .text
+  - Name:     $aa
+    Section:  .text
+  - Name:     $a.a
+    Section:  .text
+  - Name:     $d
+    Section:  .text
+  - Name:     $dd
+    Section:  .text
+  - Name:     $d.d
+    Section:  .text
+  - Name:     $m
+    Section:  .text
+  - Name:     $t
+    Section:  .text
+  - Name:     $tt
+    Section:  .text
+  - Name:     $t.t
+    Section:  .text
+  - Name:     $x
+    Section:  .text

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

if (!NewSection)
       return NewSection.takeError();
 
     FromTo[S] = *NewSection;
   }
 
   return Obj.replaceSections(FromTo);
 }
 
+static bool isAArch64MappingSymbol(const Symbol &Sym) {
+  if (Sym.Binding != STB_LOCAL || Sym.Type != STT_NOTYPE ||
+      Sym.getShndx() == SHN_UNDEF)
+    return false;
+  StringRef Name = Sym.Name;
+  if (!Name.consume_front("$x") && !Name.consume_front("$d"))
+    return false;
+  return Name.empty() || Name.startswith(".");
+}
+
+static bool isArmMappingSymbol(const Symbol &Sym) {
+  if (Sym.Binding != STB_LOCAL || Sym.Type != STT_NOTYPE ||
+      Sym.getShndx() == SHN_UNDEF)
+    return false;
+  StringRef Name = Sym.Name;
+  if (!Name.consume_front("$a") && !Name.consume_front("$d") &&
+      !Name.consume_front("$t"))
+    return false;
+  return Name.empty() || Name.startswith(".");
+}
+
+// Check if the symbol should be preserved because it is required by ABI.
+static bool isRequiredByABISymbol(const Object &Obj, const Symbol &Sym) {
+  switch (Obj.Machine) {
+  case EM_AARCH64:
+    // Mapping symbols should be preserved for a relocatable object file.
+    return Obj.isRelocatable() && isAArch64MappingSymbol(Sym);
+  case EM_ARM:
+    // Mapping symbols should be preserved for a relocatable object file.
+    return Obj.isRelocatable() && isArmMappingSymbol(Sym);
+  default:
+    return false;
+  }
+}
+
 static bool isUnneededSymbol(const Symbol &Sym) {
   return !Sym.Referenced &&
          (Sym.Binding == STB_LOCAL || Sym.getShndx() == SHN_UNDEF) &&
          Sym.Type != STT_SECTION;
 }
 
 static Error updateAndRemoveSymbols(const CommonConfig &Config,
                                     const ELFConfig &ELFConfig, Object &Obj) {
for (SectionBase &Sec : Obj.sections())
       Sec.markSymbols();
   }
 
   auto RemoveSymbolsPred = [&](const Symbol &Sym) {
     if (Config.SymbolsToKeep.matches(Sym.Name) ||
         (ELFConfig.KeepFileSymbols && Sym.Type == STT_FILE))
       return false;
 
-    if ((Config.DiscardMode == DiscardType::All ||
-         (Config.DiscardMode == DiscardType::Locals &&
-          StringRef(Sym.Name).startswith(".L"))) &&
-        Sym.Binding == STB_LOCAL && Sym.getShndx() != SHN_UNDEF &&
-        Sym.Type != STT_FILE && Sym.Type != STT_SECTION)
+    if (Config.SymbolsToRemove.matches(Sym.Name))
       return true;
 
     if (Config.StripAll || Config.StripAllGNU)
       return true;
 
+    if (isRequiredByABISymbol(Obj, Sym))
+      return false;
+
     if (Config.StripDebug && Sym.Type == STT_FILE)
       return true;
 
-    if (Config.SymbolsToRemove.matches(Sym.Name))
+    if ((Config.DiscardMode == DiscardType::All ||
+         (Config.DiscardMode == DiscardType::Locals &&
+          StringRef(Sym.Name).startswith(".L"))) &&
+        Sym.Binding == STB_LOCAL && Sym.getShndx() != SHN_UNDEF &&
+        Sym.Type != STT_FILE && Sym.Type != STT_SECTION)
       return true;
 
     if ((Config.StripUnneeded ||
          Config.UnneededSymbolsToRemove.matches(Sym.Name)) &&
         (!Obj.isRelocatable() || isUnneededSymbol(Sym)))
       return true;
 
     // We want to remove undefined symbols if all references have been stripped.