Skip to contentSkip to conversation

toolchain-external: Fix EABIhf check

PATCH Superseded Thomas De Schampheleire 2 participants 4 messages May 6, 2014 – May 9, 2014

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 toolchain-external: Fix EABIhf check

From: Stefan S?rensen <[email protected]>

Currently the check for EABI/EABIhf toolchains looks for the Tag_ABI_VFP_args attribute in the crt1.o file which gcc adds in an EABIhf toolchain.
In uClibc, however, crt1.o is not compiled from c but assembly, so the Tag_ABI_VFP_args attribute is not added in the object file. This causes the EABIhf check in the external toolchain logic to fail for uClibc-based toolchains.

Fix by compiling a dummy .c file and trying to link the object against the C library. Since it is impossible to mix EABI and EABIhf code, a mismatch between the buildroot and toolchain ABI settings will be detected during this link step.

Fixes bug #6842: https://bugs.busybox.net/show_bug.cgi?id=6842

[ThomasDS: do full link iso readelf test, update commit message]

---
v3: (ThomasDS)
- v2 properly recognized a uClibc EABIhf toolchain, but failed to detect a mismatch with buildroot settings (detected by ThomasP). Fix by performing an actual link step to detect any incompatibility.
v2: (ThomasDS)
- avoid creating a temporary source file
- update commit message

 toolchain/helpers.mk |  18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)
toolchain/helpers.mk +6 -12
@@ -297,20 +297,14 @@ check_arm_abi = \
 		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" ; \
+	TEMP_OBJ_FILE=`mktemp --tmpdir --suffix=.o`; \
+	if ! echo 'int main(void) {}' | $${__CROSS_CC} -x c -o $${TEMP_OBJ_FILE} /dev/null - 2>/dev/null; then \
+		abistr_$(BR2_ARM_EABI)='EABI'; \
+		abistr_$(BR2_ARM_EABIHF)='EABIhf'; \
+		echo "Incorrect ABI setting: $${abistr_y} selected, but toolchain is incompatible"; \
 		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
+	rm -f $${TEMP_OBJ_FILE}
 
 #
 # Check that the external toolchain supports C++
#1 lore raw

PATCH v3 1/1 toolchain-external: Fix EABIhf check

Quoted 40 lines · Thomas De Schampheleire

From: Stefan S?rensen <[email protected]>

Currently the check for EABI/EABIhf toolchains looks for the Tag_ABI_VFP_args attribute in the crt1.o file which gcc adds in an EABIhf toolchain.
In uClibc, however, crt1.o is not compiled from c but assembly, so the Tag_ABI_VFP_args attribute is not added in the object file. This causes the EABIhf check in the external toolchain logic to fail for uClibc-based toolchains.

Fix by compiling a dummy .c file and trying to link the object against the C library. Since it is impossible to mix EABI and EABIhf code, a mismatch between the buildroot and toolchain ABI settings will be detected during this link step.

Fixes bug #6842: https://bugs.busybox.net/show_bug.cgi?id=6842

[ThomasDS: do full link iso readelf test, update commit message]

---
v3: (ThomasDS)
- v2 properly recognized a uClibc EABIhf toolchain, but failed to detect a mismatch with buildroot settings (detected by ThomasP). Fix by performing an actual link step to detect any incompatibility.
v2: (ThomasDS)
- avoid creating a temporary source file
- update commit message

 toolchain/helpers.mk |  18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)
toolchain/helpers.mk +0 -0
@@ -297,20 +297,14 @@ check_arm_abi = \
 		echo "External toolchain uses the unsuported OABI" ; \
 		exit 1 ; \
 	fi ; \

toolchain/helpers.mk check_arm_abi = \ · line 301

Quoted 10 lines
toolchain/helpers.mk +2 -8
-	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" ; \
+	TEMP_OBJ_FILE=`mktemp --tmpdir --suffix=.o`; \
+	if ! echo 'int main(void) {}' | $${__CROSS_CC} -x c -o $${TEMP_OBJ_FILE} /dev/null - 2>/dev/null; then \

You can write directly to /dev/null instead of going through TEMP_OBJ_FILE, no?

toolchain/helpers.mk check_arm_abi = \ · line 304

Quoted 3 lines
toolchain/helpers.mk +3 -0
+		abistr_$(BR2_ARM_EABI)='EABI'; \
+		abistr_$(BR2_ARM_EABIHF)='EABIhf'; \
+		echo "Incorrect ABI setting: $${abistr_y} selected, but toolchain is incompatible"; \

Nice approach! But to keep the original output, you could use:

$${abistr_y} selected, but toolchain uses $${abistr_}

Regards,
Arnout

Quoted 15 lines
toolchain/helpers.mk +1 -4
 		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
+	rm -f $${TEMP_OBJ_FILE}
 
 #
 # Check that the external toolchain supports C++

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#2 lore raw

PATCH v3 1/1 toolchain-external: Fix EABIhf check

Hi Arnout,

Quoted 45 lines · Arnout Vandecappelle
Quoted 44 lines · Thomas De Schampheleire

From: Stefan S?rensen <[email protected]>

Currently the check for EABI/EABIhf toolchains looks for the Tag_ABI_VFP_args attribute in the crt1.o file which gcc adds in an EABIhf toolchain.
In uClibc, however, crt1.o is not compiled from c but assembly, so the Tag_ABI_VFP_args attribute is not added in the object file. This causes the EABIhf check in the external toolchain logic to fail for uClibc-based toolchains.

Fix by compiling a dummy .c file and trying to link the object against the C library. Since it is impossible to mix EABI and EABIhf code, a mismatch between the buildroot and toolchain ABI settings will be detected during this link step.

Fixes bug #6842: https://bugs.busybox.net/show_bug.cgi?id=6842

[ThomasDS: do full link iso readelf test, update commit message]

---
v3: (ThomasDS)
- v2 properly recognized a uClibc EABIhf toolchain, but failed to detect a mismatch with buildroot settings (detected by ThomasP). Fix by performing an actual link step to detect any incompatibility.
v2: (ThomasDS)
- avoid creating a temporary source file
- update commit message

 toolchain/helpers.mk |  18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)
toolchain/helpers.mk +0 -4
@@ -297,20 +297,14 @@ check_arm_abi = \
              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 \
Quoted 8 lines

toolchain/helpers.mk check_arm_abi = \ · line 301

Quoted 6 lines
toolchain/helpers.mk +2 -4
-             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" ; \
+     TEMP_OBJ_FILE=`mktemp --tmpdir --suffix=.o`; \
+     if ! echo 'int main(void) {}' | $${__CROSS_CC} -x c -o $${TEMP_OBJ_FILE} /dev/null - 2>/dev/null; then \

You can write directly to /dev/null instead of going through TEMP_OBJ_FILE, no?

Ah yes. When we were still using the readelf approach I tried avoiding the temporary file by using 'gcc -o -' so that I could pipe it into readelf, but gcc did not accept that.
When changing approach I did not realize I could remove it, thanks.

I also noticed now that there is an extra /dev/null on this line as input file, which is not needed.

Quoted 9 lines

toolchain/helpers.mk check_arm_abi = \ · line 304

Quoted 4 lines
toolchain/helpers.mk +3 -0
+             abistr_$(BR2_ARM_EABI)='EABI'; \
+             abistr_$(BR2_ARM_EABIHF)='EABIhf'; \
+             echo "Incorrect ABI setting: $${abistr_y} selected, but toolchain is incompatible"; \

Nice approach! But to keep the original output, you could use:

$${abistr_y} selected, but toolchain uses $${abistr_}

This did cross my mind, but it assumes that there are only two
possibilities: EABI and EABIhf. Suppose that a new EABI pops up. Or suppose the compilation/link step fails for a completely different reason. Then we have told the user that their toolchain is EABI or EABIhf while it is not true.
So here I choice correctness over completeness, but if you prefer I can change it. Do let me know if you still think we should add $${abistr_} to the message.

Thanks,
Thomas

#3 lore raw

PATCH v3 1/1 toolchain-external: Fix EABIhf check

Quoted 4 lines · Thomas De Schampheleire

Hi Arnout,

Quoted 1 line · Arnout Vandecappelle

On 06/05/14 13:25, Thomas De Schampheleire wrote:

[snip]

Quoted 10 lines
Quoted 9 lines

toolchain/helpers.mk check_arm_abi = \ · line 304

Quoted 4 lines
toolchain/helpers.mk +3 -0
+             abistr_$(BR2_ARM_EABI)='EABI'; \
+             abistr_$(BR2_ARM_EABIHF)='EABIhf'; \
+             echo "Incorrect ABI setting: $${abistr_y} selected, but toolchain is incompatible"; \

Nice approach! But to keep the original output, you could use:

$${abistr_y} selected, but toolchain uses $${abistr_}

Quoted 8 lines

This did cross my mind, but it assumes that there are only two
possibilities: EABI and EABIhf. Suppose that a new EABI pops up. Or suppose the compilation/link step fails for a completely different reason. Then we have told the user that their toolchain is EABI or EABIhf while it is not true.
So here I choice correctness over completeness, but if you prefer I can change it. Do let me know if you still think we should add $${abistr_} to the message.

Good point, let's say "incompatible" then.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F