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

Reply via email to