Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

microblaze: apply binutils patches from meta-xilinx repository #10

Open
wants to merge 2 commits into
base: zephyr-binutils-2_38
Choose a base branch
from

Conversation

alpsayin
Copy link

@alpsayin alpsayin commented Oct 1, 2023

microblaze: apply binutils patches from meta-xilinx repository

This patchset fixes many known issues on gnu-toolchain for microblaze.
But mainly the atomic-cas and binutils incorrect relocation issues.

Patches obtained from https://github.com/xilinx/meta-xilinx/ repository,
from meta-microblaze/recipes-devtools/binutils path.
Some patches that were already applied were reverted and re-applied to avoid conflicts.

Finally, I've given up and disabled microblaze64 toolchain build. meta-xilinx patches somehow manage to break the same toolchain that they introduced. I have no intention to support microblaze64 yet and this saves a few CI cycles.

@alpsayin alpsayin force-pushed the zephyr-binutils-2_38-microblaze-recipes-28082023 branch 2 times, most recently from 6a51ef5 to 1966eda Compare October 2, 2023 22:13
@alpsayin alpsayin changed the title Zephyr binutils 2 38 microblaze recipes 28082023 microblaze: apply binutils patches from meta-xilinx repository Oct 2, 2023
@alpsayin alpsayin force-pushed the zephyr-binutils-2_38-microblaze-recipes-28082023 branch from 1966eda to 8085bc5 Compare October 2, 2023 22:16
@alpsayin alpsayin marked this pull request as ready for review October 2, 2023 22:22
gdb/gdbserver/Makefile.in Outdated Show resolved Hide resolved
gdb/gdbserver/configure.srv Outdated Show resolved Hide resolved
gdb/gdbserver/linux-microblaze-low.c Outdated Show resolved Hide resolved
@alpsayin alpsayin force-pushed the zephyr-binutils-2_38-microblaze-recipes-28082023 branch from 8085bc5 to 71acd97 Compare December 2, 2023 13:21
@alpsayin
Copy link
Author

alpsayin commented Dec 2, 2023

@stephanosio resolved the marked issues and some more that I noticed irrelevant to microblaze or binutils. Zephyr(wout/ picolibc tests) and picolibc's own tests passed.

include/elf/common.h Show resolved Hide resolved
bfd/elflink.c Outdated
@@ -6580,7 +6580,6 @@ elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)

inf = (struct elf_gc_sweep_symbol_info *) data;
(*inf->hide_symbol) (inf->info, h, true);
h->def_regular = 0;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't look microblaze related; to be inspected

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upstream change to garbage collection sweep causes mb regression

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like it fixes an issue. Keeping it in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't look microblaze related; to be inspected

That was exactly my concern -- I was going to leave a comment about it then got interrupted by something else and it went off my radar ... (sorry about that).

My concern is its potential effect on other architectures -- will it not cause any regressions on other architectures relying on def_regular to be set to 0 by this function?

Unless we can prove it will not cause any regressions for other architectures, could we make this specific to Microblaze only? (maybe make the function accept bfd param and add this under if (bfd_get_arch(abfd) == bfd_arch_microblaze)?)

Copy link
Author

@alpsayin alpsayin Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll undo the patch for the time being because also from the original commit message, this seems very non-zephyr+embedded-linux related. If this gets upstreamed, and then if it finds its way down here through upstream binutils, then great, but otherwise a very edge case for an embedded application (also considering C++ is (should be) an edge case for an embedded application).

The visible issue with this change is when running a c++ application
in Petalinux which links libstdc++.so for exception handling it segfaults
on execution.
This does not occur if static linking libstdc++.a, so its during the
relocations for a shared lib with garbage collection this occurs
I'll now take a look at the others too.

gas/expr.c Show resolved Hide resolved
ld/ldexp.c Show resolved Hide resolved
ld/ldexp.h Show resolved Hide resolved
ld/ldlang.c Show resolved Hide resolved
@alpsayin alpsayin force-pushed the zephyr-binutils-2_38-microblaze-recipes-28082023 branch from ee6f0da to 04e5b36 Compare December 19, 2023 18:39
This patchset fixes many known issues on gnu-toolchain for microblaze.
But mainly the atomic-cas and binutils incorrect relocation issues.

Patches obtained from https://github.com/xilinx/meta-xilinx/ repository,
from meta-microblaze/recipes-devtools/binutils path.
Below is a list of patches applied and squashed. Some patches that were
already applied were reverted and re-applied to avoid conflicts.

0001-Add-wdc.ext.clear-and-wdc.ext.flush-insns.patch
0002-Add-mlittle-endian-and-mbig-endian-flags.patch
0003-Disable-the-warning-message-for-eh_frame_hdr.patch
0004-LOCAL-Fix-relaxation-of-assembler-resolved-reference.patch
0006-Fix-bug-in-TLSTPREL-Relocation.patch
0007-Added-Address-extension-instructions.patch
0008-Add-new-bit-field-instructions.patch
0009-Patch-Microblaze-fixed-bug-in-GCC-so-that-It-will-su.patch
0010-fixing-the-constant-range-check-issue.patch
0011-Patch-Microblaze-Compiler-will-give-error-messages-i.patch
0012-Patch-MicroBlaze-initial-support-for-MicroBlaze-64-b.patch
0013-Patch-Microblaze-negl-instruction-is-overriding-rsub.patch
0014-Added-relocations-for-MB-X.patch
0015-Fixed-MB-x-relocation-issues.patch
0016-Fixing-the-branch-related-issues.patch
0017-Fixed-address-computation-issues-with-64bit-address.patch
0018-Patch-MicroBlaze-Adding-new-relocation-to-support-64.patch
0019-fixing-the-.bss-relocation-issue.patch
0020-Fixed-the-bug-in-the-R_MICROBLAZE_64_NONE-relocation.patch
0021-Revert-ld-Remove-unused-expression-state.patch
0022-fixing-the-long-long-long-mingw-toolchain-issue.patch
0023-Added-support-to-new-arithmetic-single-register-inst.patch
0024-Patch-MicroBlaze-double-imml-generation-for-64-bit-v.patch
0025-Fixed-bug-in-generation-of-IMML-instruction-for-the.patch
0026-Patch-MicroBlaze-m64-This-patch-will-remove-imml-0-a.patch
0027-Patch-MicroBlaze-improper-address-mapping-of-PROVIDE.patch
0028-Patch-microblaze-Changing-the-long-to-long-long-as-i.patch
0029-gas-revert-moving-of-md_pseudo_table-from-const.patch
0030-ld-emulparams-elf64microblaze-Fix-emulation-generati.patch
0031-Patch-MicroBlaze-Invalid-data-offsets-pointer-after-.patch
0032-Patch-MicroBlaze-Double-free-with-ld-no-keep-memory.patch
0033-Patch-MB-MB-binutils-Upstream-port-issues.patch
0034-Patch-MicroBlaze-By-default-the-linker-will-generate.patch

Signed-off-by: Alp Sayin <alpsayin@gmail.com>
This is because I don't intend to offer Zephyr support for microblaze64 yet
but also because the toolchain is buggy to the extent that it fails to
build.

Signed-off-by: Alp Sayin <alpsayin@gmail.com>
@alpsayin alpsayin force-pushed the zephyr-binutils-2_38-microblaze-recipes-28082023 branch from 04e5b36 to 324e5d8 Compare May 29, 2024 10:50
@alpsayin
Copy link
Author

alpsayin commented Jun 3, 2024

Hi @stephanosio , would you be happy to do a re-review? (when you're available)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants