[PATCH] D98323: [lld-macho] implement options -map

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Writer.cpp:901
   if (auto e = buffer->commit())
     error("failed to write to the output file: " + toString(std::move(e)));
 }
----------------
I think `writeMapFile()` should be moved below this line -- there's no need to emit the mapfile if we can't emit the binary itself


================
Comment at: lld/test/MachO/map-file.s:6
+
+# RUN: %lld -map=%t.map %t/test.o %t/foo.o -o %t/test-map
+# RUN: FileCheck -match-full-lines -strict-whitespace %s < %t.map
----------------
The test seems to be failing, I guess it should be a `-map` without the `=`?

also I think it would be nicer to have the output go to `%t/map`


================
Comment at: lld/test/MachO/map-file.s:14
+_foo:
+
+#--- test.s
----------------
we should test common symbols (`.comm`) as well, as @thakis mentioned in an earlier comment


================
Comment at: lld/test/MachO/map-file.s:28-29
+# CHECK-NEXT: # Address  Size      Segment  Section
+# CHECK-NEXT: 0x100000308  0x00000001  __TEXT  __text
+# CHECK-NEXT: 0x100000309  0x00000000  __TEXT  obj
+# CHECK-NEXT: # Symbols:
----------------
We try not to match against hardcoded addresses because they're a bit of a pain to update. In this case I think it would be better to match this against `llvm-objdump`'s output: something like

```
RUN: llvm-objdump --syms --section-headers %t/test-map > %t/objdump
RUN: cat %t/objdump %t/map > %t/out
RUN: FileCheck %s < %t/out
```

Then we can use [[ https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-numeric-substitution-blocks | numeric substitutions ]] to check that the values line up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98323