Print C-string literals in mapfile

[Roger]

Summary

This diff has the C-string literals printed into the mapfile in the symbol table like how ld64 does.

Here is what ld64's mapfile looks like with C-string literals:

# Path: out
# Arch: x86_64
# Object files:
[  0] linker synthesized
[  1] foo.o
# Sections:
# Address       Size            Segment Section
0x100003F7D     0x0000001D      __TEXT  __text
0x100003F9A     0x0000001E      __TEXT  __cstring
0x100003FB8     0x00000048      __TEXT  __unwind_info
# Symbols:
# Address       Size            File  Name
0x100003F7D     0x0000001D      [  1] _main
0x100003F9A     0x0000000E      [  1] literal string: Hello world!\n
0x100003FA8     0x00000010      [  1] literal string: Hello, it's me\n
0x100003FB8     0x00000048      [  0] compact unwind info

Here is what the new lld's Mach-O mapfile looks like:

# Path: /Users/rgr/local/llvm-project/build/Debug/tools/lld/test/MachO/Output/map-file.s.tmp/c-string-liter
al-out
# Arch: x86_64
# Object files:
[  0] linker synthesized
[  1] /Users/rgr/local/llvm-project/build/Debug/tools/lld/test/MachO/Output/map-file.s.tmp/c-string-literal
.o
# Sections:
# Address       Size            Segment Section
0x1000002E0     0x0000001D      __TEXT  __text
0x1000002FD     0x0000001D      __TEXT  __cstring
# Symbols:
# Address           File  Name
0x1000002E0     [  1] _main
0x1000002FD     [  1] literal string: Hello world!\n
0x10000030B     [  1] literal string: Hello, it's me\n

[Roger]

Ready for review

[int3]

lld/MachO/MapFile.cpp

82–97

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 :)

lld/test/MachO/map-file.s

74–82

so most of the other tests have all the RUN and CHECK lines at the top, above the input files. I see that the other tests in this file have already interleaved them... could we fix that please? Thanks :)

[int3]

Thanks!

lld/MachO/MapFile.cpp

82–97

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.

85–86

cast<> does a runtime type check + non-null check in debug builds

87–90

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

lld/test/MachO/map-file.s

81–82

let's also have the test cover the case of dead-stripped cstrings

[MaskRay]

This may make the output much larger... The information can be achieved with llvm-readobj -p __cstring

[int3]

Well we are trying to have feature parity with ld64... I guess we can add additional LLD-specific flags to disable this behavior for builds that don't need that info from their mapfiles

[Roger]

Address comments

[Roger]

@int3 I've simplified the code to not use offsets from StringPieces and to just print the StringRef that corresponds to each StringPiece. This is because we no longer do any tail-merging, making offsets unnecessary.

[Roger]

lld/test/MachO/map-file.s

74–82

I created a new diff to do this.

[int3]

This is because we no longer do any tail-merging, making offsets unnecessary.

I believe they're technically still possible, assuming the input is like

.ascii "non-null-terminated"
_foo:
.asciz "null-terminated

Probably unlikely to happen -- plus our current implementation doesn't really work when we have strings without corresponding symbols anyway -- but it wouldn't hurt to assert that the offset is indeed zero.

lld/MachO/MapFile.cpp

85

does this not work?

[Roger]

Fix cast.

[Roger]

it wouldn't hurt to assert that the offset is indeed zero.

I realized this is not necessary as the function CStringInputSection::getStringRef should handle any complexities around offsets. Looking at that getStringRef's code right now, the logic actually assumes that there is no tail merging as it does the following:

StringRef getStringRef(size_t i) const {
    size_t begin = pieces[i].inSecOff;
    size_t end =
        (pieces.size() - 1 == i) ? data.size() : pieces[i + 1].inSecOff;
    return toStringRef(data.slice(begin, end - begin));
}

You can see that it uses the start of the next StringPiece to determine the end of the current StringPiece. This implies that StringPieces do not overlap (assuming they are ordered by their starting location and that there is 1:1 relationship between strings and StringPieces).

[int3]

lld/MachO/MapFile.cpp

86

By "assert the offset is zero" I meant assert(sym->value == piece.inSecOff)... since we are locating the StringPiece via binary search, we are not guaranteed to find a string who starts at the exact location of the symbol

Even if strings don't overlap, we can have a symbol that starts in the middle of an existing string -- that's what the assert would catch.

[Roger]

Add assert statement.

lld/MachO/MapFile.cpp

 
 #include "MapFile.h"
 #include "Config.h"
 #include "InputFiles.h"
 #include "InputSection.h"
 #include "OutputSection.h"
 #include "OutputSegment.h"
 #include "Symbols.h"
