lld hexagon ELF - 4kb aligned virtual addresses?

[androm3da]

lld exhibits different behavior regarding the virtual address selection and one of the loaders appears to have an expectation/requirement that the virtual addresses be 4kb-aligned (min page size). This represents different behavior from eld.

  echo 'int main(){}' > test.c
  clang --target=hexagon-unknown-linux-musl -c test.c -o test.o

  ld.lld test.o -o test_lld
  readelf -l test_lld | grep -A1 "LOAD.*R E"
  LLD Output:
    LOAD           0x000000 0x00010210 0x00010210 0x000123 0x000123 R E 0x10000
                                  ↑ NOT 4KB aligned

  ld.eld test.o -o test_eld
  readelf -l test_eld | grep -A1 "LOAD.*R E"
  ELD Output:
    LOAD           0x000000 0x00010000 0x00010000 0x000123 0x000123 R E 0x10000
                                  ↑ 4KB aligned

@MaskRay I can fix this with a patch like the one below. Do you think that'd be an acceptable approach?

diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h
index e121fa1183e9..72e4e6815c05 100644
--- a/lld/ELF/Target.h
+++ b/lld/ELF/Target.h
@@ -122,6 +122,8 @@ public:
   unsigned defaultCommonPageSize = 4096;
   unsigned defaultMaxPageSize = 4096;
 
+  std::optional<unsigned> minLoadSegmentAlignment;
+
   uint64_t getImageBase() const;
 
   // True if _GLOBAL_OFFSET_TABLE_ is relative to .got.plt, false if .got.
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 4fa80397cbfa..ee8d4314d82a 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2592,6 +2592,19 @@ template <class ELFT> void Writer<ELFT>::fixSectionAlignments() {
         };
       else
         cmd->addrExpr = [&ctx] {
+          // If the target specifies a minimum LOAD segment alignment, enforce
+          // true page alignment to ensure LOAD segments are aligned to the
+          // required boundaries in virtual memory, not just in the p_align
+          // field. Other architectures preserve sub-page offsets to avoid file
+          // padding.
+          if (ctx.target->minLoadSegmentAlignment.has_value()) {
+            // Enforce minimum alignment, even if user specifies smaller page
+            // size
+            uint64_t minAlign = std::max(
+                ctx.arg.maxPageSize,
+                static_cast<uint64_t>(*ctx.target->minLoadSegmentAlignment));
+            return alignToPowerOf2(ctx.script->getDot(), minAlign);
+          }
           return alignToPowerOf2(ctx.script->getDot(), ctx.arg.maxPageSize) +
                  ctx.script->getDot() % ctx.arg.maxPageSize;
         };

[llvmbot]

@llvm/issue-subscribers-backend-hexagon

Author: Brian Cain (androm3da)

lld exhibits different behavior regarding the virtual address selection and one of the loaders appears to have an expectation/requirement that the virtual addresses be 4kb-aligned (min page size). This represents different behavior from eld.

  echo 'int main(){}' > test.c
  clang --target=hexagon-unknown-linux-musl -c test.c -o test.o

  ld.lld test.o -o test_lld
  readelf -l test_lld | grep -A1 "LOAD.*R E"
  LLD Output:
    LOAD           0x000000 0x00010210 0x00010210 0x000123 0x000123 R E 0x10000
                                  ↑ NOT 4KB aligned

  ld.eld test.o -o test_eld
  readelf -l test_eld | grep -A1 "LOAD.*R E"
  ELD Output:
    LOAD           0x000000 0x00010000 0x00010000 0x000123 0x000123 R E 0x10000
                                  ↑ 4KB aligned

@MaskRay I can fix this with a patch like the one below. Do you think that'd be an acceptable approach?

diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h
index e121fa1183e9..72e4e6815c05 100644
--- a/lld/ELF/Target.h
+++ b/lld/ELF/Target.h
@@ -122,6 +122,8 @@ public:
   unsigned defaultCommonPageSize = 4096;
   unsigned defaultMaxPageSize = 4096;
 
