[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 ↗

(On Diff #96555)

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

315 ↗

(On Diff #96555)

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 ↗

(On Diff #96555)

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 ↗

(On Diff #96576)

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 ↗

(On Diff #96576)

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

399 ↗

(On Diff #96894)

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

test/ELF/incompatible-section-types2.s

7–8 ↗

(On Diff #96894)

Remove extra newlines.

[arichardson]

Addressed review comments

[arichardson]

Adapted to D32587 changes

[ruiu]

ELF/OutputSections.cpp

430 ↗

(On Diff #97421)

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

430 ↗

(On Diff #97421)

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

424 ↗

(On Diff #97611)

I think you can remove object:: from this line

426 ↗

(On Diff #97611)

and this.

[arichardson]

removed object::

[arichardson]

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

lld/trunk/ELF/OutputSections.cpp

if (getIncompatibleFlags(Sec->Flags) != getIncompatibleFlags(IS->Flags))
       error("incompatible section flags for " + Sec->Name +
             "\n>>> " + toString(IS) + ": 0x" + utohexstr(IS->Flags) +
             "\n>>> output section " + Sec->Name + ": 0x" +
             utohexstr(Sec->Flags));
     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 type mismatch for " + IS->Name +
+              "\n>>> " + toString(IS) + ": " +
+              getELFSectionTypeName(Config->EMachine, IS->Type) +
+              "\n>>> output section " + Sec->Name + ": " +
+              getELFSectionTypeName(Config->EMachine, Sec->Type));
     }
     Sec->Flags |= Flags;
   } else {
     Sec = make<OutputSection>(OutsecName, IS->Type, Flags);
     OutputSections.push_back(Sec);
   }
 
   Sec->addSection(cast<InputSection>(IS));

lld/trunk/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 type mismatch for .shstrtab
+// CHECK-NEXT: >>> <internal>:(.shstrtab): SHT_STRTAB
+// CHECK-NEXT: >>> output section .shstrtab: Unknown
 
-.section .shstrtab,""
+.section .shstrtab,"",@12345
 .short 20