Skip to contentSkip to conversation

icu: Delete inapropriate flag at link

PATCH New Maxime Hadjinlian 3 participants 9 messages Jun 19, 2014 – Jun 29, 2014

lore
Share & tools
lore

Maxime Hadjinlian

author
#0 lore raw

PATCH 1/2 icu: Delete inapropriate flag at link

From: Maxime Hadjinlian <[email protected]>

Theses flags may cause libicudata to not be loaded on ARM EABIHF because it would be built for ARM EABI only.

Signed-off-by Maxime Hadjinlian <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 .../icu/icu-004-link-icudata-as-data-only.patch    | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 package/icu/icu-004-link-icudata-as-data-only.patch
package/icu/icu-004-link-icudata-as-data-only.patch new file +32 -0
new file mode 100644
index 0000000..b69cec1
@@ -0,0 +1,32 @@
+From d5d0c4bb7cc9aa4a132ec0bea13255aee50c1cf9 Mon Sep 17 00:00:00 2001
+From: Maxime Hadjinlian <[email protected]>
+Date: Fri, 6 Jun 2014 14:55:58 +0200
+Subject: [PATCH] link icudata as data only
+
+This patch came straight from Debian.
+It fix a trouble when libicudata would not respect some flags and would
+for example end up being built for ARM EABI instead of ARM EABIHF
+
+Signed-off-by: Maxime Hadjinlian <[email protected]>
+---
+ source/config/mh-linux | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/source/config/mh-linux b/source/config/mh-linux
+index 531a3b2..5a2a7c4 100644
+--- a/source/config/mh-linux
++++ b/source/config/mh-linux
+@@ -21,7 +21,9 @@ LD_RPATH= -Wl,-zorigin,-rpath,'$$'ORIGIN
+ LD_RPATH_PRE = -Wl,-rpath,
+
+ ## These are the library specific LDFLAGS
+-LDFLAGSICUDT=-nodefaultlibs -nostdlib
++#LDFLAGSICUDT=-nodefaultlibs -nostdlib
++# Debian change: linking icudata as data only causes too many problems.
++LDFLAGSICUDT=
+
+ ## Compiler switch to embed a library name
+ # The initial tab in the next line is to prevent icu-config from reading it.
+--
+2.0.0.rc2
+

--
2.0.0

author
#1 lore raw

PATCH 2/2 devmem2: fix output when _FILE_OFFSET_BITS=64

From: Colin Didier <[email protected]>

When built with _FILE_OFFSET_BITS=64, off_t becomes an long long int and was printed as an unsigned int which lead to a garbled output.

Signed-off-by Colin Didier <[email protected]>
Signed-off-by Maxime Hadjinlian <[email protected]>
---
 .../devmem2-0001-be-coherent-in-type-usage.patch   | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 package/devmem2/devmem2-0001-be-coherent-in-type-usage.patch
