review request: implementing DW_AT_endianity

review request: implementing DW_AT_endianity

[Peeter Joot]

I've implemented support in gdb for DW_AT_endianity, DW_END_bigendian, and DW_END_littleendian, and would like to ask for gdb community review of this change.


Purpose: gcc6+ supports mixed endian attributes on structures and unions, and flags these dwarf instrumentation of these structure members with DW_END_... attributes.  As intel appears to have introduced these dwarf flags into the standard, I expect their compiler and debugger also supports them.  However, gdb does not.  The following example, compiled on a little endian system, is an example of this attribute use:


#include <stdio.h>

#include <string.h>


struct big {

    int v;

    short a[4];

} __attribute__( ( scalar_storage_order( "big-endian" ) ) );


struct little {

    int v;

    short a[4];

} __attribute__( ( scalar_storage_order( "little-endian" ) ) );


struct native {

    int v;

    short a[4];

};


int main() {

    struct big b = {3, {1, 2, 3, 4}};

    struct native n = {3, {1, 2, 3, 4}};

    struct little l = {3, {1, 2, 3, 4}};

    int cb = memcmp( &b, &n, sizeof( b ) );

    int cl = memcmp( &l, &n, sizeof( l ) );


    printf( "%d %d %d: big %s native.  little %s native\n", b.v, n.v, l.v,

            cb == 0 ? "==" : "!=", cl == 0 ? "==" : "!=" );

    return 0;

}



Running this produces the expected result, and the endianness of the underlying stores can be seen in a debugger session:


Breakpoint 1, main () at test.c:20

20          struct big b = {3, {1, 2, 3, 4}};

(gdb) n

21          struct native n = {3, {1, 2, 3, 4}};

(gdb) n

22          struct little l = {3, {1, 2, 3, 4}};

(gdb) n

23          int cb = memcmp( &b, &n, sizeof( b ) );

(gdb) p b

$1 = {v = 50331648, a = {256, 512, 768, 1024}}

(gdb) p n

$2 = {v = 3, a = {1, 2, 3, 4}}

(gdb) p l

$3 = {v = 3, a = {1, 2, 3, 4}}

(gdb) x/4x &b

0x7fffffffd96c: 0x03000000      0x02000100      0x04000300      0x00000000

(gdb) x/4x &l

0x7fffffffd954: 0x00000003      0x00020001      0x00040003      0x00000003

(gdb) x/4x &n

0x7fffffffd960: 0x00000003      0x00020001      0x00040003      0x03000000

(gdb) n

24          int cl = memcmp( &l, &n, sizeof( l ) );

(gdb)

26          printf( "%d %d %d: big %s native.  little %s native\n", b.v, n.v, l.v,

(gdb) n

3 3 3: big != native.  little == native

28          return 0;



This debugger session also shows that gdb currently ignores the DW_END_bigendian (and littleendian) attributes, showing the internal representation of the data instead of what the values that memory represents.  It turns out that propagating the DW_AT_endianity dwarf attributes to the gdb print_scalar_formatted function is fairly straightforward, which allows gdb to display the results in a user-friendly way regardless of the endian attributes:


diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 91f95ff..e79fd5e 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -2283,6 +2283,17 @@ read_and_display_attr_value (unsigned long attribute,
  }
       break;

+    case DW_AT_endianity:
+      printf ("\t");
+      switch (uvalue)
+ {
+ case DW_END_default: printf ("(default)"); break;
+ case DW_END_big: printf ("(big)"); break;
+ case DW_END_little: printf ("(little)"); break;
+ default: printf (_("(unknown endianity)")); break;
+ }
+      break;
+
     case DW_AT_virtuality:
       printf ("\t");
       switch (uvalue)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 18224e0..66a8eaf 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,19 @@
+2017-10-06  Peeter Joot  <peeter.joot@lzlabs.com>
+
+ * gdb/gdbtypes.h (global scope): define TYPE_ENDIANITY_BIG,
+ TYPE_ENDIANITY_LITTLE.
+ * binutils/dwarf.c (read_and_display_attr_value): Handle
+ DW_AT_endianity, DW_END_default, DW_END_big, DW_END_little
+ * gdb/dwarf2read.c (read_base_type): Handle DW_END_big, DW_END_little
+ * gdb/gdbtypes.c (check_types_equal): Require matching
+ TYPE_ENDIANITY_BIG, and TYPE_ENDIANITY_LITTLE if set.
+ (recursive_dump_type): Print TYPE_ENDIANITY_BIG, and
+ TYPE_ENDIANITY_LITTLE if set.
+ (struct main_type): Add flag_endianity_big, flag_endianity_little
+ * gdb/printcmd.c (print_scalar_formatted): Use compiler supplied
+ endianness instead of arch endianness if TYPE_ENDIANITY_BIG or
+ TYPE_ENDIANITY_LITTLE is set.
+
 2017-10-06  Yao Qi  <yao.qi@linaro.org>

  * Makefile.in (ALL_64_TARGET_OBS): Replace aarch64-insn.o with
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1b15adc..fa2889b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -15234,6 +15234,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *type;
   struct attribute *attr;
   int encoding = 0, bits = 0;
+  int endianity = 0;
   const char *name;

   attr = dwarf2_attr (die, DW_AT_encoding, cu);
@@ -15330,6 +15331,21 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   if (name && strcmp (name, "char") == 0)
     TYPE_NOSIGN (type) = 1;

+  attr = dwarf2_attr (die, DW_AT_endianity, cu);
+  if ( attr )
+    {
+      endianity = DW_UNSND (attr);
+      switch (endianity)
+        {
+           case DW_END_big:
+              TYPE_ENDIANITY_BIG (type) = 1;
+              break;
+           case DW_END_little:
+              TYPE_ENDIANITY_LITTLE (type) = 1;
+              break;
+        }
+    }
+
   return set_die_type (die, type, cu);
 }

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 73d4453..43f553b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3423,6 +3423,8 @@ check_types_equal (struct type *type1, struct type *type2,
       || TYPE_LENGTH (type1) != TYPE_LENGTH (type2)
       || TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)
       || TYPE_NOSIGN (type1) != TYPE_NOSIGN (type2)
+      || TYPE_ENDIANITY_BIG (type1) != TYPE_ENDIANITY_BIG (type2)
+      || TYPE_ENDIANITY_LITTLE (type1) != TYPE_ENDIANITY_LITTLE (type2)
       || TYPE_VARARGS (type1) != TYPE_VARARGS (type2)
       || TYPE_VECTOR (type1) != TYPE_VECTOR (type2)
       || TYPE_NOTTEXT (type1) != TYPE_NOTTEXT (type2)
@@ -4460,6 +4462,14 @@ recursive_dump_type (struct type *type, int spaces)
     {
       puts_filtered (" TYPE_NOSIGN");
     }
+  if (TYPE_ENDIANITY_BIG (type))
+    {
+      puts_filtered (" TYPE_ENDIANITY_BIG");
+    }
+  if (TYPE_ENDIANITY_LITTLE (type))
+    {
+      puts_filtered (" TYPE_ENDIANITY_LITTLE");
+    }
   if (TYPE_STUB (type))
     {
       puts_filtered (" TYPE_STUB");
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 009cea9..074aa2c 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -210,6 +210,16 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);

 #define TYPE_NOSIGN(t) (TYPE_MAIN_TYPE (t)->flag_nosign)

+/* * Mixed endian archetectures can supply dwarf instrumentation
+ * that indicates the desired endian interpretation of the variable.
+ * This indicates that the interpretation should be big-endian
+ * even if the cpu is running in little endian mode. */
+#define TYPE_ENDIANITY_BIG(t) (TYPE_MAIN_TYPE (t)->flag_endianity_big)
+
+/* * The type has a little endian interpretation even if the cpu
+ * is running in big endian mode. */
+#define TYPE_ENDIANITY_LITTLE(t) (TYPE_MAIN_TYPE (t)->flag_endianity_little)
+
 /* * This appears in a type's flags word if it is a stub type (e.g.,
    if someone referenced a type that wasn't defined in a source file
    via (struct sir_not_appearing_in_this_film *)).  */
@@ -616,6 +626,8 @@ struct main_type
   unsigned int flag_gnu_ifunc : 1;
   unsigned int flag_fixed_instance : 1;
   unsigned int flag_objfile_owned : 1;
+  unsigned int flag_endianity_big : 1;
+  unsigned int flag_endianity_little : 1;

   /* * True if this type was declared with "class" rather than
      "struct".  */
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index a8743f1..e714283 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -356,6 +356,15 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
   unsigned int len = TYPE_LENGTH (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);

+  if ( TYPE_ENDIANITY_BIG (type) )
+  {
+    byte_order = BFD_ENDIAN_BIG;
+  }
+  if ( TYPE_ENDIANITY_LITTLE (type) )
+  {
+    byte_order = BFD_ENDIAN_LITTLE;
+  }
+
   /* String printing should go through val_print_scalar_formatted.  */
   gdb_assert (options->format != 's');



On behalf of my employer (LzLabs), I would like to contribute this change to to the gdb project.  For a small change like this, it is not clear FSF copyright assignment is required, as I see the following in your CONTRIBUTIONS document: "Small changes can be accepted without a copyright assignment form on file.".  What counts as small?


If assignment is required, I have approval from company management and legal to formally go through that process.  Before figuring out how that assignment process works, I wanted to first ensure the changes I've made would be acceptable for contribution, and make any review driven updates required.


--

Peeter

Re: review request: implementing DW_AT_endianity

[Peeter Joot]

It looks like any talk of contribution (and perhaps review) is premature.  My initial test only had a big-endian structure, and the one I wrote describing what I wanted to address doesn't work (which I noticed only after sending my review request email).  The big-endian tagged structure does show up properly, but what I have done messes up the display of any little endian integers.  It appears that I end up setting my new big-endian flag inappropriately:


Breakpoint 1, print_scalar_formatted (valaddr=0x1e25260 "\003", type=0x1ce2670, options=0x7ffe20ac7490, size=0, stream=0x1c95a40)

    at ../../binutils-gdb/gdb/printcmd.c:355

355       struct gdbarch *gdbarch = get_type_arch (type);

(gdb) p *type->main_type

$1 = {code = TYPE_CODE_INT, flag_unsigned = 0, flag_nosign = 0, flag_stub = 0, flag_target_stub = 0, flag_static = 0,

  flag_prototyped = 0, flag_incomplete = 0, flag_varargs = 0, flag_vector = 0, flag_stub_supported = 0, flag_gnu_ifunc = 0,

  flag_fixed_instance = 0, flag_objfile_owned = 1, flag_endianity_big = 1, flag_endianity_little = 0, flag_declared_class = 0,

  flag_flag_enum = 0, type_specific_field = TYPE_SPECIFIC_NONE, nfields = 0, name = 0x1caacbc "int", tag_name = 0x0, owner = {

    objfile = 0x1c98620, gdbarch = 0x1c98620}, target_type = 0x0, flds_bnds = {fields = 0x0, bounds = 0x0}, type_specific = {

    cplus_stuff = 0x0, gnat_stuff = 0x0, floatformat = 0x0, func_stuff = 0x0, self_type = 0x0}, dyn_prop_list = 0x0}


This is when I'm printing a native endian integer, so do not want to have flag_endianity_big set.


I'm guessing that main_type is not the place for this flag, but it has to be in struct type instead.  Is that guess on the right track?


Peeter

Re: review request: implementing DW_AT_endianity

[Simon Marchi]

On 2017-10-06 11:05 AM, Peeter Joot wrote:
> I've implemented support in gdb for DW_AT_endianity, DW_END_bigendian, and DW_END_littleendian, and would like to ask for gdb community review of this change.

Hi Peeter,

Thanks for the patch.  Since the compiler supports it, it makes sense for the debugger
to support it correctly.

Please read the Contribution Checklist:

  https://sourceware.org/gdb/wiki/ContributionChecklist

Most importantly, make sure your code uses the GNU style, and use "git send-email" to
send your patch.  It will make it easier for reviewers to apply and look at your patch.

Changes to binutils should be sent the binutils mailing list (binutils@sourceware.org), so
make a separate patch for binutils/dwarf.c and send it there.

The best way to show that your contribution works is to add a test for it.  You can add it
to testsuite/gdb.base.  Copy an existing one (e.g. wchar.c/wchar.exp) and modify as needed.

You should also run the testsuite to see if your patch causes any regression:

  https://sourceware.org/gdb/wiki/TestingGDB

Note that there are many existing failures in the testsuite, so what you should do is run the
testsuite without and with your patch, and diff the before/after testsuite/gdb.sum file.

I just noted a few formatting comments below, I'll look at the code more in depth once you
send an updated version that's easier to apply.

