[PATCH 2/2] gdb: Handle alignment for C++ structures with static members

[PATCH 2/2] gdb: Handle alignment for C++ structures with static members

[Andrew Burgess]

In 'type_align' when computing the alignment of a structure we should
not consider the alignment of static structure members, these are
usually stored outside of the structure and therefore don't have any
impact on the structures alignment requirements.

I've extended the existing alignment calculating test to compile in
both C and C++ now so that we can create structures with static
members.

gdb/ChangeLog:

	* gdbtypes.c (type_align): Don't consider static members when
	computing structure alignment.

gdb/testsuite/ChangeLog:

	* gdb.base/align.exp: Extend to compile in both C and C++, and add
	tests for structs with static members.
---
 gdb/ChangeLog                    |   5 ++
 gdb/gdbtypes.c                   |  17 ++--
 gdb/testsuite/ChangeLog          |   5 ++
 gdb/testsuite/gdb.base/align.exp | 180 +++++++++++++++++++++++++++------------
 4 files changed, 147 insertions(+), 60 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 63dec3c8b7d..db9a78a80a6 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3039,15 +3039,18 @@ type_align (struct type *type)
 	  }
 	for (unsigned i = 0; i < TYPE_NFIELDS (type); ++i)
 	  {
-	    ULONGEST f_align = type_align (TYPE_FIELD_TYPE (type, i));
-	    if (f_align == 0)
+	    if (TYPE_FIELD_LOC_KIND (type, i) == FIELD_LOC_KIND_BITPOS)
 	      {
-		/* Don't pretend we know something we don't.  */
-		align = 0;
-		break;
+		ULONGEST f_align = type_align (TYPE_FIELD_TYPE (type, i));
+		if (f_align == 0)
+		  {
+		    /* Don't pretend we know something we don't.  */
+		    align = 0;
+		    break;
+		  }
+		if (f_align > align)
+		  align = f_align;
 	      }
-	    if (f_align > align)
-	      align = f_align;
 	  }
       }
       break;
diff --git a/gdb/testsuite/gdb.base/align.exp b/gdb/testsuite/gdb.base/align.exp
index e7dcf2a5480..558625d1b3e 100644
--- a/gdb/testsuite/gdb.base/align.exp
+++ b/gdb/testsuite/gdb.base/align.exp
@@ -15,8 +15,15 @@
 
 # This file is part of the gdb testsuite
 
-# This tests that C11 _Alignof works in gdb, and that it agrees with
-# the compiler.
+# This tests that C11 _Alignof and C++11 alignof works in gdb, and
+# that it agrees with the compiler.
+
+# Only test C++ if we are able.  Always use C.
+if { [skip_cplus_tests] || [get_compiler_info "c++"] } {
+    set lang {c}
+} else {
+    set lang {c c++}
+}
 
 # The types we're going to test.
 
@@ -38,72 +45,139 @@ if {[has_int128_c]} {
     lappend typelist __int128
 }
 
-# Create the test file.
+# Build source file for testing alignment handling of language LANG.
+# Returns the name of the newly created source file.
+proc prepare_test_source_file { lang } {
+    global typelist
+
+    # Create the test file.
 
-set filename [standard_output_file align.c]
-set outfile [open $filename w]
+    if { $lang == "c++" } {
+	set suffix "cpp"
+	set align_func "alignof"
+    } else {
+	set suffix "c"
+	set align_func "_Alignof"
+    }
 
-# Prologue.
-puts -nonewline $outfile "#define DEF(T,U) struct align_pair_ ## T ## _x_ ## U "
-puts $outfile "{ T one; U two; }"
-puts $outfile "unsigned a_void = _Alignof(void);"
+    set filename [standard_output_file "$lang/align.$suffix"]
+    set outfile [open $filename w]
 
-# First emit single items.
-foreach type $typelist {
-    set utype [join [split $type] _]
-    if {$type != $utype} {
-	puts $outfile "typedef $type $utype;"
+    # Prologue.
+    puts -nonewline $outfile "#define DEF(T,U) struct align_pair_ ## T ## _x_ ## U "
+    puts $outfile "{ T one; U two; }"
+    if { $lang == "c++" } {
+	puts -nonewline $outfile "#define DEF_WITH_STATIC(T,U) struct align_pair_static_ ## T ## _x_ ## U "
+	puts $outfile "{ static T one; U two; }"
+    }
+    if { $lang == "c" } {
+	puts $outfile "unsigned a_void = ${align_func} (void);"
     }
-    puts $outfile "$type item_$utype;"
-    puts $outfile "unsigned a_$utype\n  = _Alignof ($type);"
-    set utype [join [split $type] _]
-}
 
-# Now emit all pairs.
-foreach type $typelist {
-    set utype [join [split $type] _]
-    foreach inner $typelist {
-	set uinner [join [split $inner] _]
-	puts $outfile "DEF ($utype, $uinner);"
-	set joined "${utype}_x_${uinner}"
-	puts $outfile "struct align_pair_$joined item_${joined};"
-	puts $outfile "unsigned a_${joined}"
-	puts $outfile "  = _Alignof (struct align_pair_${joined});"
+    # First emit single items.
+    foreach type $typelist {
+	set utype [join [split $type] _]
+	if {$type != $utype} {
+	    puts $outfile "typedef $type $utype;"
+	}
+	puts $outfile "$type item_$utype;"
+	if { $lang == "c" } {
+	    puts $outfile "unsigned a_$utype\n  = ${align_func} ($type);"
+	}
+	set utype [join [split $type] _]
     }
-}
 
-# Epilogue.
-puts $outfile {
-    int main() {
-	return 0;
+    # Now emit all pairs.
+    foreach type $typelist {
+	set utype [join [split $type] _]
+	foreach inner $typelist {
+	    set uinner [join [split $inner] _]
+	    puts $outfile "DEF ($utype, $uinner);"
+	    set joined "${utype}_x_${uinner}"
+	    puts $outfile "struct align_pair_$joined item_${joined};"
+	    puts $outfile "unsigned a_${joined}"
+	    puts $outfile "  = ${align_func} (struct align_pair_${joined});"
+
+	    if { $lang == "c++" } {
+		puts $outfile "DEF_WITH_STATIC ($utype, $uinner);"
+		set joined "${utype}_x_${uinner}"
+		puts $outfile "struct align_pair_static_$joined item_static_${joined};"
+		puts $outfile "unsigned a_static_${joined}"
+		puts $outfile "  = ${align_func} (struct align_pair_static_${joined});"
+	    }
+	}
     }
-}
 
-close $outfile
+    # Epilogue.
+    puts $outfile {
+	int main() {
+	    return 0;
+	}
+    }
 
-standard_testfile $filename
+    close $outfile
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}]} {
-    return -1
+    return $filename
 }
 
-if {![runto_main]} {
-    perror "test suppressed"
-    return
-}
+# Run the alignment test for the language LANG.
+proc run_alignment_test { lang } {
+    global testfile srcfile typelist
+    global subdir
+
+    set filename [prepare_test_source_file $lang]
 
-foreach type $typelist {
-    set utype [join [split $type] _]
-    set expected [get_integer_valueof a_$utype 0]
-    gdb_test "print _Alignof($type)" " = $expected"
+    standard_testfile $filename
+    if {[prepare_for_testing "failed to prepare" "$lang/$testfile" $srcfile {debug}]} {
+	return -1
+    }
+
+    if {![runto_main]} {
+	perror "test suppressed"
+	return
+    }
+
+    if { $lang == "c++" } {
+	set align_func "alignof"
+    } else {
+	set align_func "_Alignof"
+    }
 
-    foreach inner $typelist {
-	set uinner [join [split $inner] _]
-	set expected [get_integer_valueof a_${utype}_x_${uinner} 0]
-	gdb_test "print _Alignof(struct align_pair_${utype}_x_${uinner})" \
-	    " = $expected"
+    foreach type $typelist {
+	set utype [join [split $type] _]
+	if { $lang == "c" } {
+	    set expected [get_integer_valueof a_$utype 0]
+	    gdb_test "print ${align_func}($type)" " = $expected"
+	}
+
+	foreach inner $typelist {
+	    set uinner [join [split $inner] _]
+	    set expected [get_integer_valueof a_${utype}_x_${uinner} 0]
+	    gdb_test "print ${align_func}(struct align_pair_${utype}_x_${uinner})" \
+		" = $expected"
+
+	    if { $lang == "c++" } {
+		set expected [get_integer_valueof a_static_${utype}_x_${uinner} 0]
+		gdb_test "print ${align_func}(struct align_pair_static_${utype}_x_${uinner})" \
+		    " = $expected"
+	    }
+	}
+    }
+
+    if { $lang == "c" } {
+	set expected [get_integer_valueof a_void 0]
+	gdb_test "print ${align_func}(void)" " = $expected"
     }
 }
 
-set expected [get_integer_valueof a_void 0]
-gdb_test "print _Alignof(void)" " = $expected"
+# Create nested 'c' and 'c++' directories within this tests directory.
+foreach l $lang {
+    set dir "$l"
+    remote_exec host "rm -rf [standard_output_file ${dir}]"
+    remote_exec host "mkdir -p [standard_output_file ${dir}]"
+}
+
+# Now run the test for each language.
+foreach_with_prefix l $lang {
+    run_alignment_test $l
+}
-- 
2.14.5

[PATCH 1/2] gdb: Restructure type_align and gdbarch_type_align

[Andrew Burgess]

This commit restructures the relationship between the type_align
function and the gdbarch_type_align method.

The problem being addressed with this commit is this; previously the
type_align function was structured so that for "basic" types (int,
float, etc) the gdbarch_type_align hook was called, which for
"compound" types (arrays, structs, etc) the common type_align code has
a fixed method for how to extract a "basic" type and would then call
itself on that "basic" type.

The problem is that if an architecture wants to modify the alignment
rules for a "compound" type then this is not currently possible.

In the revised structure, all types pass through the
gdbarch_type_align method.  If this method returns 0 then this
indicates that the architecture has no special rules for this type,
and GDB should apply the default rules for alignment.  However, the
architecture is free to provide an alignment for any type, both
"basic" and "compound".

After this commit the default alignment rules now all live in the
type_align function, the default_type_align only ever returns 0,
meaning apply the default rules.

I've updated the 3 targets (arc, i386, and nios2) that already
override the gdbarch_type_align method to fit the new scheme.

Tested on X86-64/GNU Linux with no regressions.

gdb/ChangeLog:

	* arc-tdep.c (arc_type_align): Provide alignment for basic types,
	return 0 for other types.
	* arch-utils.c (default_type_align): Always return 0.
	* gdbarch.h: Regenerate.
	* gdbarch.sh (type_align): Extend comment.
	* gdbtypes.c (type_align): Add additional comments, always call
	gdbarch_type_align before applying the default rules.
	* i386-tdep.c (i386_type_align): Return 0 as the default rule,
	generic code will then apply a suitable default.
	* nios2-tdep.c (nios2_type_align): Provide alignment for basic
	types, return 0 for other types.
---
 gdb/ChangeLog    | 14 ++++++++++++++
 gdb/arc-tdep.c   | 23 +++++++++++++++++++++--
 gdb/arch-utils.c |  2 +-
 gdb/gdbarch.h    |  5 ++++-
 gdb/gdbarch.sh   |  5 ++++-
 gdb/gdbtypes.c   | 13 ++++++++-----
 gdb/i386-tdep.c  |  2 +-
 gdb/nios2-tdep.c | 23 +++++++++++++++++++++--
 8 files changed, 74 insertions(+), 13 deletions(-)

diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index cd2762cb5cd..235fb276e47 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -1963,8 +1963,27 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
 static ULONGEST
 arc_type_align (struct gdbarch *gdbarch, struct type *type)
 {
-  type = check_typedef (type);
-  return std::min<ULONGEST> (4, TYPE_LENGTH (type));
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_PTR:
+    case TYPE_CODE_FUNC:
+    case TYPE_CODE_FLAGS:
+    case TYPE_CODE_INT:
+    case TYPE_CODE_RANGE:
+    case TYPE_CODE_FLT:
+    case TYPE_CODE_ENUM:
+    case TYPE_CODE_REF:
+    case TYPE_CODE_RVALUE_REF:
+    case TYPE_CODE_CHAR:
+    case TYPE_CODE_BOOL:
+    case TYPE_CODE_DECFLOAT:
+    case TYPE_CODE_METHODPTR:
+    case TYPE_CODE_MEMBERPTR:
+      type = check_typedef (type);
+      return std::min<ULONGEST> (4, TYPE_LENGTH (type));
+    default:
+      return 0;
+    }
 }
 
 /* Implement the "init" gdbarch method.  */
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index d2e27d95558..52a08daa3b9 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -993,7 +993,7 @@ default_in_indirect_branch_thunk (gdbarch *gdbarch, CORE_ADDR pc)
 ULONGEST
 default_type_align (struct gdbarch *gdbarch, struct type *type)
 {
-  return type_length_units (check_typedef (type));
+  return 0;
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5b265a462aa..75618376abb 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1589,7 +1589,10 @@ extern void set_gdbarch_disassembler_options (struct gdbarch *gdbarch, char ** d
 extern const disasm_options_and_args_t * gdbarch_valid_disassembler_options (struct gdbarch *gdbarch);
 extern void set_gdbarch_valid_disassembler_options (struct gdbarch *gdbarch, const disasm_options_and_args_t * valid_disassembler_options);
 
-/* Type alignment. */
+/* Type alignment override method.  Return the architecture specific
+   alignment required for TYPE.  If there is no special handling
+   required for TYPE then return the value 0, GDB will then apply the
+   default rules as laid out in gdbtypes.c:type_align. */
 
 typedef ULONGEST (gdbarch_type_align_ftype) (struct gdbarch *gdbarch, struct type *type);
 extern ULONGEST gdbarch_type_align (struct gdbarch *gdbarch, struct type *type);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index d53bbcc2ff4..48fcebd19a0 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1171,7 +1171,10 @@ v;const char *;disassembler_options_implicit;;;0;0;;0;pstring (gdbarch->disassem
 v;char **;disassembler_options;;;0;0;;0;pstring_ptr (gdbarch->disassembler_options)
 v;const disasm_options_and_args_t *;valid_disassembler_options;;;0;0;;0;host_address_to_string (gdbarch->valid_disassembler_options)
 
-# Type alignment.
+# Type alignment override method.  Return the architecture specific
+# alignment required for TYPE.  If there is no special handling
+# required for TYPE then return the value 0, GDB will then apply the
+# default rules as laid out in gdbtypes.c:type_align.
 m;ULONGEST;type_align;struct type *type;type;;default_type_align;;0
 
 EOF
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 0d293393dae..63dec3c8b7d 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2992,11 +2992,17 @@ type_raw_align (struct type *type)
 unsigned
 type_align (struct type *type)
 {
+  /* Check alignment provided in the debug information.  */
   unsigned raw_align = type_raw_align (type);
   if (raw_align != 0)
     return raw_align;
 
-  ULONGEST align = 0;
+  /* Allow the architecture to provide an alignment.  */
+  struct gdbarch *arch = get_type_arch (type);
+  ULONGEST align = gdbarch_type_align (arch, type);
+  if (align != 0)
+    return align;
+
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_PTR:
@@ -3013,10 +3019,7 @@ type_align (struct type *type)
     case TYPE_CODE_DECFLOAT:
     case TYPE_CODE_METHODPTR:
     case TYPE_CODE_MEMBERPTR:
-      {
-	struct gdbarch *arch = get_type_arch (type);
-	align = gdbarch_type_align (arch, type);
-      }
+      align = type_length_units (check_typedef (type));
       break;
 
     case TYPE_CODE_ARRAY:
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 663d510a915..bc9ba752edf 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8348,7 +8348,7 @@ i386_type_align (struct gdbarch *gdbarch, struct type *type)
 	return 4;
     }
 
-  return TYPE_LENGTH (type);
+  return 0;
 }
 
 \f
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index aa49a577dda..ee45db98af6 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -2233,8 +2233,27 @@ nios2_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
 static ULONGEST
 nios2_type_align (struct gdbarch *gdbarch, struct type *type)
 {
-  type = check_typedef (type);
-  return std::min<ULONGEST> (4, TYPE_LENGTH (type));
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_PTR:
+    case TYPE_CODE_FUNC:
+    case TYPE_CODE_FLAGS:
+    case TYPE_CODE_INT:
+    case TYPE_CODE_RANGE:
+    case TYPE_CODE_FLT:
+    case TYPE_CODE_ENUM:
+    case TYPE_CODE_REF:
+    case TYPE_CODE_RVALUE_REF:
+    case TYPE_CODE_CHAR:
+    case TYPE_CODE_BOOL:
+    case TYPE_CODE_DECFLOAT:
+    case TYPE_CODE_METHODPTR:
+    case TYPE_CODE_MEMBERPTR:
+      type = check_typedef (type);
+      return std::min<ULONGEST> (4, TYPE_LENGTH (type));
+    default:
+      return 0;
+    }
 }
 
 /* Implement the gcc_target_options gdbarch method.  */
-- 
2.14.5

[PATCH 0/2] Changes in type_align

[Andrew Burgess]

Set of changes to type_align function.  Tested on x86-64 GNU/Linux
with no regressions.

--

Andrew Burgess (2):
  gdb: Restructure type_align and gdbarch_type_align
  gdb: Handle alignment for C++ structures with static members

 gdb/ChangeLog                    |  19 +++++
 gdb/arc-tdep.c                   |  23 ++++-
 gdb/arch-utils.c                 |   2 +-
 gdb/gdbarch.h                    |   5 +-
 gdb/gdbarch.sh                   |   5 +-
 gdb/gdbtypes.c                   |  30 ++++---
 gdb/i386-tdep.c                  |   2 +-
 gdb/nios2-tdep.c                 |  23 ++++-
 gdb/testsuite/ChangeLog          |   5 ++
 gdb/testsuite/gdb.base/align.exp | 180 +++++++++++++++++++++++++++------------
 10 files changed, 221 insertions(+), 73 deletions(-)

-- 
2.14.5

Re: [PATCH 1/2] gdb: Restructure type_align and gdbarch_type_align

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This commit restructures the relationship between the type_align
Andrew> function and the gdbarch_type_align method.

Thank you, this looks good to me.

Tom

Re: [PATCH 2/2] gdb: Handle alignment for C++ structures with static members

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> 	* gdbtypes.c (type_align): Don't consider static members when
Andrew> 	computing structure alignment.

Thank you.

Andrew> +	    if (TYPE_FIELD_LOC_KIND (type, i) == FIELD_LOC_KIND_BITPOS)

This should probably use field_is_static instead.

Tom