[PATCH] D133201: [LLD][COFF] Fix Bug, occouring if Symbol has no name

[Jan Ole Hüser via Phabricator via llvm-commits]

j0le created this revision.
j0le added reviewers: mstorsjo, ruiu.
j0le added a project: lld.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
j0le requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Bug: An assertion fails:

  Assertion failed: isa<To>(Val) && "cast<Ty>() argument of incompatible type!", 
  file C:\Users\<user>\prog\llvm\llvm-git-lld-bug\llvm\include\llvm/Support/Casting.h, line 578

Bug is triggered, if

- a map file is requested with /MAP, and
- Architekture is ARMv7, Thumb, and
- a relative jump (branch instruction) is greater than 16 MiB (2^24)

The reason for the Bug is:

- a Thunk is created for the jump
- a Symbol for the Thunk is created
  - of type `DefinedSynthetic`
  - in file `Writer.cpp`
  - in function `getThunk`
- the Symbol has no name
- when creating the map file, the name of the Symbol is queried
- the function `Symbol::computeName` of the base class `Symbol` casts the `this` pointer to type `DefinedCOFF` (a derived type), but the acutal type is `DefinedSynthetic`
- The in the llvm::cast an assertion fails

Changes:

- Modify regression test to trigger this bug
- Fix bug by using dyn_cast<>() instead of cast<>()
- Give the symbol pointing to the thunk a name


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133201

Files:
  lld/COFF/Symbols.cpp
  lld/COFF/Symbols.h
  lld/COFF/Writer.cpp
  lld/test/COFF/arm-thumb-thunks.s


Index: lld/test/COFF/arm-thumb-thunks.s
===================================================================
--- lld/test/COFF/arm-thumb-thunks.s
+++ lld/test/COFF/arm-thumb-thunks.s
@@ -1,6 +1,6 @@
 // REQUIRES: arm
 // RUN: llvm-mc -filetype=obj -triple=thumbv7-windows %s -o %t.obj
-// RUN: lld-link -entry:main -subsystem:console %t.obj -out:%t.exe -verbose 2>&1 | FileCheck -check-prefix=VERBOSE %s
+// RUN: lld-link -entry:main -subsystem:console %t.obj -out:%t.exe -map -verbose 2>&1 | FileCheck -check-prefix=VERBOSE %s
 // RUN: llvm-objdump -d %t.exe --start-address=0x401000 --stop-address=0x401022 | FileCheck --check-prefix=MAIN %s
 // RUN: llvm-objdump -d %t.exe --start-address=0x501022 --stop-address=0x501032 | FileCheck --check-prefix=FUNC1 %s
 // RUN: llvm-objdump -d %t.exe --start-address=0x601032 | FileCheck --check-prefix=FUNC2 %s
Index: lld/COFF/Writer.cpp
===================================================================
--- lld/COFF/Writer.cpp
+++ lld/COFF/Writer.cpp
@@ -396,7 +396,7 @@
   default:
     llvm_unreachable("Unexpected architecture");
   }
-  Defined *d = make<DefinedSynthetic>("", c);
+  Defined *d = make<DefinedSynthetic>("range_extension_thunk", c);
   lastThunk = d;
   return {d, true};
 }
Index: lld/COFF/Symbols.h
===================================================================
--- lld/COFF/Symbols.h
+++ lld/COFF/Symbols.h
@@ -80,6 +80,9 @@
     // is a waste of time.
     if (nameData == nullptr)
       computeName();
+    if (nameData == nullptr)
+      return StringRef{}; // I don't know, if we can be sure, that nameSize is 0
+                          // in this case.
     return StringRef(nameData, nameSize);
   }
 
Index: lld/COFF/Symbols.cpp
===================================================================
--- lld/COFF/Symbols.cpp
+++ lld/COFF/Symbols.cpp
@@ -55,7 +55,9 @@
 void Symbol::computeName() {
   assert(nameData == nullptr &&
          "should only compute the name once for DefinedCOFF symbols");
-  auto *d = cast<DefinedCOFF>(this);
+  auto *d = dyn_cast<DefinedCOFF>(this);
+  if (d == nullptr)
+    return;
   StringRef nameStr =
       check(cast<ObjFile>(d->file)->getCOFFObj()->getSymbolName(d->sym));
   nameData = nameStr.data();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133201.457568.patch
Type: text/x-patch
Size: 2225 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220902/4ff1e37c/attachment.bin>