Bug#919763: adios: armhf FTBFS when built on arm64
On Mon, Jan 28, 2019 at 10:34:54AM -0800, Steve Langasek wrote: >On Mon, Jan 28, 2019 at 12:45:22PM +, Steve McIntyre wrote: >> Hi Alastair, > >> On Mon, Jan 28, 2019 at 09:50:30AM +, Alastair McKinstry wrote: > >> >Yes, thanka for this patch. I'm reluctant to consider that binaries built on >> >armhf failing on arm64 is an _RC Bug_, but its definitely undesirable. > >> It's not *yet* RC, but it's going to go that way soon. Our plan is to >> switch to building for armel and armhf using arm64 machines before too >> long, which would reliably break all packages with bugs like this. > >If it were only about the buildds, then Alastair's solution (disabling the >testsuite on this arch), though suboptimal, would probably be acceptable. >But I think it is increasingly likely that people who are running armhf >Debian binaries will be doing so on 64-bit chips with 64-bit kernels, where >unaligned fixups will not happen. > >This is also the configuration that android 32-bit kernels shipped in since >forever. > >So there is a small but significant and increasing set of configurations >where armhf binaries need to avoid unalign access in order to be useful. Yup, agreed also. People should also *not* be disabling test suites that are showing real issues being found! -- Steve McIntyre, Cambridge, UK.st...@einval.com < liw> everything I know about UK hotels I learned from "Fawlty Towers"
Bug#919763: adios: armhf FTBFS when built on arm64
On Mon, Jan 28, 2019 at 12:45:22PM +, Steve McIntyre wrote: > Hi Alastair, > On Mon, Jan 28, 2019 at 09:50:30AM +, Alastair McKinstry wrote: > >Yes, thanka for this patch. I'm reluctant to consider that binaries built on > >armhf failing on arm64 is an _RC Bug_, but its definitely undesirable. > It's not *yet* RC, but it's going to go that way soon. Our plan is to > switch to building for armel and armhf using arm64 machines before too > long, which would reliably break all packages with bugs like this. If it were only about the buildds, then Alastair's solution (disabling the testsuite on this arch), though suboptimal, would probably be acceptable. But I think it is increasingly likely that people who are running armhf Debian binaries will be doing so on 64-bit chips with 64-bit kernels, where unaligned fixups will not happen. This is also the configuration that android 32-bit kernels shipped in since forever. So there is a small but significant and increasing set of configurations where armhf binaries need to avoid unalign access in order to be useful. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer https://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: PGP signature
Bug#919763: adios: armhf FTBFS when built on arm64
Hi Alastair, On Mon, Jan 28, 2019 at 09:50:30AM +, Alastair McKinstry wrote: >Hi > >Yes, thanka for this patch. I'm reluctant to consider that binaries built on >armhf failing on arm64 is an _RC Bug_, but its definitely undesirable. It's not *yet* RC, but it's going to go that way soon. Our plan is to switch to building for armel and armhf using arm64 machines before too long, which would reliably break all packages with bugs like this. -- Steve McIntyre, Cambridge, UK.st...@einval.com Into the distance, a ribbon of black Stretched to the point of no turning back
Bug#919763: adios: armhf FTBFS when built on arm64
Hi Yes, thanka for this patch. I'm reluctant to consider that binaries built on armhf failing on arm64 is an _RC Bug_, but its definitely undesirable. I'll forward the patch to upstream, and let them optimize it better. regards Alastair On 25/01/2019 18:02, Steve Langasek wrote: Package: adios Followup-For: Bug #919763 User: ubuntu-de...@lists.ubuntu.com Usertags: origin-ubuntu disco ubuntu-patch Hi Alastair, I'm reopening this bug report because I see that you have closed it by disabling the testsuite. This is a wrong fix for the issue given that the tests are correctly identifying CPU incompatibility in the code. This only results in willingly shipping binaries that will not work on more recent ARM CPUs (and even older CPUs, when using a kernel in a particular mode). If you as maintainer don't want to support adios across the range of CPUs that the Debian armhf port is expected to work on, then it would be better to remove the adios binaries for this architecture instead. (And btw, this issue would also affect sparc64 as an architecture, not only armhf, due to similar alignment constraints.) However, I've just prepared a patch that fixes the unaligned access problems in adios's code. Would you consider applying this as a solution instead? It could stand to be improved - in particular, for cases where we are byteswapping, the code currently writes the variable twice where this could be optimized to be a single write. But I presume the byteswap case is only of interest on big-endian architectures, so not of great concern on modern targets. Thanks for considering, -- Alastair McKinstry, , , https://diaspora.sceal.ie/u/amckinstry Misentropy: doubting that the Universe is becoming more disordered.
Bug#919763: adios: armhf FTBFS when built on arm64
Package: adios
Followup-For: Bug #919763
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu disco ubuntu-patch
Hi Alastair,
I'm reopening this bug report because I see that you have closed it by
disabling the testsuite.
This is a wrong fix for the issue given that the tests are correctly
identifying CPU incompatibility in the code. This only results in willingly
shipping binaries that will not work on more recent ARM CPUs (and even older
CPUs, when using a kernel in a particular mode).
If you as maintainer don't want to support adios across the range of CPUs
that the Debian armhf port is expected to work on, then it would be better
to remove the adios binaries for this architecture instead.
(And btw, this issue would also affect sparc64 as an architecture, not only
armhf, due to similar alignment constraints.)
However, I've just prepared a patch that fixes the unaligned access problems
in adios's code. Would you consider applying this as a solution instead?
It could stand to be improved - in particular, for cases where we are
byteswapping, the code currently writes the variable twice where this could
be optimized to be a single write. But I presume the byteswap case is only
of interest on big-endian architectures, so not of great concern on modern
targets.
Thanks for considering,
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer https://www.debian.org/
slanga...@ubuntu.com vor...@debian.org
diff -Nru adios-1.13.1/debian/patches/no-unaligned-access.patch
adios-1.13.1/debian/patches/no-unaligned-access.patch
--- adios-1.13.1/debian/patches/no-unaligned-access.patch 1969-12-31
16:00:00.0 -0800
+++ adios-1.13.1/debian/patches/no-unaligned-access.patch 2019-01-25
09:21:48.0 -0800
@@ -0,0 +1,248 @@
+Description: use alignment-safe buffer handling.
+ Currently we are using assignment to copy the contents of a variable of
+ arbitrary type into a buffer, but this is not portable because the buffer
+ address may not be aligned. Use memcpy() instead, which should be
+ comparable performance but alignment-safe.
+Author: Steve Langasek
+Last-Modified: 2019-01-25
+Bug-Debian: https://bugs.debian.org/919763
+
+Index: adios-1.13.1/src/core/adios_bp_v1.c
+===
+--- adios-1.13.1.orig/src/core/adios_bp_v1.c
adios-1.13.1/src/core/adios_bp_v1.c
+@@ -155,21 +155,21 @@
+
+ uint64_t attrs_end = b->file_size - 28;
+
+-b->pg_index_offset = *(uint64_t *) (b->buff + b->offset);
++memcpy(>pg_index_offset, b->buff + b->offset, sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64(b->pg_index_offset);
+ }
+
+ b->offset += 8;
+
+-b->vars_index_offset = *(uint64_t *) (b->buff + b->offset);
++memcpy(>vars_index_offset, b->buff + b->offset, sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64(b->vars_index_offset);
+ }
+
+ b->offset += 8;
+
+-b->attrs_index_offset = *(uint64_t *) (b->buff + b->offset);
++memcpy(>attrs_index_offset, b->buff + b->offset, sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64(b->attrs_index_offset);
+ }
+@@ -203,13 +203,13 @@
+ uint64_t process_groups_count;
+ uint64_t process_groups_length;
+
+-process_groups_count = *(uint64_t *) (b->buff + b->offset);
++memcpy(_groups_count, b->buff + b->offset, sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64(process_groups_count);
+ }
+ b->offset += 8;
+
+-process_groups_length = *(uint64_t *) (b->buff + b->offset);
++memcpy(_groups_length, b->buff + b->offset, sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64(process_groups_length);
+ }
+@@ -275,7 +275,8 @@
+ }
+ b->offset += 4;
+
+-(*root)->offset_in_file = *(uint64_t *) (b->buff + b->offset);
++memcpy(&((*root)->offset_in_file), b->buff + b->offset,
++ sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64((*root)->offset_in_file);
+ }
+@@ -321,7 +322,7 @@
+ }
+ b->offset += 4;
+
+-vars_length = *(uint64_t *) (b->buff + b->offset);
++memcpy(_length, b->buff + b->offset, sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+ swap_64(vars_length);
+ }
+@@ -390,7 +391,8 @@
+ (*root)->type = (enum ADIOS_DATATYPES) flag;
+ b->offset += 1;
+
+-characteristics_sets_count = *(uint64_t *) (b->buff + b->offset);
++memcpy(_sets_count, b->buff + b->offset,
++ sizeof(uint64_t));
+ if(b->change_endianness == adios_flag_yes) {
+
Bug#919763: adios: armhf FTBFS when built on arm64
Source: adios Version: 1.13.1-11 Severity: serious Tags: ftbfs https://buildd.debian.org/status/logs.php?pkg=adios=armhf ... make check-TESTS make[5]: Entering directory '/<>/build-mpich/tests/test_src' make[6]: Entering directory '/<>/build-mpich/tests/test_src' PASS: copy_subvolume PASS: test_strutil PASS: text_to_pairstruct PASS: trim_spaces PASS: group_free_test PASS: transforms_specparse ../../../config/test-driver: line 107: 15948 Bus error "$@" > $log_file 2>&1 ../../../config/test-driver: line 107: 15949 Bus error "$@" > $log_file 2>&1 FAIL: read_points_2d FAIL: array_attribute PASS: selection_api ../../../config/test-driver: line 107: 15944 Bus error "$@" > $log_file 2>&1 FAIL: query_minmax ../../../config/test-driver: line 107: 15950 Bus error "$@" > $log_file 2>&1 FAIL: read_points_3d PASS: points_1DtoND PASS: hashtest = adios 1.13.1: tests/test_src/test-suite.log = # TOTAL: 14 # PASS: 9 # SKIP: 0 # XFAIL: 0 # FAIL: 5 # XPASS: 0 # ERROR: 0 .. contents:: :depth: 2 FAIL: query_minmax == Running query_minmax [rank=000, line 196]: Write step 0 to query_minmax.bp [rank=000, line 200]: groupsize calculated = 48 [rank=000, line 202]: groupsize calculated = 448 [rank=000, line 205]: groupsize 448, totalsize 1844 [rank=000, line 237]: Write time for step 0 was 0.000 seconds [rank=000, line 196]: Write step 1 to query_minmax.bp FAIL query_minmax (exit status: 135) FAIL: read_points_2d Running read_points [rank=000, line 221]: Write step 0 to read_points_2d.bp [rank=000, line 264]: Write time for step 0 was 0.000 seconds [rank=000, line 221]: Write step 1 to read_points_2d.bp FAIL read_points_2d (exit status: 135) FAIL: read_points_3d Running read_points [rank=000, line 238]: Write step 0 to read_points_3d.bp [rank=000, line 274]: Write time for step 0 was 0.001 seconds [rank=000, line 238]: Write step 1 to read_points_3d.bp FAIL read_points_3d (exit status: 135) FAIL: array_attribute = FAIL array_attribute (exit status: 135) FAIL: array_attribute = FAIL array_attribute (exit status: 135) Testsuite summary for adios 1.13.1 # TOTAL: 14 # PASS: 9 # SKIP: 0 # XFAIL: 0 # FAIL: 5 # XPASS: 0 # ERROR: 0 See tests/test_src/test-suite.log make[6]: *** [Makefile:1343: test-suite.log] Error 1 These are likely alignment problems. Backtrace of array_attribute: #0 0x00b6ac76 in bp_parse_vars (fh=fh@entry=0x14dc940) at ../../src/core/bp_utils.c:1585 #1 0x00b6bc1e in bp_open (fname=, comm=comm@entry=1, fh=fh@entry=0x14dc940) at ../../src/core/bp_utils.c:384 #2 0x00b780ae in open_stream (timeout_sec=, comm=1, fname=, fp=0x14ddfb0) at ../../src/read/read_bp.c:1685 #3 adios_read_bp_open (fname=, comm=1, lock_mode=, timeout_sec=) at ../../src/read/read_bp.c:1713 #4 0x00b6d76e in common_read_open (fname=0xbbe394 "array_attribute.bp", method=ADIOS_READ_METHOD_BP, comm=1, lock_mode=ADIOS_LOCKMODE_NONE, timeout_sec=0) at ../../src/core/common_read.c:424 #5 0x00b50698 in read_attrs () at ../../../tests/test_src/array_attribute.c:267 #6 0x00b4ff1c in main (argc=, argv=) at ../../../tests/test_src/array_attribute.c:115
