Skip to contentSkip to conversation

EABI vs EABIhf check

PATCH Superseded Thomas Petazzoni 2 participants 9 messages Jul 17, 2013

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Thomas Petazzoni

author
#0 lore raw

v2 cover EABI vs EABIhf check

Hello,

Second version of the EABI vs. EABIhf check, using a different technique, as discussed with Yann E. Morin, and also additional cleanups.

I've also included a somewhat unrelated fix to the arch/Config.in.arm file.

Thomas

Thomas Petazzoni (4):
toolchain/helpers: don't use the x$(...) = x"value" syntax
toolchain: check ARM EABI vs. EABIhf for external toolchains toolchain/toolchain-external: don't use x$(...) construct or == arch/arm: update VFPv2 comment to mention ARMv5

 arch/Config.in.arm                       |  4 ++--
 toolchain/helpers.mk                     | 21 ++++++++++++++++++---
 toolchain/toolchain-external/ext-tool.mk | 13 ++++++++-----
 3 files changed, 28 insertions(+), 10 deletions(-)
Signature · 1 line
1.8.1.2
author
#1 lore raw

PATCH v2 1/4 toolchain/helpers: don't use the x$(...) = x"value" syntax

As noted by Yann E. Morin, the x$(...) = x"value" syntax is old and ugly, and the easier to read "$(...)" = "value" can now be used without problems.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 toolchain/helpers.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
toolchain/helpers.mk +3 -3
index 95217e7..94fe015 100644
@@ -181,7 +181,7 @@ create_lib64_symlinks = \
 # $2: feature description
 #
 check_glibc_feature = \
-	if [ x$($(1)) != x"y" ] ; then \
+	if [ "$($(1))" != "y" ] ; then \
 		echo "$(2) available in C library, please enable $(1)" ; \
 		exit 1 ; \
 	fi

@@ -236,11 +236,11 @@ check_glibc = \
 #
 check_uclibc_feature = \
 	IS_IN_LIBC=`grep -q "\#define $(1) 1" $(3) && echo y` ; \
-	if [ x$($(2)) != x"y" -a x$${IS_IN_LIBC} = x"y" ] ; then \
+	if [ "$($(2))" != "y" -a $${IS_IN_LIBC} = "y" ] ; then \
 		echo "$(4) available in C library, please enable $(2)" ; \
 		exit 1 ; \
 	fi ; \
-	if [ x$($(2)) = x"y" -a x$${IS_IN_LIBC} != x"y" ] ; then \
+	if [ "$($(2))" = "y" -a $${IS_IN_LIBC} != "y" ] ; then \
 		echo "$(4) not available in C library, please disable $(2)" ; \
 		exit 1 ; \
 	fi
Signature · 1 line
1.8.1.2
#2 lore raw

PATCH v2 1/4 toolchain/helpers: don't use the x$(...) = x"value" syntax

Thomas, All,

On 2013-07-17 10:13 +0200, Thomas Petazzoni spake thusly:

Quoted 23 lines

As noted by Yann E. Morin, the x$(...) = x"value" syntax is old and ugly, and the easier to read "$(...)" = "value" can now be used without problems.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 toolchain/helpers.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
toolchain/helpers.mk +1 -1
index 95217e7..94fe015 100644
@@ -181,7 +181,7 @@ create_lib64_symlinks = \
 # $2: feature description
 #
 check_glibc_feature = \
-	if [ x$($(1)) != x"y" ] ; then \
+	if [ "$($(1))" != "y" ] ; then \
 		echo "$(2) available in C library, please enable $(1)" ; \
 		exit 1 ; \
 	fi

toolchain/helpers.mk check_glibc = \ · line 239

Quoted 5 lines
toolchain/helpers.mk +1 -1
@@ -236,11 +236,11 @@ check_glibc = \
 #
 check_uclibc_feature = \
 	IS_IN_LIBC=`grep -q "\#define $(1) 1" $(3) && echo y` ; \
-	if [ x$($(2)) != x"y" -a x$${IS_IN_LIBC} = x"y" ] ; then \
+	if [ "$($(2))" != "y" -a $${IS_IN_LIBC} = "y" ] ; then \

				 ^^^^^^^^^^^^^^

You'd have to quote this, too, since it can be the empty string.

toolchain/helpers.mk check_glibc = \ · line 243

Quoted 5 lines
toolchain/helpers.mk +1 -1
 		echo "$(4) available in C library, please enable $(2)" ; \
 		exit 1 ; \
 	fi ; \
-	if [ x$($(2)) = x"y" -a x$${IS_IN_LIBC} != x"y" ] ; then \
+	if [ "$($(2))" = "y" -a $${IS_IN_LIBC} != "y" ] ; then \

				^^^^^^^^^^^^^^

Ditto.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
author
#3 lore raw

PATCH v2 2/4 toolchain: check ARM EABI vs. EABIhf for external toolchains

In reply to Thomas Petazzoni

Following the introduction of the support of EABIhf as a second ARM ABI, it is important to check whether the external toolchain provided by the user actually uses the ABI that has been selected in the Buildroot configuration. This commit introduces such a check by looking at the 'Tag_ABI_VFP_args' tag of the architecture-specific section of the ELF headers. This assumes that ELF is the binary format used on ARM, which may not be the case on ARM noMMU systems (they use the FLAT binary format), but Buildroot doesn't have support for such systems at the moment.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 toolchain/helpers.mk                     | 15 +++++++++++++++
 toolchain/toolchain-external/ext-tool.mk |  5 ++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
toolchain/helpers.mk +15 -0
index 94fe015..a8dab90 100644
@@ -279,10 +279,25 @@ check_uclibc = \
 #
 check_arm_abi = \
 	__CROSS_CC=$(strip $1) ; \
+	__CROSS_READELF=$(strip $2) ; \
 	EXT_TOOLCHAIN_TARGET=`LANG=C $${__CROSS_CC} -v 2>&1 | grep ^Target | cut -f2 -d ' '` ; \
 	if ! echo $${EXT_TOOLCHAIN_TARGET} | grep -qE 'eabi(hf)?$$' ; then \
 		echo "External toolchain uses the unsuported OABI" ; \
 		exit 1 ; \
+	fi ; \
+	EXT_TOOLCHAIN_CRT1=`LANG=C $${__CROSS_CC} -print-file-name=crt1.o` ; \
+	if $${__CROSS_READELF} -A $${EXT_TOOLCHAIN_CRT1} | grep -q "Tag_ABI_VFP_args:" ; then \
+		EXT_TOOLCHAIN_ABI="eabihf" ; \
+	else \
+		EXT_TOOLCHAIN_ABI="eabi" ; \
+	fi ; \
+	if [ "$(BR2_ARM_EABI)" = "y" -a $${EXT_TOOLCHAIN_ABI} = "eabihf" ] ; then \
+		echo "Incorrect ABI setting: EABI selected, but toolchain uses EABIhf" ; \
+		exit 1 ; \
+	fi ; \
+	if [ "$(BR2_ARM_EABIHF)" = "y" -a $${EXT_TOOLCHAIN_ABI} = "eabi" ] ; then \
+		echo "Incorrect ABI setting: EABIhf selected, but toolchain uses EABI" ; \
+		exit 1 ; \
 	fi
 
 #
toolchain/toolchain-external/ext-tool.mk +4 -1
index b9ae68f..80e03b9 100644
@@ -125,6 +125,7 @@ endif
 TOOLCHAIN_EXTERNAL_CROSS=$(TOOLCHAIN_EXTERNAL_BIN)/$(TOOLCHAIN_EXTERNAL_PREFIX)-
 TOOLCHAIN_EXTERNAL_CC=$(TOOLCHAIN_EXTERNAL_CROSS)gcc
 TOOLCHAIN_EXTERNAL_CXX=$(TOOLCHAIN_EXTERNAL_CROSS)g++
+TOOLCHAIN_EXTERNAL_READELF=$(TOOLCHAIN_EXTERNAL_CROSS)readelf
 TOOLCHAIN_EXTERNAL_WRAPPER_ARGS = -DBR_SYSROOT='"$(STAGING_SUBDIR)"'
 
 ifeq ($(filter $(HOST_DIR)/%,$(TOOLCHAIN_EXTERNAL_BIN)),)

@@ -368,7 +369,9 @@ $(STAMP_DIR)/ext-toolchain-checked: $(TOOLCHAIN_EXTERNAL_DEPENDENCIES)
 		exit 1 ; \
 	fi ; \
 	if test x$(BR2_arm) == x"y" ; then \
-		$(call check_arm_abi,$(TOOLCHAIN_EXTERNAL_CC)) ; \
+		$(call check_arm_abi,\
+			"$(TOOLCHAIN_EXTERNAL_CC) $(TOOLCHAIN_EXTERNAL_CFLAGS)",\
+			$(TOOLCHAIN_EXTERNAL_READELF)) ; \
 	fi ; \
 	if test x$(BR2_INSTALL_LIBSTDCPP) == x"y" ; then \
 		$(call check_cplusplus,$(TOOLCHAIN_EXTERNAL_CXX)) ; \
Signature · 1 line
1.8.1.2
#4 lore raw

