gdb/testsuite: Fix py-format-string.exp on big-endian platforms

Message ID m3h84t4q05.fsf@oc0404454431.ibm.com
State New
Headers show
Series
  • gdb/testsuite: Fix py-format-string.exp on big-endian platforms
Related show

Commit Message

Andreas Arnez Sept. 30, 2019, 6:07 p.m.
GDB's py-format-string test case depends on endianness.  In particular it
relies on the first byte of the machine representation of 42 (as an int)
to be 42 as well.  While this is indeed the case for little-endian
machines, big-endian machines store a zero in the first byte instead.  The
wrong assumption leads to lots of FAILs on such architectures.

Fix this by using a 'palindrome' integer instead, i.e., one that has the
same machine representation on little- and big-endian architectures.  Use
the value 0x2a00002a, such that the expected character in the first byte
remains the same as before.

gdb/testsuite/ChangeLog:

	* gdb.python/py-format-string.c (main): Initialize
	a_struct_with_union.the_union.an_int with a "palindrome" integer
	instead of just 42, for endianness-independence.
	* gdb.python/py-format-string.exp (default_regexp_dict)
	(test_pretty_structs, test_format): Adjust expected output to the
	changed integer value.
---
 gdb/testsuite/gdb.python/py-format-string.c   | 3 ++-
 gdb/testsuite/gdb.python/py-format-string.exp | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.22.0

Comments

Tom Tromey Oct. 1, 2019, 5:09 p.m. | #1
>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:


Andreas> 	* gdb.python/py-format-string.c (main): Initialize
Andreas> 	a_struct_with_union.the_union.an_int with a "palindrome" integer
Andreas> 	instead of just 42, for endianness-independence.
Andreas> 	* gdb.python/py-format-string.exp (default_regexp_dict)
Andreas> 	(test_pretty_structs, test_format): Adjust expected output to the
Andreas> 	changed integer value.

Thank you for doing this.

Andreas> +  /* Choose a value whose first byte (little-/big-endian) is 42.  */
Andreas> +  a_struct_with_union.the_union.an_int = 0x2a00002a;
 
Maybe for size-independence it would be better to memset the entire
union to 0x2a?

Tom
Andreas Arnez Oct. 2, 2019, 1:22 p.m. | #2
On Tue, Oct 01 2019, Tom Tromey wrote:

>>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:

>

> Andreas> 	* gdb.python/py-format-string.c (main): Initialize

> Andreas> 	a_struct_with_union.the_union.an_int with a "palindrome" integer

> Andreas> 	instead of just 42, for endianness-independence.

> Andreas> 	* gdb.python/py-format-string.exp (default_regexp_dict)

> Andreas> 	(test_pretty_structs, test_format): Adjust expected output to the

> Andreas> 	changed integer value.

>

> Thank you for doing this.


No problem.  I'd just like the number of testsuite FAILs on s390x to
drop to a reasonable amount again.

>

> Andreas> +  /* Choose a value whose first byte (little-/big-endian) is 42.  */

> Andreas> +  a_struct_with_union.the_union.an_int = 0x2a00002a;

>  

> Maybe for size-independence it would be better to memset the entire

> union to 0x2a?


You mean in anticipation of this union being extended in the future?
Sure, I can do that; see updated patch below.  OK to apply?

--
Andreas

--- >8 ---

Subject: [PATCH v2] gdb/testsuite: Fix py-format-string.exp on
 big-endian platforms

GDB's py-format-string test case depends on endianness.  In particular it
relies on the first byte of the machine representation of 42 (as an int)
to be 42 as well.  While this is indeed the case for little-endian
machines, big-endian machines store a zero in the first byte instead.  The
wrong assumption leads to lots of FAILs on such architectures.

Fix this by filling the affected union with bytes of the same value, such
that endianness does not matter.  Use the value 42, to keep the character
in the first byte unchanged.

gdb/testsuite/ChangeLog:

	* gdb.python/py-format-string.c (string.h): New include.
	(main): Fill a_struct_with_union.the_union.an_int with bytes of
	the same value, for endianness-independence.
	* gdb.python/py-format-string.exp (default_regexp_dict)
	(test_pretty_structs, test_format): Adjust expected output to the
	changed initialization.
