[PATCH] D118077: Print C-string literals in mapfile

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

Thanks!



================
Comment at: lld/MachO/MapFile.cpp:82-97
+    switch (sym->isec->kind()) {
+    case InputSection::CStringLiteralKind: {
+      // Output "literal string: <string literal>"
+      auto *isec = static_cast<CStringInputSection *>(sym->isec);
+      assert(isec != nullptr);
+      auto &piece = isec->getStringPiece(sym->value);
+      auto offset = sym->value - piece.outSecOff;
----------------
int3 wrote:
> we don't need to match cstrings with symbols -- in fact, to follow ld64's behavior, we shouldn't. Given an input like
> ```
> .cstring
> _y:
> .asciz "hi"
> .asciz "unnamed"
> ```
> 
> The mapfile contains separate entries for "hi" and "unnamed", even though "unnamed" doesn't have a symbol associated with it.
> 
> So basically we can iterate from `i=0...isec->pieces.size()-1` and call `isec->getStringRef(i)` on each one. No need to work directly with the string pieces either :)
talked offline. Roger pointed out that in order to sort the cstrings along with the rest of the symbols in address order, we would need some sort of `Symbol *` key in the map. We could create fake Symbols for this purpose, but that doesn't make for any simpler code, so let's stick with this.

This does mean that we won't be able to deal with "unnamed" strings, but I suspect that in practice it's a rare enough case. I don't believe clang generates any such strings in its assembly output.


================
Comment at: lld/MachO/MapFile.cpp:85-86
+      // Output "literal string: <string literal>"
+      auto *isec = static_cast<CStringInputSection *>(sym->isec);
+      assert(isec != nullptr);
+      auto &piece = isec->getStringPiece(sym->value);
----------------
`cast<>` does a runtime type check + non-null check in debug builds


================
Comment at: lld/MachO/MapFile.cpp:87-90
+      auto &piece = isec->getStringPiece(sym->value);
+      auto offset = sym->value - piece.outSecOff;
+      auto pieceIndex = &piece - &(*isec->pieces.begin());
+      auto str = isec->getStringRef(pieceIndex);
----------------
codebase convention is to favor explicit types over `auto`. Exceptions are if the RHS has the type explicitly mentioned already (e.g. via a cast), or if the type is some messy thing (e.g. iterator types)

also, let's add `const` wherever possible


================
Comment at: lld/test/MachO/map-file.s:81-82
+# CSTRING-DAG: _main
+# CSTRING-DAG: literal string: Hello world!\n
+# CSTRING-DAG: literal string: Hello, it's me
+
----------------
let's also have the test cover the case of dead-stripped cstrings


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118077/new/

https://reviews.llvm.org/D118077