| Message ID | 20241227214756.1059146-1-thiago.bauermann@linaro.org |
|---|---|
| State | Accepted |
| Commit | 36eee5a74eb6d4c48a3a22cd71b3944bac499d0a |
| Headers | show |
| Series | gcc/configure: Fix check for assembler section merging support on Arm | expand |
Hello, Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes: > This problem was noticed because the recent binutils commit > d5cbf916be4a ("gas/ELF: also reject merge entity size being zero") caused > gas to be stricter about mergeable sections without an entity size: > > configure:27013: checking assembler for section merging support > configure:27022: /path/to/as --fatal-warnings -o conftest.o conftest.s >&5 > conftest.s: Assembler messages: > conftest.s:1: Warning: invalid merge / string entity size > conftest.s: Error: 1 warning, treating warnings as errors > configure:27025: $? = 1 > configure: failed program was > .section .rodata.str, "aMS", @progbits, 1 > configure:27036: result: no > > In previous versions of gas the conftest.s program above was accepted > and configure detected support for section merging. I just posted a patch for gas implementing a suggestion by Alan Modra that makes it accept the syntax above: https://inbox.sourceware.org/binutils/20250103032831.622617-1-thiago.bauermann@linaro.org/ However, I think it's still a good idea to commit this patch. -- Thiago
On 27/12/2024 21:47, Thiago Jung Bauermann wrote: > In 32-bit Arm assembly, the @ character is the start of a comment so > the section type needs to use the % character instead. > > configure.ac attempts to account for this difference by doing a second > try when checking the assembler for section merging support. > Unfortunately there is a bug: because the gcc_GAS_CHECK_FEATURE macro > has a call to AC_CACHE_CHECK, it will actually skip the second try > because the gcc_cv_as_shf_merge variable has already been set: > > checking assembler for section merging support... no > checking assembler for section merging support... (cached) no > > Fix by using a separate variable for the second try, as is done in the > check for COMDAT group support. > > This problem was noticed because the recent binutils commit > d5cbf916be4a ("gas/ELF: also reject merge entity size being zero") caused > gas to be stricter about mergeable sections without an entity size: > > configure:27013: checking assembler for section merging support > configure:27022: /path/to/as --fatal-warnings -o conftest.o conftest.s >&5 > conftest.s: Assembler messages: > conftest.s:1: Warning: invalid merge / string entity size > conftest.s: Error: 1 warning, treating warnings as errors > configure:27025: $? = 1 > configure: failed program was > .section .rodata.str, "aMS", @progbits, 1 > configure:27036: result: no > > In previous versions of gas the conftest.s program above was accepted > and configure detected support for section merging. > > See also: > https://linaro.atlassian.net/browse/GNU-1427 > https://sourceware.org/bugzilla/show_bug.cgi?id=32491 > > Tested on armv8l-linux-gnueabihf. > > gcc/ChangeLog: > * configure.ac: Fix check for HAVE_GAS_SHF_MERGE on Arm targets. > * configure: Regenerate. OK. R.
"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > On 27/12/2024 21:47, Thiago Jung Bauermann wrote: >> In 32-bit Arm assembly, the @ character is the start of a comment so >> the section type needs to use the % character instead. >> >> configure.ac attempts to account for this difference by doing a second >> try when checking the assembler for section merging support. >> Unfortunately there is a bug: because the gcc_GAS_CHECK_FEATURE macro >> has a call to AC_CACHE_CHECK, it will actually skip the second try >> because the gcc_cv_as_shf_merge variable has already been set: >> >> checking assembler for section merging support... no >> checking assembler for section merging support... (cached) no >> >> Fix by using a separate variable for the second try, as is done in the >> check for COMDAT group support. >> >> This problem was noticed because the recent binutils commit >> d5cbf916be4a ("gas/ELF: also reject merge entity size being zero") caused >> gas to be stricter about mergeable sections without an entity size: >> >> configure:27013: checking assembler for section merging support >> configure:27022: /path/to/as --fatal-warnings -o conftest.o conftest.s >&5 >> conftest.s: Assembler messages: >> conftest.s:1: Warning: invalid merge / string entity size >> conftest.s: Error: 1 warning, treating warnings as errors >> configure:27025: $? = 1 >> configure: failed program was >> .section .rodata.str, "aMS", @progbits, 1 >> configure:27036: result: no >> >> In previous versions of gas the conftest.s program above was accepted >> and configure detected support for section merging. >> >> See also: >> https://linaro.atlassian.net/browse/GNU-1427 >> https://sourceware.org/bugzilla/show_bug.cgi?id=32491 >> >> Tested on armv8l-linux-gnueabihf. >> >> gcc/ChangeLog: >> * configure.ac: Fix check for HAVE_GAS_SHF_MERGE on Arm targets. >> * configure: Regenerate. > > OK. > > R. Thank you! Pushed as commit 36eee5a74eb6. Is it ok if I also push it to the supported branches releases/gcc-12, releases/gcc-13 and releases/gcc-14? -- Thiago
On 08/01/2025 21:47, Thiago Jung Bauermann wrote: > "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > >> On 27/12/2024 21:47, Thiago Jung Bauermann wrote: >>> In 32-bit Arm assembly, the @ character is the start of a comment so >>> the section type needs to use the % character instead. >>> >>> configure.ac attempts to account for this difference by doing a second >>> try when checking the assembler for section merging support. >>> Unfortunately there is a bug: because the gcc_GAS_CHECK_FEATURE macro >>> has a call to AC_CACHE_CHECK, it will actually skip the second try >>> because the gcc_cv_as_shf_merge variable has already been set: >>> >>> checking assembler for section merging support... no >>> checking assembler for section merging support... (cached) no >>> >>> Fix by using a separate variable for the second try, as is done in the >>> check for COMDAT group support. >>> >>> This problem was noticed because the recent binutils commit >>> d5cbf916be4a ("gas/ELF: also reject merge entity size being zero") caused >>> gas to be stricter about mergeable sections without an entity size: >>> >>> configure:27013: checking assembler for section merging support >>> configure:27022: /path/to/as --fatal-warnings -o conftest.o conftest.s >&5 >>> conftest.s: Assembler messages: >>> conftest.s:1: Warning: invalid merge / string entity size >>> conftest.s: Error: 1 warning, treating warnings as errors >>> configure:27025: $? = 1 >>> configure: failed program was >>> .section .rodata.str, "aMS", @progbits, 1 >>> configure:27036: result: no >>> >>> In previous versions of gas the conftest.s program above was accepted >>> and configure detected support for section merging. >>> >>> See also: >>> https://linaro.atlassian.net/browse/GNU-1427 >>> https://sourceware.org/bugzilla/show_bug.cgi?id=32491 >>> >>> Tested on armv8l-linux-gnueabihf. >>> >>> gcc/ChangeLog: >>> * configure.ac: Fix check for HAVE_GAS_SHF_MERGE on Arm targets. >>> * configure: Regenerate. >> >> OK. >> >> R. > > Thank you! Pushed as commit 36eee5a74eb6. > > Is it ok if I also push it to the supported branches releases/gcc-12, > releases/gcc-13 and releases/gcc-14? > Yes, but please give it a few days to allow the various builders to check that there's no unexpected fallout first. R.
diff --git a/gcc/configure b/gcc/configure index a8b531d8fae0..0bc33f0ede18 100755 --- a/gcc/configure +++ b/gcc/configure @@ -27038,12 +27038,12 @@ $as_echo "$gcc_cv_as_shf_merge" >&6; } if test $gcc_cv_as_shf_merge = no; then - { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for section merging support" >&5 -$as_echo_n "checking assembler for section merging support... " >&6; } -if ${gcc_cv_as_shf_merge+:} false; then : + { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for section merging support (%progbits)" >&5 +$as_echo_n "checking assembler for section merging support (%progbits)... " >&6; } +if ${gcc_cv_as_shf_merge_percent+:} false; then : $as_echo_n "(cached) " >&6 else - gcc_cv_as_shf_merge=no + gcc_cv_as_shf_merge_percent=no if test x$gcc_cv_as != x; then $as_echo '.section .rodata.str, "aMS", %progbits, 1' > conftest.s if { ac_try='$gcc_cv_as $gcc_cv_as_flags --fatal-warnings -o conftest.o conftest.s >&5' @@ -27053,7 +27053,7 @@ else $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 test $ac_status = 0; }; } then - gcc_cv_as_shf_merge=yes + gcc_cv_as_shf_merge_percent=yes else echo "configure: failed program was" >&5 cat conftest.s >&5 @@ -27061,14 +27061,15 @@ else rm -f conftest.o conftest.s fi fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_shf_merge" >&5 -$as_echo "$gcc_cv_as_shf_merge" >&6; } +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_shf_merge_percent" >&5 +$as_echo "$gcc_cv_as_shf_merge_percent" >&6; } fi cat >>confdefs.h <<_ACEOF -#define HAVE_GAS_SHF_MERGE `if test $gcc_cv_as_shf_merge = yes; then echo 1; else echo 0; fi` +#define HAVE_GAS_SHF_MERGE `if test $gcc_cv_as_shf_merge = yes \ + || test $gcc_cv_as_shf_merge_percent = yes; then echo 1; else echo 0; fi` _ACEOF diff --git a/gcc/configure.ac b/gcc/configure.ac index 77fab885a428..1407c86e355b 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -3612,12 +3612,14 @@ gcc_GAS_CHECK_FEATURE(section merging support, gcc_cv_as_shf_merge, [--fatal-warnings], [.section .rodata.str, "aMS", @progbits, 1]) if test $gcc_cv_as_shf_merge = no; then - gcc_GAS_CHECK_FEATURE(section merging support, gcc_cv_as_shf_merge, + gcc_GAS_CHECK_FEATURE(section merging support (%progbits), + gcc_cv_as_shf_merge_percent, [--fatal-warnings], [.section .rodata.str, "aMS", %progbits, 1]) fi AC_DEFINE_UNQUOTED(HAVE_GAS_SHF_MERGE, - [`if test $gcc_cv_as_shf_merge = yes; then echo 1; else echo 0; fi`], + [`if test $gcc_cv_as_shf_merge = yes \ + || test $gcc_cv_as_shf_merge_percent = yes; then echo 1; else echo 0; fi`], [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.]) gcc_GAS_CHECK_FEATURE([COMDAT group support (GNU as)],
In 32-bit Arm assembly, the @ character is the start of a comment so the section type needs to use the % character instead. configure.ac attempts to account for this difference by doing a second try when checking the assembler for section merging support. Unfortunately there is a bug: because the gcc_GAS_CHECK_FEATURE macro has a call to AC_CACHE_CHECK, it will actually skip the second try because the gcc_cv_as_shf_merge variable has already been set: checking assembler for section merging support... no checking assembler for section merging support... (cached) no Fix by using a separate variable for the second try, as is done in the check for COMDAT group support. This problem was noticed because the recent binutils commit d5cbf916be4a ("gas/ELF: also reject merge entity size being zero") caused gas to be stricter about mergeable sections without an entity size: configure:27013: checking assembler for section merging support configure:27022: /path/to/as --fatal-warnings -o conftest.o conftest.s >&5 conftest.s: Assembler messages: conftest.s:1: Warning: invalid merge / string entity size conftest.s: Error: 1 warning, treating warnings as errors configure:27025: $? = 1 configure: failed program was .section .rodata.str, "aMS", @progbits, 1 configure:27036: result: no In previous versions of gas the conftest.s program above was accepted and configure detected support for section merging. See also: https://linaro.atlassian.net/browse/GNU-1427 https://sourceware.org/bugzilla/show_bug.cgi?id=32491 Tested on armv8l-linux-gnueabihf. gcc/ChangeLog: * configure.ac: Fix check for HAVE_GAS_SHF_MERGE on Arm targets. * configure: Regenerate. --- gcc/configure | 17 +++++++++-------- gcc/configure.ac | 6 ++++-- 2 files changed, 13 insertions(+), 10 deletions(-) base-commit: d09628742bb17719360ff447a8e604f5c6801bdf