[PATCH] D19272: [ELF] - linkerscript AT keyword (in output section description) implemented.

[Rui Ueyama via llvm-commits]

ruiu added a comment.

This change needs a test.


================
Comment at: ELF/LinkerScript.cpp:334
@@ -333,1 +333,3 @@
 
+template <class ELFT> bool LinkerScript<ELFT>::hasLma(StringRef Name) {
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
----------------
Small but noticeable duplicate code. Please define `hasLma` using `getLma`.

  return getLma(Name) != -1;


================
Comment at: ELF/Writer.cpp:1643
@@ +1642,3 @@
+
+    // Now set the segment's physical address.
+    if (H.p_type == PT_LOAD && First)
----------------
Remove this comment. I think it is too terse and obvious.

================
Comment at: ELF/Writer.cpp:993
@@ -989,3 +992,3 @@
     uintX_t NewFlags = toPhdrFlags(Sec->getFlags());
-    if (Flags != NewFlags) {
+    if (Script<ELFT>::X->hasLma(Sec->getName()) || Flags != NewFlags) {
       Load = AddHdr(PT_LOAD, NewFlags);
----------------
I don't think this will create a valid PHDR if there's a section with an AT command followed by another section without AT.

  .foo 0x1000 : AT(0) { ... }
  .bar 0x2000 { ... }

For example, if .bar's flags are the same as .foo's, then this code would put both .foo and .bar into the same segment.


https://reviews.llvm.org/D19272