---
 gdb/testsuite/gdb.python/py-format-string.c   | 6 +++++-
 gdb/testsuite/gdb.python/py-format-string.exp | 8 ++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.python/py-format-string.c b/gdb/testsuite/gdb.python/py-format-string.c
index 120ecce989..017f92080b 100644
--- a/gdb/testsuite/gdb.python/py-format-string.c
+++ b/gdb/testsuite/gdb.python/py-format-string.c
@@ -15,6 +15,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include <string.h>
+
 typedef struct point
 {
   int x;
@@ -84,7 +86,9 @@ main ()
   struct point another_point = { 123, 456 };
 
   struct_union_t a_struct_with_union;
-  a_struct_with_union.the_union.an_int = 42;
+  /* Fill the union in an endianness-independent way.  */
+  memset (&a_struct_with_union.the_union, 42,
+	  sizeof (a_struct_with_union.the_union));
 
   enum_t an_enum = ENUM_BAR;
 
diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
index ebb2074ce8..ad71fe0e51 100644
--- a/gdb/testsuite/gdb.python/py-format-string.exp
+++ b/gdb/testsuite/gdb.python/py-format-string.exp
@@ -124,7 +124,7 @@ set default_regexp_dict [dict create \
   "a_point_t_pointer"		$default_pointer_regexp \
   "a_point_t_ref"		"Pretty Point \\(42, 12\\)" \
   "another_point"		"Pretty Point \\(123, 456\\)" \
-  "a_struct_with_union"		"\\{the_union = \\{an_int = 42, a_char = 42 '\\*'\\}\\}" \
+  "a_struct_with_union"		"\\{the_union = \\{an_int = 707406378, a_char = 42 '\\*'\\}\\}" \
   "an_enum"			"ENUM_BAR" \
   "a_string"			"${default_pointer_regexp} \"hello world\"" \
   "a_binary_string"		"${default_pointer_regexp} \"hello\"" \
@@ -333,7 +333,7 @@ proc test_pretty_structs {} {
   global current_lang
 
   set a_struct_with_union_pretty \
-    "\\{\[\r\n\]+  the_union = \\{\[\r\n\]+    an_int = 42,\[\r\n\]+    a_char = 42 '\\*'\[\r\n\]+  \\}\[\r\n\]+\\}"
+    "\\{\[\r\n\]+  the_union = \\{\[\r\n\]+    an_int = 707406378,\[\r\n\]+    a_char = 42 '\\*'\[\r\n\]+  \\}\[\r\n\]+\\}"
 
   check_var_with_bool_opt "pretty_structs" "a_point_t"
   check_var_with_bool_opt "pretty_structs" "a_point_t_pointer"
@@ -814,7 +814,7 @@ proc test_format {} {
     check_format_string "a_point_t_pointer" $opts
     check_format_string "another_point" $opts
     check_format_string "a_struct_with_union" $opts \
-      "\\{the_union = \\{an_int = 0x2a, a_char = 0x2a\\}\\}"
+      "\\{the_union = \\{an_int = 0x2a2a2a2a, a_char = 0x2a\\}\\}"
     check_format_string "an_enum" $opts \
       "0x1"
     check_format_string "a_string" $opts \
@@ -851,7 +851,7 @@ proc test_format {} {
       $decimal_pointer_regexp
     check_format_string "another_point" $opts
     check_format_string "a_struct_with_union" $opts \
-      "\\{the_union = \\{an_int = 42, a_char = 42\\}\\}"
+      "\\{the_union = \\{an_int = 707406378, a_char = 42\\}\\}"
     check_format_string "an_enum" $opts \
       "1"
     check_format_string "a_string" $opts \
-- 
2.17.0
Tom Tromey Oct. 2, 2019, 3:58 p.m. | #3
>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:


Andreas> You mean in anticipation of this union being extended in the future?
Andreas> Sure, I can do that; see updated patch below.  OK to apply?

Well, I was thinking more if the size of "int" was different.
But that's probably a pedantic suggestion... sorry about that.

Andreas> +  "a_struct_with_union"		"\\{the_union = \\{an_int = 707406378, a_char = 42 '\\*'\\}\\}" \

.. especially seeing this and realizing that we can't avoid the issue.

This patch is ok.  Thanks for bearing with me.

Tom
Andreas Arnez Oct. 2, 2019, 6:06 p.m. | #4
On Wed, Oct 02 2019, Tom Tromey wrote:

>>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:

>

> Andreas> You mean in anticipation of this union being extended in the future?

> Andreas> Sure, I can do that; see updated patch below.  OK to apply?

>

> Well, I was thinking more if the size of "int" was different.

> But that's probably a pedantic suggestion... sorry about that.

>

> Andreas> +  "a_struct_with_union"		"\\{the_union = \\{an_int = 707406378, a_char = 42 '\\*'\\}\\}" \

>

> .. especially seeing this and realizing that we can't avoid the issue.


Right, not that simply, anyway.

>

> This patch is ok.  Thanks for bearing with me.


Thanks for reviewing!

Pushed as git commit #9ef62df072d85d9cd0fadc90a3bd7f05e32ba9fd.

--
Andreas

Patch

diff --git a/gdb/testsuite/gdb.python/py-format-string.c b/gdb/testsuite/gdb.python/py-format-string.c
index 120ecce989..f2cac280af 100644
--- a/gdb/testsuite/gdb.python/py-format-string.c
+++ b/gdb/testsuite/gdb.python/py-format-string.c
@@ -84,7 +84,8 @@  main ()
   struct point another_point = { 123, 456 };
 
   struct_union_t a_struct_with_union;
-  a_struct_with_union.the_union.an_int = 42;
+  /* Choose a value whose first byte (little-/big-endian) is 42.  */
+  a_struct_with_union.the_union.an_int = 0x2a00002a;
 
   enum_t an_enum = ENUM_BAR;
 
diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
index ebb2074ce8..5512b54e00 100644
--- a/gdb/testsuite/gdb.python/py-format-string.exp
+++ b/gdb/testsuite/gdb.python/py-format-string.exp
@@ -124,7 +124,7 @@  set default_regexp_dict [dict create \
   "a_point_t_pointer"		$default_pointer_regexp \
   "a_point_t_ref"		"Pretty Point \\(42, 12\\)" \
   "another_point"		"Pretty Point \\(123, 456\\)" \
-  "a_struct_with_union"		"\\{the_union = \\{an_int = 42, a_char = 42 '\\*'\\}\\}" \
+  "a_struct_with_union"		"\\{the_union = \\{an_int = 704643114, a_char = 42 '\\*'\\}\\}" \
   "an_enum"			"ENUM_BAR" \
   "a_string"			"${default_pointer_regexp} \"hello world\"" \
   "a_binary_string"		"${default_pointer_regexp} \"hello\"" \
@@ -333,7 +333,7 @@  proc test_pretty_structs {} {
   global current_lang
 
   set a_struct_with_union_pretty \
-    "\\{\[\r\n\]+  the_union = \\{\[\r\n\]+    an_int = 42,\[\r\n\]+    a_char = 42 '\\*'\[\r\n\]+  \\}\[\r\n\]+\\}"
+    "\\{\[\r\n\]+  the_union = \\{\[\r\n\]+    an_int = 704643114,\[\r\n\]+    a_char = 42 '\\*'\[\r\n\]+  \\}\[\r\n\]+\\}"
 
   check_var_with_bool_opt "pretty_structs" "a_point_t"
   check_var_with_bool_opt "pretty_structs" "a_point_t_pointer"
@@ -814,7 +814,7 @@  proc test_format {} {
     check_format_string "a_point_t_pointer" $opts
     check_format_string "another_point" $opts
     check_format_string "a_struct_with_union" $opts \
-      "\\{the_union = \\{an_int = 0x2a, a_char = 0x2a\\}\\}"
+      "\\{the_union = \\{an_int = 0x2a00002a, a_char = 0x2a\\}\\}"
     check_format_string "an_enum" $opts \
       "0x1"
     check_format_string "a_string" $opts \
@@ -851,7 +851,7 @@  proc test_format {} {
       $decimal_pointer_regexp
     check_format_string "another_point" $opts
     check_format_string "a_struct_with_union" $opts \
-      "\\{the_union = \\{an_int = 42, a_char = 42\\}\\}"
+      "\\{the_union = \\{an_int = 704643114, a_char = 42\\}\\}"
     check_format_string "an_enum" $opts \
       "1"
     check_format_string "a_string" $opts \