+  std::optional<unsigned> minLoadSegmentAlignment;
+
   uint64_t getImageBase() const;
 
   // True if _GLOBAL_OFFSET_TABLE_ is relative to .got.plt, false if .got.
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 4fa80397cbfa..ee8d4314d82a 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2592,6 +2592,19 @@ template <class ELFT> void Writer<ELFT>::fixSectionAlignments() {
         };
       else
         cmd->addrExpr = [&ctx] {
+          // If the target specifies a minimum LOAD segment alignment, enforce
+          // true page alignment to ensure LOAD segments are aligned to the
+          // required boundaries in virtual memory, not just in the p_align
+          // field. Other architectures preserve sub-page offsets to avoid file
+          // padding.
+          if (ctx.target->minLoadSegmentAlignment.has_value()) {
+            // Enforce minimum alignment, even if user specifies smaller page
+            // size
+            uint64_t minAlign = std::max(
+                ctx.arg.maxPageSize,
+                static_cast<uint64_t>(*ctx.target->minLoadSegmentAlignment));
+            return alignToPowerOf2(ctx.script->getDot(), minAlign);
+          }
           return alignToPowerOf2(ctx.script->getDot(), ctx.arg.maxPageSize) +
                  ctx.script->getDot() % ctx.arg.maxPageSize;
         };

[llvmbot]

@llvm/issue-subscribers-lld-elf

Author: Brian Cain (androm3da)

lld exhibits different behavior regarding the virtual address selection and one of the loaders appears to have an expectation/requirement that the virtual addresses be 4kb-aligned (min page size). This represents different behavior from eld.

  echo 'int main(){}' > test.c
  clang --target=hexagon-unknown-linux-musl -c test.c -o test.o

  ld.lld test.o -o test_lld
  readelf -l test_lld | grep -A1 "LOAD.*R E"
  LLD Output:
    LOAD           0x000000 0x00010210 0x00010210 0x000123 0x000123 R E 0x10000
                                  ↑ NOT 4KB aligned

  ld.eld test.o -o test_eld
  readelf -l test_eld | grep -A1 "LOAD.*R E"
  ELD Output:
    LOAD           0x000000 0x00010000 0x00010000 0x000123 0x000123 R E 0x10000
                                  ↑ 4KB aligned

@MaskRay I can fix this with a patch like the one below. Do you think that'd be an acceptable approach?

diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h
index e121fa1183e9..72e4e6815c05 100644
--- a/lld/ELF/Target.h
+++ b/lld/ELF/Target.h
@@ -122,6 +122,8 @@ public:
   unsigned defaultCommonPageSize = 4096;
   unsigned defaultMaxPageSize = 4096;
 
+  std::optional<unsigned> minLoadSegmentAlignment;
+
   uint64_t getImageBase() const;
 
   // True if _GLOBAL_OFFSET_TABLE_ is relative to .got.plt, false if .got.
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 4fa80397cbfa..ee8d4314d82a 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2592,6 +2592,19 @@ template <class ELFT> void Writer<ELFT>::fixSectionAlignments() {
         };
       else
         cmd->addrExpr = [&ctx] {
+          // If the target specifies a minimum LOAD segment alignment, enforce
+          // true page alignment to ensure LOAD segments are aligned to the
+          // required boundaries in virtual memory, not just in the p_align
+          // field. Other architectures preserve sub-page offsets to avoid file
+          // padding.
+          if (ctx.target->minLoadSegmentAlignment.has_value()) {
+            // Enforce minimum alignment, even if user specifies smaller page
+            // size
+            uint64_t minAlign = std::max(
+                ctx.arg.maxPageSize,
+                static_cast<uint64_t>(*ctx.target->minLoadSegmentAlignment));
+            return alignToPowerOf2(ctx.script->getDot(), minAlign);
+          }
           return alignToPowerOf2(ctx.script->getDot(), ctx.arg.maxPageSize) +
                  ctx.script->getDot() % ctx.arg.maxPageSize;
         };