PATCH v2 2/4 toolchain: check ARM EABI vs. EABIhf for external toolchains

Thomas, All,

On 2013-07-17 10:13 +0200, Thomas Petazzoni spake thusly:

Quoted 27 lines

Following the introduction of the support of EABIhf as a second ARM ABI, it is important to check whether the external toolchain provided by the user actually uses the ABI that has been selected in the Buildroot configuration. This commit introduces such a check by looking at the 'Tag_ABI_VFP_args' tag of the architecture-specific section of the ELF headers. This assumes that ELF is the binary format used on ARM, which may not be the case on ARM noMMU systems (they use the FLAT binary format), but Buildroot doesn't have support for such systems at the moment.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 toolchain/helpers.mk                     | 15 +++++++++++++++
 toolchain/toolchain-external/ext-tool.mk |  5 ++++-
 2 files changed, 19 insertions(+), 1 deletion(-)
toolchain/helpers.mk +1 -0
index 94fe015..a8dab90 100644
@@ -279,10 +279,25 @@ check_uclibc = \
 #
 check_arm_abi = \
 	__CROSS_CC=$(strip $1) ; \
+	__CROSS_READELF=$(strip $2) ; \
 	EXT_TOOLCHAIN_TARGET=`LANG=C $${__CROSS_CC} -v 2>&1 | grep ^Target | cut -f2 -d ' '` ; \
 	if ! echo $${EXT_TOOLCHAIN_TARGET} | grep -qE 'eabi(hf)?$$' ; then \

toolchain/helpers.mk check_uclibc = \ · line 294

Quoted 10 lines
toolchain/helpers.mk +8 -0
 		echo "External toolchain uses the unsuported OABI" ; \
 		exit 1 ; \
+	fi ; \
+	EXT_TOOLCHAIN_CRT1=`LANG=C $${__CROSS_CC} -print-file-name=crt1.o` ; \
+	if $${__CROSS_READELF} -A $${EXT_TOOLCHAIN_CRT1} | grep -q "Tag_ABI_VFP_args:" ; then \
+		EXT_TOOLCHAIN_ABI="eabihf" ; \
+	else \
+		EXT_TOOLCHAIN_ABI="eabi" ; \
+	fi ; \
+	if [ "$(BR2_ARM_EABI)" = "y" -a $${EXT_TOOLCHAIN_ABI} = "eabihf" ] ; then \

	                                ^^^^^^^^^^^^^^^^^^^^^

Obviously, EXT_TOOLCHAIN_ABI can't be empty, but for consistecy, it would be better to quote it, too.

toolchain/helpers.mk check_uclibc = \ · line 298

Quoted 4 lines
toolchain/helpers.mk +4 -0
+		echo "Incorrect ABI setting: EABI selected, but toolchain uses EABIhf" ; \
+		exit 1 ; \
+	fi ; \
+	if [ "$(BR2_ARM_EABIHF)" = "y" -a $${EXT_TOOLCHAIN_ABI} = "eabi" ] ; then \

	                                  ^^^^^^^^^^^^^^^^^^^^^

Ditto.

Otherwise:

Acked-by "Yann E. MORIN" <[email protected]>

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
author
#5 lore raw

PATCH v2 3/4 toolchain/toolchain-external: don't use x$(...) construct or ==

In reply to Thomas Petazzoni

With modern shells, we can simply do test using the "$(...)" = "value" form. This commit gets rid of the x$(...) = x"value" constructs and replaces == by =, which is the correct operator to test the equality of two strings with the test program.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 toolchain/toolchain-external/ext-tool.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
toolchain/toolchain-external/ext-tool.mk +4 -4
index 80e03b9..8aac50e 100644
@@ -368,15 +368,15 @@ $(STAMP_DIR)/ext-toolchain-checked: $(TOOLCHAIN_EXTERNAL_DEPENDENCIES)
 		@echo "External toolchain doesn't support --sysroot. Cannot use." ; \
 		exit 1 ; \
 	fi ; \
-	if test x$(BR2_arm) == x"y" ; then \
+	if test "$(BR2_arm)" = "y" ; then \
 		$(call check_arm_abi,\
 			"$(TOOLCHAIN_EXTERNAL_CC) $(TOOLCHAIN_EXTERNAL_CFLAGS)",\
 			$(TOOLCHAIN_EXTERNAL_READELF)) ; \
 	fi ; \
-	if test x$(BR2_INSTALL_LIBSTDCPP) == x"y" ; then \
+	if test "$(BR2_INSTALL_LIBSTDCPP)" = "y" ; then \
 		$(call check_cplusplus,$(TOOLCHAIN_EXTERNAL_CXX)) ; \
 	fi ; \
-	if test x$(BR2_TOOLCHAIN_EXTERNAL_UCLIBC) == x"y" ; then \
+	if test "$(BR2_TOOLCHAIN_EXTERNAL_UCLIBC)" = "y" ; then \
 		$(call check_uclibc,$${SYSROOT_DIR}) ; \
 	else \
 		$(call check_glibc,$${SYSROOT_DIR}) ; \

@@ -464,7 +464,7 @@ $(STAMP_DIR)/ext-toolchain-installed: $(STAMP_DIR)/ext-toolchain-checked
 	if [ -L $${ARCH_SYSROOT_DIR}/lib64 -o -d $${ARCH_SYSROOT_DIR}/lib64 ] ; then \
 		$(call create_lib64_symlinks) ; \
 	fi ; \
-	if test x"$(BR2_TOOLCHAIN_EXTERNAL_GDB_SERVER_COPY)" == x"y"; then \
+	if test "$(BR2_TOOLCHAIN_EXTERNAL_GDB_SERVER_COPY)" = "y"; then \
 		$(call MESSAGE,"Copying gdbserver") ; \
 		gdbserver_found=0 ; \
 		for d in $${ARCH_SYSROOT_DIR}/usr $${ARCH_SYSROOT_DIR}/../debug-root/usr $${ARCH_SYSROOT_DIR}/usr/$${ARCH_LIB_DIR} ; do \
Signature · 1 line
1.8.1.2
#6 lore raw

PATCH v2 3/4 toolchain/toolchain-external: don't use x$(...) construct or ==

Thomas, All,

On 2013-07-17 10:13 +0200, Thomas Petazzoni spake thusly:

Quoted 6 lines

With modern shells, we can simply do test using the "$(...)" = "value" form. This commit gets rid of the x$(...) = x"value" constructs and replaces == by =, which is the correct operator to test the equality of two strings with the test program.

Signed-off-by Thomas Petazzoni <[email protected]>
Acked-by "Yann E. MORIN" <[email protected]>

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
author
#7 lore raw

PATCH v2 4/4 arch/arm: update VFPv2 comment to mention ARMv5

In reply to Thomas Petazzoni

Commit 6b3a0417c4 ('arch/arm: arm926 may have VFP') forgot to update the help text of the VFPv2 option to mention ARMv5. This commit fixes that.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 arch/Config.in.arm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/Config.in.arm +2 -2
index b2fe257..096fc48 100644
@@ -193,8 +193,8 @@ config BR2_ARM_FPU_VFPV2
 	depends on BR2_ARM_CPU_HAS_VFPV2 || BR2_ARM_CPU_MAYBE_HAS_VFPV2
 	help
 	  This option allows to use the VFPv2 floating point unit, as
-	  available in some ARMv6 processors (ARM1136JF-S,
-	  ARM1176JZF-S and ARM11 MPCore).
+	  available in some ARMv5 processors (ARM926EJ-S) ARMv6
+	  processors (ARM1136JF-S, ARM1176JZF-S and ARM11 MPCore).
 
 	  Note that this option is also safe to use for newer cores
 	  such as Cortex-A, because the VFPv3 and VFPv4 units are
Signature · 1 line
1.8.1.2
#8 lore raw

PATCH v2 4/4 arch/arm: update VFPv2 comment to mention ARMv5

Thomas, All,

On 2013-07-17 10:13 +0200, Thomas Petazzoni spake thusly:

Quoted 14 lines

Commit 6b3a0417c4 ('arch/arm: arm926 may have VFP') forgot to update the help text of the VFPv2 option to mention ARMv5. This commit fixes that.

Signed-off-by Thomas Petazzoni <[email protected]>
---
 arch/Config.in.arm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/Config.in.arm +0 -0
index b2fe257..096fc48 100644

arch/Config.in.arm config BR2_ARM_FPU_VFPV2 · line 196

Quoted 6 lines
arch/Config.in.arm +1 -2
@@ -193,8 +193,8 @@ config BR2_ARM_FPU_VFPV2
 	depends on BR2_ARM_CPU_HAS_VFPV2 || BR2_ARM_CPU_MAYBE_HAS_VFPV2
 	help
 	  This option allows to use the VFPv2 floating point unit, as
-	  available in some ARMv6 processors (ARM1136JF-S,
-	  ARM1176JZF-S and ARM11 MPCore).
+	  available in some ARMv5 processors (ARM926EJ-S) ARMv6

                                                        ^^^
Weird sentence construction. What about:
    [...] in some ARMv5 processors (ARM926EJ-S), and in some ARMv6 [...]
                                               ^^^^^^^^^^^^^

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'