package/devmem2/devmem2-0001-be-coherent-in-type-usage.patch new file +49 -0
new file mode 100644
index 0000000..14c99a2
@@ -0,0 +1,49 @@
+From 15eae3dbb3d6824a0fd478e2ce0fe879496bb72c Mon Sep 17 00:00:00 2001
+From: Colin Didier <[email protected]>
+Date: Thu, 19 Jun 2014 16:58:19 +0200
+Subject: [PATCH] be coherent in type usage
+
+- no need to use off_t because it is read from strtoul()
+- print unsigned int type correctly
+
+When built with _FILE_OFFSET_BITS=64, off_t becomes an long long int
+and was printed as an unsigned int which lead to a garbled output.
+
+Signed-off-by: Colin Didier <[email protected]>
+---
+ devmem2.c | 7 +++----
+ 1 file changed, 3 insertions(+), 4 deletions(-)
+
+diff --git a/devmem2.c b/devmem2.c
+index 7732ecb..1ae06ef 100644
+--- a/devmem2.c
++++ b/devmem2.c
+@@ -58,8 +58,7 @@
+ int main(int argc, char **argv) {
+     int fd;
+     void *map_base, *virt_addr;
+-	unsigned long read_result, writeval;
+-	off_t target;
++	unsigned long target, read_result, writeval;
+ 	int access_type = 'w';
+
+ 	if(argc < 2) {
+@@ -101,7 +100,7 @@ int main(int argc, char **argv) {
+ 			fprintf(stderr, "Illegal data type '%c'.\n", access_type);
+ 			exit(2);
+ 	}
+-    printf("Value at address 0x%X (%p): 0x%X\n", target, virt_addr, read_result);
++    printf("Value@address 0x%lX (%p): 0x%lX\n", target, virt_addr, read_result);
+     fflush(stdout);
+
+ 	if(argc > 3) {
+@@ -120,7 +119,7 @@ int main(int argc, char **argv) {
+ 				read_result = *((unsigned long *) virt_addr);
+ 				break;
+ 		}
+-		printf("Written 0x%X; readback 0x%X\n", writeval, read_result);
++		printf("Written 0x%lX; readback 0x%lX\n", writeval, read_result);
+ 		fflush(stdout);
+ 	}
+
+--

+2.0.0
+
--
2.0.0

Applied
#2 lore raw

PATCH 2/2 devmem2: fix output when _FILE_OFFSET_BITS=64

Dear Maxime Hadjinlian,

Quoted 7 lines · Maxime Hadjinlian

From: Colin Didier <[email protected]>

When built with _FILE_OFFSET_BITS=64, off_t becomes an long long int and was printed as an unsigned int which lead to a garbled output.

Signed-off-by Colin Didier <[email protected]>
Signed-off-by Maxime Hadjinlian <[email protected]>

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH 2/2 devmem2: fix output when _FILE_OFFSET_BITS=64

Hello,

Quoted 5 lines · Thomas Petazzoni

Dear Maxime Hadjinlian,

Quoted 2 lines · Maxime Hadjinlian

From: Colin Didier <[email protected]>

Quoted 7 lines
Quoted 5 lines

When built with _FILE_OFFSET_BITS=64, off_t becomes an long long int and was printed as an unsigned int which lead to a garbled output.

Signed-off-by Colin Didier <[email protected]>
Signed-off-by Maxime Hadjinlian <[email protected]>

Applied, thanks.

And the patch you provided against devmem did not apply... see
http://autobuild.buildroot.org/results/c45/c45eb706fb35ffc87fede777c3d20479a90872ae/build-end.log.
Looks like some whitespace issue, I've fixed that up, but it seems like something is screwed in your setup to format or send patches.

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH 1/2 icu: Delete inapropriate flag at link

In reply to Maxime Hadjinlian
Quoted 18 lines · Maxime Hadjinlian

From: Maxime Hadjinlian <[email protected]>

Theses flags may cause libicudata to not be loaded on ARM EABIHF because it would be built for ARM EABI only.

Signed-off-by Maxime Hadjinlian <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 .../icu/icu-004-link-icudata-as-data-only.patch    | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 package/icu/icu-004-link-icudata-as-data-only.patch
package/icu/icu-004-link-icudata-as-data-only.patch new file +0 -0
new file mode 100644
index 0000000..b69cec1

package/icu/icu-004-link-icudata-as-data-only.patch line 4

Quoted 4 lines
package/icu/icu-004-link-icudata-as-data-only.patch new file +4 -0
@@ -0,0 +1,32 @@
+From d5d0c4bb7cc9aa4a132ec0bea13255aee50c1cf9 Mon Sep 17 00:00:00 2001
+From: Maxime Hadjinlian <[email protected]>
+Date: Fri, 6 Jun 2014 14:55:58 +0200
+Subject: [PATCH] link icudata as data only

I guess this should be "Don't link icudata as data only"

package/icu/icu-004-link-icudata-as-data-only.patch line 8

Quoted 4 lines
package/icu/icu-004-link-icudata-as-data-only.patch new file +4 -0
+
+This patch came straight from Debian.
+It fix a trouble when libicudata would not respect some flags and would
+for example end up being built for ARM EABI instead of ARM EABIHF

I don't see how -nostdlib -nodefaultlibs could have that effect - especially in
buildroot, where we always force the abi. Do you have an example configuration where this happens?

That said, I also don't see a reason why you would want the -nostdlib
-nodefaultlibs because in the end it will be linked with those libraries anyway...

Regards,
Arnout

Quoted 33 lines
package/icu/icu-004-link-icudata-as-data-only.patch new file +24 -1
+
+Signed-off-by: Maxime Hadjinlian <[email protected]>
+---
+ source/config/mh-linux | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/source/config/mh-linux b/source/config/mh-linux
+index 531a3b2..5a2a7c4 100644
+--- a/source/config/mh-linux
++++ b/source/config/mh-linux
+@@ -21,7 +21,9 @@ LD_RPATH= -Wl,-zorigin,-rpath,'$$'ORIGIN
+ LD_RPATH_PRE = -Wl,-rpath,
+
+ ## These are the library specific LDFLAGS
+-LDFLAGSICUDT=-nodefaultlibs -nostdlib
++#LDFLAGSICUDT=-nodefaultlibs -nostdlib
++# Debian change: linking icudata as data only causes too many problems.
++LDFLAGSICUDT=
+
+ ## Compiler switch to embed a library name
+ # The initial tab in the next line is to prevent icu-config from reading it.
+--
+2.0.0.rc2
+
--

2.0.0

_______________________________________________
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
author
#5 lore raw

PATCH 1/2 icu: Delete inapropriate flag at link

Quoted 13 lines · Arnout Vandecappelle
Quoted 12 lines · Maxime Hadjinlian

From: Maxime Hadjinlian <[email protected]>

Theses flags may cause libicudata to not be loaded on ARM EABIHF because it would be built for ARM EABI only.

Signed-off-by Maxime Hadjinlian <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 .../icu/icu-004-link-icudata-as-data-only.patch    | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 package/icu/icu-004-link-icudata-as-data-only.patch
Quoted 12 lines

package/icu/icu-004-link-icudata-as-data-only.patch line 4

Quoted 10 lines
package/icu/icu-004-link-icudata-as-data-only.patch new file +4 -0
new file mode 100644
index 0000000..b69cec1
@@ -0,0 +1,32 @@
+From d5d0c4bb7cc9aa4a132ec0bea13255aee50c1cf9 Mon Sep 17 00:00:00 2001
+From: Maxime Hadjinlian <[email protected]>
+Date: Fri, 6 Jun 2014 14:55:58 +0200
+Subject: [PATCH] link icudata as data only

I guess this should be "Don't link icudata as data only"

Ah yes !

Quoted 9 lines

package/icu/icu-004-link-icudata-as-data-only.patch line 8

Quoted 5 lines
package/icu/icu-004-link-icudata-as-data-only.patch new file +4 -0
+
+This patch came straight from Debian.
+It fix a trouble when libicudata would not respect some flags and would
+for example end up being built for ARM EABI instead of ARM EABIHF

I don't see how -nostdlib -nodefaultlibs could have that effect - especially in
buildroot, where we always force the abi. Do you have an example configuration where this happens?

When I build a Qt5 apps for ARM EABIHF, the libicudata.so ends up being built without the hard float, which leads to unusable binary.

Quoted 3 lines

That said, I also don't see a reason why you would want the -nostdlib
-nodefaultlibs because in the end it will be linked with those libraries anyway...

As said, this patch come straight from the Debian packages and seemed like a good idea, and it proved to solve the issues. I haven't digged enough to explain the 'why' it solves it.

Quoted 47 lines

Regards,
Arnout

Quoted 27 lines
package/icu/icu-004-link-icudata-as-data-only.patch new file +24 -1
+
+Signed-off-by: Maxime Hadjinlian <[email protected]>
+---
+ source/config/mh-linux | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/source/config/mh-linux b/source/config/mh-linux
+index 531a3b2..5a2a7c4 100644
+--- a/source/config/mh-linux
++++ b/source/config/mh-linux
+@@ -21,7 +21,9 @@ LD_RPATH= -Wl,-zorigin,-rpath,'$$'ORIGIN
+ LD_RPATH_PRE = -Wl,-rpath,
+
+ ## These are the library specific LDFLAGS
+-LDFLAGSICUDT=-nodefaultlibs -nostdlib
++#LDFLAGSICUDT=-nodefaultlibs -nostdlib
++# Debian change: linking icudata as data only causes too many problems.
++LDFLAGSICUDT=
+
+ ## Compiler switch to embed a library name
+ # The initial tab in the next line is to prevent icu-config from reading it.
+--
+2.0.0.rc2
+
--

2.0.0

Quoted 6 lines

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

--

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
#6 lore raw

PATCH 1/2 icu: Delete inapropriate flag at link

In reply to Arnout Vandecappelle

Dear Arnout Vandecappelle,

Quoted 3 lines · Arnout Vandecappelle

I don't see how -nostdlib -nodefaultlibs could have that effect - especially in
buildroot, where we always force the abi. Do you have an example configuration where this happens?

Yes:

BR2_arm=y
BR2_cortex_a8=y
BR2_ARM_EABIHF=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_PACKAGE_ICU=y

With this configuration, all libraries are properly built EABIhf. For example, libicutu.so:

$ LANG=C arm-linux-gnueabihf-readelf -A output/target/usr/lib/libicutu.so.51.2 
Attribute Section: aeabi
File Attributes
  Tag_CPU_name: "7-A"
  Tag_CPU_arch: v7
  Tag_CPU_arch_profile: Application
  Tag_ARM_ISA_use: Yes
  Tag_THUMB_ISA_use: Thumb-2
  Tag_FP_arch: VFPv3-D16
  Tag_ABI_PCS_wchar_t: 4
  Tag_ABI_FP_rounding: Needed
  Tag_ABI_FP_denormal: Needed
  Tag_ABI_FP_exceptions: Needed
  Tag_ABI_FP_number_model: IEEE 754
  Tag_ABI_align_needed: 8-byte
  Tag_ABI_align_preserved: 8-byte, except leaf SP
  Tag_ABI_enum_size: int
  Tag_ABI_HardFP_use: SP and DP
  Tag_ABI_VFP_args: VFP registers
  Tag_CPU_unaligned_access: v6
  Tag_Virtualization_use: TrustZone

However, libicudata.so is not built properly:

$ LANG=C arm-linux-gnueabihf-readelf -A output/target/usr/lib/libicudata.so.51.2 
Attribute Section: aeabi
File Attributes
  Tag_CPU_name: "Cortex-A8"
  Tag_CPU_arch: v7
  Tag_CPU_arch_profile: Application
  Tag_ARM_ISA_use: Yes
  Tag_THUMB_ISA_use: Thumb-2
  Tag_FP_arch: VFPv3-D16
  Tag_Virtualization_use: TrustZone

I was actually with Maxime in Paris a couple of weeks ago to debug this issue. He had a Qt application that was not starting properly due to this problem, because the dynamic library loader was refusing to load the libicudata library. After applying the patch for Debian to remove the problematic flags and rebuilding icu, we were able to start his Qt application with no problem.

Also, the Debian changelog is pretty clear about what they've done, and explicitly mentions the armhf problem. From
http://metadata.ftp-master.debian.org/changelogs//main/i/icu/icu_52.1-3_changelog:

icu (4.8.1.1-2) unstable; urgency=low

* debian/patches/icudata-stdlibs.patch: Link stdlibs to libicudata so we get reasonably sane ELF headers on armhf. Thanks Adam Conrad <[email protected]>. (Closes: #653457)

-- Jay Berkenbilt <[email protected]> Wed, 04 Jan 2012 09:52:11 -0500

See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=653457 for more details.

Hopefully, these details will be enough to justify the patch. Though I agree with you that Maxime's description of the patch should have been more detailed.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#7 lore raw

PATCH 1/2 icu: Delete inapropriate flag at link

On 21/06/14 19:14, Thomas Petazzoni wrote:
[snip]

Quoted 2 lines

See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=653457 for more details.

That reference (and the failing config) indeed helps. Therefore:

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Regards,
Arnout

Quoted 4 lines

Hopefully, these details will be enough to justify the patch. Though I agree with you that Maxime's description of the patch should have been more detailed.

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
#8 lore raw

PATCH 1/2 icu: Delete inapropriate flag at link

In reply to Maxime Hadjinlian

Dear Maxime Hadjinlian,

Quoted 11 lines · Maxime Hadjinlian

From: Maxime Hadjinlian <[email protected]>

Theses flags may cause libicudata to not be loaded on ARM EABIHF because it would be built for ARM EABI only.

Signed-off-by Maxime Hadjinlian <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 .../icu/icu-004-link-icudata-as-data-only.patch    | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 package/icu/icu-004-link-icudata-as-data-only.patch

Thanks, patch applied, after adding more details following the discussion with Arnout.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com