[ELF] Improve error message for incompatible section types

[arichardson]

Summary

Previously we were not printing out the type of the incompatible section which made it difficult to determine what the problem was.

The error message format has been change to the following:

error: section type mismatch for .shstrtab
>>> <internal>:(.shstrtab): SHT_STRTAB
>>> output section .shstrtab: Unknown

[ruiu]

ELF/OutputSections.cpp

313

We generally avoid C macros even if it means we need to repeat code multiple times. Please expand this macro.

315

It feels to me that this should live somewhere in libObject or libSupport, so that you can use this both from LLD and llvm-readobj.

324

Do you really want to fall through?

[arichardson]

Start message with lower case letter

[arichardson]

change error message format and remove macro

[ruiu]

ELF/OutputSections.cpp

312

Can you move this to llvm/Object/ELF.cpp? We already have this function in that file, and I think can do the same.

StringRef getELFRelocationTypeName(uint32_t Machine, uint32_t Type);

[arichardson]

ELF/OutputSections.cpp

312

Will do. Should I open a new review for that or add it to this one?

[arichardson]

Rebased on https://reviews.llvm.org/D32587

[ruiu]

ELF/OutputSections.cpp

456

I think you can return S since StringRef can be converted to std::string implicitly, no?

test/ELF/incompatible-section-types2.s

7–8

Remove extra newlines.

[arichardson]

Addressed review comments

[arichardson]

Adapted to D32587 changes

[ruiu]

ELF/OutputSections.cpp

472

I'm not sure if you want printing "Unknown" with a hex code, as I think it is rare to see unknown sections in object files. If you don't do that, you can directly use getELFSectionTypeName here. I'd think I prefer that over adding a new function.

[arichardson]

Remove helper function and call getELFSectionTypeName directly

ELF/OutputSections.cpp

472

As we now know which object file section it is we can just look at it to get the hex value so I agree we don't need it. The other option would be to always print the hex value (like llvm-readobj does).

[ruiu]

LGTM with these changes. Thank you for doing this!

ELF/OutputSections.cpp

473

I think you can remove object:: from this line

475

and this.

[arichardson]

removed object::

[arichardson]

I don't have commit access, could you please commit for me?

ELF/OutputSections.cpp

parallelFor(0, Sections.size(), [=](size_t I) {
     }
   });
 
   // Linker scripts may have BYTE()-family commands with which you
   // can write arbitrary bytes to the output. Process them if any.
   Script->writeDataBytes(Name, Buf);
 }
 
