[Buildroot] [PATCH 0/2] fix elf2flt on m68k

[Buildroot] [PATCH 0/2] fix elf2flt on m68k

[Niklas Cassel via buildroot]

Hello buildroot folks,

This series fixes a segfault at link time on m68k.

With this series, we have bootable m68k and riscv64.

(riscv64 was booting with the previous patch, but we
were simply lucky that we didn't observe the same
segfault there as well.)


Kind regards,
Niklas

Niklas Cassel (2):
  package/elf2flt: remove broken patch
  package/elf2flt: add another .eh_frame section handling patch

 ...3-elf2flt-add-riscv-64-bits-support.patch} |  0
 ...f2flt-fix-.eh_frame-section-handling.patch | 73 ------------------
 ...2flt-create-a-common-helper-function.patch | 76 +++++++++++++++++++
 ...l-error-regression-on-m68k-and-riscv.patch | 72 ++++++++++++++++++
 4 files changed, 148 insertions(+), 73 deletions(-)
 rename package/elf2flt/{0004-elf2flt-add-riscv-64-bits-support.patch => 0003-elf2flt-add-riscv-64-bits-support.patch} (100%)
 delete mode 100644 package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch
 create mode 100644 package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch
 create mode 100644 package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch

-- 
2.37.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/2] package/elf2flt: remove broken patch

[Niklas Cassel via buildroot]

Remove broken elf2flt patch:
package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch

The patch modifies main(), but does not perform a similar change in
output_relocs(), which might cause sectionp in output_relocs() to be
initialized with an invalid address, causing random memory to be
overwritten.

This leads to a segfault when linking e.g. busybox for m68k.

We cannot perform a similar change in output_relocs(), since at this
point in time, data_len will always be non-zero.

Revert this patch completely, so that we can add a working patch.

Note that even with this patch reverted, m68k results in a fatal error:
ERROR: text=0x3bab8 overlaps data=0x33f60 ?

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 ...3-elf2flt-add-riscv-64-bits-support.patch} |  0
 ...f2flt-fix-.eh_frame-section-handling.patch | 73 -------------------
 2 files changed, 73 deletions(-)
 rename package/elf2flt/{0004-elf2flt-add-riscv-64-bits-support.patch => 0003-elf2flt-add-riscv-64-bits-support.patch} (100%)
 delete mode 100644 package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch

diff --git a/package/elf2flt/0004-elf2flt-add-riscv-64-bits-support.patch b/package/elf2flt/0003-elf2flt-add-riscv-64-bits-support.patch
similarity index 100%
rename from package/elf2flt/0004-elf2flt-add-riscv-64-bits-support.patch
rename to package/elf2flt/0003-elf2flt-add-riscv-64-bits-support.patch
diff --git a/package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch b/package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch
deleted file mode 100644
index e7e8c6f25b..0000000000
--- a/package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch
+++ /dev/null
@@ -1,73 +0,0 @@
-From 8b7fdb1dedfb8a6e858b46e5af33029fe0462ab8 Mon Sep 17 00:00:00 2001
-From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
-Date: Tue, 10 May 2022 23:14:36 +0900
-Subject: [PATCH] elf2flt: fix .eh_frame section handling
-
-elf2flt.ld linker script positions the .eh_frame section in an output
-section after the .text and .data output sections.
-
-However, when elf2flt.c is supplied the ELF linked using the elf2flt.ld
-linker script, it only looks at the flags for each input section, and
-then puts it in either a bFLT text, data or bss output section.
-
-Commit ba379d08bb7 ("elf2flt: fix for segfault on some ARM ELFs")
-modified the section scanning loop of elf2flt main() function to put
-read-only relocation data sections in the bFLT text output section so
-that the .ARM.exidx section is placed in the .text flat output section.
-Previously a read-only relocation data section would be put in the data
-output section.
-
-On ARM, the .eh_frame section does not have the SEC_RELOC flag set, so
-it will still end up in the data output section. However, on
-architectures that generates the .eh_frame section with the SEC_RELOC
-flag set, this section will now be placed in the text output section.
-
-The logic in elf2flt will handle all sections in order, and since the
-input order is .text, .data, and .eh_frame, putting .eh_frame in text
-output section does not work, since elf2flt.c has already put the .data
-input section in the bFLT data output section. This leads to the
-following print (example for riscv64 architecture):
-
-buildroot/output/host/riscv64-buildroot-linux-uclibc/bin/elf2flt:
-ERROR: text=0x3bab8 overlaps data=0x33f60 ?
-
-The way that elf2flt is written, we cannot append to the text output
-section after an input section has been added to the data output
-section. It might be possible to change this, but that would require
-moving all the code the was already placed in the data output section.
-
-Instead, let's allow putting a read-only relocation data section in the
-text output section (so that .ARM.exidx will still be placed correctly),
-but only if there has not yet been anything placed in the data output
-section.
-
-That way .ARM.exidx will still be placed correctly, and .eh_frame will
-be placed correctly in the .data output section, regardless if it has
-flag SEC_RELOC set or not.
-
-Fixes: ba379d08bb7 ("elf2flt: fix for segfault on some ARM ELFs")
-Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
-Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
----
- elf2flt.c | 5 +++--
- 1 file changed, 3 insertions(+), 2 deletions(-)
-
-diff --git a/elf2flt.c b/elf2flt.c
-index 7ac0617..da25e93 100644
---- a/elf2flt.c
-+++ b/elf2flt.c
-@@ -1877,8 +1877,9 @@ int main(int argc, char *argv[])
-     bfd_vma sec_vma;
- 
-     if ((s->flags & SEC_CODE) ||
--       ((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
--                    (SEC_DATA | SEC_READONLY | SEC_RELOC))) {
-+        (((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
-+                     (SEC_DATA | SEC_READONLY | SEC_RELOC)) &&
-+          !data_len)) {
-       vma = &text_vma;
-       len = &text_len;
-     } else if (s->flags & SEC_DATA) {
--- 
-2.35.1
-
-- 
2.37.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/2] package/elf2flt: add another .eh_frame section handling patch

[Niklas Cassel via buildroot]

This adds another .eh_frame section handling patch.

Without this patch, both m68k and riscv64 would result in a fatal error:
ERROR: text=0x3bab8 overlaps data=0x33f60 ?

With this patch, qemu_m68k_mcf5208_defconfig and
qemu_riscv64_nommu_virt_defconfig both builds and boots properly.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 ...2flt-create-a-common-helper-function.patch | 76 +++++++++++++++++++
 ...l-error-regression-on-m68k-and-riscv.patch | 72 ++++++++++++++++++
 2 files changed, 148 insertions(+)
 create mode 100644 package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch
 create mode 100644 package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch

diff --git a/package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch b/package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch
new file mode 100644
index 0000000000..49d305f35a
--- /dev/null
+++ b/package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch
@@ -0,0 +1,76 @@
+From d1dde619e277369782ccf50e4b54484520e3a6b6 Mon Sep 17 00:00:00 2001
+From: Niklas Cassel <niklas.cassel@wdc.com>
+Date: Tue, 9 Aug 2022 12:13:50 +0200
+Subject: [PATCH 1/2] elf2flt: create a common helper function
+
+In order to make the code more maintainable,
+move duplicated code to a common helper function.
+
+No functional change intended.
+
+Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
+---
+ elf2flt.c | 19 +++++++++++--------
+ 1 file changed, 11 insertions(+), 8 deletions(-)
+
+diff --git a/elf2flt.c b/elf2flt.c
+index 669591e..9c32f9a 100644
+--- a/elf2flt.c
++++ b/elf2flt.c
+@@ -337,6 +337,13 @@ compare_relocs (const void *pa, const void *pb)
+ }
+ #endif
+ 
++static bool
++ro_reloc_data_section_should_be_in_text(asection *s)
++{
++  return (s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
++	  (SEC_DATA | SEC_READONLY | SEC_RELOC);
++}
++
+ static uint32_t *
+ output_relocs (
+   bfd *abs_bfd,
+@@ -428,8 +435,7 @@ output_relocs (
+ 	 */
+ 	if ((!pic_with_got || ALWAYS_RELOC_TEXT) &&
+ 	    ((a->flags & SEC_CODE) ||
+-	    ((a->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
+-		         (SEC_DATA | SEC_READONLY | SEC_RELOC))))
++	     ro_reloc_data_section_should_be_in_text(a)))
+ 		sectionp = text + (a->vma - text_vma);
+ 	else if (a->flags & SEC_DATA)
+ 		sectionp = data + (a->vma - data_vma);
+@@ -1893,8 +1899,7 @@ int main(int argc, char *argv[])
+     bfd_vma sec_vma;
+ 
+     if ((s->flags & SEC_CODE) ||
+-       ((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
+-                    (SEC_DATA | SEC_READONLY | SEC_RELOC))) {
++	ro_reloc_data_section_should_be_in_text(s)) {
+       vma = &text_vma;
+       len = &text_len;
+     } else if (s->flags & SEC_DATA) {
+@@ -1932,8 +1937,7 @@ int main(int argc, char *argv[])
+    * data sections.*/
+   for (s = abs_bfd->sections; s != NULL; s = s->next)
+     if ((s->flags & SEC_CODE) ||
+-       ((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
+-                    (SEC_DATA | SEC_READONLY | SEC_RELOC)))
++	ro_reloc_data_section_should_be_in_text(s))
+       if (!bfd_get_section_contents(abs_bfd, s,
+ 				   text + (s->vma - text_vma), 0,
+ 				   bfd_section_size(abs_bfd, s)))
+@@ -1962,8 +1966,7 @@ int main(int argc, char *argv[])
+    * data sections already included in the text output section.*/
+   for (s = abs_bfd->sections; s != NULL; s = s->next)
+     if ((s->flags & SEC_DATA) &&
+-       ((s->flags & (SEC_READONLY | SEC_RELOC)) !=
+-                    (SEC_READONLY | SEC_RELOC)))
++	!ro_reloc_data_section_should_be_in_text(s))
+       if (!bfd_get_section_contents(abs_bfd, s,
+ 				   data + (s->vma - data_vma), 0,
+ 				   bfd_section_size(abs_bfd, s)))
+-- 
+2.37.1
+
diff --git a/package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch b/package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch
new file mode 100644
index 0000000000..d2c9cbd2e2
--- /dev/null
+++ b/package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch
@@ -0,0 +1,72 @@
+From 0285d5093798eaaba1069db1b36272ac1975c205 Mon Sep 17 00:00:00 2001
+From: Niklas Cassel <niklas.cassel@wdc.com>
+Date: Tue, 9 Aug 2022 21:06:05 +0200
+Subject: [PATCH 2/2] elf2flt: fix fatal error regression on m68k and riscv64
+
+Commit ba379d08bb78 ("elf2flt: fix for segfault on some ARM ELFs")
+changed the condition of which input sections that should be included
+in the .text output section from:
+((a->flags & (SEC_DATA | SEC_READONLY)) == (SEC_DATA | SEC_READONLY))
+to:
+((a->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
+(SEC_DATA | SEC_READONLY | SEC_RELOC))
+
+On ARM, the .eh_frame input section does not have the SEC_RELOC flag
+set, so this specific change had no effect on ARM.
+
+However, on e.g. m68k and riscv64, the .eh_frame input section does
+have the SEC_RELOG flag set, which means that after commit ba379d08bb78
+("elf2flt: fix for segfault on some ARM ELFs"), read-only relocation
+data sections were placed in .text output section, instead of .data
+output section.
+
+This will result in a fatal error on m68k and riscv64:
+ERROR: text=0x3bab8 overlaps data=0x33f60 ?
+
+This is because elf2flt cannot append to .text after .data has been
+appended to.
+
+Note that the binutils maintainer says that the correct thing is
+to put read-only relocation data sections in .text:
+https://sourceware.org/legacy-ml/binutils/2019-10/msg00132.html
+
+So the proper fix is probably to rewrite elf2flt so that it can append
+to .text after .data has been appended to (which will require elf2flt
+to move/relocate everything that has already been appended to .data,
+since the virtual addresses are contiguous).
+
+However, for now, add an exception for m68k and riscv64 (specifically
+for the problematic input section, .eh_frame), so that we get the same
+behavior as older elf2flt releases, where we put read-only relocation
+data in .data, which was working perfectly fine.
+
+Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
+---
+ elf2flt.c | 11 +++++++++--
+ 1 file changed, 9 insertions(+), 2 deletions(-)
+
+diff --git a/elf2flt.c b/elf2flt.c
+index 9c32f9a..a680c89 100644
+--- a/elf2flt.c
++++ b/elf2flt.c
+@@ -340,8 +340,15 @@ compare_relocs (const void *pa, const void *pb)
+ static bool
+ ro_reloc_data_section_should_be_in_text(asection *s)
+ {
+-  return (s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
+-	  (SEC_DATA | SEC_READONLY | SEC_RELOC);
++  if ((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
++      (SEC_DATA | SEC_READONLY | SEC_RELOC)) {
++#if defined(TARGET_m68k) || defined(TARGET_riscv64)
++    if (!strcmp(".eh_frame", s->name))
++      return false;
++#endif
++    return true;
++  }
++  return false;
+ }
+ 
+ static uint32_t *
+-- 
+2.37.1
+
-- 
2.37.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/2] fix elf2flt on m68k

[Waldemar Brodkorb]

Hi Niklas,

it still fails for qemu_xtensa_lx60_nommu_defconfig with:
/home/wbx/buildroot/output/host/xtensa-buildroot-uclinux-uclibc/bin/elf2flt:
ERROR: text=0x69904 overlaps data=0x569a0 ?

best regards
 Waldemar

Niklas Cassel via buildroot wrote,

> Hello buildroot folks,
> 
> This series fixes a segfault at link time on m68k.
> 
> With this series, we have bootable m68k and riscv64.
> 
> (riscv64 was booting with the previous patch, but we
> were simply lucky that we didn't observe the same
> segfault there as well.)
> 
> 
> Kind regards,
> Niklas
> 
> Niklas Cassel (2):
>   package/elf2flt: remove broken patch
>   package/elf2flt: add another .eh_frame section handling patch
> 
>  ...3-elf2flt-add-riscv-64-bits-support.patch} |  0
>  ...f2flt-fix-.eh_frame-section-handling.patch | 73 ------------------
>  ...2flt-create-a-common-helper-function.patch | 76 +++++++++++++++++++
>  ...l-error-regression-on-m68k-and-riscv.patch | 72 ++++++++++++++++++
>  4 files changed, 148 insertions(+), 73 deletions(-)
>  rename package/elf2flt/{0004-elf2flt-add-riscv-64-bits-support.patch => 0003-elf2flt-add-riscv-64-bits-support.patch} (100%)
>  delete mode 100644 package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch
>  create mode 100644 package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch
>  create mode 100644 package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch
> 
> -- 
> 2.37.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/2] fix elf2flt on m68k

[Niklas Cassel via buildroot]

On Thu, Aug 11, 2022 at 01:38:33AM +0200, Waldemar Brodkorb wrote:
> Hi Niklas,
> 
> it still fails for qemu_xtensa_lx60_nommu_defconfig with:
> /home/wbx/buildroot/output/host/xtensa-buildroot-uclinux-uclibc/bin/elf2flt:
> ERROR: text=0x69904 overlaps data=0x569a0 ?

This is because of the same bad upstream elf2flt commit
ba379d08bb78 ("elf2flt: fix for segfault on some ARM ELFs"),
which fixed segfaults on ARM, but seems to have introduced
segfaults on some previously working archs.

The solution is to also include xtensa in my patch:

+  if ((s->flags & (SEC_DATA | SEC_READONLY | SEC_RELOC)) ==
+      (SEC_DATA | SEC_READONLY | SEC_RELOC)) {
+#if defined(TARGET_m68k) || defined(TARGET_riscv64) || defined(TARGET_xtensa)
+    if (!strcmp(".eh_frame", s->name))
+      return false;
+#endif
+    return true;
+  }
+  return false;

Let me send a v2 with this shortly.

However, if we keep adding additional, then we might
eventually be able to change it into a:

+#if defined (TARGET_arm)
+#else
+#endif


Kind regards,
Niklas

> 
> best regards
>  Waldemar
> 
> Niklas Cassel via buildroot wrote,
> 
> > Hello buildroot folks,
> > 
> > This series fixes a segfault at link time on m68k.
> > 
> > With this series, we have bootable m68k and riscv64.
> > 
> > (riscv64 was booting with the previous patch, but we
> > were simply lucky that we didn't observe the same
> > segfault there as well.)
> > 
> > 
> > Kind regards,
> > Niklas
> > 
> > Niklas Cassel (2):
> >   package/elf2flt: remove broken patch
> >   package/elf2flt: add another .eh_frame section handling patch
> > 
> >  ...3-elf2flt-add-riscv-64-bits-support.patch} |  0
> >  ...f2flt-fix-.eh_frame-section-handling.patch | 73 ------------------
> >  ...2flt-create-a-common-helper-function.patch | 76 +++++++++++++++++++
> >  ...l-error-regression-on-m68k-and-riscv.patch | 72 ++++++++++++++++++
> >  4 files changed, 148 insertions(+), 73 deletions(-)
> >  rename package/elf2flt/{0004-elf2flt-add-riscv-64-bits-support.patch => 0003-elf2flt-add-riscv-64-bits-support.patch} (100%)
> >  delete mode 100644 package/elf2flt/0003-elf2flt-fix-.eh_frame-section-handling.patch
> >  create mode 100644 package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch
> >  create mode 100644 package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-and-riscv.patch
> > 
> > -- 
> > 2.37.1
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> > 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot