| Message ID | 20220811094049.798010-3-niklas.cassel@wdc.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | fix elf2flt on m68k, xtensa, riscv | expand |
Hello Max, On Thu, 11 Aug 2022 11:40:49 +0200 Niklas Cassel via buildroot <buildroot@buildroot.org> wrote: > This series fixes a fatal error at link time on m68k, xtensa, > and riscv64, caused by a bad upstream elf2flt commit. > > Without this patch, m68k, xtensa, and riscv64 would result in > a fatal error: > ERROR: text=0x3bab8 overlaps data=0x33f60 ? > > With this patch, qemu_m68k_mcf5208_defconfig, > qemu_riscv64_nommu_virt_defconfig, and > qemu_xtensa_lx60_nommu_defconfig builds properly. > > riscv64 and m68k boots to login prompt. > xtensa crashes when loading init, the same behavior as when > reverting the bad upstream elf2flt commit completely. As you can see here, apparently the xtensa noMMU defconfig in Buildroot no longer boots. We had a regression in elf2flt, but even with this regression fixed the problem persists, and we're apparently back to what it was before the elf2flt regression was introduced. Do you think you could have a look? Thanks a lot, Thomas
Hi Thomas, On Thu, Aug 11, 2022 at 1:30 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > On Thu, 11 Aug 2022 11:40:49 +0200 > Niklas Cassel via buildroot <buildroot@buildroot.org> wrote: > > > This series fixes a fatal error at link time on m68k, xtensa, > > and riscv64, caused by a bad upstream elf2flt commit. > > > > Without this patch, m68k, xtensa, and riscv64 would result in > > a fatal error: > > ERROR: text=0x3bab8 overlaps data=0x33f60 ? > > > > With this patch, qemu_m68k_mcf5208_defconfig, > > qemu_riscv64_nommu_virt_defconfig, and > > qemu_xtensa_lx60_nommu_defconfig builds properly. > > > > riscv64 and m68k boots to login prompt. > > xtensa crashes when loading init, the same behavior as when > > reverting the bad upstream elf2flt commit completely. > > As you can see here, apparently the xtensa noMMU defconfig in Buildroot > no longer boots. We had a regression in elf2flt, but even with this > regression fixed the problem persists, and we're apparently back to > what it was before the elf2flt regression was introduced. > > Do you think you could have a look? Sure, I will. Thank you for the heads up.
>>>>> "Niklas" == Niklas Cassel via buildroot <buildroot@buildroot.org> writes: > This series fixes a fatal error at link time on m68k, xtensa, > and riscv64, caused by a bad upstream elf2flt commit. > Without this patch, m68k, xtensa, and riscv64 would result in > a fatal error: > ERROR: text=0x3bab8 overlaps data=0x33f60 ? > With this patch, qemu_m68k_mcf5208_defconfig, > qemu_riscv64_nommu_virt_defconfig, and > qemu_xtensa_lx60_nommu_defconfig builds properly. > riscv64 and m68k boots to login prompt. > xtensa crashes when loading init, the same behavior as when > reverting the bad upstream elf2flt commit completely. > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> When was this regression introduced? With the update to elf2flt 2021.08? Are 2022.02.x and 2022.05.x also affected?
On Thu, Sep 15, 2022 at 09:21:52AM +0200, Peter Korsgaard wrote: > >>>>> "Niklas" == Niklas Cassel via buildroot <buildroot@buildroot.org> writes: > > > This series fixes a fatal error at link time on m68k, xtensa, > > and riscv64, caused by a bad upstream elf2flt commit. > > > Without this patch, m68k, xtensa, and riscv64 would result in > > a fatal error: > > ERROR: text=0x3bab8 overlaps data=0x33f60 ? > > > With this patch, qemu_m68k_mcf5208_defconfig, > > qemu_riscv64_nommu_virt_defconfig, and > > qemu_xtensa_lx60_nommu_defconfig builds properly. > > > riscv64 and m68k boots to login prompt. > > xtensa crashes when loading init, the same behavior as when > > reverting the bad upstream elf2flt commit completely. > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> Hello Peter, > > When was this regression introduced? With the update to elf2flt 2021.08? While I didn't include the SHA1 in the buildroot patch, the SHA1 is included in the elf2flt patch embedded in the buildroot patch. ba379d08bb78 ("elf2flt: fix for segfault on some ARM ELFs") $ git tag --contains ba379d08bb78 v2021.08 So yes, the regression was introduced with elf2flt 2021.08. > Are 2022.02.x and 2022.05.x also affected? Buildroot branches 2022.02.x and 2022.05.x both includes the patch the upgrades elf2flt to 2021.08, so these branches should also be affected. I've tried to upstream my patches to elf2flt: https://github.com/uclinux-dev/elf2flt/pull/24 But maintainer seems to have gone AWOL :) Kind regards, Niklas
>>>>> "Niklas" == Niklas Cassel <Niklas.Cassel@wdc.com> writes: Hi, >> Are 2022.02.x and 2022.05.x also affected? > Buildroot branches 2022.02.x and 2022.05.x both includes > the patch the upgrades elf2flt to 2021.08, so these branches > should also be affected. Thanks for the feedback! Committed to 2022.05.x and 2022.02.x, thanks.
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..9fa5ff986f --- /dev/null +++ b/package/elf2flt/0004-elf2flt-create-a-common-helper-function.patch @@ -0,0 +1,76 @@ +From 37e1e0ace8ccebf54ec2f5522bfc1f9db86946ad 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-xtensa-ri.patch b/package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-xtensa-ri.patch new file mode 100644 index 0000000000..4224eacd9f --- /dev/null +++ b/package/elf2flt/0005-elf2flt-fix-fatal-error-regression-on-m68k-xtensa-ri.patch @@ -0,0 +1,73 @@ +From 65ac5f9e69cfb989d970da74c41e478774d29be5 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, xtensa, + 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_RELOC 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, xtensa 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, xtensa 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) || defined(TARGET_xtensa) ++ if (!strcmp(".eh_frame", s->name)) ++ return false; ++#endif ++ return true; ++ } ++ return false; + } + + static uint32_t * +-- +2.37.1 +
This series fixes a fatal error at link time on m68k, xtensa, and riscv64, caused by a bad upstream elf2flt commit. Without this patch, m68k, xtensa, and riscv64 would result in a fatal error: ERROR: text=0x3bab8 overlaps data=0x33f60 ? With this patch, qemu_m68k_mcf5208_defconfig, qemu_riscv64_nommu_virt_defconfig, and qemu_xtensa_lx60_nommu_defconfig builds properly. riscv64 and m68k boots to login prompt. xtensa crashes when loading init, the same behavior as when reverting the bad upstream elf2flt commit completely. Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> --- ...2flt-create-a-common-helper-function.patch | 76 +++++++++++++++++++ ...l-error-regression-on-m68k-xtensa-ri.patch | 73 ++++++++++++++++++ 2 files changed, 149 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-xtensa-ri.patch