+// TODO: the following function is almost identical with llvm-readobj and
+// should probably be shared
+#define LLD_STRINGIFY_ENUM_CASE(flag) case flag: return #flag
+
+static std::string getElfSectionType(unsigned Type) {
+  switch (Config->EMachine) {
+  case ELF::EM_ARM:
+    switch (Type) {
+      LLD_STRINGIFY_ENUM_CASE(SHT_ARM_EXIDX);
+      LLD_STRINGIFY_ENUM_CASE(SHT_ARM_PREEMPTMAP);
+      LLD_STRINGIFY_ENUM_CASE(SHT_ARM_ATTRIBUTES);
+      LLD_STRINGIFY_ENUM_CASE(SHT_ARM_DEBUGOVERLAY);
+      LLD_STRINGIFY_ENUM_CASE(SHT_ARM_OVERLAYSECTION);
+    }
+  case ELF::EM_HEXAGON:
+    switch (Type) { LLD_STRINGIFY_ENUM_CASE(SHT_HEX_ORDERED); }
+  case ELF::EM_X86_64:
+    switch (Type) { LLD_STRINGIFY_ENUM_CASE(SHT_X86_64_UNWIND); }
+  case ELF::EM_MIPS:
+  case ELF::EM_MIPS_RS3_LE:
+    switch (Type) {
+      LLD_STRINGIFY_ENUM_CASE(SHT_MIPS_REGINFO);
+      LLD_STRINGIFY_ENUM_CASE(SHT_MIPS_OPTIONS);
+      LLD_STRINGIFY_ENUM_CASE(SHT_MIPS_ABIFLAGS);
+      LLD_STRINGIFY_ENUM_CASE(SHT_MIPS_DWARF);
+    }
+  }
+
+  switch (Type) {
+    LLD_STRINGIFY_ENUM_CASE(SHT_NULL              );
+    LLD_STRINGIFY_ENUM_CASE(SHT_PROGBITS          );
+    LLD_STRINGIFY_ENUM_CASE(SHT_SYMTAB            );
+    LLD_STRINGIFY_ENUM_CASE(SHT_STRTAB            );
+    LLD_STRINGIFY_ENUM_CASE(SHT_RELA              );
+    LLD_STRINGIFY_ENUM_CASE(SHT_HASH              );
+    LLD_STRINGIFY_ENUM_CASE(SHT_DYNAMIC           );
+    LLD_STRINGIFY_ENUM_CASE(SHT_NOTE              );
+    LLD_STRINGIFY_ENUM_CASE(SHT_NOBITS            );
+    LLD_STRINGIFY_ENUM_CASE(SHT_REL               );
+    LLD_STRINGIFY_ENUM_CASE(SHT_SHLIB             );
+    LLD_STRINGIFY_ENUM_CASE(SHT_DYNSYM            );
+    LLD_STRINGIFY_ENUM_CASE(SHT_INIT_ARRAY        );
+    LLD_STRINGIFY_ENUM_CASE(SHT_FINI_ARRAY        );
+    LLD_STRINGIFY_ENUM_CASE(SHT_PREINIT_ARRAY     );
+    LLD_STRINGIFY_ENUM_CASE(SHT_GROUP             );
+    LLD_STRINGIFY_ENUM_CASE(SHT_SYMTAB_SHNDX      );
+    LLD_STRINGIFY_ENUM_CASE(SHT_GNU_ATTRIBUTES    );
+    LLD_STRINGIFY_ENUM_CASE(SHT_GNU_HASH          );
+    LLD_STRINGIFY_ENUM_CASE(SHT_GNU_verdef        );
+    LLD_STRINGIFY_ENUM_CASE(SHT_GNU_verneed       );
+    LLD_STRINGIFY_ENUM_CASE(SHT_GNU_versym        );
+  default:
+    return "0x" + utohexstr(Type);
+  }
+}
+#undef LLD_STRINGIFY_ENUM_CASE
+
 static uint64_t getOutFlags(InputSectionBase *S) {
   return S->Flags & ~SHF_GROUP & ~SHF_COMPRESSED;
 }
 
 static SectionKey createKey(InputSectionBase *C, StringRef OutsecName) {
   //  The ELF spec just says
   // ----------------------------------------------------------------
   // In the first phase, input sections that match in name, type and
if (!Config->PrintGcSections)
     return;
   message("removing unused section from '" + IS->Name + "' in file '" +
           IS->File->getName());
 }
 
 void OutputSectionFactory::addInputSec(InputSectionBase *IS,
                                        StringRef OutsecName) {
   if (!IS->Live) {
     reportDiscarded(IS);
     return;
   }
 
   SectionKey Key = createKey(IS, OutsecName);
   uint64_t Flags = getOutFlags(IS);
   OutputSection *&Sec = Map[Key];
   if (Sec) {
     if (getIncompatibleFlags(Sec->Flags) != getIncompatibleFlags(IS->Flags))
       error("Section has flags incompatible with others with the same name " +
             toString(IS));
     if (Sec->Type != IS->Type) {
       if (canMergeToProgbits(Sec->Type) && canMergeToProgbits(IS->Type))
         Sec->Type = SHT_PROGBITS;
       else
-        error("Section has different type from others with the same name " +
-              toString(IS));
+        error("section " + IS->Name + " has different type (" +
+              getElfSectionType(IS->Type) + ") from output section " +
+              Sec->Name + " (" + getElfSectionType(Sec->Type) +
+              ")\n>>> input section " + IS->Name + " is defined in " +
+              toString(IS->File));
     }
     Sec->Flags |= Flags;
   } else {
     Sec = make<OutputSection>(Key.Name, IS->Type, Flags);
     OutputSections.push_back(Sec);
   }
 
   Sec->addSection(cast<InputSection>(IS));

test/ELF/incompatible-section-types2.s

 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck %s
 
-// CHECK: error: Section has different type from others with the same name <internal>:(.shstrtab)
+// CHECK:      error: section .shstrtab has different type (SHT_STRTAB) from output section .shstrtab (SHT_PROGBITS)
+// CHECK-NEXT: >>> input section .shstrtab is defined in (internal)
+
 
 .section .shstrtab,""
 .short 20