[MaskRay]

You probably just need -z separate-code or -z separate-loadable-segments. musl rtld supports the lld default -z noseparate-code layout.

If Hexagon really wants to use the alternative layout, it's preferred to specify the option in the compiler driver.

[androm3da]

@iajbar can you modify the hexagon driver to use -z separate-loadable-segments as a link argument for hexagon-unknown-elf? Chenna Raju is requesting this change in behavior so that lld gets the same behavior as eld.

[llvmbot]

@llvm/issue-subscribers-clang-driver

Author: Brian Cain (androm3da)

lld exhibits different behavior regarding the virtual address selection and one of the loaders appears to have an expectation/requirement that the virtual addresses be 4kb-aligned (min page size). This represents different behavior from eld.

  echo 'int main(){}' > test.c
  clang --target=hexagon-unknown-linux-musl -c test.c -o test.o

  ld.lld test.o -o test_lld
  readelf -l test_lld | grep -A1 "LOAD.*R E"
  LLD Output:
    LOAD           0x000000 0x00010210 0x00010210 0x000123 0x000123 R E 0x10000
                                  ↑ NOT 4KB aligned

  ld.eld test.o -o test_eld
  readelf -l test_eld | grep -A1 "LOAD.*R E"
  ELD Output:
    LOAD           0x000000 0x00010000 0x00010000 0x000123 0x000123 R E 0x10000
                                  ↑ 4KB aligned

@MaskRay I can fix this with a patch like the one below. Do you think that'd be an acceptable approach?

diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h
index e121fa1183e9..72e4e6815c05 100644
--- a/lld/ELF/Target.h
+++ b/lld/ELF/Target.h
@@ -122,6 +122,8 @@ public:
   unsigned defaultCommonPageSize = 4096;
   unsigned defaultMaxPageSize = 4096;
 
+  std::optional<unsigned> minLoadSegmentAlignment;
+
   uint64_t getImageBase() const;
 
   // True if _GLOBAL_OFFSET_TABLE_ is relative to .got.plt, false if .got.
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 4fa80397cbfa..ee8d4314d82a 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2592,6 +2592,19 @@ template <class ELFT> void Writer<ELFT>::fixSectionAlignments() {
         };
       else
         cmd->addrExpr = [&ctx] {
+          // If the target specifies a minimum LOAD segment alignment, enforce
+          // true page alignment to ensure LOAD segments are aligned to the
+          // required boundaries in virtual memory, not just in the p_align
+          // field. Other architectures preserve sub-page offsets to avoid file
+          // padding.
+          if (ctx.target->minLoadSegmentAlignment.has_value()) {
+            // Enforce minimum alignment, even if user specifies smaller page
+            // size
+            uint64_t minAlign = std::max(
+                ctx.arg.maxPageSize,
+                static_cast<uint64_t>(*ctx.target->minLoadSegmentAlignment));
+            return alignToPowerOf2(ctx.script->getDot(), minAlign);
+          }
           return alignToPowerOf2(ctx.script->getDot(), ctx.arg.maxPageSize) +
                  ctx.script->getDot() % ctx.arg.maxPageSize;
         };

[quic-seaswara]

Why should clang default to -z separate-loadable-segments only for Hexagon ? shouldn't the build be using -Wl,-z,separate-loadable-segments

[androm3da]

Why should clang default to -z separate-loadable-segments only for Hexagon ? shouldn't the build be using -Wl,-z,separate-loadable-segments

Sure - that's even better still IMO. @Chennak-quic had previously ruled out a change to a linker script that would have addressed this. So I assumed that a change to the linker arguments might also be refused.

IIUC Chenna's planning to explore -Wl,-z,separate-loadable-segments anyways - so perhaps like you say, the driver shouldn't change? Although if people are targeting hexagon-unknown-elf for shared objects and the popular commercial loader can't cope with too small aligned segments, maybe it's not a bad default?