[PATCH] D124266: [lld/elf] fix quote usage in section names

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/test/ELF/linkerscript/section-quotes.test:19
+  ".text" : AT(ADDR(".text")) {
+    LONG (ALIGNOF(".text"))
+    LONG (LOADADDR(".text"))
----------------
royger wrote:
> MaskRay wrote:
> > Just keep ALIGNOF for .text and keep LOADADDR for .data? That provides sufficient coverage without being too verbose.
> The point is to test all possible combinations of quoted section name definition and usage of quotes section names in functions. Hence the tree combinations for .text .data and .bss:
> 
> * text: quoted section name used for both definition and as function parameter
> * data: section definition not quoted, section parameter in functions quoted.
> * bss: section definition quoted, section parameter in functions not quoted.
One `LONG (ALIGNOF("..."))` and one `LONG (LOADADDR(".text"))` are sufficient. No need to test `LONG (ALIGNOF("..."))` in two output section descriptions.


================
Comment at: lld/test/ELF/linkerscript/section-quotes.test:23
+  }
+  text_size = SIZEOF(".text");
+  .data : AT(ADDR(".data")) {
----------------
royger wrote:
> MaskRay wrote:
> > Unused. Delete
> It's unused, but the point (here and in usages below) is to test that the SIZEOF function can correctly deal with quoted and unquoted section names. 
OK, keeping `text_size` and `data_text` sounds good. But change `llvm-readelf -S %t` to `llvm-readelf -S -s %t` and additionally test the values of the two symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124266