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

elf2flt: handle binutils >= 2.34 #15

Closed

Conversation

RomainNaour
Copy link
Contributor

The latest Binutils release (2.34) is not compatible with elf2flt due
to a change in bfd_section_* macros. The issue has been reported to
the Binutils mailing list but Alan Modra recommend to bundle libbfd
library sources into each projects using it [1]. That's because the
API is not stable over the time without any backward compatibility
guaranties.

On the other hand, the elf2flt tools needs to support modified
version of binutils for specific arch/target [2].

Add two tests in the configure script to detect this API change
in order to support binutils < 2.34 and binutils >= 2.34.

[1] https://sourceware.org/ml/binutils/2020-02/msg00044.html
[2] #14

Signed-off-by: Romain Naour romain.naour@smile.fr

Copy link
Member

@vapier vapier left a comment

Choose a reason for hiding this comment

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

some nits

configure.ac Outdated

AC_TRY_COMPILE([#include <bfd.h>],
[const asection *sec; bfd_section_size(sec);],
bfd_section_size_macro_has_one_arg=yes,
Copy link
Member

Choose a reason for hiding this comment

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

name this more generically like bfd_section_api_takes_bfd or something. they converted all the APIs at once, so there's no need to test each one individually.

elf2flt.c Outdated
@@ -149,6 +149,17 @@ const char *elf2flt_progname;
#define O_BINARY 0
#endif

#if defined(HAVE_BFD_SECTION_SIZE_MACRO_HAS_ONE_ARG)
#define elf2flt_bfd_section_size(abs_bfd, s) bfd_section_size(s)
Copy link
Member

Choose a reason for hiding this comment

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

the point of the API change is that the bfd isn't actually used by any of the bfd_section funcs. so we should structure our API similarly.

#define elf2flt_bfd_section_size(s) bfd_section_size(s)
#else
#define elf2flt_bfd_section_size(s) bfd_section_size(NULL, s)

elf2flt.c Outdated
@@ -149,6 +149,17 @@ const char *elf2flt_progname;
#define O_BINARY 0
#endif

#if defined(HAVE_BFD_SECTION_SIZE_MACRO_HAS_ONE_ARG)
Copy link
Member

Choose a reason for hiding this comment

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

you can use #ifdef here instead

configure.ac Outdated
@@ -212,6 +212,31 @@ AC_CHECK_FUNCS([ \
strsignal \
])

dnl Various bfd section macros and functions like bfd_section_size() has been
Copy link
Member

Choose a reason for hiding this comment

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

has->have

configure.ac Outdated
@@ -212,6 +212,31 @@ AC_CHECK_FUNCS([ \
strsignal \
])

dnl Various bfd section macros and functions like bfd_section_size() has been
dnl modified starting binutils >= 2.34.
Copy link
Member

Choose a reason for hiding this comment

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

"starting with binutils"

configure.ac Outdated
@@ -212,6 +212,31 @@ AC_CHECK_FUNCS([ \
strsignal \
])

dnl Various bfd section macros and functions like bfd_section_size() has been
dnl modified starting binutils >= 2.34.
dnl Check if the prototype is "bfd_section_size (sec)" or "bfd_section_size(bfd, ptr)"
Copy link
Member

Choose a reason for hiding this comment

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

"Check if the prototypes take a bfd argument."

The latest Binutils release (2.34) is not compatible with elf2flt due
to a change in bfd_section_* macros [1]. The issue has been reported
to the Binutils mailing list but Alan Modra recommend to bundle
libbfd library sources into each projects using it [2]. That's
because the API is not stable over the time without any backward
compatibility guaranties.

On the other hand, the elf2flt tools needs to support modified
version of binutils for specific arch/target [3].

Add two tests in the configure script to detect this API change
in order to support binutils < 2.34 and binutils >= 2.34.

Upstream status: [4]

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=fd3619828e94a24a92cddec42cbc0ab33352eeb4
[2] https://sourceware.org/ml/binutils/2020-02/msg00044.html
[3] uclinux-dev#14
[4] uclinux-dev#15

Signed-off-by: Romain Naour <romain.naour@smile.fr>
@vapier vapier changed the base branch from master to main January 13, 2021 22:48
@vapier
Copy link
Member

vapier commented Aug 18, 2021

were you planning on refreshing this ?

atishp04 pushed a commit to atishp04/elf2flt that referenced this pull request Sep 29, 2021
The latest Binutils release (2.34) is not compatible with elf2flt due
to a change in bfd_section_* macros [1]. The issue has been reported
to the Binutils mailing list but Alan Modra recommend to bundle
libbfd library sources into each projects using it [2]. That's
because the API is not stable over the time without any backward
compatibility guaranties.

On the other hand, the elf2flt tools needs to support modified
version of binutils for specific arch/target [3].

Add two tests in the configure script to detect this API change
in order to support binutils < 2.34 and binutils >= 2.34.

Upstream status: [4]

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=fd3619828e94a24a92cddec42cbc0ab33352eeb4
[2] https://sourceware.org/ml/binutils/2020-02/msg00044.html
[3] uclinux-dev#14
[4] uclinux-dev#15

Signed-off-by: Romain Naour <romain.naour@smile.fr>
damien-lemoal pushed a commit to damien-lemoal/elf2flt that referenced this pull request Apr 21, 2022
The latest Binutils release (2.34) is not compatible with elf2flt due
to a change in bfd_section_* macros [1]. The issue has been reported
to the Binutils mailing list but Alan Modra recommend to bundle
libbfd library sources into each projects using it [2]. That's
because the API is not stable over the time without any backward
compatibility guaranties.

On the other hand, the elf2flt tools needs to support modified
version of binutils for specific arch/target [3].

Add two tests in the configure script to detect this API change
in order to support binutils < 2.34 and binutils >= 2.34.

Upstream status: [4]

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=fd3619828e94a24a92cddec42cbc0ab33352eeb4
[2] https://sourceware.org/ml/binutils/2020-02/msg00044.html
[3] uclinux-dev#14
[4] uclinux-dev#15

Signed-off-by: Romain Naour <romain.naour@smile.fr>
michael131468 pushed a commit to michael131468/elf2flt that referenced this pull request Jul 5, 2022
The latest Binutils release (2.34) is not compatible with elf2flt due
to a change in bfd_section_* macros [1]. The issue has been reported
to the Binutils mailing list but Alan Modra recommend to bundle
libbfd library sources into each projects using it [2]. That's
because the API is not stable over the time without any backward
compatibility guaranties.

On the other hand, the elf2flt tools needs to support modified
version of binutils for specific arch/target [3].

Add two tests in the configure script to detect this API change
in order to support binutils < 2.34 and binutils >= 2.34.

Upstream status: [4]

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=fd3619828e94a24a92cddec42cbc0ab33352eeb4
[2] https://sourceware.org/ml/binutils/2020-02/msg00044.html
[3] uclinux-dev#14
[4] uclinux-dev#15

Signed-off-by: Romain Naour <romain.naour@smile.fr>
@gregungerer
Copy link
Contributor

This looks to have been sitting here for quite a while now. I have manually resolved the conflict, so it is at least clean and ready to be applied. As far as I can tell Romain addressed all feedback.

@vapier
Copy link
Member

vapier commented Mar 14, 2023

tbh, i didn't really notice the updated pushes since the open comments weren't marked resolved, so was waiting for OP to do so. if you think issues are resolved, and you've rebased the work, then let's merge it.

should update the unittest coverage (i.e. uclinux-dev/prebuilts-binutils-libs) to make sure things work & stay working.

@gregungerer
Copy link
Contributor

Further testing has found at least one problem. This only works if you use the single --with-binutils-build-dir configure form. If you specify each of the --with-libbfd/--with-libiberty/etc configure options (but no --with-binutils-build-dir option) it will fail to detect the version of binutils correctly.

@gregungerer
Copy link
Contributor

Oh, and the in-tree configure script needs to be regenerated too.

@vapier
Copy link
Member

vapier commented Mar 23, 2023

hmm, we should add those scenarios to our CI tests

gregungerer pushed a commit that referenced this pull request Apr 4, 2023
The latest Binutils release (2.34) is not compatible with elf2flt due
to a change in bfd_section_* macros [1]. The issue has been reported
to the Binutils mailing list but Alan Modra recommend to bundle
libbfd library sources into each projects using it [2]. That's
because the API is not stable over the time without any backward
compatibility guaranties.

On the other hand, the elf2flt tools needs to support modified
version of binutils for specific arch/target [3].

Add two tests in the configure script to detect this API change
in order to support binutils < 2.34 and binutils >= 2.34.

Upstream status: [4]

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=fd3619828e94a24a92cddec42cbc0ab33352eeb4
[2] https://sourceware.org/ml/binutils/2020-02/msg00044.html
[3] #14
[4] #15

Signed-off-by: Romain Naour <romain.naour@smile.fr>

Two changes made to original patch:
. $binutils_include_dir used for binutils version check
. configure script regenerated (run autoconf)

Signed-off-by: Greg Ungerer <gerg@kernel.org>
@gregungerer
Copy link
Contributor

This pull request updated to fix --with-binutils-build-dir case and the configure script re-generated and applied to main.

@gregungerer gregungerer closed this Apr 4, 2023
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.

None yet

3 participants