From 8090737d564e2ed5b85f14ca6f29041bfe699744 Mon Sep 17 00:00:00 2001
From: Rui Ueyama <ruiu@cs.stanford.edu>
Date: Wed, 21 Feb 2024 18:35:06 +0900
Subject: [PATCH] Merge sections with different flags into a single section

Previously, mold didn't merge sections with the same name but different
section flags into a single section. This behavior is different from other
linkers and caused a compatibility issue.

Now, mold merges input sections by their names and types.

Fixes https://github.com/rui314/mold/issues/1196
---
 elf/mold.h                     |  7 +++-
 elf/output-chunks.cc           | 30 -----------------
 elf/passes.cc                  | 61 +++++++++++++++++++++++-----------
 test/elf/section-attributes.sh | 26 +++++++++++++++
 4 files changed, 74 insertions(+), 50 deletions(-)
 create mode 100755 test/elf/section-attributes.sh

diff --git a/elf/mold.h b/elf/mold.h
index 6bafb7be26..b2e99194ab 100644
--- a/elf/mold.h
+++ b/elf/mold.h
@@ -449,7 +449,11 @@ class InterpSection : public Chunk<E> {
 template <typename E>
 class OutputSection : public Chunk<E> {
 public:
-  OutputSection(Context<E> &ctx, std::string_view name, u32 type, u64 flags);
+  OutputSection(std::string_view name, u32 type) {
+    this->name = name;
+    this->shdr.sh_type = type;
+  }
+
   ChunkKind kind() override { return OUTPUT_SECTION; }
   OutputSection<E> *to_osec() override { return this; }
   void construct_relr(Context<E> &ctx) override;
@@ -464,6 +468,7 @@ class OutputSection : public Chunk<E> {
   std::vector<InputSection<E> *> members;
   std::vector<std::unique_ptr<Thunk<E>>> thunks;
   std::unique_ptr<RelocSection<E>> reloc_sec;
+  Atomic<u32> sh_flags;
 };
 
 template <typename E>
diff --git a/elf/output-chunks.cc b/elf/output-chunks.cc
index 52ea8e8644..027a3bad8c 100644
--- a/elf/output-chunks.cc
+++ b/elf/output-chunks.cc
@@ -859,36 +859,6 @@ void DynamicSection<E>::copy_buf(Context<E> &ctx) {
   write_vector(ctx.buf + this->shdr.sh_offset, contents);
 }
 
-template <typename E>
-OutputSection<E>::OutputSection(Context<E> &ctx, std::string_view name,
-                                u32 type, u64 flags) {
-  this->name = name;
-  this->shdr.sh_type = type;
-  this->shdr.sh_flags = flags & ~SHF_MERGE & ~SHF_STRINGS & ~SHF_COMPRESSED;
-
-  if (auto it = ctx.arg.section_align.find(name);
-      it != ctx.arg.section_align.end())
-    this->shdr.sh_addralign = it->second;
-
-  // PT_GNU_RELRO segment is a security mechanism to make more pages
-  // read-only than we could have done without it.
-  //
-  // Traditionally, sections are either read-only or read-write. If a
-  // section contains dynamic relocations, it must have been put into a
-  // read-write segment so that the program loader can mutate its
-  // contents in memory, even if no one will write to it at runtime.
-  //
-  // RELRO segment allows us to make such pages writable only when a
-  // program is being loaded. After that, the page becomes read-only.
-  //
-  // Some sections, such as .init, .fini, .got, .dynamic, contain
-  // dynamic relocations but doesn't have to be writable at runtime,
-  // so they are put into a RELRO segment.
-  this->is_relro = (name == ".toc" || name.ends_with(".rel.ro") ||
-                    type == SHT_INIT_ARRAY || type == SHT_FINI_ARRAY ||
-                    type == SHT_PREINIT_ARRAY || (flags & SHF_TLS));
-}
-
 template <typename E>
 void OutputSection<E>::copy_buf(Context<E> &ctx) {
   if (this->shdr.sh_type != SHT_NOBITS)
diff --git a/elf/passes.cc b/elf/passes.cc
index bf6715d471..3a16dd42ae 100644
--- a/elf/passes.cc
+++ b/elf/passes.cc
@@ -458,7 +458,6 @@ struct OutputSectionKey {
   bool operator==(const OutputSectionKey &) const = default;
   std::string_view name;
   u64 type;
-  u64 flags;
 };
 
 template <typename E>
@@ -527,23 +526,15 @@ get_output_section_key(Context<E> &ctx, InputSection<E> &isec) {
   if (ctx.has_init_array && !isec.get_rels(ctx).empty()) {
     std::string_view name = isec.name();
     if (name == ".ctors" || name.starts_with(".ctors."))
-      return {".init_array", SHT_INIT_ARRAY, SHF_ALLOC | SHF_WRITE};
+      return {".init_array", SHT_INIT_ARRAY};
     if (name == ".dtors" || name.starts_with(".dtors."))
-      return {".fini_array", SHT_FINI_ARRAY, SHF_ALLOC | SHF_WRITE};
+      return {".fini_array", SHT_FINI_ARRAY};
   }
 
   const ElfShdr<E> &shdr = isec.shdr();
   std::string_view name = get_output_name(ctx, isec.name(), shdr.sh_flags);
   u64 type = canonicalize_type<E>(name, shdr.sh_type);
-  u64 flags = shdr.sh_flags & ~(u64)(SHF_COMPRESSED | SHF_GROUP | SHF_GNU_RETAIN);
-
-  // .init_array is usually writable. We don't want to create multiple
-  // .init_array output sections, so make it always writable.
-  // So is .fini_array.
-  if (type == SHT_INIT_ARRAY || type == SHT_FINI_ARRAY)
-    flags |= SHF_WRITE;
-
-  return {name, type, flags};
+  return {name, type};
 }
 
 // Create output sections for input sections.
@@ -553,10 +544,7 @@ void create_output_sections(Context<E> &ctx) {
 
   struct Hash {
     size_t operator()(const OutputSectionKey &k) const {
-      u64 h = hash_string(k.name);
-      h = combine_hash(h, std::hash<u64>{}(k.type));
-      h = combine_hash(h, std::hash<u64>{}(k.flags));
-      return h;
+      return combine_hash(hash_string(k.name), std::hash<u64>{}(k.type));
     }
   };
 
@@ -580,9 +568,10 @@ void create_output_sections(Context<E> &ctx) {
         continue;
 
       const ElfShdr<E> &shdr = isec->shdr();
+
       if (ctx.arg.relocatable && (shdr.sh_flags & SHF_GROUP)) {
-        OutputSection<E> *osec =
-          new OutputSection<E>(ctx, isec->name(), shdr.sh_type, shdr.sh_flags);
+        OutputSection<E> *osec = new OutputSection<E>(isec->name(), shdr.sh_type);
+        osec->sh_flags = shdr.sh_flags;
         isec->output_section = osec;
         ctx.osec_pool.emplace_back(osec);
         continue;
@@ -603,7 +592,7 @@ void create_output_sections(Context<E> &ctx) {
         }
 
         std::unique_ptr<OutputSection<E>> osec =
-          std::make_unique<OutputSection<E>>(ctx, key.name, key.type, key.flags);
+          std::make_unique<OutputSection<E>>(key.name, key.type);
 
         std::unique_lock lock(mu);
         auto [it, inserted] = map.insert({key, osec.get()});
@@ -614,12 +603,46 @@ void create_output_sections(Context<E> &ctx) {
         return ret;
       };
 
+      u32 flags = shdr.sh_flags & ~SHF_MERGE & ~SHF_STRINGS & ~SHF_COMPRESSED &
+                  ~SHF_GROUP & ~SHF_GNU_RETAIN;
+
       OutputSection<E> *osec = get_or_insert();
+      osec->sh_flags |= flags;
       isec->output_section = osec;
       cache.insert({key, osec});
     }
   });
 
+  for (std::unique_ptr<OutputSection<E>> &osec : ctx.osec_pool) {
+    osec->shdr.sh_flags = osec->sh_flags;
+
+    // Handle --section-align
+    if (!ctx.arg.section_align.empty())
+      if (auto it = ctx.arg.section_align.find(osec->name);
+          it != ctx.arg.section_align.end())
+        osec->shdr.sh_addralign = it->second;
+
+    // PT_GNU_RELRO segment is a security mechanism to make more pages
+    // read-only than we could have done without it.
+    //
+    // Traditionally, sections are either read-only or read-write. If a
+    // section contains dynamic relocations, it must have been put into a
+    // read-write segment so that the program loader can mutate its
+    // contents in memory, even if no one will write to it at runtime.
+    //
+    // RELRO segment allows us to make such pages writable only when a
+    // program is being loaded. After that, the page becomes read-only.
+    //
+    // Some sections, such as .init, .fini, .got, .dynamic, contain
+    // dynamic relocations but doesn't have to be writable at runtime,
+    // so they are put into a RELRO segment.
+    u32 type = osec->shdr.sh_type;
+    u32 flags = osec->shdr.sh_flags;
+    osec->is_relro = (osec->name == ".toc" || osec->name.ends_with(".rel.ro") ||
+                      type == SHT_INIT_ARRAY || type == SHT_FINI_ARRAY ||
+                      type == SHT_PREINIT_ARRAY || (flags & SHF_TLS));
+  }
+
   // Add input sections to output sections
   std::vector<Chunk<E> *> chunks;
   for (i64 i = size; i < ctx.osec_pool.size(); i++)
diff --git a/test/elf/section-attributes.sh b/test/elf/section-attributes.sh
new file mode 100755
index 0000000000..58b8c1b0f3
--- /dev/null
+++ b/test/elf/section-attributes.sh
@@ -0,0 +1,26 @@
+#!/bin/bash
+. $(dirname $0)/common.inc
+
+cat <<EOF | $CC -o $t/a.o -c -xassembler -
+.section .foobar,"aw"
+.ascii "foo\0"
+EOF
+
+cat <<EOF | $CC -o $t/b.o -c -xassembler -
+.section .foobar,"a"
+.ascii "bar\0"
+EOF
+
+cat <<EOF | $CC -o $t/c.o -c -xassembler -
+.section .foobar,"axT"
+.ascii "bar\0"
+EOF
+
+cat <<EOF | $CC -o $t/d.o -c -xc -
+int main() {}
+EOF
+
+$CC -B. -o $t/exe $t/a.o $t/b.o $t/c.o $t/d.o
+$CC -o $t/exe $t/a.o $t/b.o $t/c.o $t/d.o
+readelf -WS $t/exe
+# readelf -WS $t/exe | grep -q 'foobar.*WAX'