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.000000000 -0800 +++ adios-1.13.1/debian/patches/no-unaligned-access.patch 2019-01-25 09:21:48.000000000 -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 <steve.langa...@ubuntu.com> +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(&b->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(&b->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(&b->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(&process_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(&process_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(&vars_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(&characteristics_sets_count, b->buff + b->offset, ++ sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(characteristics_sets_count); + } +@@ -672,8 +674,8 @@ + + case adios_characteristic_offset: + { +- (*root)->characteristics [j].offset = +- *(uint64_t *) (b->buff + b->offset); ++ memcpy(&((*root)->characteristics [j].offset), ++ b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->characteristics [j].offset); + } +@@ -684,8 +686,8 @@ + + case adios_characteristic_payload_offset: + { +- (*root)->characteristics [j].payload_offset = +- *(uint64_t *) (b->buff + b->offset); ++ memcpy(&((*root)->characteristics [j].payload_offset), ++ b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->characteristics [j].payload_offset); + } +@@ -814,7 +816,7 @@ + } + b->offset += 4; + +- attrs_length = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&attrs_length, b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(attrs_length); + } +@@ -884,7 +886,8 @@ + (*root)->type = (enum ADIOS_DATATYPES) flag; + b->offset += 1; + +- characteristics_sets_count = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&characteristics_sets_count, b->buff + b->offset, ++ sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(characteristics_sets_count); + } +@@ -1050,8 +1053,8 @@ + + case adios_characteristic_offset: + { +- (*root)->characteristics [j].offset = +- *(uint64_t *) (b->buff + b->offset); ++ memcpy(&((*root)->characteristics [j].offset), ++ b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->characteristics [j].offset); + } +@@ -1074,8 +1077,8 @@ + + case adios_characteristic_payload_offset: + { +- (*root)->characteristics [j].payload_offset = +- *(uint64_t *) (b->buff + b->offset); ++ memcpy(&((*root)->characteristics [j].payload_offset), ++ b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->characteristics [j].payload_offset); + } +@@ -1187,7 +1190,7 @@ + } + + uint64_t size; +- size = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&size, b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(size); + } +@@ -1298,7 +1301,7 @@ + swap_32(vars_header->count); + } + b->offset += 4; +- vars_header->length = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&vars_header->length, b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(vars_header->length); + } +@@ -1325,7 +1328,7 @@ + uint16_t len; + uint8_t flag; + +- length_of_var = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&length_of_var, b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(length_of_var); + } +@@ -1425,7 +1428,8 @@ + } + else + { +- (*root)->dimension.rank = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&((*root)->dimension.rank), b->buff + b->offset, ++ sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->dimension.rank); + } +@@ -1449,8 +1453,8 @@ + } + else + { +- (*root)->global_dimension.rank = *(uint64_t *) +- (b->buff + b->offset); ++ memcpy(&((*root)->global_dimension.rank), b->buff + b->offset, ++ sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->global_dimension.rank); + } +@@ -1473,7 +1477,8 @@ + } + else + { +- (*root)->local_offset.rank = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&((*root)->local_offset.rank), b->buff + b->offset, ++ sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64((*root)->local_offset.rank); + } +@@ -1520,8 +1525,8 @@ + switch (c) + { + case adios_characteristic_offset: +- var_header->characteristics.offset = *(uint64_t *) +- (b->buff + b->offset); ++ memcpy(&var_header->characteristics.offset, ++ b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(var_header->characteristics.offset); + } +@@ -1529,8 +1534,8 @@ + break; + + case adios_characteristic_payload_offset: +- var_header->characteristics.payload_offset = *(uint64_t *) +- (b->buff + b->offset); ++ memcpy(&var_header->characteristics.payload_offset, ++ b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(var_header->characteristics.payload_offset); + } +@@ -1955,7 +1960,7 @@ + } + b->offset += 4; + +- attrs_header->length = *(uint64_t *) (b->buff + b->offset); ++ memcpy(&attrs_header->length, b->buff + b->offset, sizeof(uint64_t)); + if(b->change_endianness == adios_flag_yes) { + swap_64(attrs_header->length); + } +Index: adios-1.13.1/src/core/bp_utils.c +=================================================================== +--- adios-1.13.1.orig/src/core/bp_utils.c ++++ adios-1.13.1/src/core/bp_utils.c +@@ -51,7 +51,7 @@ + swap_32(var); \ + b->offset += 4; + +-#define BUFREAD64(b,var) var = *(uint64_t *) (b->buff + b->offset); \ ++#define BUFREAD64(b,var) memcpy(&(var), b->buff + b->offset, sizeof(uint64_t));\ + if (b->change_endianness == adios_flag_yes) \ + swap_64(var); \ + b->offset += 8; diff -Nru adios-1.13.1/debian/patches/series adios-1.13.1/debian/patches/series --- adios-1.13.1/debian/patches/series 2019-01-05 23:28:16.000000000 -0800 +++ adios-1.13.1/debian/patches/series 2019-01-24 16:33:38.000000000 -0800 @@ -7,3 +7,4 @@ exclude_cmake.patch python_wrapper.patch missing-header.patch +no-unaligned-access.patch