> Purpose: gcc6+ supports mixed endian attributes on structures and unions, and flags these dwarf instrumentation of these structure members with DW_END_... attributes.  As intel appears to have introduced these dwarf flags into the standard, I expect their compiler and debugger also supports them.  However, gdb does not.  The following example, compiled on a little endian system, is an example of this attribute use:
> 
> 
> #include <stdio.h>
> 
> #include <string.h>
> 
> 
> struct big {
> 
>     int v;
> 
>     short a[4];
> 
> } __attribute__( ( scalar_storage_order( "big-endian" ) ) );
> 
> 
> struct little {
> 
>     int v;
> 
>     short a[4];
> 
> } __attribute__( ( scalar_storage_order( "little-endian" ) ) );
> 
> 
> struct native {
> 
>     int v;
> 
>     short a[4];
> 
> };
> 
> 
> int main() {
> 
>     struct big b = {3, {1, 2, 3, 4}};
> 
>     struct native n = {3, {1, 2, 3, 4}};
> 
>     struct little l = {3, {1, 2, 3, 4}};
> 
>     int cb = memcmp( &b, &n, sizeof( b ) );
> 
>     int cl = memcmp( &l, &n, sizeof( l ) );
> 
> 
>     printf( "%d %d %d: big %s native.  little %s native\n", b.v, n.v, l.v,
> 
>             cb == 0 ? "==" : "!=", cl == 0 ? "==" : "!=" );
> 
>     return 0;
> 
> }
> 
> 
> 
> Running this produces the expected result, and the endianness of the underlying stores can be seen in a debugger session:
> 
> 
> Breakpoint 1, main () at test.c:20
> 
> 20          struct big b = {3, {1, 2, 3, 4}};
> 
> (gdb) n
> 
> 21          struct native n = {3, {1, 2, 3, 4}};
> 
> (gdb) n
> 
> 22          struct little l = {3, {1, 2, 3, 4}};
> 
> (gdb) n
> 
> 23          int cb = memcmp( &b, &n, sizeof( b ) );
> 
> (gdb) p b
> 
> $1 = {v = 50331648, a = {256, 512, 768, 1024}}
> 
> (gdb) p n
> 
> $2 = {v = 3, a = {1, 2, 3, 4}}
> 
> (gdb) p l
> 
> $3 = {v = 3, a = {1, 2, 3, 4}}
> 
> (gdb) x/4x &b
> 
> 0x7fffffffd96c: 0x03000000      0x02000100      0x04000300      0x00000000
> 
> (gdb) x/4x &l
> 
> 0x7fffffffd954: 0x00000003      0x00020001      0x00040003      0x00000003
> 
> (gdb) x/4x &n
> 
> 0x7fffffffd960: 0x00000003      0x00020001      0x00040003      0x03000000
> 
> (gdb) n
> 
> 24          int cl = memcmp( &l, &n, sizeof( l ) );
> 
> (gdb)
> 
> 26          printf( "%d %d %d: big %s native.  little %s native\n", b.v, n.v, l.v,
> 
> (gdb) n
> 
> 3 3 3: big != native.  little == native
> 
> 28          return 0;
> 
> 
> 
> This debugger session also shows that gdb currently ignores the DW_END_bigendian (and littleendian) attributes, showing the internal representation of the data instead of what the values that memory represents.  It turns out that propagating the DW_AT_endianity dwarf attributes to the gdb print_scalar_formatted function is fairly straightforward, which allows gdb to display the results in a user-friendly way regardless of the endian attributes:
> 
> 
> diff --git a/binutils/dwarf.c b/binutils/dwarf.c
> index 91f95ff..e79fd5e 100644
> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -2283,6 +2283,17 @@ read_and_display_attr_value (unsigned long attribute,
>   }
>        break;
> 
> +    case DW_AT_endianity:
> +      printf ("\t");
> +      switch (uvalue)
> + {
> + case DW_END_default: printf ("(default)"); break;
> + case DW_END_big: printf ("(big)"); break;
> + case DW_END_little: printf ("(little)"); break;
> + default: printf (_("(unknown endianity)")); break;

Put each statement on its own line:

case DW_END_default:
  printf ("(default)");
  break;

> + }
> +      break;
> +
>      case DW_AT_virtuality:
>        printf ("\t");
>        switch (uvalue)
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 18224e0..66a8eaf 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,19 @@
> +2017-10-06  Peeter Joot  <peeter.joot@lzlabs.com>
> +
> + * gdb/gdbtypes.h (global scope): define TYPE_ENDIANITY_BIG,
> + TYPE_ENDIANITY_LITTLE.
> + * binutils/dwarf.c (read_and_display_attr_value): Handle
> + DW_AT_endianity, DW_END_default, DW_END_big, DW_END_little
> + * gdb/dwarf2read.c (read_base_type): Handle DW_END_big, DW_END_little
> + * gdb/gdbtypes.c (check_types_equal): Require matching
> + TYPE_ENDIANITY_BIG, and TYPE_ENDIANITY_LITTLE if set.
> + (recursive_dump_type): Print TYPE_ENDIANITY_BIG, and
> + TYPE_ENDIANITY_LITTLE if set.
> + (struct main_type): Add flag_endianity_big, flag_endianity_little
> + * gdb/printcmd.c (print_scalar_formatted): Use compiler supplied
> + endianness instead of arch endianness if TYPE_ENDIANITY_BIG or
> + TYPE_ENDIANITY_LITTLE is set.
> +
>  2017-10-06  Yao Qi  <yao.qi@linaro.org>
> 
>   * Makefile.in (ALL_64_TARGET_OBS): Replace aarch64-insn.o with
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 1b15adc..fa2889b 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -15234,6 +15234,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
>    struct type *type;
>    struct attribute *attr;
>    int encoding = 0, bits = 0;
> +  int endianity = 0;
>    const char *name;
> 
>    attr = dwarf2_attr (die, DW_AT_encoding, cu);
> @@ -15330,6 +15331,21 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
>    if (name && strcmp (name, "char") == 0)
>      TYPE_NOSIGN (type) = 1;
> 
> +  attr = dwarf2_attr (die, DW_AT_endianity, cu);
> +  if ( attr )
> +    {
> +      endianity = DW_UNSND (attr);
> +      switch (endianity)
> +        {
> +           case DW_END_big:
> +              TYPE_ENDIANITY_BIG (type) = 1;
> +              break;
> +           case DW_END_little:
> +              TYPE_ENDIANITY_LITTLE (type) = 1;
> +              break;
> +        }
> +    }
> +
>    return set_die_type (die, type, cu);
>  }
> 
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 73d4453..43f553b 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -3423,6 +3423,8 @@ check_types_equal (struct type *type1, struct type *type2,
>        || TYPE_LENGTH (type1) != TYPE_LENGTH (type2)
>        || TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)
>        || TYPE_NOSIGN (type1) != TYPE_NOSIGN (type2)
> +      || TYPE_ENDIANITY_BIG (type1) != TYPE_ENDIANITY_BIG (type2)
> +      || TYPE_ENDIANITY_LITTLE (type1) != TYPE_ENDIANITY_LITTLE (type2)
>        || TYPE_VARARGS (type1) != TYPE_VARARGS (type2)
>        || TYPE_VECTOR (type1) != TYPE_VECTOR (type2)
>        || TYPE_NOTTEXT (type1) != TYPE_NOTTEXT (type2)
> @@ -4460,6 +4462,14 @@ recursive_dump_type (struct type *type, int spaces)
>      {
>        puts_filtered (" TYPE_NOSIGN");
>      }
> +  if (TYPE_ENDIANITY_BIG (type))
> +    {
> +      puts_filtered (" TYPE_ENDIANITY_BIG");
> +    }
> +  if (TYPE_ENDIANITY_LITTLE (type))
> +    {
> +      puts_filtered (" TYPE_ENDIANITY_LITTLE");
> +    }
>    if (TYPE_STUB (type))
>      {
>        puts_filtered (" TYPE_STUB");
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 009cea9..074aa2c 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -210,6 +210,16 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
> 
>  #define TYPE_NOSIGN(t) (TYPE_MAIN_TYPE (t)->flag_nosign)
> 
> +/* * Mixed endian archetectures can supply dwarf instrumentation
> + * that indicates the desired endian interpretation of the variable.
> + * This indicates that the interpretation should be big-endian
> + * even if the cpu is running in little endian mode. */
> +#define TYPE_ENDIANITY_BIG(t) (TYPE_MAIN_TYPE (t)->flag_endianity_big)
> +
> +/* * The type has a little endian interpretation even if the cpu
> + * is running in big endian mode. */
> +#define TYPE_ENDIANITY_LITTLE(t) (TYPE_MAIN_TYPE (t)->flag_endianity_little)
> +
>  /* * This appears in a type's flags word if it is a stub type (e.g.,
>     if someone referenced a type that wasn't defined in a source file
>     via (struct sir_not_appearing_in_this_film *)).  */
> @@ -616,6 +626,8 @@ struct main_type
>    unsigned int flag_gnu_ifunc : 1;
>    unsigned int flag_fixed_instance : 1;
>    unsigned int flag_objfile_owned : 1;
> +  unsigned int flag_endianity_big : 1;
> +  unsigned int flag_endianity_little : 1;
> 
>    /* * True if this type was declared with "class" rather than
>       "struct".  */
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index a8743f1..e714283 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -356,6 +356,15 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>    unsigned int len = TYPE_LENGTH (type);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> 
> +  if ( TYPE_ENDIANITY_BIG (type) )
> +  {
> +    byte_order = BFD_ENDIAN_BIG;
> +  }
> +  if ( TYPE_ENDIANITY_LITTLE (type) )

Don't put spaces inside parentheses.

> +  {
> +    byte_order = BFD_ENDIAN_LITTLE;
> +  }

Don't use curly braces for a single statement.

> +
>    /* String printing should go through val_print_scalar_formatted.  */
>    gdb_assert (options->format != 's');
> 
> 
> 
> On behalf of my employer (LzLabs), I would like to contribute this change to to the gdb project.  For a small change like this, it is not clear FSF copyright assignment is required, as I see the following in your CONTRIBUTIONS document: "Small changes can be accepted without a copyright assignment form on file.".  What counts as small?
> 
> 
> If assignment is required, I have approval from company management and legal to formally go through that process.  Before figuring out how that assignment process works, I wanted to first ensure the changes I've made would be acceptable for contribution, and make any review driven updates required.

I would consider a change like that to be significant enough to require an assignment.

Thanks!

Simon

Re: review request: implementing DW_AT_endianity

[Peeter Joot]

Hi Simon,


> Please read the Contribution Checklist:

>

>   https://sourceware.org/gdb/wiki/ContributionChecklist

>

> Most importantly, make sure your code uses the GNU style, and use "git send-email" to

> send your patch.  It will make it easier for reviewers to apply and look at your patch.

>

> Changes to binutils should be sent the binutils mailing list (binutils@sourceware.org), so

> make a separate patch for binutils/dwarf.c and send it there.

>

> The best way to show that your contribution works is to add a test for it.  You can add it

> to testsuite/gdb.base.  Copy an existing one (e.g. wchar.c/wchar.exp) and modify as needed.

>

> You should also run the testsuite to see if your patch causes any regression:

>

>   https://sourceware.org/gdb/wiki/TestingGDB

>

> Note that there are many existing failures in the testsuite, so what you should do is run the

> testsuite without and with your patch, and diff the before/after testsuite/gdb.sum file.

Thanks for the new-developer tips.


> > --- a/binutils/dwarf.c

> > +++ b/binutils/dwarf.c

...

> Put each statement on its own line:

>

> case DW_END_default:

>   printf ("(default)");

>   break;



Note that if I did this, the addition wouldn't match any of the case statements in the surrounding code (read_and_display_attr_value).  There are case statements matching your suggested style earlier in this function, but others that do not (like all the ones immediately surrounding my addition).  I've temporarily adjusted my addition to match the immediately surrounding code (i.e. using tabs instead of spaces, which I hadn't initially noticed).  Shall I adjust all of that surrounding "non-standard formatting" so that each statement is on its own line, or be consistent locally with the conventions used in this file?


I've fixed up all the other formatting issues that you've pointed out, and will submit a new patch with git send-email after running the test suite, and adding my own new test.  The next patch I send will have an additional mechanical change, altering blocks of code from:


gdbarch_byte_order (get_type_arch (type1))


to:


gdbarch_byte_order_for_type (NULL, type1)


where the following helper function has been added to gdbtypes.c:


enum bfd_endian

gdbarch_byte_order_for_type (struct gdbarch * gdbarch, struct type *type)

{

  if (TYPE_ENDIANITY_BIG (type))

    return BFD_ENDIAN_BIG;

  else if (TYPE_ENDIANITY_LITTLE (type))

    return BFD_ENDIAN_LITTLE;

  if (!gdbarch)

    gdbarch = get_type_arch (type);

  return gdbarch_byte_order (gdbarch);

}


When the caller of this already had a gdbarch variable handy, I've used that, instead of passing NULL.  This transformation is enough to make gdb assignment to a mixed endian field, as in:


(gdb) p b.v = 4


work properly, storing the value in big-endian format.  This is the using the same example from my original review request email, assuming a little endian target and a structure with a big-endian attribute.


In the process of doing this debug testing, I've noticed that gcc appears to have some bugs with respect to its dwarf endianity attribute tagging.  For example:


#include <stdio.h>

#include <string.h>


typedef int int32be_t;

typedef short int16be_t;


struct big {

    int32be_t v;

    int16be_t a[4];

} __attribute__( ( scalar_storage_order( "big-endian" ) ) );


struct native {

    int v;

    short a[4];

};


int main() {

    struct big b = {3, {1, 2, 3, 4}};

    struct native n = {3, {1, 2, 3, 4}};


    printf( "%d\n", b.v );

    printf( "%d\n", n.v );


    return 0;

}


A version of this code that does not use typedefs for the field types does get tagged with DW_AT_endianity/DW_END_big, but when typedefs are used as above, the object code ends up with no endianity attributes at all (although the compiler does insert the desired bswap operations).  It appears that it may take a few iterations of compiler/debugger enhancements to really get this working nicely together.


There is a bigger issue of the DW_AT_name that gets emitted for a field with non-native endianity.  I think it would be best for the compiler to choose a different DW_AT_name than the default (int.be for example).  If that is not done it looks like it breaks gdb's assumption that there is one set of attributes for each type in question.  This could be considered a gdb bug, but I think it would be better for the compiler to cater to gdb in this case.  I'm curious what other developers (especially anybody that works on both gcc and gdb) think about this.


> I would consider a change like that to be significant enough to require an assignment.


Okay.  I've contacted the FSF assignment representative to start this process.


Peeter

Re: review request: implementing DW_AT_endianity

[Simon Marchi]

On 2017-10-09 05:11 AM, Peeter Joot wrote:
> Note that if I did this, the addition wouldn't match any of the case statements in the surrounding code (read_and_display_attr_value).  There are case statements matching your suggested style earlier in this function, but others that do not (like all the ones immediately surrounding my addition).  I've temporarily adjusted my addition to match the immediately surrounding code (i.e. using tabs instead of spaces, which I hadn't initially noticed).  Shall I adjust all of that surrounding "non-standard formatting" so that each statement is on its own line, or be consistent locally with the conventions used in this file?

Ok, I see this is binutils code, and I hadn't see that the surrounding code did like that.  You
can leave it as is, and the binutils people will tell you if that's not ok.

> I've fixed up all the other formatting issues that you've pointed out, and will submit a new patch with git send-email after running the test suite, and adding my own new test.  The next patch I send will have an additional mechanical change, altering blocks of code from:
> 
> 
> gdbarch_byte_order (get_type_arch (type1))
> 
> 
> to:
> 
> 
> gdbarch_byte_order_for_type (NULL, type1)
> 
> 
> where the following helper function has been added to gdbtypes.c:
> 
> 
> enum bfd_endian
> 
> gdbarch_byte_order_for_type (struct gdbarch * gdbarch, struct type *type)
> 
> {
> 
>   if (TYPE_ENDIANITY_BIG (type))
> 
>     return BFD_ENDIAN_BIG;
> 
>   else if (TYPE_ENDIANITY_LITTLE (type))
> 
>     return BFD_ENDIAN_LITTLE;
> 
>   if (!gdbarch)
> 
>     gdbarch = get_type_arch (type);
> 
>   return gdbarch_byte_order (gdbarch);
> 
> }

I suggest naming this function type_byte_order.  Functions named "gdbarch_*" are usually
those part of the gdbarch interface (defined in gdbarch.sh/.h/.c).

> 
> 
> When the caller of this already had a gdbarch variable handy, I've used that, instead of passing NULL.  This transformation is enough to make gdb assignment to a mixed endian field, as in:
> 
> 
> (gdb) p b.v = 4

Nice.  Assginment of fields by GDB would be a good thing to check in the test.

> work properly, storing the value in big-endian format.  This is the using the same example from my original review request email, assuming a little endian target and a structure with a big-endian attribute.
> 
> 
> In the process of doing this debug testing, I've noticed that gcc appears to have some bugs with respect to its dwarf endianity attribute tagging.  For example:
> 
> 
> #include <stdio.h>
> 
> #include <string.h>
> 
> 
> typedef int int32be_t;
> 
> typedef short int16be_t;
> 
> 
> struct big {
> 
>     int32be_t v;
> 
>     int16be_t a[4];
> 
> } __attribute__( ( scalar_storage_order( "big-endian" ) ) );
> 
> 
> struct native {
> 
>     int v;
> 
>     short a[4];
> 
> };
> 
> 
> int main() {
> 
>     struct big b = {3, {1, 2, 3, 4}};
> 
>     struct native n = {3, {1, 2, 3, 4}};
> 
> 
>     printf( "%d\n", b.v );
> 
>     printf( "%d\n", n.v );
> 
> 
>     return 0;
> 
> }
> 
> 
> A version of this code that does not use typedefs for the field types does get tagged with DW_AT_endianity/DW_END_big, but when typedefs are used as above, the object code ends up with no endianity attributes at all (although the compiler does insert the desired bswap operations).  It appears that it may take a few iterations of compiler/debugger enhancements to really get this working nicely together.

Ah indeed.  Do you report the gcc bugs you find to them?

> There is a bigger issue of the DW_AT_name that gets emitted for a field with non-native endianity.  I think it would be best for the compiler to choose a different DW_AT_name than the default (int.be for example).  If that is not done it looks like it breaks gdb's assumption that there is one set of attributes for each type in question.  This could be considered a gdb bug, but I think it would be better for the compiler to cater to gdb in this case.  I'm curious what other developers (especially anybody that works on both gcc and gdb) think about this.
> 
> 
>> I would consider a change like that to be significant enough to require an assignment.
> 
> 
> Okay.  I've contacted the FSF assignment representative to start this process.
Great, thanks,

Simon

Re: review request: implementing DW_AT_endianity

[Peeter Joot]

> I suggest naming this function type_byte_order.  Functions named "gdbarch_*" are usually

> those part of the gdbarch interface (defined in gdbarch.sh/.h/.c).


done.

> Nice.  Assginment of fields by GDB would be a good thing to check in the test.

done.

> Ah indeed.  Do you report the gcc bugs you find to them?

I will verify first on the dev version of gcc8 that this is still an issue before submitting a report.

> testsuite.

It is normal to see the number of tests vary when running the test suite (make check -j8)?  My before and after runs had an unexpected difference in the numbers of tests:

                === gdb Summary ===



-# of expected passes           40087

-# of unexpected failures       96

+# of expected passes           40082

+# of unexpected failures       98

 # of unexpected successes      1

 # of expected failures         67

 # of unknown successes         3


My test added 4 additional expected passes (and I verified that my new tests ran in gdb/testsuite/gdb.log), so the number of expected successes should have grown by 4, not decreased by 5?  Some of the failures differences look like buggy tests (outputting pids and so forth).

I clearly didn't regress anything significant, but didn't expect the baseline to vary run to run.

Peeter

Re: review request: implementing DW_AT_endianity

[Simon Marchi]

On 2017-10-10 14:16, Peeter Joot wrote:
>> I suggest naming this function type_byte_order.  Functions named 
>> "gdbarch_*" are usually
> 
>> those part of the gdbarch interface (defined in gdbarch.sh/.h/.c).
> 
> 
> done.
> 
>> Nice.  Assginment of fields by GDB would be a good thing to check in 
>> the test.
> 
> done.
> 
>> Ah indeed.  Do you report the gcc bugs you find to them?
> 
> I will verify first on the dev version of gcc8 that this is still an
> issue before submitting a report.
> 
>> testsuite.
> 
> It is normal to see the number of tests vary when running the test
> suite (make check -j8)?  My before and after runs had an unexpected
> difference in the numbers of tests:
> 
>                 === gdb Summary ===
> 
> 
> 
> -# of expected passes           40087
> 
> -# of unexpected failures       96
> 
> +# of expected passes           40082
> 
> +# of unexpected failures       98
> 
>  # of unexpected successes      1
> 
>  # of expected failures         67
> 
>  # of unknown successes         3
> 
> 
> My test added 4 additional expected passes (and I verified that my new
> tests ran in gdb/testsuite/gdb.log), so the number of expected
> successes should have grown by 4, not decreased by 5?  Some of the
> failures differences look like buggy tests (outputting pids and so
> forth).
> 
> I clearly didn't regress anything significant, but didn't expect the
> baseline to vary run to run.
> 
> Peeter

Yeah, it's possible for the total number of test to vary when tests 
start failing.  For example, you can have:

if [something] {
   fail "couldn't fetch variable"
   return
}

gdb_test "..."
gdb_test "..."

If it succeeds, you'll have 2 pass, 0 fail.  If [something] fails, 
you'll have 0 pass, 1 fail.  If your case you have 2 more unexpected 
failures, I would look into these, see if they are related.  Which tests 
are they from?

Simon

Re: review request: implementing DW_AT_endianity

[Peeter Joot]

These were the differences in the gdb.sum output for the baseline run vs. mine:


-PASS: gdb.base/step-over-syscall.exp: clone: displaced=off: single step over clone

-PASS: gdb.base/step-over-syscall.exp: clone: displaced=off: get hexadecimal valueof "$pc"

-PASS: gdb.base/step-over-syscall.exp: clone: displaced=off: single step over clone final pc

-PASS: gdb.base/step-over-syscall.exp: clone: displaced=off: break marker

-PASS: gdb.base/step-over-syscall.exp: clone: displaced=off: continue to marker (clone)

-KFAIL: gdb.base/step-over-syscall.exp: clone: displaced=on: single step over clone (PRMS: gdb/19675)

+FAIL: gdb.base/step-over-syscall.exp: clone: displaced=off: single step over clone

-PASS: gdb.mi/list-thread-groups-available.exp: list available thread groups

+FAIL: gdb.mi/list-thread-groups-available.exp: list available thread groups (unexpected output)

-PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attempt 1: attach (pass 1), pending signal catch

-PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attempt 3: attach (pass 2), pending signal catch

+PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attempt 4: attach (pass 1), pending signal catch

+PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attempt 4: attach (pass 2), pending signal catch

-PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 4: attach (pass 1), pending signal catch

-PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 7: attach (pass 2), pending signal catch

+PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 3: attach (pass 1), pending signal catch

+PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 3: attach (pass 2), pending signal catch

-PASS: gdb.threads/process-dies-while-detaching.exp: single-process: detach: killed outside: get integer valueof "mypid" (9535)

+PASS: gdb.threads/process-dies-while-detaching.exp: single-process: detach: killed outside: get integer valueof "mypid" (6194)

-PASS: gdb.threads/process-dies-while-detaching.exp: single-process: continue: killed outside: get integer valueof "mypid" (8865)

+PASS: gdb.threads/process-dies-while-detaching.exp: single-process: continue: killed outside: get integer valueof "mypid" (19681)

-PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: detach: killed outside: get integer valueof "mypid" (443)

+PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: detach: killed outside: get integer valueof "mypid" (11368)

-PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (24003)

+PASS: gdb.threads/process-dies-while-detaching.exp: multi-process: continue: killed outside: get integer valueof "mypid" (2640)

-PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=0: inferior 1 exited

-PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=0: no threads left

+KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=0: inferior 1 exited (prompt) (PRMS: gdb/18749)

-PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: inferior 1 exited

-PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: no threads left

+KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: inferior 1 exited (prompt) (PRMS: gdb/18749)

-PASS: gdb.threads/signal-while-stepping-over-bp-other-thread.exp: step

+FAIL: gdb.threads/signal-while-stepping-over-bp-other-thread.exp: step (pattern 3)

-FAIL: gdb.threads/watchpoint-fork.exp: child: multithreaded: breakpoint (A) after the second fork (timeout)

+PASS: gdb.threads/watchpoint-fork.exp: child: multithreaded: breakpoint (A) after the second fork

-KFAIL: gdb.threads/watchthreads2.exp: gdb can drop watchpoints in multithreaded app (PRMS: gdb/10116)

+PASS: gdb.threads/watchthreads2.exp: all threads incremented x



(there were also the 4 additional PASS: records from my new test, and no failures from that test).


Peeter

Re: review request: implementing DW_AT_endianity

[Simon Marchi]

On 2017-10-10 14:38, Peeter Joot wrote:

About these:

> -PASS: gdb.threads/process-dies-while-detaching.exp: single-process:
> detach: killed outside: get integer valueof "mypid" (9535)
> 
> +PASS: gdb.threads/process-dies-while-detaching.exp: single-process:
> detach: killed outside: get integer valueof "mypid" (6194)

It's weird that get_integer_valueof outputs the value in the test name.  
I can't remember see this variation, but it's bad anyhow.  I'll post a 
patch for that.

The rest seems to be known racy test cases (not saying it's good, just 
saying it's known).

> (there were also the 4 additional PASS: records from my new test, and
> no failures from that test).

Looks good then!

Simon

Re: review request: implementing DW_AT_endianity

[Peeter Joot]

re: the gcc bug report.  Have now opened:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82509


Peeter

[PATCH] review request: implementing DW_AT_endianity

[Peeter Joot]

 * binutils/dwarf.c (read_and_display_attr_value): Handle
 DW_AT_endianity, DW_END_default, DW_END_big, DW_END_little
 * gdb/gdbtypes.h (global scope): define TYPE_ENDIANITY_BIG,
 TYPE_ENDIANITY_LITTLE.  New function: type_byte_order
 * gdb/dwarf2read.c (read_base_type): Handle DW_END_big, DW_END_little
 * gdb/gdbtypes.c (check_types_equal): Require matching
 TYPE_ENDIANITY_BIG, and TYPE_ENDIANITY_LITTLE if set.
 New function: type_byte_order.
 (recursive_dump_type): Print TYPE_ENDIANITY_BIG, and
 TYPE_ENDIANITY_LITTLE if set.
 (struct main_type): Add flag_endianity_big, flag_endianity_little
 * gdb/ada-lang.c (ada_value_binop)
 * gdb/ada-valprint.c (printstr, ada_val_print_string)
 * gdb/c-lang.c (c_get_string)
 * gdb/c-valprint.c (c_val_print_array)
 * gdb/cp-valprint.c (cp_print_class_member)
 * gdb/dwarf2loc.c (rw_pieced_value)
 * gdb/f-lang.c (f_get_encoding)
 * gdb/f-valprint.c (f_val_print)
 * gdb/findvar.c (default_read_var_value)
 * gdb/infcmd.c (default_print_one_register_info)
 * gdb/p-lang.c (pascal_printstr)
 * gdb/p-valprint.c (pascal_val_print)
 * gdb/printcmd.c (print_scalar_formatted, printf_decfloat)
 * gdb/python/py-value.c (valpy_nonzero)
 * gdb/solib-darwin.c (open_symbol_file_object)
 * gdb/solib-svr4.c (solib_svr4_r_brk)
 * gdb/stap-probe.c (stap_modify_semaphore)
 * gdb/valarith.c (value_args_as_decimal, scalar_binop, value_logical_not, value_neg)
 * gdb/valops.c (value_cast, value_one)
 * gdb/valprint.c (print_decimal_floating, generic_emit_char, generic_printstr, val_print_string)
 * gdb/value.c (value_as_address, unpack_long, unpack_bits_as_long, unpack_value_bitfield, modify_field, pack_long, pack_unsigned_long)
 use new helper function type_byte_order(, type) to replace gdbarch_byte_order (get_type_arch (type));
 * gdb/testsuite/gdb.base/endianity.c, gdb/testsuite/gdb.base/endianity.exp
 New test.  print a value in a struct with non-native endianity.
 Modify that value in a print statement and re-print the structure.
---
 ChangeLog                            |  5 ++++
 binutils/dwarf.c                     | 11 +++++++++
 gdb/ChangeLog                        | 37 ++++++++++++++++++++++++++++++
 gdb/ada-lang.c                       |  2 +-
 gdb/ada-valprint.c                   |  4 ++--
 gdb/c-lang.c                         |  2 +-
 gdb/c-valprint.c                     |  2 +-
 gdb/cp-valprint.c                    |  2 +-
 gdb/dwarf2loc.c                      |  5 ++--
 gdb/dwarf2read.c                     | 18 +++++++++++++++
 gdb/f-lang.c                         |  2 +-
 gdb/f-valprint.c                     |  2 +-
 gdb/findvar.c                        |  2 +-
 gdb/gdbtypes.c                       | 22 ++++++++++++++++++
 gdb/gdbtypes.h                       | 14 ++++++++++++
 gdb/infcmd.c                         |  2 +-
 gdb/p-lang.c                         |  2 +-
 gdb/p-valprint.c                     |  2 +-
 gdb/printcmd.c                       |  4 ++--
 gdb/python/py-value.c                |  2 +-
 gdb/solib-darwin.c                   |  5 ++--
 gdb/solib-svr4.c                     |  5 ++--
 gdb/stap-probe.c                     |  7 +++---
 gdb/testsuite/gdb.base/endianity.c   | 44 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/endianity.exp | 34 ++++++++++++++++++++++++++++
 gdb/valarith.c                       | 25 ++++++++++----------
 gdb/valops.c                         |  6 ++---
 gdb/valprint.c                       |  8 +++----
 gdb/value.c                          | 14 ++++++------
 29 files changed, 239 insertions(+), 51 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/endianity.c
 create mode 100644 gdb/testsuite/gdb.base/endianity.exp

diff --git a/ChangeLog b/ChangeLog
index b5c224ecc9..a0a2c8fe52 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-10-06  Peeter Joot  <peeter.joot@lzlabs.com>
+
+ * binutils/dwarf.c (read_and_display_attr_value): Handle
+ DW_AT_endianity, DW_END_default, DW_END_big, DW_END_little
+
 2017-09-15  Nick Clifton  <nickc@redhat.com>
 
 	* src-release.sh (LZIPPROG): New define.  Provides the name of the
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 91f95ff068..e79fd5e4b4 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -2283,6 +2283,17 @@ read_and_display_attr_value (unsigned long attribute,
 	}
       break;
 
+    case DW_AT_endianity:
+      printf ("\t");
+      switch (uvalue)
+	{
+	case DW_END_default:		printf ("(default)"); break;
+	case DW_END_big:		printf ("(big)"); break;
+	case DW_END_little:		printf ("(little)"); break;
+	default:			printf (_("(unknown endianity)")); break;
+	}
+      break;
+
     case DW_AT_virtuality:
       printf ("\t");
       switch (uvalue)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 18224e0305..6719cbace5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,40 @@
+2017-10-06  Peeter Joot  <peeter.joot@lzlabs.com>
+
+ * gdb/gdbtypes.h (global scope): define TYPE_ENDIANITY_BIG,
+ TYPE_ENDIANITY_LITTLE.  New function: type_byte_order
+ * gdb/dwarf2read.c (read_base_type): Handle DW_END_big, DW_END_little
+ * gdb/gdbtypes.c (check_types_equal): Require matching
+ TYPE_ENDIANITY_BIG, and TYPE_ENDIANITY_LITTLE if set.
+ New function: type_byte_order.
+ (recursive_dump_type): Print TYPE_ENDIANITY_BIG, and
+ TYPE_ENDIANITY_LITTLE if set.
+ (struct main_type): Add flag_endianity_big, flag_endianity_little
+ * gdb/ada-lang.c (ada_value_binop)
+ * gdb/ada-valprint.c (printstr, ada_val_print_string)
+ * gdb/c-lang.c (c_get_string)
+ * gdb/c-valprint.c (c_val_print_array)
+ * gdb/cp-valprint.c (cp_print_class_member)
+ * gdb/dwarf2loc.c (rw_pieced_value)
+ * gdb/f-lang.c (f_get_encoding)
+ * gdb/f-valprint.c (f_val_print)
+ * gdb/findvar.c (default_read_var_value)
+ * gdb/infcmd.c (default_print_one_register_info)
+ * gdb/p-lang.c (pascal_printstr)
+ * gdb/p-valprint.c (pascal_val_print)
+ * gdb/printcmd.c (print_scalar_formatted, printf_decfloat)
+ * gdb/python/py-value.c (valpy_nonzero)
+ * gdb/solib-darwin.c (open_symbol_file_object)
+ * gdb/solib-svr4.c (solib_svr4_r_brk)
+ * gdb/stap-probe.c (stap_modify_semaphore)
+ * gdb/valarith.c (value_args_as_decimal, scalar_binop, value_logical_not, value_neg)
+ * gdb/valops.c (value_cast, value_one)
+ * gdb/valprint.c (print_decimal_floating, generic_emit_char, generic_printstr, val_print_string)
+ * gdb/value.c (value_as_address, unpack_long, unpack_bits_as_long, unpack_value_bitfield, modify_field, pack_long, pack_unsigned_long)
+ use new helper function type_byte_order(, type) to replace gdbarch_byte_order (get_type_arch (type));
+ * gdb/testsuite/gdb.base/endianity.c, gdb/testsuite/gdb.base/endianity.exp
+ New test.  print a value in a struct with non-native endianity.
+ Modify that value in a print statement and re-print the structure.
+
 2017-10-06  Yao Qi  <yao.qi@linaro.org>
 
 	* Makefile.in (ALL_64_TARGET_OBS): Replace aarch64-insn.o with
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1a0c769594..cfe0d39e81 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -9761,7 +9761,7 @@ ada_value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
   val = allocate_value (type1);
   store_unsigned_integer (value_contents_raw (val),
                           TYPE_LENGTH (value_type (val)),
-			  gdbarch_byte_order (get_type_arch (type1)), v);
+			  type_byte_order (NULL, type1), v);
   return val;
 }
 
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 8095eede72..0584745d14 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -442,7 +442,7 @@ printstr (struct ui_file *stream, struct type *elttype, const gdb_byte *string,
 	  unsigned int length, int force_ellipses, int type_len,
 	  const struct value_print_options *options)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (elttype));
+  enum bfd_endian byte_order = type_byte_order (NULL, elttype);
   unsigned int i;
   unsigned int things_printed = 0;
   int in_quotes = 0;
@@ -683,7 +683,7 @@ ada_val_print_string (struct type *type, const gdb_byte *valaddr,
 		      struct value *original_value,
 		      const struct value_print_options *options)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   struct type *elttype = TYPE_TARGET_TYPE (type);
   unsigned int eltlen;
   unsigned int len;
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index f86e26ed5f..b6b863df4c 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -243,7 +243,7 @@ c_get_string (struct value *value, gdb_byte **buffer,
   struct type *element_type = TYPE_TARGET_TYPE (type);
   int req_length = *length;
   enum bfd_endian byte_order
-    = gdbarch_byte_order (get_type_arch (type));
+    = type_byte_order (NULL, type);
 
   if (element_type == NULL)
     goto error;
diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 653fed657a..767fa73907 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -246,7 +246,7 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
       LONGEST low_bound, high_bound;
       int eltlen, len;
       struct gdbarch *gdbarch = get_type_arch (type);
-      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+      enum bfd_endian byte_order = type_byte_order (gdbarch, type);
       unsigned int i = 0;	/* Number of characters printed.  */
 
       if (!get_array_bounds (type, &low_bound, &high_bound))
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index fb9bfd904f..a94391b8ef 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -755,7 +755,7 @@ void
 cp_print_class_member (const gdb_byte *valaddr, struct type *type,
 		       struct ui_file *stream, const char *prefix)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
 
   /* VAL is a byte offset into the structure type SELF_TYPE.
      Find the name of the field for that offset and
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index c485eaf8b0..9ea85f497f 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1773,8 +1773,9 @@ rw_pieced_value (struct value *v, struct value *from)
   struct piece_closure *c
     = (struct piece_closure *) value_computed_closure (v);
   gdb::byte_vector buffer;
+  struct gdbarch *gdbarch = get_type_arch (value_type (v));
   int bits_big_endian
-    = gdbarch_bits_big_endian (get_type_arch (value_type (v)));
+    = gdbarch_bits_big_endian (gdbarch);
 
   if (from != NULL)
     {
@@ -1797,7 +1798,7 @@ rw_pieced_value (struct value *v, struct value *from)
       bits_to_skip += (8 * value_offset (value_parent (v))
 		       + value_bitpos (v));
       if (from != NULL
-	  && (gdbarch_byte_order (get_type_arch (value_type (from)))
+	  && (type_byte_order (gdbarch, value_type (from))
 	      == BFD_ENDIAN_BIG))
 	{
 	  /* Use the least significant bits of FROM.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1b15adced6..0883bf1b0f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -15234,6 +15234,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   struct type *type;
   struct attribute *attr;
   int encoding = 0, bits = 0;
+  int endianity = 0;
   const char *name;
 
   attr = dwarf2_attr (die, DW_AT_encoding, cu);
@@ -15252,6 +15253,11 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
       complaint (&symfile_complaints,
 		 _("DW_AT_name missing from DW_TAG_base_type"));
     }
+  attr = dwarf2_attr (die, DW_AT_endianity, cu);
+  if (attr)
+    {
+      endianity = DW_UNSND (attr); // break here
+    }
 
   switch (encoding)
     {
@@ -15330,6 +15336,18 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
   if (name && strcmp (name, "char") == 0)
     TYPE_NOSIGN (type) = 1;
 
+  TYPE_ENDIANITY_BIG (type) = 0;
+  TYPE_ENDIANITY_LITTLE (type) = 0;
+  switch (endianity)
+    {
+      case DW_END_big:
+        TYPE_ENDIANITY_BIG (type) = 1; // break here
+        break;
+      case DW_END_little:
+        TYPE_ENDIANITY_LITTLE (type) = 1; // break here
+        break;
+    }
+
   return set_die_type (die, type, cu);
 }
 
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 073d5291f7..79ea9ff8c1 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -55,7 +55,7 @@ f_get_encoding (struct type *type)
       encoding = target_charset (get_type_arch (type));
       break;
     case 4:
-      if (gdbarch_byte_order (get_type_arch (type)) == BFD_ENDIAN_BIG)
+      if (type_byte_order (NULL, type) == BFD_ENDIAN_BIG)
 	encoding = "UTF-32BE";
       else
 	encoding = "UTF-32LE";
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index 5bcab9d525..6e6c001c30 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -215,7 +215,7 @@ f_val_print (struct type *type, int embedded_offset,
 	     const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
   int printed_field = 0; /* Number of fields printed.  */
   struct type *elttype;
   CORE_ADDR addr;
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 2bc2095bf7..f980f1cbf8 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -622,7 +622,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
       /* Put the constant back in target format. */
       v = allocate_value (type);
       store_signed_integer (value_contents_raw (v), TYPE_LENGTH (type),
-			    gdbarch_byte_order (get_type_arch (type)),
+			    type_byte_order (NULL, type),
 			    (LONGEST) SYMBOL_VALUE (var));
       VALUE_LVAL (v) = not_lval;
       return v;
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 73d445361d..5519ca5372 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3423,6 +3423,8 @@ check_types_equal (struct type *type1, struct type *type2,
       || TYPE_LENGTH (type1) != TYPE_LENGTH (type2)
       || TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)
       || TYPE_NOSIGN (type1) != TYPE_NOSIGN (type2)
+      || TYPE_ENDIANITY_BIG (type1) != TYPE_ENDIANITY_BIG (type2)
+      || TYPE_ENDIANITY_LITTLE (type1) != TYPE_ENDIANITY_LITTLE (type2)
       || TYPE_VARARGS (type1) != TYPE_VARARGS (type2)
       || TYPE_VECTOR (type1) != TYPE_VECTOR (type2)
       || TYPE_NOTTEXT (type1) != TYPE_NOTTEXT (type2)
@@ -4460,6 +4462,14 @@ recursive_dump_type (struct type *type, int spaces)
     {
       puts_filtered (" TYPE_NOSIGN");
     }
+  if (TYPE_ENDIANITY_BIG (type))
+    {
+      puts_filtered (" TYPE_ENDIANITY_BIG");
+    }
+  if (TYPE_ENDIANITY_LITTLE (type))
+    {
+      puts_filtered (" TYPE_ENDIANITY_LITTLE");
+    }
   if (TYPE_STUB (type))
     {
       puts_filtered (" TYPE_STUB");
@@ -5401,3 +5411,15 @@ _initialize_gdbtypes (void)
 			   show_strict_type_checking,
 			   &setchecklist, &showchecklist);
 }
+
+enum bfd_endian
+type_byte_order (struct gdbarch * gdbarch, struct type *type)
+{
+  if (TYPE_ENDIANITY_BIG (type))
+    return BFD_ENDIAN_BIG;
+  else if (TYPE_ENDIANITY_LITTLE (type))
+    return BFD_ENDIAN_LITTLE;
+  if (!gdbarch)
+    gdbarch = get_type_arch (type);
+  return gdbarch_byte_order (gdbarch);
+}
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 009cea90d9..28ff20c6d9 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -210,6 +210,16 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
 
 #define TYPE_NOSIGN(t)		(TYPE_MAIN_TYPE (t)->flag_nosign)
 
+/* * Mixed endian archetectures can supply dwarf instrumentation
+ * that indicates the desired endian interpretation of the variable.
+ * This indicates that the interpretation should be big-endian
+ * even if the cpu is running in little endian mode. */
+#define TYPE_ENDIANITY_BIG(t) (TYPE_MAIN_TYPE (t)->flag_endianity_big)
+
+/* * The type has a little endian interpretation even if the cpu
+ * is running in big endian mode. */
+#define TYPE_ENDIANITY_LITTLE(t) (TYPE_MAIN_TYPE (t)->flag_endianity_little)
+
 /* * This appears in a type's flags word if it is a stub type (e.g.,
    if someone referenced a type that wasn't defined in a source file
    via (struct sir_not_appearing_in_this_film *)).  */
@@ -616,6 +626,8 @@ struct main_type
   unsigned int flag_gnu_ifunc : 1;
   unsigned int flag_fixed_instance : 1;
   unsigned int flag_objfile_owned : 1;
+  unsigned int flag_endianity_big : 1;
+  unsigned int flag_endianity_little : 1;
 
   /* * True if this type was declared with "class" rather than
      "struct".  */
@@ -1951,4 +1963,6 @@ extern int type_not_allocated (const struct type *type);
 
 extern int type_not_associated (const struct type *type);
 
+extern enum bfd_endian type_byte_order (struct gdbarch *, struct type *type);
+
 #endif /* GDBTYPES_H */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 187c71f344..d31a9c8ddd 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2364,7 +2364,7 @@ default_print_one_register_info (struct ui_file *file,
     {
       struct value_print_options opts;
       const gdb_byte *valaddr = value_contents_for_printing (val);
-      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (regtype));
+      enum bfd_endian byte_order = type_byte_order (NULL, regtype);
 
       get_user_print_options (&opts);
       opts.deref_ref = 1;
diff --git a/gdb/p-lang.c b/gdb/p-lang.c
index 439a3772cb..ec2df7f29a 100644
--- a/gdb/p-lang.c
+++ b/gdb/p-lang.c
@@ -218,7 +218,7 @@ pascal_printstr (struct ui_file *stream, struct type *type,
 		 const char *encoding, int force_ellipses,
 		 const struct value_print_options *options)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   unsigned int i;
   unsigned int things_printed = 0;
   int in_quotes = 0;
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index d12b63638b..7941b0875f 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -66,7 +66,7 @@ pascal_val_print (struct type *type,
 		  const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
   unsigned int i = 0;	/* Number of characters printed */
   unsigned len;
   LONGEST low_bound, high_bound;
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index a8743f1f71..0e027f1b0b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -354,7 +354,7 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
 {
   struct gdbarch *gdbarch = get_type_arch (type);
   unsigned int len = TYPE_LENGTH (type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
 
   /* String printing should go through val_print_scalar_formatted.  */
   gdb_assert (options->format != 's');
@@ -2299,7 +2299,7 @@ printf_decfloat (struct ui_file *stream, const char *format,
   /* Parameter data.  */
   struct type *param_type = value_type (value);
   struct gdbarch *gdbarch = get_type_arch (param_type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum bfd_endian byte_order = type_byte_order (NULL, param_type);
 
   /* DFP output data.  */
   struct value *dfp_value = NULL;
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index cbbb9362ec..60683b906a 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1322,7 +1322,7 @@ valpy_nonzero (PyObject *self)
       else if (TYPE_CODE (type) == TYPE_CODE_DECFLOAT)
 	nonzero = !decimal_is_zero (value_contents (self_value->value),
 				 TYPE_LENGTH (type),
-				 gdbarch_byte_order (get_type_arch (type)));
+				 type_byte_order (NULL, type));
       else
 	/* All other values are True.  */
 	nonzero = 1;
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index 04bbf86cf1..c6ca869860 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -232,8 +232,9 @@ open_symbol_file_object (void *from_ttyp)
 static struct so_list *
 darwin_current_sos (void)
 {
-  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
-  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+  struct gdbarch *gdbarch = target_gdbarch ();
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  enum bfd_endian byte_order = type_byte_order (gdbarch, ptr_type);
   int ptr_len = TYPE_LENGTH (ptr_type);
   unsigned int image_info_size;
   struct so_list *head = NULL;
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 405de37aa5..24e523116c 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -920,9 +920,10 @@ solib_svr4_r_brk (struct svr4_info *info)
 static CORE_ADDR
 solib_svr4_r_ldsomap (struct svr4_info *info)
 {
+  struct gdbarch *gdbarch = target_gdbarch ();
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
-  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
-  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  enum bfd_endian byte_order = type_byte_order (gdbarch, ptr_type);
   ULONGEST version = 0;
 
   TRY
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index 6fa0d20280..ea51d1ae49 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1403,8 +1403,8 @@ stap_modify_semaphore (CORE_ADDR address, int set, struct gdbarch *gdbarch)
       return;
     }
 
-  value = extract_unsigned_integer (bytes, TYPE_LENGTH (type),
-				    gdbarch_byte_order (gdbarch));
+  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
+  value = extract_unsigned_integer (bytes, TYPE_LENGTH (type), byte_order);
   /* Note that we explicitly don't worry about overflow or
      underflow.  */
   if (set)
@@ -1412,8 +1412,7 @@ stap_modify_semaphore (CORE_ADDR address, int set, struct gdbarch *gdbarch)
   else
     --value;
 
-  store_unsigned_integer (bytes, TYPE_LENGTH (type),
-			  gdbarch_byte_order (gdbarch), value);
+  store_unsigned_integer (bytes, TYPE_LENGTH (type), byte_order, value);
 
   if (target_write_memory (address, bytes, TYPE_LENGTH (type)) != 0)
     warning (_("Could not write the value of a SystemTap semaphore."));
diff --git a/gdb/testsuite/gdb.base/endianity.c b/gdb/testsuite/gdb.base/endianity.c
new file mode 100644
index 0000000000..330395a970
--- /dev/null
+++ b/gdb/testsuite/gdb.base/endianity.c
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This tests the handling of dwarf attributes:
+    DW_AT_endianity, DW_END_big, and DW_END_little.  */
+struct otherendian
+{
+   int v;
+}
+#if defined __GNUC__ && (__GNUC__ >= 6)
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+__attribute__( ( scalar_storage_order( "big-endian" ) ) )
+#else
+__attribute__( ( scalar_storage_order( "little-endian" ) ) )
+#endif
+#endif
+;
+
+void
+do_nothing (struct otherendian *c)
+{
+}
+
+int
+main (void)
+{
+  struct otherendian o = {3};
+
+  do_nothing (&o); /* START */
+}
diff --git a/gdb/testsuite/gdb.base/endianity.exp b/gdb/testsuite/gdb.base/endianity.exp
new file mode 100644
index 0000000000..e125838dfd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/endianity.exp
@@ -0,0 +1,34 @@
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile .c
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+set bp_location [gdb_get_line_number "START"]
+if ![runto "endianity.c:$bp_location" ] then {
+  return -1
+}
+
+gdb_test "print o" "= {v = 3}"
+
+gdb_test "print o.v = 4" "= 4"
+
+# expect this to fail if compiled with < gcc6.
+gdb_test "x/x &o.v" "0x04000000"
+
+gdb_test "print o" "= {v = 4}"
diff --git a/gdb/valarith.c b/gdb/valarith.c
index ede60e4b68..bcb0372830 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -873,13 +873,13 @@ value_args_as_decimal (struct value *arg1, struct value *arg2,
 
   if (TYPE_CODE (type1) == TYPE_CODE_DECFLOAT)
     {
-      *byte_order_x = gdbarch_byte_order (get_type_arch (type1));
+      *byte_order_x = type_byte_order (NULL, type1);
       *len_x = TYPE_LENGTH (type1);
       memcpy (x, value_contents (arg1), *len_x);
     }
   else if (is_integral_type (type1))
     {
-      *byte_order_x = gdbarch_byte_order (get_type_arch (type2));
+      *byte_order_x = type_byte_order (NULL, type2);
       *len_x = TYPE_LENGTH (type2);
       if (TYPE_UNSIGNED (type1))
 	decimal_from_ulongest (value_as_long (arg1), x, *len_x, *byte_order_x);
@@ -895,13 +895,13 @@ value_args_as_decimal (struct value *arg1, struct value *arg2,
 
   if (TYPE_CODE (type2) == TYPE_CODE_DECFLOAT)
     {
-      *byte_order_y = gdbarch_byte_order (get_type_arch (type2));
+      *byte_order_y = type_byte_order (NULL, type2);
       *len_y = TYPE_LENGTH (type2);
       memcpy (y, value_contents (arg2), *len_y);
     }
   else if (is_integral_type (type2))
     {
-      *byte_order_y = gdbarch_byte_order (get_type_arch (type1));
+      *byte_order_y = type_byte_order (NULL, type1);
       *len_y = TYPE_LENGTH (type1);
       if (TYPE_UNSIGNED (type2))
 	decimal_from_ulongest (value_as_long (arg2), y, *len_y, *byte_order_y);
@@ -930,6 +930,7 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 
   type1 = check_typedef (value_type (arg1));
   type2 = check_typedef (value_type (arg2));
+  struct gdbarch *gdbarch = get_type_arch (type1);
 
   if ((TYPE_CODE (type1) != TYPE_CODE_FLT
        && TYPE_CODE (type1) != TYPE_CODE_DECFLOAT
@@ -959,7 +960,7 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 	result_type = type1;
 
       len_v = TYPE_LENGTH (result_type);
-      byte_order_v = gdbarch_byte_order (get_type_arch (result_type));
+      byte_order_v = type_byte_order (gdbarch, result_type);
 
       value_args_as_decimal (arg1, arg2, v1, &len_v1, &byte_order_v1,
 					 v2, &len_v2, &byte_order_v2);
@@ -1084,7 +1085,7 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
       val = allocate_value (result_type);
       store_signed_integer (value_contents_raw (val),
 			    TYPE_LENGTH (result_type),
-			    gdbarch_byte_order (get_type_arch (result_type)),
+			    type_byte_order (gdbarch, result_type),
 			    v);
     }
   else
@@ -1232,8 +1233,8 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 	  val = allocate_value (result_type);
 	  store_unsigned_integer (value_contents_raw (val),
 				  TYPE_LENGTH (value_type (val)),
-				  gdbarch_byte_order
-				    (get_type_arch (result_type)),
+				  type_byte_order
+				    (gdbarch, result_type),
 				  v);
 	}
       else
@@ -1362,8 +1363,8 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 	  val = allocate_value (result_type);
 	  store_signed_integer (value_contents_raw (val),
 				TYPE_LENGTH (value_type (val)),
-				gdbarch_byte_order
-				  (get_type_arch (result_type)),
+				type_byte_order
+				  (gdbarch, result_type),
 				v);
 	}
     }
@@ -1518,7 +1519,7 @@ value_logical_not (struct value *arg1)
     return 0 == value_as_double (arg1);
   else if (TYPE_CODE (type1) == TYPE_CODE_DECFLOAT)
     return decimal_is_zero (value_contents (arg1), TYPE_LENGTH (type1),
-			    gdbarch_byte_order (get_type_arch (type1)));
+			    type_byte_order (NULL, type1));
 
   len = TYPE_LENGTH (type1);
   p = value_contents (arg1);
@@ -1774,7 +1775,7 @@ value_neg (struct value *arg1)
 
       memcpy (decbytes, value_contents (arg1), len);
 
-      if (gdbarch_byte_order (get_type_arch (type)) == BFD_ENDIAN_LITTLE)
+      if (type_byte_order (NULL, type) == BFD_ENDIAN_LITTLE)
 	decbytes[len-1] = decbytes[len - 1] | 0x80;
       else
 	decbytes[0] = decbytes[0] | 0x80;
diff --git a/gdb/valops.c b/gdb/valops.c
index de4544cd29..c12cd17b2e 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -466,7 +466,7 @@ value_cast (struct type *type, struct value *arg2)
     return value_from_double (to_type, value_as_double (arg2));
   else if (code1 == TYPE_CODE_DECFLOAT && scalar)
     {
-      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+      enum bfd_endian byte_order = type_byte_order (NULL, type);
       int dec_len = TYPE_LENGTH (type);
       gdb_byte dec[16];
 
@@ -502,7 +502,7 @@ value_cast (struct type *type, struct value *arg2)
       if (code2 == TYPE_CODE_PTR)
         longest = extract_unsigned_integer
 		    (value_contents (arg2), TYPE_LENGTH (type2),
-		     gdbarch_byte_order (get_type_arch (type2)));
+		     type_byte_order (NULL, type2));
       else
         longest = value_as_long (arg2);
       return value_from_longest (to_type, convert_to_boolean ?
@@ -870,7 +870,7 @@ value_one (struct type *type)
 
   if (TYPE_CODE (type1) == TYPE_CODE_DECFLOAT)
     {
-      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+      enum bfd_endian byte_order = type_byte_order (NULL, type);
       gdb_byte v[16];
 
       decimal_from_string (v, TYPE_LENGTH (type), byte_order, "1");
diff --git a/gdb/valprint.c b/gdb/valprint.c
index ca0c4768c9..73b7306fab 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1471,7 +1471,7 @@ void
 print_decimal_floating (const gdb_byte *valaddr, struct type *type,
 			struct ui_file *stream)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   unsigned len = TYPE_LENGTH (type);
 
   std::string str = decimal_to_string (valaddr, len, byte_order);
@@ -2478,7 +2478,7 @@ generic_emit_char (int c, struct type *type, struct ui_file *stream,
 		   int quoter, const char *encoding)
 {
   enum bfd_endian byte_order
-    = gdbarch_byte_order (get_type_arch (type));
+    = type_byte_order (NULL, type);
   gdb_byte *buf;
   int need_escape = 0;
 
@@ -2799,7 +2799,7 @@ generic_printstr (struct ui_file *stream, struct type *type,
 		  int quote_char, int c_style_terminator,
 		  const struct value_print_options *options)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   unsigned int i;
   int width = TYPE_LENGTH (type);
   struct cleanup *cleanup;
@@ -2918,7 +2918,7 @@ val_print_string (struct type *elttype, const char *encoding,
   gdb_byte *buffer = NULL;	/* Dynamically growable fetch buffer.  */
   struct cleanup *old_chain = NULL;	/* Top of the old cleanup chain.  */
   struct gdbarch *gdbarch = get_type_arch (elttype);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum bfd_endian byte_order = type_byte_order (gdbarch, elttype);
   int width = TYPE_LENGTH (elttype);
 
   /* First we need to figure out the limit on the number of characters we are
diff --git a/gdb/value.c b/gdb/value.c
index 90423ed002..82e0065256 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2900,7 +2900,7 @@ value_as_address (struct value *val)
 LONGEST
 unpack_long (struct type *type, const gdb_byte *valaddr)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   enum type_code code = TYPE_CODE (type);
   int len = TYPE_LENGTH (type);
   int nosign = TYPE_UNSIGNED (type);
@@ -2949,7 +2949,7 @@ unpack_long (struct type *type, const gdb_byte *valaddr)
 DOUBLEST
 unpack_double (struct type *type, const gdb_byte *valaddr, int *invp)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   enum type_code code;
   int len;
   int nosign;
@@ -3305,7 +3305,7 @@ static LONGEST
 unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr,
 		     LONGEST bitpos, LONGEST bitsize)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (field_type));
+  enum bfd_endian byte_order = type_byte_order (NULL, field_type);
   ULONGEST val;
   ULONGEST valmask;
   int lsbcount;
@@ -3411,7 +3411,7 @@ unpack_value_bitfield (struct value *dest_val,
   int dst_bit_offset;
   struct type *field_type = value_type (dest_val);
 
-  byte_order = gdbarch_byte_order (get_type_arch (field_type));
+  byte_order = type_byte_order (NULL, field_type);
 
   /* First, unpack and sign extend the bitfield as if it was wholly
      valid.  Optimized out/unavailable bits are read as zero, but
@@ -3471,7 +3471,7 @@ void
 modify_field (struct type *type, gdb_byte *addr,
 	      LONGEST fieldval, LONGEST bitpos, LONGEST bitsize)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   ULONGEST oword;
   ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);
   LONGEST bytesize;
@@ -3517,7 +3517,7 @@ modify_field (struct type *type, gdb_byte *addr,
 void
 pack_long (gdb_byte *buf, struct type *type, LONGEST num)
 {
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+  enum bfd_endian byte_order = type_byte_order (NULL, type);
   LONGEST len;
 
   type = check_typedef (type);
@@ -3558,7 +3558,7 @@ pack_unsigned_long (gdb_byte *buf, struct type *type, ULONGEST num)
 
   type = check_typedef (type);
   len = TYPE_LENGTH (type);
-  byte_order = gdbarch_byte_order (get_type_arch (type));
+  byte_order = type_byte_order (NULL, type);
 
   switch (TYPE_CODE (type))
     {
-- 
2.13.5 (Apple Git-94)

Re: [PATCH] review request: implementing DW_AT_endianity

[Peeter Joot]

- ignore the binutils/dwarf.c change which I've now submitted separately to binutils@sourceware.

- also ignore the // break here comments, which I've also removed from my branch (left over from debuggin).

Re: [PATCH] review request: implementing DW_AT_endianity

[Simon Marchi]

Hi Peter,

Thanks for the update.  Your commit should include a message that summarizes what the
code does, the limitations, anything you think would be useful to a reader.

A few words about the ChangeLog.  First, we generally don't include the ChangeLog in the patch/diff
itself when sending it to the list.  The reason is that trying to apply the patch will almost
certainly result in a conflict.  Instead, we only include it in the commit message, and then put it
in the real ChangeLog file when pushing the code.

Second, each entry in the ChangeLog should be place in the ChangeLog closest to it in the hierarchy:

$ find binutils gdb -name ChangeLog | sort
binutils/ChangeLog
gdb/ChangeLog
gdb/doc/ChangeLog
gdb/gdbserver/ChangeLog
gdb/stubs/ChangeLog
gdb/testsuite/ChangeLog

For example, this entry:

	* gdb/ada-lang.c (ada_value_binop)

should go in gdb/ChangeLog, and omit the gdb/ part:

	* ada-lang.c (ada_value_binop): ...

Finally, make sure the lines start with a tab, and are not too long (74 columns).  Read this for more
details: https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog

See more comments below.  Once you addressed them, you can send a new version again with git-send-email,
make sure to pass --subject-prefix="PATCH v2".

On 2017-10-10 07:30 PM, Peeter Joot wrote:
>  * binutils/dwarf.c (read_and_display_attr_value): Handle
>  DW_AT_endianity, DW_END_default, DW_END_big, DW_END_little
>  * gdb/gdbtypes.h (global scope): define TYPE_ENDIANITY_BIG,
>  TYPE_ENDIANITY_LITTLE.  New function: type_byte_order

No need to say "global scope".  Here's how I would do it:

	* gdbtypes.h (TYPE_ENDIANITY_BIG): New macro.
	(TYPE_ENDIANITY_LITTLE): New macro.
	(struct main_type) <flag_endianity_big>: New field.
	<flag_endianity_little>: New field.
	(type_byte_order): New function.

You can also collapse multiple items that have the same message, but it is optional:

	* gdbtypes.h (TYPE_ENDIANITY_BIG, TYPE_ENDIANITY_LITTLE): New
	macros.
	(struct main_type) <flag_endianity_big, flag_endianity_little>:
	New fields.
	(type_byte_order): New function.

>  * gdb/dwarf2read.c (read_base_type): Handle DW_END_big, DW_END_little
>  * gdb/gdbtypes.c (check_types_equal): Require matching
>  TYPE_ENDIANITY_BIG, and TYPE_ENDIANITY_LITTLE if set.
>  New function: type_byte_order.
>  (recursive_dump_type): Print TYPE_ENDIANITY_BIG, and
>  TYPE_ENDIANITY_LITTLE if set.
>  (struct main_type): Add flag_endianity_big, flag_endianity_little

This last one should go in gdbtypes.h.

>  * gdb/ada-lang.c (ada_value_binop)

Each entry should have some text and finish with a period.  For example,

	* ada-lang.c (ada_value_binop): Use type_byte_order instead of
	gdbarch_byte_order.

>  * gdb/ada-valprint.c (printstr, ada_val_print_string)
>  * gdb/c-lang.c (c_get_string)
>  * gdb/c-valprint.c (c_val_print_array)
>  * gdb/cp-valprint.c (cp_print_class_member)
>  * gdb/dwarf2loc.c (rw_pieced_value)
>  * gdb/f-lang.c (f_get_encoding)
>  * gdb/f-valprint.c (f_val_print)
>  * gdb/findvar.c (default_read_var_value)
>  * gdb/infcmd.c (default_print_one_register_info)
>  * gdb/p-lang.c (pascal_printstr)
>  * gdb/p-valprint.c (pascal_val_print)
>  * gdb/printcmd.c (print_scalar_formatted, printf_decfloat)
>  * gdb/python/py-value.c (valpy_nonzero)
>  * gdb/solib-darwin.c (open_symbol_file_object)
>  * gdb/solib-svr4.c (solib_svr4_r_brk)
>  * gdb/stap-probe.c (stap_modify_semaphore)
>  * gdb/valarith.c (value_args_as_decimal, scalar_binop, value_logical_not, value_neg)
>  * gdb/valops.c (value_cast, value_one)
>  * gdb/valprint.c (print_decimal_floating, generic_emit_char, generic_printstr, val_print_string)
>  * gdb/value.c (value_as_address, unpack_long, unpack_bits_as_long, unpack_value_bitfield, modify_field, pack_long, pack_unsigned_long)
>  use new helper function type_byte_order(, type) to replace gdbarch_byte_order (get_type_arch (type));
>  * gdb/testsuite/gdb.base/endianity.c, gdb/testsuite/gdb.base/endianity.exp
>  New test.  print a value in a struct with non-native endianity.
>  Modify that value in a print statement and re-print the structure.

When adding a file, you can be brief and just say "New file.".

> ---
>  ChangeLog                            |  5 ++++
>  binutils/dwarf.c                     | 11 +++++++++
>  gdb/ChangeLog                        | 37 ++++++++++++++++++++++++++++++
>  gdb/ada-lang.c                       |  2 +-
>  gdb/ada-valprint.c                   |  4 ++--
>  gdb/c-lang.c                         |  2 +-
>  gdb/c-valprint.c                     |  2 +-
>  gdb/cp-valprint.c                    |  2 +-
>  gdb/dwarf2loc.c                      |  5 ++--
>  gdb/dwarf2read.c                     | 18 +++++++++++++++
>  gdb/f-lang.c                         |  2 +-
>  gdb/f-valprint.c                     |  2 +-
>  gdb/findvar.c                        |  2 +-
>  gdb/gdbtypes.c                       | 22 ++++++++++++++++++
>  gdb/gdbtypes.h                       | 14 ++++++++++++
>  gdb/infcmd.c                         |  2 +-
>  gdb/p-lang.c                         |  2 +-
>  gdb/p-valprint.c                     |  2 +-
>  gdb/printcmd.c                       |  4 ++--
>  gdb/python/py-value.c                |  2 +-
>  gdb/solib-darwin.c                   |  5 ++--
>  gdb/solib-svr4.c                     |  5 ++--
>  gdb/stap-probe.c                     |  7 +++---
>  gdb/testsuite/gdb.base/endianity.c   | 44 ++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/endianity.exp | 34 ++++++++++++++++++++++++++++
>  gdb/valarith.c                       | 25 ++++++++++----------
>  gdb/valops.c                         |  6 ++---
>  gdb/valprint.c                       |  8 +++----
>  gdb/value.c                          | 14 ++++++------
>  29 files changed, 239 insertions(+), 51 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/endianity.c
>  create mode 100644 gdb/testsuite/gdb.base/endianity.exp
> 
> diff --git a/ChangeLog b/ChangeLog
> index b5c224ecc9..a0a2c8fe52 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2017-10-06  Peeter Joot  <peeter.joot@lzlabs.com>
> +
> + * binutils/dwarf.c (read_and_display_attr_value): Handle
> + DW_AT_endianity, DW_END_default, DW_END_big, DW_END_little
> +
>  2017-09-15  Nick Clifton  <nickc@redhat.com>
>  
>  	* src-release.sh (LZIPPROG): New define.  Provides the name of the
> diff --git a/binutils/dwarf.c b/binutils/dwarf.c
> index 91f95ff068..e79fd5e4b4 100644
> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -2283,6 +2283,17 @@ read_and_display_attr_value (unsigned long attribute,
>  	}
>        break;
>  
> +    case DW_AT_endianity:
> +      printf ("\t");
> +      switch (uvalue)
> +	{
> +	case DW_END_default:		printf ("(default)"); break;
> +	case DW_END_big:		printf ("(big)"); break;
> +	case DW_END_little:		printf ("(little)"); break;
> +	default:			printf (_("(unknown endianity)")); break;
> +	}
> +      break;
> +
>      case DW_AT_virtuality:
>        printf ("\t");
>        switch (uvalue)
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 18224e0305..6719cbace5 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,40 @@
> +2017-10-06  Peeter Joot  <peeter.joot@lzlabs.com>
> +
> + * gdb/gdbtypes.h (global scope): define TYPE_ENDIANITY_BIG,
> + TYPE_ENDIANITY_LITTLE.  New function: type_byte_order
> + * gdb/dwarf2read.c (read_base_type): Handle DW_END_big, DW_END_little
> + * gdb/gdbtypes.c (check_types_equal): Require matching
> + TYPE_ENDIANITY_BIG, and TYPE_ENDIANITY_LITTLE if set.
> + New function: type_byte_order.
> + (recursive_dump_type): Print TYPE_ENDIANITY_BIG, and
> + TYPE_ENDIANITY_LITTLE if set.
> + (struct main_type): Add flag_endianity_big, flag_endianity_little
> + * gdb/ada-lang.c (ada_value_binop)
> + * gdb/ada-valprint.c (printstr, ada_val_print_string)
> + * gdb/c-lang.c (c_get_string)
> + * gdb/c-valprint.c (c_val_print_array)
> + * gdb/cp-valprint.c (cp_print_class_member)
> + * gdb/dwarf2loc.c (rw_pieced_value)
> + * gdb/f-lang.c (f_get_encoding)
> + * gdb/f-valprint.c (f_val_print)
> + * gdb/findvar.c (default_read_var_value)
> + * gdb/infcmd.c (default_print_one_register_info)
> + * gdb/p-lang.c (pascal_printstr)
> + * gdb/p-valprint.c (pascal_val_print)
> + * gdb/printcmd.c (print_scalar_formatted, printf_decfloat)
> + * gdb/python/py-value.c (valpy_nonzero)
> + * gdb/solib-darwin.c (open_symbol_file_object)
> + * gdb/solib-svr4.c (solib_svr4_r_brk)
> + * gdb/stap-probe.c (stap_modify_semaphore)
> + * gdb/valarith.c (value_args_as_decimal, scalar_binop, value_logical_not, value_neg)
> + * gdb/valops.c (value_cast, value_one)
> + * gdb/valprint.c (print_decimal_floating, generic_emit_char, generic_printstr, val_print_string)
> + * gdb/value.c (value_as_address, unpack_long, unpack_bits_as_long, unpack_value_bitfield, modify_field, pack_long, pack_unsigned_long)
> + use new helper function type_byte_order(, type) to replace gdbarch_byte_order (get_type_arch (type));
> + * gdb/testsuite/gdb.base/endianity.c, gdb/testsuite/gdb.base/endianity.exp
> + New test.  print a value in a struct with non-native endianity.
> + Modify that value in a print statement and re-print the structure.
> +
>  2017-10-06  Yao Qi  <yao.qi@linaro.org>
>  
>  	* Makefile.in (ALL_64_TARGET_OBS): Replace aarch64-insn.o with
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 1a0c769594..cfe0d39e81 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -9761,7 +9761,7 @@ ada_value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>    val = allocate_value (type1);
>    store_unsigned_integer (value_contents_raw (val),
>                            TYPE_LENGTH (value_type (val)),
> -			  gdbarch_byte_order (get_type_arch (type1)), v);
> +			  type_byte_order (NULL, type1), v);
>    return val;
>  }
>  
> diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
> index 8095eede72..0584745d14 100644
> --- a/gdb/ada-valprint.c
> +++ b/gdb/ada-valprint.c
> @@ -442,7 +442,7 @@ printstr (struct ui_file *stream, struct type *elttype, const gdb_byte *string,
>  	  unsigned int length, int force_ellipses, int type_len,
>  	  const struct value_print_options *options)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (elttype));
> +  enum bfd_endian byte_order = type_byte_order (NULL, elttype);
>    unsigned int i;
>    unsigned int things_printed = 0;
>    int in_quotes = 0;
> @@ -683,7 +683,7 @@ ada_val_print_string (struct type *type, const gdb_byte *valaddr,
>  		      struct value *original_value,
>  		      const struct value_print_options *options)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    struct type *elttype = TYPE_TARGET_TYPE (type);
>    unsigned int eltlen;
>    unsigned int len;
> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index f86e26ed5f..b6b863df4c 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -243,7 +243,7 @@ c_get_string (struct value *value, gdb_byte **buffer,
>    struct type *element_type = TYPE_TARGET_TYPE (type);
>    int req_length = *length;
>    enum bfd_endian byte_order
> -    = gdbarch_byte_order (get_type_arch (type));
> +    = type_byte_order (NULL, type);
>  
>    if (element_type == NULL)
>      goto error;
> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index 653fed657a..767fa73907 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -246,7 +246,7 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
>        LONGEST low_bound, high_bound;
>        int eltlen, len;
>        struct gdbarch *gdbarch = get_type_arch (type);
> -      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +      enum bfd_endian byte_order = type_byte_order (gdbarch, type);

Here, gdbarch is only used for passing here, so you could pass NULL instead
and remove the gdbarch variable.

>        unsigned int i = 0;	/* Number of characters printed.  */
>  
>        if (!get_array_bounds (type, &low_bound, &high_bound))
> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
> index fb9bfd904f..a94391b8ef 100644
> --- a/gdb/cp-valprint.c
> +++ b/gdb/cp-valprint.c
> @@ -755,7 +755,7 @@ void
>  cp_print_class_member (const gdb_byte *valaddr, struct type *type,
>  		       struct ui_file *stream, const char *prefix)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>  
>    /* VAL is a byte offset into the structure type SELF_TYPE.
>       Find the name of the field for that offset and
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index c485eaf8b0..9ea85f497f 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -1773,8 +1773,9 @@ rw_pieced_value (struct value *v, struct value *from)
>    struct piece_closure *c
>      = (struct piece_closure *) value_computed_closure (v);
>    gdb::byte_vector buffer;
> +  struct gdbarch *gdbarch = get_type_arch (value_type (v));
>    int bits_big_endian
> -    = gdbarch_bits_big_endian (get_type_arch (value_type (v)));
> +    = gdbarch_bits_big_endian (gdbarch);
>  
>    if (from != NULL)
>      {
> @@ -1797,7 +1798,7 @@ rw_pieced_value (struct value *v, struct value *from)
>        bits_to_skip += (8 * value_offset (value_parent (v))
>  		       + value_bitpos (v));
>        if (from != NULL
> -	  && (gdbarch_byte_order (get_type_arch (value_type (from)))
> +	  && (type_byte_order (gdbarch, value_type (from))
>  	      == BFD_ENDIAN_BIG))
>  	{
>  	  /* Use the least significant bits of FROM.  */
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 1b15adced6..0883bf1b0f 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -15234,6 +15234,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
>    struct type *type;
>    struct attribute *attr;
>    int encoding = 0, bits = 0;
> +  int endianity = 0;
>    const char *name;
>  
>    attr = dwarf2_attr (die, DW_AT_encoding, cu);
> @@ -15252,6 +15253,11 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
>        complaint (&symfile_complaints,
>  		 _("DW_AT_name missing from DW_TAG_base_type"));
>      }
> +  attr = dwarf2_attr (die, DW_AT_endianity, cu);
> +  if (attr)
> +    {
> +      endianity = DW_UNSND (attr); // break here

Remove the comment and the curly braces.

> +    }
>  
>    switch (encoding)
>      {
> @@ -15330,6 +15336,18 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
>    if (name && strcmp (name, "char") == 0)
>      TYPE_NOSIGN (type) = 1;
>  
> +  TYPE_ENDIANITY_BIG (type) = 0;
> +  TYPE_ENDIANITY_LITTLE (type) = 0;

This shouldn't be necessary, as it's assumed that the type object is
zero-ed out (see alloc_type).

> +  switch (endianity)
> +    {
> +      case DW_END_big:
> +        TYPE_ENDIANITY_BIG (type) = 1; // break here
> +        break;
> +      case DW_END_little:
> +        TYPE_ENDIANITY_LITTLE (type) = 1; // break here

Remove comments.

> +        break;
> +    }
> +
>    return set_die_type (die, type, cu);
>  }
>  
> diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> index 073d5291f7..79ea9ff8c1 100644
> --- a/gdb/f-lang.c
> +++ b/gdb/f-lang.c
> @@ -55,7 +55,7 @@ f_get_encoding (struct type *type)
>        encoding = target_charset (get_type_arch (type));
>        break;
>      case 4:
> -      if (gdbarch_byte_order (get_type_arch (type)) == BFD_ENDIAN_BIG)
> +      if (type_byte_order (NULL, type) == BFD_ENDIAN_BIG)
>  	encoding = "UTF-32BE";
>        else
>  	encoding = "UTF-32LE";
> diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
> index 5bcab9d525..6e6c001c30 100644
> --- a/gdb/f-valprint.c
> +++ b/gdb/f-valprint.c
> @@ -215,7 +215,7 @@ f_val_print (struct type *type, int embedded_offset,
>  	     const struct value_print_options *options)
>  {
>    struct gdbarch *gdbarch = get_type_arch (type);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
>    int printed_field = 0; /* Number of fields printed.  */
>    struct type *elttype;
>    CORE_ADDR addr;
> diff --git a/gdb/findvar.c b/gdb/findvar.c
> index 2bc2095bf7..f980f1cbf8 100644
> --- a/gdb/findvar.c
> +++ b/gdb/findvar.c
> @@ -622,7 +622,7 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
>        /* Put the constant back in target format. */
>        v = allocate_value (type);
>        store_signed_integer (value_contents_raw (v), TYPE_LENGTH (type),
> -			    gdbarch_byte_order (get_type_arch (type)),
> +			    type_byte_order (NULL, type),
>  			    (LONGEST) SYMBOL_VALUE (var));
>        VALUE_LVAL (v) = not_lval;
>        return v;
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 73d445361d..5519ca5372 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -3423,6 +3423,8 @@ check_types_equal (struct type *type1, struct type *type2,
>        || TYPE_LENGTH (type1) != TYPE_LENGTH (type2)
>        || TYPE_UNSIGNED (type1) != TYPE_UNSIGNED (type2)
>        || TYPE_NOSIGN (type1) != TYPE_NOSIGN (type2)
> +      || TYPE_ENDIANITY_BIG (type1) != TYPE_ENDIANITY_BIG (type2)
> +      || TYPE_ENDIANITY_LITTLE (type1) != TYPE_ENDIANITY_LITTLE (type2)
>        || TYPE_VARARGS (type1) != TYPE_VARARGS (type2)
>        || TYPE_VECTOR (type1) != TYPE_VECTOR (type2)
>        || TYPE_NOTTEXT (type1) != TYPE_NOTTEXT (type2)
> @@ -4460,6 +4462,14 @@ recursive_dump_type (struct type *type, int spaces)
>      {
>        puts_filtered (" TYPE_NOSIGN");
>      }
> +  if (TYPE_ENDIANITY_BIG (type))
> +    {
> +      puts_filtered (" TYPE_ENDIANITY_BIG");
> +    }
> +  if (TYPE_ENDIANITY_LITTLE (type))
> +    {
> +      puts_filtered (" TYPE_ENDIANITY_LITTLE");
> +    }
>    if (TYPE_STUB (type))
>      {
>        puts_filtered (" TYPE_STUB");
> @@ -5401,3 +5411,15 @@ _initialize_gdbtypes (void)
>  			   show_strict_type_checking,
>  			   &setchecklist, &showchecklist);
>  }
> +
> +enum bfd_endian
> +type_byte_order (struct gdbarch * gdbarch, struct type *type)

Remove the space after *.

Add a comment above this definition:

/* See gdbtypes.h.  */

> +{
> +  if (TYPE_ENDIANITY_BIG (type))
> +    return BFD_ENDIAN_BIG;
> +  else if (TYPE_ENDIANITY_LITTLE (type))
> +    return BFD_ENDIAN_LITTLE;
> +  if (!gdbarch)
> +    gdbarch = get_type_arch (type);
> +  return gdbarch_byte_order (gdbarch);

To simplify the API, I think we could omit passing the gdbarch, and
always call get_type_arch.  It's not very costly, and would avoid having
to pass NULL at many places.

> +}
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 009cea90d9..28ff20c6d9 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -210,6 +210,16 @@ DEF_ENUM_FLAGS_TYPE (enum type_instance_flag_value, type_instance_flags);
>  
>  #define TYPE_NOSIGN(t)		(TYPE_MAIN_TYPE (t)->flag_nosign)
>  
> +/* * Mixed endian archetectures can supply dwarf instrumentation

"archetectures"

> + * that indicates the desired endian interpretation of the variable.
> + * This indicates that the interpretation should be big-endian
> + * even if the cpu is running in little endian mode. */

Remove the stars at the beginning of each line, finish with two spaces:

/* Mixed endian architectures can supply dwarf instrumentation
   that indicates the desired endian interpretation of the variable.
   This indicates that the interpretation should be big-endian
   even if the cpu is running in little endian mode.  */

For consistency with what's around, add an empty line before the macro
(although I'm not entirely sure what's the rule for that).

> +#define TYPE_ENDIANITY_BIG(t) (TYPE_MAIN_TYPE (t)->flag_endianity_big)
> +
> +/* * The type has a little endian interpretation even if the cpu
> + * is running in big endian mode. */

Same here.

> +#define TYPE_ENDIANITY_LITTLE(t) (TYPE_MAIN_TYPE (t)->flag_endianity_little)
> +
>  /* * This appears in a type's flags word if it is a stub type (e.g.,
>     if someone referenced a type that wasn't defined in a source file
>     via (struct sir_not_appearing_in_this_film *)).  */
> @@ -616,6 +626,8 @@ struct main_type
>    unsigned int flag_gnu_ifunc : 1;
>    unsigned int flag_fixed_instance : 1;
>    unsigned int flag_objfile_owned : 1;
> +  unsigned int flag_endianity_big : 1;
> +  unsigned int flag_endianity_little : 1;
>  
>    /* * True if this type was declared with "class" rather than
>       "struct".  */
> @@ -1951,4 +1963,6 @@ extern int type_not_allocated (const struct type *type);
>  
>  extern int type_not_associated (const struct type *type);
>  
> +extern enum bfd_endian type_byte_order (struct gdbarch *, struct type *type);

Add a comment above this declaration to describe what it does.

> +
>  #endif /* GDBTYPES_H */
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 187c71f344..d31a9c8ddd 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -2364,7 +2364,7 @@ default_print_one_register_info (struct ui_file *file,
>      {
>        struct value_print_options opts;
>        const gdb_byte *valaddr = value_contents_for_printing (val);
> -      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (regtype));
> +      enum bfd_endian byte_order = type_byte_order (NULL, regtype);
>  
>        get_user_print_options (&opts);
>        opts.deref_ref = 1;
> diff --git a/gdb/p-lang.c b/gdb/p-lang.c
> index 439a3772cb..ec2df7f29a 100644
> --- a/gdb/p-lang.c
> +++ b/gdb/p-lang.c
> @@ -218,7 +218,7 @@ pascal_printstr (struct ui_file *stream, struct type *type,
>  		 const char *encoding, int force_ellipses,
>  		 const struct value_print_options *options)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    unsigned int i;
>    unsigned int things_printed = 0;
>    int in_quotes = 0;
> diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
> index d12b63638b..7941b0875f 100644
> --- a/gdb/p-valprint.c
> +++ b/gdb/p-valprint.c
> @@ -66,7 +66,7 @@ pascal_val_print (struct type *type,
>  		  const struct value_print_options *options)
>  {
>    struct gdbarch *gdbarch = get_type_arch (type);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
>    unsigned int i = 0;	/* Number of characters printed */
>    unsigned len;
>    LONGEST low_bound, high_bound;
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index a8743f1f71..0e027f1b0b 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -354,7 +354,7 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>  {
>    struct gdbarch *gdbarch = get_type_arch (type);
>    unsigned int len = TYPE_LENGTH (type);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
>  
>    /* String printing should go through val_print_scalar_formatted.  */
>    gdb_assert (options->format != 's');
> @@ -2299,7 +2299,7 @@ printf_decfloat (struct ui_file *stream, const char *format,
>    /* Parameter data.  */
>    struct type *param_type = value_type (value);
>    struct gdbarch *gdbarch = get_type_arch (param_type);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum bfd_endian byte_order = type_byte_order (NULL, param_type);
>  
>    /* DFP output data.  */
>    struct value *dfp_value = NULL;
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index cbbb9362ec..60683b906a 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -1322,7 +1322,7 @@ valpy_nonzero (PyObject *self)
>        else if (TYPE_CODE (type) == TYPE_CODE_DECFLOAT)
>  	nonzero = !decimal_is_zero (value_contents (self_value->value),
>  				 TYPE_LENGTH (type),
> -				 gdbarch_byte_order (get_type_arch (type)));
> +				 type_byte_order (NULL, type));
>        else
>  	/* All other values are True.  */
>  	nonzero = 1;
> diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
> index 04bbf86cf1..c6ca869860 100644
> --- a/gdb/solib-darwin.c
> +++ b/gdb/solib-darwin.c
> @@ -232,8 +232,9 @@ open_symbol_file_object (void *from_ttyp)
>  static struct so_list *
>  darwin_current_sos (void)
>  {
> -  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
> -  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
> +  struct gdbarch *gdbarch = target_gdbarch ();
> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, ptr_type);
>    int ptr_len = TYPE_LENGTH (ptr_type);
>    unsigned int image_info_size;
>    struct so_list *head = NULL;
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 405de37aa5..24e523116c 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -920,9 +920,10 @@ solib_svr4_r_brk (struct svr4_info *info)
>  static CORE_ADDR
>  solib_svr4_r_ldsomap (struct svr4_info *info)
>  {
> +  struct gdbarch *gdbarch = target_gdbarch ();
>    struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
> -  struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
> -  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, ptr_type);
>    ULONGEST version = 0;
>  
>    TRY
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index 6fa0d20280..ea51d1ae49 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -1403,8 +1403,8 @@ stap_modify_semaphore (CORE_ADDR address, int set, struct gdbarch *gdbarch)
>        return;
>      }
>  
> -  value = extract_unsigned_integer (bytes, TYPE_LENGTH (type),
> -				    gdbarch_byte_order (gdbarch));
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, type);
> +  value = extract_unsigned_integer (bytes, TYPE_LENGTH (type), byte_order);
>    /* Note that we explicitly don't worry about overflow or
>       underflow.  */
>    if (set)
> @@ -1412,8 +1412,7 @@ stap_modify_semaphore (CORE_ADDR address, int set, struct gdbarch *gdbarch)
>    else
>      --value;
>  
> -  store_unsigned_integer (bytes, TYPE_LENGTH (type),
> -			  gdbarch_byte_order (gdbarch), value);
> +  store_unsigned_integer (bytes, TYPE_LENGTH (type), byte_order, value);
>  
>    if (target_write_memory (address, bytes, TYPE_LENGTH (type)) != 0)
>      warning (_("Could not write the value of a SystemTap semaphore."));
> diff --git a/gdb/testsuite/gdb.base/endianity.c b/gdb/testsuite/gdb.base/endianity.c
> new file mode 100644
> index 0000000000..330395a970
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/endianity.c
> @@ -0,0 +1,44 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2017 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* This tests the handling of dwarf attributes:
> +    DW_AT_endianity, DW_END_big, and DW_END_little.  */
> +struct otherendian
> +{
> +   int v;

Remove one space at the beginning of this line.

> +}
> +#if defined __GNUC__ && (__GNUC__ >= 6)
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +__attribute__( ( scalar_storage_order( "big-endian" ) ) )
> +#else
> +__attribute__( ( scalar_storage_order( "little-endian" ) ) )
> +#endif
> +#endif
> +;
> +
> +void
> +do_nothing (struct otherendian *c)
> +{
> +}
> +
> +int
> +main (void)
> +{
> +  struct otherendian o = {3};
> +
> +  do_nothing (&o); /* START */
> +}
> diff --git a/gdb/testsuite/gdb.base/endianity.exp b/gdb/testsuite/gdb.base/endianity.exp
> new file mode 100644
> index 0000000000..e125838dfd
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/endianity.exp
> @@ -0,0 +1,34 @@
> +# Copyright 2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +standard_testfile .c
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +set bp_location [gdb_get_line_number "START"]
> +if ![runto "endianity.c:$bp_location" ] then {

Add a fail message, like

  fail "couldn't run to start"

> +  return -1
> +}
> +
> +gdb_test "print o" "= {v = 3}"
> +
> +gdb_test "print o.v = 4" "= 4"
> +
> +# expect this to fail if compiled with < gcc6.
> +gdb_test "x/x &o.v" "0x04000000"

You can check the compiler version with test_compiler_info, look
in the testsuite for examples.  If the compiler is not gcc or is
gcc < 6, then you want to call setup_xfail.

Alternatively, you could add a check at the beginning of the .exp
file, and skip the file altogether if the compiler doesn't support
endianity attributes.  You can call "untested" with a message, and
return.

> +
> +gdb_test "print o" "= {v = 4}"

We want each test to have a unique name.  If you omit the test name
(third argument to gdb_test), it will default to the command.  Since
you have two "print o", there will be duplicates.

I suggest always giving a name to tests, like:

  gdb_test "print o" "= {v = 3}" "print o before assignment"

> diff --git a/gdb/valarith.c b/gdb/valarith.c
> index ede60e4b68..bcb0372830 100644
> --- a/gdb/valarith.c
> +++ b/gdb/valarith.c
> @@ -873,13 +873,13 @@ value_args_as_decimal (struct value *arg1, struct value *arg2,
>  
>    if (TYPE_CODE (type1) == TYPE_CODE_DECFLOAT)
>      {
> -      *byte_order_x = gdbarch_byte_order (get_type_arch (type1));
> +      *byte_order_x = type_byte_order (NULL, type1);
>        *len_x = TYPE_LENGTH (type1);
>        memcpy (x, value_contents (arg1), *len_x);
>      }
>    else if (is_integral_type (type1))
>      {
> -      *byte_order_x = gdbarch_byte_order (get_type_arch (type2));
> +      *byte_order_x = type_byte_order (NULL, type2);
>        *len_x = TYPE_LENGTH (type2);
>        if (TYPE_UNSIGNED (type1))
>  	decimal_from_ulongest (value_as_long (arg1), x, *len_x, *byte_order_x);
> @@ -895,13 +895,13 @@ value_args_as_decimal (struct value *arg1, struct value *arg2,
>  
>    if (TYPE_CODE (type2) == TYPE_CODE_DECFLOAT)
>      {
> -      *byte_order_y = gdbarch_byte_order (get_type_arch (type2));
> +      *byte_order_y = type_byte_order (NULL, type2);
>        *len_y = TYPE_LENGTH (type2);
>        memcpy (y, value_contents (arg2), *len_y);
>      }
>    else if (is_integral_type (type2))
>      {
> -      *byte_order_y = gdbarch_byte_order (get_type_arch (type1));
> +      *byte_order_y = type_byte_order (NULL, type1);
>        *len_y = TYPE_LENGTH (type1);
>        if (TYPE_UNSIGNED (type2))
>  	decimal_from_ulongest (value_as_long (arg2), y, *len_y, *byte_order_y);
> @@ -930,6 +930,7 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>  
>    type1 = check_typedef (value_type (arg1));
>    type2 = check_typedef (value_type (arg2));
> +  struct gdbarch *gdbarch = get_type_arch (type1);
>  
>    if ((TYPE_CODE (type1) != TYPE_CODE_FLT
>         && TYPE_CODE (type1) != TYPE_CODE_DECFLOAT
> @@ -959,7 +960,7 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>  	result_type = type1;
>  
>        len_v = TYPE_LENGTH (result_type);
> -      byte_order_v = gdbarch_byte_order (get_type_arch (result_type));
> +      byte_order_v = type_byte_order (gdbarch, result_type);
>  
>        value_args_as_decimal (arg1, arg2, v1, &len_v1, &byte_order_v1,
>  					 v2, &len_v2, &byte_order_v2);
> @@ -1084,7 +1085,7 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>        val = allocate_value (result_type);
>        store_signed_integer (value_contents_raw (val),
>  			    TYPE_LENGTH (result_type),
> -			    gdbarch_byte_order (get_type_arch (result_type)),
> +			    type_byte_order (gdbarch, result_type),
>  			    v);
>      }
>    else
> @@ -1232,8 +1233,8 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>  	  val = allocate_value (result_type);
>  	  store_unsigned_integer (value_contents_raw (val),
>  				  TYPE_LENGTH (value_type (val)),
> -				  gdbarch_byte_order
> -				    (get_type_arch (result_type)),
> +				  type_byte_order
> +				    (gdbarch, result_type),
>  				  v);
>  	}
>        else
> @@ -1362,8 +1363,8 @@ scalar_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>  	  val = allocate_value (result_type);
>  	  store_signed_integer (value_contents_raw (val),
>  				TYPE_LENGTH (value_type (val)),
> -				gdbarch_byte_order
> -				  (get_type_arch (result_type)),
> +				type_byte_order
> +				  (gdbarch, result_type),
>  				v);
>  	}
>      }
> @@ -1518,7 +1519,7 @@ value_logical_not (struct value *arg1)
>      return 0 == value_as_double (arg1);
>    else if (TYPE_CODE (type1) == TYPE_CODE_DECFLOAT)
>      return decimal_is_zero (value_contents (arg1), TYPE_LENGTH (type1),
> -			    gdbarch_byte_order (get_type_arch (type1)));
> +			    type_byte_order (NULL, type1));
>  
>    len = TYPE_LENGTH (type1);
>    p = value_contents (arg1);
> @@ -1774,7 +1775,7 @@ value_neg (struct value *arg1)
>  
>        memcpy (decbytes, value_contents (arg1), len);
>  
> -      if (gdbarch_byte_order (get_type_arch (type)) == BFD_ENDIAN_LITTLE)
> +      if (type_byte_order (NULL, type) == BFD_ENDIAN_LITTLE)
>  	decbytes[len-1] = decbytes[len - 1] | 0x80;
>        else
>  	decbytes[0] = decbytes[0] | 0x80;
> diff --git a/gdb/valops.c b/gdb/valops.c
> index de4544cd29..c12cd17b2e 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -466,7 +466,7 @@ value_cast (struct type *type, struct value *arg2)
>      return value_from_double (to_type, value_as_double (arg2));
>    else if (code1 == TYPE_CODE_DECFLOAT && scalar)
>      {
> -      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +      enum bfd_endian byte_order = type_byte_order (NULL, type);
>        int dec_len = TYPE_LENGTH (type);
>        gdb_byte dec[16];
>  
> @@ -502,7 +502,7 @@ value_cast (struct type *type, struct value *arg2)
>        if (code2 == TYPE_CODE_PTR)
>          longest = extract_unsigned_integer
>  		    (value_contents (arg2), TYPE_LENGTH (type2),
> -		     gdbarch_byte_order (get_type_arch (type2)));
> +		     type_byte_order (NULL, type2));
>        else
>          longest = value_as_long (arg2);
>        return value_from_longest (to_type, convert_to_boolean ?
> @@ -870,7 +870,7 @@ value_one (struct type *type)
>  
>    if (TYPE_CODE (type1) == TYPE_CODE_DECFLOAT)
>      {
> -      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +      enum bfd_endian byte_order = type_byte_order (NULL, type);
>        gdb_byte v[16];
>  
>        decimal_from_string (v, TYPE_LENGTH (type), byte_order, "1");
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index ca0c4768c9..73b7306fab 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1471,7 +1471,7 @@ void
>  print_decimal_floating (const gdb_byte *valaddr, struct type *type,
>  			struct ui_file *stream)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    unsigned len = TYPE_LENGTH (type);
>  
>    std::string str = decimal_to_string (valaddr, len, byte_order);
> @@ -2478,7 +2478,7 @@ generic_emit_char (int c, struct type *type, struct ui_file *stream,
>  		   int quoter, const char *encoding)
>  {
>    enum bfd_endian byte_order
> -    = gdbarch_byte_order (get_type_arch (type));
> +    = type_byte_order (NULL, type);
>    gdb_byte *buf;
>    int need_escape = 0;
>  
> @@ -2799,7 +2799,7 @@ generic_printstr (struct ui_file *stream, struct type *type,
>  		  int quote_char, int c_style_terminator,
>  		  const struct value_print_options *options)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    unsigned int i;
>    int width = TYPE_LENGTH (type);
>    struct cleanup *cleanup;
> @@ -2918,7 +2918,7 @@ val_print_string (struct type *elttype, const char *encoding,
>    gdb_byte *buffer = NULL;	/* Dynamically growable fetch buffer.  */
>    struct cleanup *old_chain = NULL;	/* Top of the old cleanup chain.  */
>    struct gdbarch *gdbarch = get_type_arch (elttype);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum bfd_endian byte_order = type_byte_order (gdbarch, elttype);
>    int width = TYPE_LENGTH (elttype);
>  
>    /* First we need to figure out the limit on the number of characters we are
> diff --git a/gdb/value.c b/gdb/value.c
> index 90423ed002..82e0065256 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -2900,7 +2900,7 @@ value_as_address (struct value *val)
>  LONGEST
>  unpack_long (struct type *type, const gdb_byte *valaddr)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    enum type_code code = TYPE_CODE (type);
>    int len = TYPE_LENGTH (type);
>    int nosign = TYPE_UNSIGNED (type);
> @@ -2949,7 +2949,7 @@ unpack_long (struct type *type, const gdb_byte *valaddr)
>  DOUBLEST
>  unpack_double (struct type *type, const gdb_byte *valaddr, int *invp)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    enum type_code code;
>    int len;
>    int nosign;
> @@ -3305,7 +3305,7 @@ static LONGEST
>  unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr,
>  		     LONGEST bitpos, LONGEST bitsize)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (field_type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, field_type);
>    ULONGEST val;
>    ULONGEST valmask;
>    int lsbcount;
> @@ -3411,7 +3411,7 @@ unpack_value_bitfield (struct value *dest_val,
>    int dst_bit_offset;
>    struct type *field_type = value_type (dest_val);
>  
> -  byte_order = gdbarch_byte_order (get_type_arch (field_type));
> +  byte_order = type_byte_order (NULL, field_type);
>  
>    /* First, unpack and sign extend the bitfield as if it was wholly
>       valid.  Optimized out/unavailable bits are read as zero, but
> @@ -3471,7 +3471,7 @@ void
>  modify_field (struct type *type, gdb_byte *addr,
>  	      LONGEST fieldval, LONGEST bitpos, LONGEST bitsize)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    ULONGEST oword;
>    ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);
>    LONGEST bytesize;
> @@ -3517,7 +3517,7 @@ modify_field (struct type *type, gdb_byte *addr,
>  void
>  pack_long (gdb_byte *buf, struct type *type, LONGEST num)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  enum bfd_endian byte_order = type_byte_order (NULL, type);
>    LONGEST len;
>  
>    type = check_typedef (type);
> @@ -3558,7 +3558,7 @@ pack_unsigned_long (gdb_byte *buf, struct type *type, ULONGEST num)
>  
>    type = check_typedef (type);
>    len = TYPE_LENGTH (type);
> -  byte_order = gdbarch_byte_order (get_type_arch (type));
> +  byte_order = type_byte_order (NULL, type);
>  
>    switch (TYPE_CODE (type))
>      {
> 

Thanks,

Simon

Re: [PATCH] review request: implementing DW_AT_endianity

[Tom Tromey]

>>>>> "Peeter" == Peeter Joot <peeter.joot@gmail.com> writes:

I happened to run across this patch today.

What's the status of it?

Peeter> +enum bfd_endian
Peeter> +type_byte_order (struct gdbarch * gdbarch, struct type *type)
Peeter> +{
Peeter> +  if (TYPE_ENDIANITY_BIG (type))
Peeter> +    return BFD_ENDIAN_BIG;
Peeter> +  else if (TYPE_ENDIANITY_LITTLE (type))
Peeter> +    return BFD_ENDIAN_LITTLE;
Peeter> +  if (!gdbarch)
Peeter> +    gdbarch = get_type_arch (type);
Peeter> +  return gdbarch_byte_order (gdbarch);

Does it ever make sense to call type_byte_order with a gdbarch other
than the type's gdbarch?  I would assume not but I'm not really sure.
What would this mean?

Anyway, if it doesn't make sense, then I'd suggest just removing the
gdbarch parameter.

Peeter> +  unsigned int flag_endianity_big : 1;
Peeter> +  unsigned int flag_endianity_little : 1;

It also seems to me that perhaps only a single bit is needed --
something like:

   unsigned int flag_endian_differs_from_arch : 1;

Then type_byte_order could do:

  enum bfd_endian endian = gdbarch_byte_order (...);
  if (blah blah flag_endian_differs_from_arch)
    endian = (endian == BFD_ENDIAN_LITTLE) ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE;

Of course this only makes sense if the arch endianness can't change
somehow, and if only the type's arch can be used to get the endianness.

Tom

Re: [PATCH] review request: implementing DW_AT_endianity

[Peeter Joot]

I'm still holding the patch for submission.  I'd handled all the reviewer
comments I received, implemented the tests suggestion by the reviewers, and
verified that I introduced no test regressions.  Also, the gcc side issues
I noticed testing this are already fixed in the latest version (trunk, but
perhaps one of the releases now too).

Unfortunately, the FSF attribution required for me to submit this formally
on behalf of the company I work for has been delayed repeatedly.  The last
I heard from the company lawyer who was arranging the attribution is that
there's just a few more i's and t's to dot and cross, but it should be done
soon.

Let me try your differs from arch suggestion.  With respect to the gdbarch
parameter, I'm not sure, as it's been a while since I worked on this, and
will have to inspect what I was doing before I can answer.

Peeter

--
Peeter

Note that my preferred email address is now: peeterjoot@protonmail.com

On Thu, Feb 22, 2018 at 12:20 PM, Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Peeter" == Peeter Joot <peeter.joot@gmail.com> writes:
>
> I happened to run across this patch today.
>
> What's the status of it?
>
> Peeter> +enum bfd_endian
> Peeter> +type_byte_order (struct gdbarch * gdbarch, struct type *type)
> Peeter> +{
> Peeter> +  if (TYPE_ENDIANITY_BIG (type))
> Peeter> +    return BFD_ENDIAN_BIG;
> Peeter> +  else if (TYPE_ENDIANITY_LITTLE (type))
> Peeter> +    return BFD_ENDIAN_LITTLE;
> Peeter> +  if (!gdbarch)
> Peeter> +    gdbarch = get_type_arch (type);
> Peeter> +  return gdbarch_byte_order (gdbarch);
>
> Does it ever make sense to call type_byte_order with a gdbarch other
> than the type's gdbarch?  I would assume not but I'm not really sure.
> What would this mean?
>
> Anyway, if it doesn't make sense, then I'd suggest just removing the
> gdbarch parameter.
>
> Peeter> +  unsigned int flag_endianity_big : 1;
> Peeter> +  unsigned int flag_endianity_little : 1;
>
> It also seems to me that perhaps only a single bit is needed --
> something like:
>
>    unsigned int flag_endian_differs_from_arch : 1;
>
> Then type_byte_order could do:
>
>   enum bfd_endian endian = gdbarch_byte_order (...);
>   if (blah blah flag_endian_differs_from_arch)
>     endian = (endian == BFD_ENDIAN_LITTLE) ? BFD_ENDIAN_BIG :
> BFD_ENDIAN_LITTLE;
>
> Of course this only makes sense if the arch endianness can't change
> somehow, and if only the type's arch can be used to get the endianness.
>
> Tom
>

Re: [PATCH] review request: implementing DW_AT_endianity

[Tom Tromey]

>>>>> "Peeter" == Peeter Joot <peeter.joot@gmail.com> writes:

Peeter> I'm still holding the patch for submission.  I'd handled all the reviewer
Peeter> comments I received, implemented the tests suggestion by the reviewers, and
Peeter> verified that I introduced no test regressions.  Also, the gcc side issues
Peeter> I noticed testing this are already fixed in the latest version (trunk, but
Peeter> perhaps one of the releases now too).

Peeter> Unfortunately, the FSF attribution required for me to submit this formally
Peeter> on behalf of the company I work for has been delayed repeatedly.  The last
Peeter> I heard from the company lawyer who was arranging the attribution is that
Peeter> there's just a few more i's and t's to dot and cross, but it should be done
Peeter> soon.

Hi!  What is the status of this?  I'm still interested in getting this
patch into gdb.

thanks,
Tom

Re: [PATCH] review request: implementing DW_AT_endianity

[Peeter Joot]

> Hi!  What is the status of this?  I'm still interested in getting this
> patch into gdb.
>

Hi Tom,

FSF legal is essentially unresponsive about the contribution paperwork that
we submitted (Oct 2017.)

We last heard from the FSF Licensing & Compliance Manager August 31st last
year, who stated they were hiring a new administrator, and asked for some
time to review.  Our company legal asked if we could provide any additional
information to help things along Oct 1st last year, and again Jan 14th this
year -- I don't believe we received any reply from the FSF.

The delays were so extensive that we've opted to use lldb as our backend
debugger, and have started contributing changes to the lldb/clang/llvm
stack instead.  That said, if the FSF does process our contribution
paperwork, it is still my intention to rebase my changes against
binutils/LATEST and submit a patch.

--
Peeter

Re: [PATCH] review request: implementing DW_AT_endianity

[Pedro Alves]

On 02/13/2019 02:11 PM, Peeter Joot wrote:
>> Hi!  What is the status of this?  I'm still interested in getting this
>> patch into gdb.
>>
> 
> Hi Tom,
> 
> FSF legal is essentially unresponsive about the contribution paperwork that
> we submitted (Oct 2017.)
> 
> We last heard from the FSF Licensing & Compliance Manager August 31st last
> year, who stated they were hiring a new administrator, and asked for some
> time to review.  Our company legal asked if we could provide any additional
> information to help things along Oct 1st last year, and again Jan 14th this
> year -- I don't believe we received any reply from the FSF.

I'm very sorry to hear that.  I'll get in touch with the FSF, see if
we can help unblock this.  In my experience copyright assignments are
pretty quick nowadays, except that last year the FSF went through
that administration change, which unfortunately delayed things.

> 
> The delays were so extensive that we've opted to use lldb as our backend
> debugger, and have started contributing changes to the lldb/clang/llvm
> stack instead.  That said, if the FSF does process our contribution
> paperwork, it is still my intention to rebase my changes against
> binutils/LATEST and submit a patch.
Thanks,
Pedro Alves

Re: [PATCH] review request: implementing DW_AT_endianity

[Pedro Alves]

On 02/13/2019 02:47 PM, Pedro Alves wrote:
> On 02/13/2019 02:11 PM, Peeter Joot wrote:
>>> Hi!  What is the status of this?  I'm still interested in getting this
>>> patch into gdb.
>>>
>>
>> Hi Tom,
>>
>> FSF legal is essentially unresponsive about the contribution paperwork that
>> we submitted (Oct 2017.)
>>
>> We last heard from the FSF Licensing & Compliance Manager August 31st last
>> year, who stated they were hiring a new administrator, and asked for some
>> time to review.  Our company legal asked if we could provide any additional
>> information to help things along Oct 1st last year, and again Jan 14th this
>> year -- I don't believe we received any reply from the FSF.
> 
> I'm very sorry to hear that.  I'll get in touch with the FSF, see if
> we can help unblock this.  In my experience copyright assignments are
> pretty quick nowadays, except that last year the FSF went through
> that administration change, which unfortunately delayed things.

Just to sort of close a loop here -- I got a response back.  I'll follow up
more offlist, but to summarize: The FSF apologizes for the delays but also explains
that there's a bit more to it than described above.  The process isn't actually
stuck, as the FSF responded as recently as Jan 17, and the delay mainly
revolves around the fact that EzLabs wants a non-standard assignment.
Hopefully it'll all be sorted out soon.

Thanks,
Pedro Alves