[PATCH v1] GAS: Keep symbols in their own section [PR27566]

[Xiao Zeng]

2024-10-25 14:47  Fangrui Song <i@maskray.me> wrote:
>
>On Thu, Oct 24, 2024 at 10:54 PM Xiao Zeng <zengxiao@eswincomputing.com> wrote:
>>
>> Hi Jeff and Nelson, an issue has arisen in GCC, described here:
>> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115166>
>>
>> Afterward, it was thought to be an ld issue, described here:
>> <https://sourceware.org/bugzilla/show_bug.cgi?id=27566>
>>
>> Nelson has done some work on ld, but it hasn’t completely resolved the problem. I understand that this issue may actually stem from as. I have tested the following modification, which can resolve the current issue.
>>
>> Here is the emails exchange I had with Alan:
>> <https://sourceware.org/pipermail/binutils/2024-October/137360.html>
>>
>> Alan provided some excellent insights.
>>
>> To help drive the resolution of this issue, could you also offer some advice when you have time?
>>
>> Thank you both very much.
>>
>> 2024-10-22 17:43  Xiao Zeng <zengxiao@eswincomputing.com> wrote:
>> >
>> >PR27566 is categorized under ld:
>> ><https://sourceware.org/bugzilla/show_bug.cgi?id=27566>
>> >but the root cause of this issue is in gas.
>> >
>> >The following assembly example is provided:
>> >        .section        .rodata
>> >        .globl Sym
>> >        .type  Sym, @object
>> >        .set   Sym, . + 0x1200
>> >        .size  Sym, 8
>> >
>> >readelf reads ELF information:
>> >Section Headers:
>> >  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>> >  [ 4] .rodata           PROGBITS        0000000000000000 000040 000000 00   A  0   0  1
>> >
>> >Symbol table '.symtab' contains 7 entries:
>> >   Num:    Value          Size Type    Bind   Vis      Ndx Name
>> >     6: 0000000000001200     8 OBJECT  GLOBAL DEFAULT    4 Sym
>> >
>> >The Sym belongs to the .rodata section, but the size of .rodata is 0.
>> >
>> >        PR gas/27566
>> >       * write.c (size_seg): Update section size.
>> >       * testsuite/gas/elf/elf.exp: New test.
>> >       * testsuite/gas/elf/pr27566.d: New file.
>> >       * testsuite/gas/elf/pr27566.s: Likewise.
>> >
>> >Signed-off-by: Xiao Zeng <zengxiao@eswincomputing.com>
>> >---
>> > gas/testsuite/gas/elf/elf.exp   |  1 +
>> > gas/testsuite/gas/elf/pr27566.d | 11 +++++++++++
>> > gas/testsuite/gas/elf/pr27566.s |  5 +++++
>> > gas/write.c                     |  7 +++++++
>> > 4 files changed, 24 insertions(+)
>> > create mode 100644 gas/testsuite/gas/elf/pr27566.d
>> > create mode 100644 gas/testsuite/gas/elf/pr27566.s
>> >
>> >diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
>> >index c828c3af25c..6a69acbc109 100644
>> >--- a/gas/testsuite/gas/elf/elf.exp
>> >+++ b/gas/testsuite/gas/elf/elf.exp
>> >@@ -387,4 +387,5 @@ if { [is_elf_format] } then {
>> >     run_dump_test "bignums" $dump_opts
>> >     run_dump_test "section-symbol-redef"
>> >     run_dump_test "pr27228"
>> >+    run_dump_test "pr27566"
>> > }
>> >diff --git a/gas/testsuite/gas/elf/pr27566.d b/gas/testsuite/gas/elf/pr27566.d
>> >new file mode 100644
>> >index 00000000000..75268cefb20
>> >--- /dev/null
>> >+++ b/gas/testsuite/gas/elf/pr27566.d
>> >@@ -0,0 +1,11 @@
>> >+#as:
>> >+#name: Keep symbols in their own section (PR 27566)
>> >+#readelf: -S --wide
>> >+
>> >+There are [0-9]+ section headers, starting at offset 0x[0-9a-f]+:
>> >+
>> >+Section Headers:
>> >+ +\[Nr\] Name +Type +Addr(ess|) +Off +Size +ES +Flg +Lk +Inf +Al
>> >+#...
>> >+ *\[ [1-9]\] *\.rodata +PROGBITS +0*0 +0[0-9a-f]* +0*1208 .*
>> >+#pass
>> >diff --git a/gas/testsuite/gas/elf/pr27566.s b/gas/testsuite/gas/elf/pr27566.s
>> >new file mode 100644
>> >index 00000000000..4232df21494
>> >--- /dev/null
>> >+++ b/gas/testsuite/gas/elf/pr27566.s
>> >@@ -0,0 +1,5 @@
>> >+        .section        .rodata
>> >+        .globl Sym
>> >+        .type  Sym, @object
>> >+        .set   Sym, . + 0x1200
>> >+        .size Sym, 8
>> >diff --git a/gas/write.c b/gas/write.c
>> >index 18cf18fc830..7f69d7aad44 100644
>> >--- a/gas/write.c
>> >+++ b/gas/write.c
>> >@@ -559,6 +559,7 @@ size_seg (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *xxx ATTRIBUTE_UNUSED)
>> >   segment_info_type *seginfo;
>> >   int x;
>> >   valueT size, newsize;
>> >+  symbolS *symp;
>> >
>> >   subseg_change (sec, 0);
>> >
>> >@@ -577,6 +578,12 @@ size_seg (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *xxx ATTRIBUTE_UNUSED)
>> >   else
>> >     size = 0;
>> >
>> >+  /* Keep symbols in their own section.  */
>> >+  for (symp = symbol_rootP; symp; symp = symbol_next (symp))
>> >+    if (S_GET_SEGMENT (symp) == sec
>> >+      && size < (S_GET_VALUE (symp) + S_GET_SIZE (symp)))
>> >+      size = S_GET_VALUE (symp) + S_GET_SIZE (symp);
>> >+
>> >   flags = bfd_section_flags (sec);
>> >   if (size == 0 && bfd_section_size (sec) != 0 &&
>> >     (flags & SEC_HAS_CONTENTS) != 0)
>> >--
>> >2.17.1
>> Thanks
>> Xiao Zeng
>
>
>I commented on the GNU ld bug report
>https://sourceware.org/bugzilla/show_bug.cgi?id=27566#c11 .
>This looks like a linker relaxation bug in ld riscv. I agree with
>Alan's rejection of the assembler patch. 
Alright, thank you, Fangrui, for the code review feedback.
I'll take a look at the linker when I have time and continue to work on this issue.

Thanks
Xiao Zeng