[PATCH] D104919: [lld-macho] Preserve alignment for non-deduplicated cstrings

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

> Oh, I didn't realize this breaks -order_file for strings

I think it's fine actually -- in theory, cstrings can be reordered based on their literal values (instead of via symbol names) -- or at least that's what ld64's manpage suggests. In practice it doesn't actually seem to work. So we can probably come up with an alternative ordering mechanism for cstrings at some point

> (I kind of wish we could put make InputSection small and fast enough that we could just do one regular InputSection per string....)

FWIW, I belatedly realized that my benchmarking diff D104159 <https://reviews.llvm.org/D104159> (and the current StringPiece implementation) has a flaw: they create one InputSection/StringPiece respectively for every null byte, and since the Mach-O format employs zero padding extensively for alignment, we're actually allocating a bunch of useless stuff for zero-length strings. I guess this isn't an issue with ELF since it creates separate sections for strings that need to be aligned.



================
Comment at: lld/test/MachO/dead-strip-align.s:1
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: %lld -lSystem -o %t.out %t.o -dead_strip
----------------
needs a `# REQUIRES: x86` line above


================
Comment at: lld/test/MachO/dead-strip-align.s:13
+
+# CHECK: 0  \303Q043\005\376\334\272\230vT2\020\001
+
----------------
should we test for a non-zero offset as well?


================
Comment at: lld/test/MachO/dead-strip-align.s:20
+
+.section __TEXT,__cstring,cstring_literals
+.globl _hi
----------------
this section header is redundant


================
Comment at: lld/test/MachO/dead-strip-align.s:25
+
+.section __TEXT,__cstring,cstring_literals
+.p2align 4
----------------
ditto


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

https://reviews.llvm.org/D104919