+#include "SyntheticSections.h"
 #include "Target.h"
 #include "llvm/Support/Parallel.h"
 #include "llvm/Support/TimeProfiler.h"
 
 using namespace llvm;
 using namespace llvm::sys;
 using namespace lld;
 using namespace lld::macho;

 // Construct a map from symbols to their stringified representations.
 // Demangling symbols (which is what toString() does) is slow, so
 // we do that in batch using parallel-for.
 static DenseMap<Symbol *, std::string>
 getSymbolStrings(ArrayRef<Defined *> syms) {
   std::vector<std::string> str(syms.size());
   parallelForEachN(0, syms.size(), [&](size_t i) {
     raw_string_ostream os(str[i]);
-    os << toString(*syms[i]);
+    Defined *sym = syms[i];
+
+    switch (sym->isec->kind()) {
+    case InputSection::CStringLiteralKind: {
+      // Output "literal string: <string literal>"
+      const auto *isec = cast<CStringInputSection>(sym->isec);
+      const StringPiece &piece = isec->getStringPiece(sym->value);
+      assert(
+          sym->value == piece.inSecOff &&
+          "We expect symbols to always point to the start of a StringPiece.");
+      StringRef str = isec->getStringRef(&piece - &(*isec->pieces.begin()));
+      assert(str.back() == '\000');
+      (os << "literal string: ")
+          // Remove null sequence at the end
+          .write_escaped(str.substr(0, str.size() - 1));
+      break;
+    }
+    case InputSection::ConcatKind:
+    case InputSection::WordLiteralKind:
+      os << toString(*sym);
+    }
   });
 
   DenseMap<Symbol *, std::string> ret;
   for (size_t i = 0, e = syms.size(); i < e; ++i)
     ret[syms[i]] = std::move(str[i]);
   return ret;
 }
 

lld/test/MachO/map-file.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/c-string-literal.s -o %t/c-string-literal.o
 
 # RUN: %lld -map %t/map %t/test.o %t/foo.o --time-trace -o %t/test-map
 # RUN: llvm-objdump --syms --section-headers %t/test-map > %t/objdump
 # RUN: cat %t/objdump %t/map > %t/out
 # RUN: FileCheck %s < %t/out
 # RUN: FileCheck %s --check-prefix=MAPFILE < %t/test-map.time-trace
 
 #--- foo.s

 # CHECK-NEXT: 0x[[#BSS]]   0x{{[0-9a-f]+}}   __DATA  __common
 
 # CHECK-NEXT: # Symbols:
 # CHECK-NEXT: # Address        File  Name
 # CHECK-NEXT: 0x[[#MAIN]]      [  1]  _main
 # CHECK-NEXT: 0x[[#FOO]]       [  2]  _foo
 # CHECK-NEXT: 0x[[#NUMBER]]    [  1]  _number
 
+#--- c-string-literal.s
+.section __TEXT,__cstring
+.globl _hello_world, _hello_its_me, _main
+
+_hello_world:
+.asciz "Hello world!\n"
+
+_hello_its_me:
+.asciz "Hello, it's me"
+
+.text
+_main:
+  movl $0x2000004, %eax # write() syscall
+  mov $1, %rdi # stdout
+  leaq _hello_world(%rip), %rsi
+  mov $13, %rdx # length of str
+  syscall
+  ret
+
+# RUN: %lld -map %t/c-string-literal-map %t/c-string-literal.o -o %t/c-string-literal-out
+# RUN: FileCheck --check-prefix=CSTRING %s < %t/c-string-literal-map
+
+## C-string literals should be printed as "literal string: <C string literal>"
+# CSTRING-LABEL: Symbols:
+# CSTRING-DAG: _main
+# CSTRING-DAG: literal string: Hello world!\n
+# CSTRING-DAG: literal string: Hello, it's me
+
+# RUN: %lld -dead_strip -map %t/dead-c-string-literal-map %t/c-string-literal.o -o %t/dead-c-string-literal-out
+# RUN: FileCheck --check-prefix=DEADCSTRING %s < %t/dead-c-string-literal-map
+
+## C-string literals should be printed as "literal string: <C string literal>"
+# DEADCSTRING-LABEL: Symbols:
+# DEADCSTRING-DAG: _main
+# DEADCSTRING-DAG: literal string: Hello world!\n
+# DEADCSTRING-LABEL: Dead Stripped Symbols:
+# DEADCSTRING-DAG: literal string: Hello, it's me
+
 # MAPFILE: "name":"Total Write map file"