[ELF] - Do not crash on absolute local symbol starting from ".L".

[grimar]

Summary

I had a bunch of crashes during last AFL runs.

Problem is next. Object contains local symbol of type STT_NOTYPE
(it just should not be STT_FILE or STT_SECTION to crash).

Has section index greater than SHN_LORESERVE, so next code returns 0
template <class ELFT>

uint32_t ELFFileBase<ELFT>::getSectionIndex(const Elf_Sym &Sym) const {
...
  if (I >= ELF::SHN_LORESERVE)
    return 0;
  return I;
}

Then DefinedRegular is created:

  if (Sym->st_shndx == SHN_UNDEF)
    return new (this->Alloc)
        Undefined(Sym->st_name, Sym->st_other, Sym->getType(), this);
  return new (this->Alloc) DefinedRegular<ELFT>(*Sym, Sec);
}

And finally code is crashes in shouldKeepInSymtab() because Sec is null there.

As was noticed by Rafael, the same happens for absolute
local symbols with name staring from ".L".

Patch fixes that.

[grimar]

• Updated.

[• rafael]

LGTM with a nit.

ELF/Writer.cpp

311

You don't need F.

[davide]

lg

[ruiu]

ELF/Writer.cpp

339

Add () around &. This is probably better.

return !Sec || !(Sec->getSectionHdr()->sh_flags & SHF_MERGE);

[grimar]

Revision: 283731

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::run() {
   writeBuildId();
   if (HasError)
     return;
   if (auto EC = Buffer->commit())
     error(EC, "failed to write to the output file");
 }
 
 template <class ELFT>
-static bool shouldKeepInSymtab(InputSectionBase<ELFT> *Sec, StringRef SymName,
+static bool shouldKeepInSymtab(elf::ObjectFile<ELFT> *F,
+                               InputSectionBase<ELFT> *Sec, StringRef SymName,
                                const SymbolBody &B) {
   if (B.isFile())
     return false;
 
   // We keep sections in symtab for relocatable output.
   if (B.isSection())
     return Config->Relocatable;
 
static bool shouldKeepInSymtab(elf::ObjectFile<ELFT> *F,
   // * The symbol is in a SHF_MERGE section, which is normally the reason for
   //   the assembler keeping the .L symbol.
   if (!SymName.startswith(".L") && !SymName.empty())
     return true;
 
   if (Config->Discard == DiscardPolicy::Locals)
     return false;
 
-  return !(Sec->getSectionHdr()->sh_flags & SHF_MERGE);
+  return !(Sec && Sec->getSectionHdr()->sh_flags & SHF_MERGE);
 }
 
 template <class ELFT> static bool includeInSymtab(const SymbolBody &B) {
   if (!B.isLocal() && !B.symbol()->IsUsedInRegularObj)
     return false;
 
   if (auto *D = dyn_cast<DefinedRegular<ELFT>>(&B)) {
     // Always include absolute symbols.
for (SymbolBody *B : F->getLocalSymbols()) {
       if (!DR)
         continue;
       if (!includeInSymtab<ELFT>(*B))
         continue;
       if (B->getNameOffset() >= StrTab.size())
         fatal(getFilename(F) + ": invalid symbol name offset");
       StringRef SymName(StrTab.data() + B->getNameOffset());
       InputSectionBase<ELFT> *Sec = DR->Section;
-      if (!shouldKeepInSymtab<ELFT>(Sec, SymName, *B))
+      if (!shouldKeepInSymtab<ELFT>(F, Sec, SymName, *B))
         continue;
       ++Out<ELFT>::SymTab->NumLocals;
       if (Config->Relocatable)
         B->DynsymIndex = Out<ELFT>::SymTab->NumLocals;
       F->KeptLocalSyms.push_back(
           std::make_pair(DR, Out<ELFT>::SymTab->StrTabSec.addString(SymName)));
     }
   }

test/ELF/local.s

 // Check that symbol table is correctly populated with local symbols.
-// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+// RUN: llvm-mc -save-temp-labels -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: ld.lld %t -o %t1
 // RUN: llvm-readobj -t -s %t1 | FileCheck %s
 // REQUIRES: x86
 
 // Check that Info is equal to the number of local symbols.
 // CHECK:   Section {
 // CHECK:     Name: .symtab
 // CHECK-NEXT:     Type: SHT_SYMTAB
 // CHECK-NEXT:     Flags [
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Address:
 // CHECK-NEXT:     Offset:
 // CHECK-NEXT:     Size:
 // CHECK-NEXT:     Link:
-// CHECK-NEXT:     Info: 5
+// CHECK-NEXT:     Info: 6
 
 // CHECK: Symbols [
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name:
 // CHECK-NEXT:     Value: 0x0
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name: .Labs
+// CHECK-NEXT:     Value: 0x2B
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Local
+// CHECK-NEXT:     Type: None
+// CHECK-NEXT:     Other: 0
+// CHECK-NEXT:     Section: Absolute
+// CHECK-NEXT:   }
+// CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: abs
 // CHECK-NEXT:     Value: 0x2A
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
 // CHECK-NEXT:     Type: None
 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Absolute
 // CHECK-NEXT:   }

 
 .global _start
 _start:
 
 blah:
 foo:
 goo:
 abs = 42
+.Labs = 43