* [PATCH 1/2] Fix ptype/o comment formatting
2019-04-29 18:31 [PATCH 0/2] two ptype/o changes Tom Tromey
@ 2019-04-29 18:31 ` Tom Tromey
2019-04-29 18:31 ` [PATCH 2/2] Change ptype/o to print bit offset Tom Tromey
2019-05-08 16:14 ` [PATCH 0/2] two ptype/o changes Tom Tromey
2 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2019-04-29 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed that ptype/o will print:
/* 3: 3 | 1 */ signed char a4 : 2;
/* XXX 3-bit hole */
That is, "*/" at the end of the "hole" message does not line up with
the other comment ends. I thought it would be a bit nicer if this did
line up, so I fixed it. Then, to my surprise, I found that I could
not make ptype-offsets.exp fail.
I still am not sure why it doesn't fail, but changing the tests to use
string_to_regexp and changing the quoting helped. This in turn showed
that some of the existing test cases were wrong, so I've also updated
them here.
gdb/ChangeLog
2019-04-29 Tom Tromey <tromey@adacore.com>
* typeprint.c (print_offset_data::maybe_print_hole): Add extra
padding at end of comment.
gdb/testsuite/ChangeLog
2019-04-29 Tom Tromey <tromey@adacore.com>
* gdb.base/ptype-offsets.exp: Use string_to_regexp. Fix test
cases.
* gdb.base/ptype-offsets.cc (struct abc) <my_int_type>: Now
"short".
---
gdb/ChangeLog | 5 +
gdb/testsuite/ChangeLog | 7 +
gdb/testsuite/gdb.base/ptype-offsets.cc | 2 +-
gdb/testsuite/gdb.base/ptype-offsets.exp | 501 ++++++++++++-----------
gdb/typeprint.c | 4 +-
5 files changed, 266 insertions(+), 253 deletions(-)
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.cc b/gdb/testsuite/gdb.base/ptype-offsets.cc
index 39236a60c75..1731545f97b 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.cc
+++ b/gdb/testsuite/gdb.base/ptype-offsets.cc
@@ -66,7 +66,7 @@ struct abc
{}
/* Typedef defined in-struct. */
- typedef int my_int_type;
+ typedef short my_int_type;
my_int_type field9;
};
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index 5e3c59c4184..12b3a746005 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -34,217 +34,217 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
# Test general offset printing, ctor/dtor printing, union, formatting.
gdb_test "ptype /o struct abc" \
- [multi_line \
-{/\* offset | size \*/ type = struct abc \{} \
-{ public:} \
-{/\* 8 | 8 \*/ void \*field1;} \
-{/\* 16:31 | 4 \*/ unsigned int field2 : 1;} \
-{/\* XXX 7-bit hole \*/} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 20 | 4 \*/ int field3;} \
-{/\* 24 | 1 \*/ signed char field4;} \
-{/\* XXX 7-byte hole \*/} \
-{/\* 32 | 8 \*/ uint64_t field5;} \
-{/\* 40 | 8 \*/ union \{} \
-{/\* 8 \*/ void \*field6;} \
-{/\* 4 \*/ int field7;} \
-{} \
-{ /\* total size \(bytes\): 8 \*/} \
-{ \} field8;} \
-{/\* 48 | 4 \*/ my_int_type field9;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 56 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct abc \{" \
+" public:" \
+"/* 8 | 8 */ void *field1;" \
+"/* 16:31 | 4 */ unsigned int field2 : 1;" \
+"/* XXX 7-bit hole */" \
+"/* XXX 3-byte hole */" \
+"/* 20 | 4 */ int field3;" \
+"/* 24 | 1 */ signed char field4;" \
+"/* XXX 7-byte hole */" \
+"/* 32 | 8 */ uint64_t field5;" \
+"/* 40 | 8 */ union \{" \
+"/* 8 */ void *field6;" \
+"/* 4 */ int field7;" \
+"" \
+" /* total size (bytes): 8 */" \
+" \} field8;" \
+"/* 48 | 2 */ my_int_type field9;" \
+"/* XXX 6-byte padding */" \
+"" \
+" /* total size (bytes): 56 */" \
+" \}"]]
# Test "ptype /oTM".
gdb_test "ptype /oTM struct abc" \
- [multi_line \
-{/\* offset | size \*/ type = struct abc \{} \
-{ public:} \
-{/\* 8 | 8 \*/ void \*field1;} \
-{/\* 16:31 | 4 \*/ unsigned int field2 : 1;} \
-{/\* XXX 7-bit hole \*/} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 20 | 4 \*/ int field3;} \
-{/\* 24 | 1 \*/ signed char field4;} \
-{/\* XXX 7-byte hole \*/} \
-{/\* 32 | 8 \*/ uint64_t field5;} \
-{/\* 40 | 8 \*/ union \{} \
-{/\* 8 \*/ void \*field6;} \
-{/\* 4 \*/ int field7;} \
-{} \
-{ /\* total size \(bytes\): 8 \*/} \
-{ \} field8;} \
-{/\* 48 | 4 \*/ my_int_type field9;} \
-{} \
-{ abc\(void\);} \
-{ ~abc\(\);} \
-{} \
-{ typedef int my_int_type;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 56 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct abc \{" \
+" public:" \
+"/* 8 | 8 */ void *field1;" \
+"/* 16:31 | 4 */ unsigned int field2 : 1;" \
+"/* XXX 7-bit hole */" \
+"/* XXX 3-byte hole */" \
+"/* 20 | 4 */ int field3;" \
+"/* 24 | 1 */ signed char field4;" \
+"/* XXX 7-byte hole */" \
+"/* 32 | 8 */ uint64_t field5;" \
+"/* 40 | 8 */ union \{" \
+"/* 8 */ void *field6;" \
+"/* 4 */ int field7;" \
+"" \
+" /* total size (bytes): 8 */" \
+" \} field8;" \
+"/* 48 | 2 */ my_int_type field9;" \
+"" \
+" abc(void);" \
+" ~abc();" \
+"" \
+" typedef short my_int_type;" \
+"/* XXX 6-byte padding */" \
+"" \
+" /* total size (bytes): 56 */" \
+" \}"]]
# Test "ptype /TMo". This should be the same as "ptype /o".
gdb_test "ptype /TMo struct abc" \
- [multi_line \
-{/\* offset | size \*/ type = struct abc \{} \
-{ public:} \
-{/\* 8 | 8 \*/ void \*field1;} \
-{/\* 16:31 | 4 \*/ unsigned int field2 : 1;} \
-{/\* XXX 7-bit hole \*/} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 20 | 4 \*/ int field3;} \
-{/\* 24 | 1 \*/ signed char field4;} \
-{/\* XXX 7-byte hole \*/} \
-{/\* 32 | 8 \*/ uint64_t field5;} \
-{/\* 40 | 8 \*/ union \{} \
-{/\* 8 \*/ void \*field6;} \
-{/\* 4 \*/ int field7;} \
-{} \
-{ /\* total size \(bytes\): 8 \*/} \
-{ \} field8;} \
-{/\* 48 | 4 \*/ my_int_type field9;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 56 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct abc \{" \
+" public:" \
+"/* 8 | 8 */ void *field1;" \
+"/* 16:31 | 4 */ unsigned int field2 : 1;" \
+"/* XXX 7-bit hole */" \
+"/* XXX 3-byte hole */" \
+"/* 20 | 4 */ int field3;" \
+"/* 24 | 1 */ signed char field4;" \
+"/* XXX 7-byte hole */" \
+"/* 32 | 8 */ uint64_t field5;" \
+"/* 40 | 8 */ union \{" \
+"/* 8 */ void *field6;" \
+"/* 4 */ int field7;" \
+"" \
+" /* total size (bytes): 8 */" \
+" \} field8;" \
+"/* 48 | 2 */ my_int_type field9;" \
+"/* XXX 6-byte padding */" \
+"" \
+" /* total size (bytes): 56 */" \
+" \}"]]
# Test nested structs.
gdb_test "ptype /o struct pqr" \
- [multi_line \
-{/\* offset | size \*/ type = struct pqr \{} \
-{/\* 0 | 4 \*/ int ff1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 8 | 40 \*/ struct xyz \{} \
-{/\* 8 | 4 \*/ int f1;} \
-{/\* 12 | 1 \*/ signed char f2;} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 16 | 8 \*/ void \*f3;} \
-{/\* 24 | 24 \*/ struct tuv \{} \
-{/\* 24 | 4 \*/ int a1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 32 | 8 \*/ signed char \*a2;} \
-{/\* 40 | 4 \*/ int a3;} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \} f4;} \
-{} \
-{ /\* total size \(bytes\): 40 \*/} \
-{ \} ff2;} \
-{/\* XXX 28-byte hole \*/} \
-{/\* 72 | 1 \*/ signed char ff3;} \
-{/\* XXX 7-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 56 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct pqr \{" \
+"/* 0 | 4 */ int ff1;" \
+"/* XXX 4-byte hole */" \
+"/* 8 | 40 */ struct xyz \{" \
+"/* 8 | 4 */ int f1;" \
+"/* 12 | 1 */ signed char f2;" \
+"/* XXX 3-byte hole */" \
+"/* 16 | 8 */ void *f3;" \
+"/* 24 | 24 */ struct tuv \{" \
+"/* 24 | 4 */ int a1;" \
+"/* XXX 4-byte hole */" \
+"/* 32 | 8 */ signed char *a2;" \
+"/* 40 | 4 */ int a3;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \} f4;" \
+"" \
+" /* total size (bytes): 40 */" \
+" \} ff2;" \
+"/* 48 | 1 */ signed char ff3;" \
+"/* XXX 7-byte padding */" \
+"" \
+" /* total size (bytes): 56 */" \
+" \}"]]
# Test that the offset is properly reset when we are printing a union
# and go inside two inner structs.
# This also tests a struct inside a struct inside a union.
gdb_test "ptype /o union qwe" \
- [multi_line \
-{/\* offset | size \*/ type = union qwe \{} \
-{/\* 24 \*/ struct tuv \{} \
-{/\* 0 | 4 \*/ int a1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 8 | 8 \*/ signed char \*a2;} \
-{/\* 16 | 4 \*/ int a3;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \} fff1;} \
-{/\* 40 \*/ struct xyz \{} \
-{/\* 0 | 4 \*/ int f1;} \
-{/\* 4 | 1 \*/ signed char f2;} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 8 | 8 \*/ void \*f3;} \
-{/\* 16 | 24 \*/ struct tuv \{} \
-{/\* 16 | 4 \*/ int a1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 24 | 8 \*/ signed char \*a2;} \
-{/\* 32 | 4 \*/ int a3;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \} f4;} \
-{} \
-{ /\* total size \(bytes\): 40 \*/} \
-{ \} fff2;} \
-{} \
-{ /\* total size \(bytes\): 40 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = union qwe \{" \
+"/* 24 */ struct tuv \{" \
+"/* 0 | 4 */ int a1;" \
+"/* XXX 4-byte hole */" \
+"/* 8 | 8 */ signed char *a2;" \
+"/* 16 | 4 */ int a3;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \} fff1;" \
+"/* 40 */ struct xyz \{" \
+"/* 0 | 4 */ int f1;" \
+"/* 4 | 1 */ signed char f2;" \
+"/* XXX 3-byte hole */" \
+"/* 8 | 8 */ void *f3;" \
+"/* 16 | 24 */ struct tuv \{" \
+"/* 16 | 4 */ int a1;" \
+"/* XXX 4-byte hole */" \
+"/* 24 | 8 */ signed char *a2;" \
+"/* 32 | 4 */ int a3;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \} f4;" \
+"" \
+" /* total size (bytes): 40 */" \
+" \} fff2;" \
+"" \
+" /* total size (bytes): 40 */" \
+" \}"]]
# Test printing a struct that contains a union, and that also
# contains a struct.
gdb_test "ptype /o struct poi" \
- [multi_line \
-{/\* offset | size \*/ type = struct poi \{} \
-{/\* 0 | 4 \*/ int f1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 8 | 40 \*/ union qwe \{} \
-{/\* 24 \*/ struct tuv \{} \
-{/\* 8 | 4 \*/ int a1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 16 | 8 \*/ signed char \*a2;} \
-{/\* 24 | 4 \*/ int a3;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \} fff1;} \
-{/\* 40 \*/ struct xyz \{} \
-{/\* 8 | 4 \*/ int f1;} \
-{/\* 12 | 1 \*/ signed char f2;} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 16 | 8 \*/ void \*f3;} \
-{/\* 24 | 24 \*/ struct tuv \{} \
-{/\* 24 | 4 \*/ int a1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 32 | 8 \*/ signed char \*a2;} \
-{/\* 40 | 4 \*/ int a3;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \} f4;} \
-{/\* XXX 32-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 40 \*/} \
-{ \} fff2;} \
-{} \
-{ /\* total size \(bytes\): 40 \*/} \
-{ \} f2;} \
-{/\* 72 | 2 \*/ uint16_t f3;} \
-{/\* XXX 6-byte hole \*/} \
-{/\* 80 | 56 \*/ struct pqr \{} \
-{/\* 80 | 4 \*/ int ff1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 88 | 40 \*/ struct xyz \{} \
-{/\* 88 | 4 \*/ int f1;} \
-{/\* 92 | 1 \*/ signed char f2;} \
-{/\* XXX 3-byte hole \*/} \
-{/\* 96 | 8 \*/ void \*f3;} \
-{/\* 104 | 24 \*/ struct tuv \{} \
-{/\* 104 | 4 \*/ int a1;} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 112 | 8 \*/ signed char \*a2;} \
-{/\* 120 | 4 \*/ int a3;} \
-{/\* XXX 4-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \} f4;} \
-{} \
-{ /\* total size \(bytes\): 40 \*/} \
-{ \} ff2;} \
-{/\* 152 | 1 \*/ signed char ff3;} \
-{/\* XXX 7-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 56 \*/} \
-{ \} f4;} \
-{} \
-{ /\* total size \(bytes\): 112 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct poi \{" \
+"/* 0 | 4 */ int f1;" \
+"/* XXX 4-byte hole */" \
+"/* 8 | 40 */ union qwe \{" \
+"/* 24 */ struct tuv \{" \
+"/* 8 | 4 */ int a1;" \
+"/* XXX 4-byte hole */" \
+"/* 16 | 8 */ signed char *a2;" \
+"/* 24 | 4 */ int a3;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \} fff1;" \
+"/* 40 */ struct xyz \{" \
+"/* 8 | 4 */ int f1;" \
+"/* 12 | 1 */ signed char f2;" \
+"/* XXX 3-byte hole */" \
+"/* 16 | 8 */ void *f3;" \
+"/* 24 | 24 */ struct tuv \{" \
+"/* 24 | 4 */ int a1;" \
+"/* XXX 4-byte hole */" \
+"/* 32 | 8 */ signed char *a2;" \
+"/* 40 | 4 */ int a3;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \} f4;" \
+"" \
+" /* total size (bytes): 40 */" \
+" \} fff2;" \
+"/* XXX 32-byte padding */" \
+"" \
+" /* total size (bytes): 40 */" \
+" \} f2;" \
+"/* 48 | 2 */ uint16_t f3;" \
+"/* XXX 6-byte hole */" \
+"/* 56 | 56 */ struct pqr \{" \
+"/* 56 | 4 */ int ff1;" \
+"/* XXX 4-byte hole */" \
+"/* 64 | 40 */ struct xyz \{" \
+"/* 64 | 4 */ int f1;" \
+"/* 68 | 1 */ signed char f2;" \
+"/* XXX 3-byte hole */" \
+"/* 72 | 8 */ void *f3;" \
+"/* 80 | 24 */ struct tuv \{" \
+"/* 80 | 4 */ int a1;" \
+"/* XXX 4-byte hole */" \
+"/* 88 | 8 */ signed char *a2;" \
+"/* 96 | 4 */ int a3;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \} f4;" \
+"" \
+" /* total size (bytes): 40 */" \
+" \} ff2;" \
+"/* 104 | 1 */ signed char ff3;" \
+"/* XXX 7-byte padding */" \
+"" \
+" /* total size (bytes): 56 */" \
+" \} f4;" \
+"" \
+" /* total size (bytes): 112 */" \
+" \}"]]
# Test printing a struct with several bitfields, laid out in various
# ways.
@@ -262,58 +262,59 @@ gdb_test "ptype /o struct poi" \
# 0x7fffffffd548: 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
# 0x7fffffffd550: 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00
gdb_test "ptype /o struct tyu" \
- [multi_line \
-{/\* offset | size \*/ type = struct tyu \{} \
-{/\* 0:31 | 4 \*/ int a1 : 1;} \
-{/\* 0:28 | 4 \*/ int a2 : 3;} \
-{/\* 0: 5 | 4 \*/ int a3 : 23;} \
-{/\* 3: 3 | 1 \*/ signed char a4 : 2;} \
-{/\* XXX 3-bit hole \*/} \
-{/\* XXX 4-byte hole \*/} \
-{/\* 8 | 8 \*/ int64_t a5;} \
-{/\* 16:27 | 4 \*/ int a6 : 5;} \
-{/\* 16:56 | 8 \*/ int64_t a7 : 3;} \
-{/\* XXX 7-byte padding \*/} \
-{} \
-{ /\* total size \(bytes\): 24 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct tyu \{" \
+"/* 0:31 | 4 */ int a1 : 1;" \
+"/* 0:28 | 4 */ int a2 : 3;" \
+"/* 0: 5 | 4 */ int a3 : 23;" \
+"/* 3: 3 | 1 */ signed char a4 : 2;" \
+"/* XXX 3-bit hole */" \
+"/* XXX 4-byte hole */" \
+"/* 8 | 8 */ int64_t a5;" \
+"/* 16:27 | 4 */ int a6 : 5;" \
+"/* 16:56 | 8 */ int64_t a7 : 3;" \
+"/* XXX 7-byte padding */" \
+"" \
+" /* total size (bytes): 24 */" \
+" \}"]]
gdb_test "ptype /o struct asd" \
- [multi_line \
-{/\* offset | size \*/ type = struct asd \{} \
-{/\* 0 | 32 \*/ struct asd::jkl \{} \
-{/\* 0 | 8 \*/ signed char \*f1;} \
-{/\* 8 | 8 \*/ union \{} \
-{/\* 8 \*/ void \*ff1;} \
-{} \
-{ /\* total size \(bytes\): 8 \*/} \
-{ \} f2;} \
-{/\* 16 | 8 \*/ union \{} \
-{/\* 8 \*/ signed char \*ff2;} \
-{} \
-{ /\* total size \(bytes\): 8 \*/} \
-{ \} f3;} \
-{/\* 24:27 | 4 \*/ int f4 : 5;} \
-{/\* 24:26 | 4 \*/ unsigned int f5 : 1;} \
-{/\* XXX 2-bit hole \*/} \
-{/\* XXX 1-byte hole \*/} \
-{/\* 26 | 2 \*/ short f6;} \
-{} \
-{ /\* total size \(bytes\): 32 \*/} \
-{ \} f7;} \
-{/\* 32 | 8 \*/ unsigned long f8;} \
-{/\* 40 | 8 \*/ signed char \*f9;} \
-{/\* 48:28 | 4 \*/ int f10 : 4;} \
-{/\* 48:27 | 4 \*/ unsigned int f11 : 1;} \
-{/\* 48:26 | 4 \*/ unsigned int f12 : 1;} \
-{/\* 48:25 | 4 \*/ unsigned int f13 : 1;} \
-{/\* 48:24 | 4 \*/ unsigned int f14 : 1;} \
-{/\* XXX 7-byte hole \*/} \
-{/\* 56 | 8 \*/ void \*f15;} \
-{/\* 64 | 8 \*/ void \*f16;} \
-{} \
-{ /\* total size \(bytes\): 72 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct asd \{" \
+"/* 0 | 32 */ struct asd::jkl \{" \
+"/* 0 | 8 */ signed char *f1;" \
+"/* 8 | 8 */ union \{" \
+"/* 8 */ void *ff1;" \
+"" \
+" /* total size (bytes): 8 */" \
+" \} f2;" \
+"/* 16 | 8 */ union \{" \
+"/* 8 */ signed char *ff2;" \
+"" \
+" /* total size (bytes): 8 */" \
+" \} f3;" \
+"/* 24:27 | 4 */ int f4 : 5;" \
+"/* 24:26 | 4 */ unsigned int f5 : 1;" \
+"/* XXX 2-bit hole */" \
+"/* XXX 1-byte hole */" \
+"/* 26 | 2 */ short f6;" \
+"/* XXX 4-byte padding */" \
+"" \
+" /* total size (bytes): 32 */" \
+" \} f7;" \
+"/* 32 | 8 */ unsigned long f8;" \
+"/* 40 | 8 */ signed char *f9;" \
+"/* 48:28 | 4 */ int f10 : 4;" \
+"/* 48:27 | 4 */ unsigned int f11 : 1;" \
+"/* 48:26 | 4 */ unsigned int f12 : 1;" \
+"/* 48:25 | 4 */ unsigned int f13 : 1;" \
+"/* 48:24 | 4 */ unsigned int f14 : 1;" \
+"/* XXX 7-byte hole */" \
+"/* 56 | 8 */ void *f15;" \
+"/* 64 | 8 */ void *f16;" \
+"" \
+" /* total size (bytes): 72 */" \
+" \}"]]
# Test that we don't print any header when issuing a "ptype /o" on a
# non-struct, non-union, non-class type.
@@ -332,10 +333,10 @@ gdb_test_multiple "$test" "$test" {
# Test that printing a struct with a static member of itself doesn't
# get us into an infinite loop.
gdb_test "ptype/o static_member" \
- [multi_line \
-{/\* offset | size \*/ type = struct static_member \{} \
-{ static static_member Empty;} \
-{\/* 0 | 4 \*/ int abc;} \
-{} \
-{ /\* total size \(bytes\): 4 \*/} \
-{ \}}]
+ [string_to_regexp [multi_line \
+"/* offset | size */ type = struct static_member \{" \
+" static static_member Empty;" \
+"\/* 0 | 4 */ int abc;" \
+"" \
+" /* total size (bytes): 4 */" \
+" \}"]]
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 02c04514303..d1cdfe11cc0 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -94,11 +94,11 @@ print_offset_data::maybe_print_hole (struct ui_file *stream,
unsigned int hole_bit = hole % TARGET_CHAR_BIT;
if (hole_bit > 0)
- fprintf_filtered (stream, "/* XXX %2u-bit %s */\n", hole_bit,
+ fprintf_filtered (stream, "/* XXX %2u-bit %s */\n", hole_bit,
for_what);
if (hole_byte > 0)
- fprintf_filtered (stream, "/* XXX %2u-byte %s */\n", hole_byte,
+ fprintf_filtered (stream, "/* XXX %2u-byte %s */\n", hole_byte,
for_what);
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/2] two ptype/o changes
@ 2019-04-29 18:31 Tom Tromey
2019-04-29 18:31 ` [PATCH 1/2] Fix ptype/o comment formatting Tom Tromey
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tom Tromey @ 2019-04-29 18:31 UTC (permalink / raw)
To: gdb-patches
This series changes ptype/o in a couple of ways.
Last week I spent quite some time puzzling over the meaning of the
'offset' for a bitfield in ptype/o output. After talking with Sergio
on irc, I ended up concluding that gdb should instead print the bit
offset, not the number of bits remaining in the field's allocation.
That is patch #2. Then, I noticed that the tests did not fail,
despite the output changing. It turns out the tests weren't working
correctly, so I wrote patch #1.
Tested on x86-64 Fedora 29. Let me know what you think.
Patch #2 includes a doc change.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] Change ptype/o to print bit offset
2019-04-29 18:31 [PATCH 0/2] two ptype/o changes Tom Tromey
2019-04-29 18:31 ` [PATCH 1/2] Fix ptype/o comment formatting Tom Tromey
@ 2019-04-29 18:31 ` Tom Tromey
2019-04-29 18:52 ` Eli Zaretskii
2019-04-29 20:10 ` John Baldwin
2019-05-08 16:14 ` [PATCH 0/2] two ptype/o changes Tom Tromey
2 siblings, 2 replies; 7+ messages in thread
From: Tom Tromey @ 2019-04-29 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Consider this short C example:
struct inner
{
unsigned x;
unsigned y : 3;
unsigned z : 3;
};
struct outer
{
unsigned char o : 3;
struct inner i __attribute__ ((packed));
};
When I use "ptype/o" on this, I get:
(gdb) ptype/o struct outer
/* offset | size */ type = struct outer {
/* 0: 5 | 1 */ unsigned char o : 3;
/* XXX 5-bit hole */
/* 1 | 8 */ struct inner {
/* 1 | 4 */ unsigned int x;
/* 5:29 | 4 */ unsigned int y : 3;
/* 5:26 | 4 */ unsigned int z : 3;
/* XXX 2-bit padding */
/* XXX 3-byte padding */
/* total size (bytes): 8 */
} i;
/* total size (bytes): 9 */
}
In the location of "o" ("0: 5"), the "5" means "there are 5 bits left
relative to the size of the underlying type.
I find this very difficult to follow. On irc, Sergio said that this
choice came because it is what pahole does. However, I think it's not
very useful, and maybe is just an artifact of the way that
DW_AT_bit_offset was defined in DWARF 3.
This patch changes ptype/o to print the offset of a bitfield in a more
natural way, that is, using the bit number according to the platform's
bit numbering.
With this patch, the output is now:
(gdb) ptype/o struct outer
/* offset | size */ type = struct outer {
/* 0: 0 | 1 */ unsigned char o : 3;
/* XXX 5-bit hole */
/* 1 | 8 */ struct inner {
/* 1 | 4 */ unsigned int x;
/* 5: 0 | 4 */ unsigned int y : 3;
/* 5: 3 | 4 */ unsigned int z : 3;
/* XXX 2-bit padding */
/* XXX 3-byte padding */
/* total size (bytes): 8 */
} i;
/* total size (bytes): 9 */
}
This is better, IMO, because now the "offset" of a bitfield is
consistent with the offset of an ordinary member, referring to its
offset from the start of the structure.
gdb/ChangeLog
2019-04-29 Tom Tromey <tromey@adacore.com>
* typeprint.c (print_offset_data::update): Print the bit offset,
not the number of bits remaining.
gdb/doc/ChangeLog
2019-04-29 Tom Tromey <tromey@adacore.com>
* gdb.texinfo (Symbols): Document change to ptype/o.
gdb/testsuite/ChangeLog
2019-04-29 Tom Tromey <tromey@adacore.com>
* gdb.base/ptype-offsets.exp: Update tests.
---
gdb/ChangeLog | 5 ++++
gdb/doc/ChangeLog | 4 ++++
gdb/doc/gdb.texinfo | 9 +++----
gdb/testsuite/ChangeLog | 4 ++++
gdb/testsuite/gdb.base/ptype-offsets.exp | 30 ++++++++++++------------
gdb/typeprint.c | 30 ++++++------------------
6 files changed, 40 insertions(+), 42 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cf8333d86be..83528a3385e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17803,15 +17803,16 @@ bitfields:
/* XXX 3-bit hole */
/* XXX 4-byte hole */
/* 8 | 8 */ int64_t a5;
-/* 16:27 | 4 */ int a6 : 5;
-/* 16:56 | 8 */ int64_t a7 : 3;
+/* 16: 0 | 4 */ int a6 : 5;
+/* 16: 5 | 8 */ int64_t a7 : 3;
+"/* XXX 7-byte padding */
/* total size (bytes): 24 */
@}
@end smallexample
-Note how the offset information is now extended to also include how
-many bits are left to be used in each bitfield.
+Note how the offset information is now extended to also include the
+first bit of the bitfield.
@end table
@kindex ptype
diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
index 12b3a746005..6116520bbd7 100644
--- a/gdb/testsuite/gdb.base/ptype-offsets.exp
+++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
@@ -38,7 +38,7 @@ gdb_test "ptype /o struct abc" \
"/* offset | size */ type = struct abc \{" \
" public:" \
"/* 8 | 8 */ void *field1;" \
-"/* 16:31 | 4 */ unsigned int field2 : 1;" \
+"/* 16: 0 | 4 */ unsigned int field2 : 1;" \
"/* XXX 7-bit hole */" \
"/* XXX 3-byte hole */" \
"/* 20 | 4 */ int field3;" \
@@ -63,7 +63,7 @@ gdb_test "ptype /oTM struct abc" \
"/* offset | size */ type = struct abc \{" \
" public:" \
"/* 8 | 8 */ void *field1;" \
-"/* 16:31 | 4 */ unsigned int field2 : 1;" \
+"/* 16: 0 | 4 */ unsigned int field2 : 1;" \
"/* XXX 7-bit hole */" \
"/* XXX 3-byte hole */" \
"/* 20 | 4 */ int field3;" \
@@ -93,7 +93,7 @@ gdb_test "ptype /TMo struct abc" \
"/* offset | size */ type = struct abc \{" \
" public:" \
"/* 8 | 8 */ void *field1;" \
-"/* 16:31 | 4 */ unsigned int field2 : 1;" \
+"/* 16: 0 | 4 */ unsigned int field2 : 1;" \
"/* XXX 7-bit hole */" \
"/* XXX 3-byte hole */" \
"/* 20 | 4 */ int field3;" \
@@ -264,15 +264,15 @@ gdb_test "ptype /o struct poi" \
gdb_test "ptype /o struct tyu" \
[string_to_regexp [multi_line \
"/* offset | size */ type = struct tyu \{" \
-"/* 0:31 | 4 */ int a1 : 1;" \
-"/* 0:28 | 4 */ int a2 : 3;" \
-"/* 0: 5 | 4 */ int a3 : 23;" \
+"/* 0: 0 | 4 */ int a1 : 1;" \
+"/* 0: 1 | 4 */ int a2 : 3;" \
+"/* 0: 4 | 4 */ int a3 : 23;" \
"/* 3: 3 | 1 */ signed char a4 : 2;" \
"/* XXX 3-bit hole */" \
"/* XXX 4-byte hole */" \
"/* 8 | 8 */ int64_t a5;" \
-"/* 16:27 | 4 */ int a6 : 5;" \
-"/* 16:56 | 8 */ int64_t a7 : 3;" \
+"/* 16: 0 | 4 */ int a6 : 5;" \
+"/* 16: 5 | 8 */ int64_t a7 : 3;" \
"/* XXX 7-byte padding */" \
"" \
" /* total size (bytes): 24 */" \
@@ -293,8 +293,8 @@ gdb_test "ptype /o struct asd" \
"" \
" /* total size (bytes): 8 */" \
" \} f3;" \
-"/* 24:27 | 4 */ int f4 : 5;" \
-"/* 24:26 | 4 */ unsigned int f5 : 1;" \
+"/* 24: 0 | 4 */ int f4 : 5;" \
+"/* 24: 5 | 4 */ unsigned int f5 : 1;" \
"/* XXX 2-bit hole */" \
"/* XXX 1-byte hole */" \
"/* 26 | 2 */ short f6;" \
@@ -304,11 +304,11 @@ gdb_test "ptype /o struct asd" \
" \} f7;" \
"/* 32 | 8 */ unsigned long f8;" \
"/* 40 | 8 */ signed char *f9;" \
-"/* 48:28 | 4 */ int f10 : 4;" \
-"/* 48:27 | 4 */ unsigned int f11 : 1;" \
-"/* 48:26 | 4 */ unsigned int f12 : 1;" \
-"/* 48:25 | 4 */ unsigned int f13 : 1;" \
-"/* 48:24 | 4 */ unsigned int f14 : 1;" \
+"/* 48: 0 | 4 */ int f10 : 4;" \
+"/* 48: 4 | 4 */ unsigned int f11 : 1;" \
+"/* 48: 5 | 4 */ unsigned int f12 : 1;" \
+"/* 48: 6 | 4 */ unsigned int f13 : 1;" \
+"/* 48: 7 | 4 */ unsigned int f14 : 1;" \
"/* XXX 7-byte hole */" \
"/* 56 | 8 */ void *f15;" \
"/* 64 | 8 */ void *f16;" \
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index d1cdfe11cc0..7a44dbdb313 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -131,32 +131,16 @@ print_offset_data::update (struct type *type, unsigned int field_idx,
maybe_print_hole (stream, bitpos, "hole");
- if (TYPE_FIELD_PACKED (type, field_idx))
+ if (TYPE_FIELD_PACKED (type, field_idx)
+ || offset_bitpos % TARGET_CHAR_BIT != 0)
{
- /* We're dealing with a bitfield. Print how many bits are left
- to be used. */
- unsigned int bitsize = TYPE_FIELD_BITSIZE (type, field_idx);
- /* The bitpos relative to the beginning of our container
- field. */
- unsigned int relative_bitpos;
-
- /* The following was copied from
- value.c:value_primitive_field. */
- if ((bitpos % fieldsize_bit) + bitsize <= fieldsize_bit)
- relative_bitpos = bitpos % fieldsize_bit;
- else
- relative_bitpos = bitpos % TARGET_CHAR_BIT;
+ /* We're dealing with a bitfield. Print the bit offset. */
+ fieldsize_bit = TYPE_FIELD_BITSIZE (type, field_idx);
- /* This is the exact offset (in bits) of this bitfield. */
- unsigned int bit_offset
- = (bitpos - relative_bitpos) + offset_bitpos;
+ unsigned real_bitpos = bitpos + offset_bitpos;
- /* The position of the field, relative to the beginning of the
- struct, and how many bits are left to be used in this
- container. */
- fprintf_filtered (stream, "/* %4u:%2u", bit_offset / TARGET_CHAR_BIT,
- fieldsize_bit - (relative_bitpos + bitsize));
- fieldsize_bit = bitsize;
+ fprintf_filtered (stream, "/* %4u:%2u", real_bitpos / TARGET_CHAR_BIT,
+ real_bitpos % TARGET_CHAR_BIT);
}
else
{
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Change ptype/o to print bit offset
2019-04-29 18:31 ` [PATCH 2/2] Change ptype/o to print bit offset Tom Tromey
@ 2019-04-29 18:52 ` Eli Zaretskii
2019-04-29 18:57 ` Tom Tromey
2019-04-29 20:10 ` John Baldwin
1 sibling, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2019-04-29 18:52 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>
> Date: Mon, 29 Apr 2019 12:31:05 -0600
>
> With this patch, the output is now:
>
> (gdb) ptype/o struct outer
> /* offset | size */ type = struct outer {
> /* 0: 0 | 1 */ unsigned char o : 3;
> /* XXX 5-bit hole */
> /* 1 | 8 */ struct inner {
> /* 1 | 4 */ unsigned int x;
> /* 5: 0 | 4 */ unsigned int y : 3;
> /* 5: 3 | 4 */ unsigned int z : 3;
> /* XXX 2-bit padding */
> /* XXX 3-byte padding */
This loses information, because now the bits part is just a trivial
conversion of the field size in the declaration. The current display
shows something that cannot be trivially gleaned by looking at the
bitfield sizes.
I'm not objecting to the change, I'm just saying we lose something
here.
> gdb/doc/ChangeLog
> 2019-04-29 Tom Tromey <tromey@adacore.com>
>
> * gdb.texinfo (Symbols): Document change to ptype/o.
This seems to be a mechanical change, so OK.
Do we need to call this out in NEWS?
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Change ptype/o to print bit offset
2019-04-29 18:52 ` Eli Zaretskii
@ 2019-04-29 18:57 ` Tom Tromey
0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2019-04-29 18:57 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>> /* 5: 0 | 4 */ unsigned int y : 3;
>> /* 5: 3 | 4 */ unsigned int z : 3;
>> /* XXX 2-bit padding */
>> /* XXX 3-byte padding */
Eli> This loses information, because now the bits part is just a trivial
Eli> conversion of the field size in the declaration. The current display
Eli> shows something that cannot be trivially gleaned by looking at the
Eli> bitfield sizes.
Eli> I'm not objecting to the change, I'm just saying we lose something
Eli> here.
We don't actually lose anything here, because the padding is spelled out
explicitly in the comments that are printed.
Also, consider an example like this, from an Ada test case:
/* offset | size */ type = struct aggregates__nested_packed {
/* 0: 5 | 1 */ <range type> q000 : 3;
/* 0:23 | 8 */ struct aggregates__packed_rec {
/* 0 | 4 */ integer packed_array_assign_w;
/* 4: 5 | 1 */ <range type> packed_array_assign_x : 3;
/* 4: 2 | 1 */ <range type> packed_array_assign_y : 3;
/* XXX 2-bit padding */
/* XXX 3-byte padding */
/* total size (bytes): 8 */
} r000 : 38;
The "0:23" here is, IMO, actively confusing.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Change ptype/o to print bit offset
2019-04-29 18:31 ` [PATCH 2/2] Change ptype/o to print bit offset Tom Tromey
2019-04-29 18:52 ` Eli Zaretskii
@ 2019-04-29 20:10 ` John Baldwin
1 sibling, 0 replies; 7+ messages in thread
From: John Baldwin @ 2019-04-29 20:10 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 4/29/19 11:31 AM, Tom Tromey wrote:
> This is better, IMO, because now the "offset" of a bitfield is
> consistent with the offset of an ordinary member, referring to its
> offset from the start of the structure.
I agree with this. This is what I use ptype /o for (to get starting
offsets). pahole's format may make sense if the bits were numbered
N -> 0 instead of 0 -> N, or if bitfields used "high" bits instead of
"low" bits, but the convention I always see is to use 0 for LSB and
N for MSB. I'm also assuming that compilers start with the LSB when
assigning bits to bitfield members.
--
John Baldwin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] two ptype/o changes
2019-04-29 18:31 [PATCH 0/2] two ptype/o changes Tom Tromey
2019-04-29 18:31 ` [PATCH 1/2] Fix ptype/o comment formatting Tom Tromey
2019-04-29 18:31 ` [PATCH 2/2] Change ptype/o to print bit offset Tom Tromey
@ 2019-05-08 16:14 ` Tom Tromey
2 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2019-05-08 16:14 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> This series changes ptype/o in a couple of ways.
Tom> Last week I spent quite some time puzzling over the meaning of the
Tom> 'offset' for a bitfield in ptype/o output. After talking with Sergio
Tom> on irc, I ended up concluding that gdb should instead print the bit
Tom> offset, not the number of bits remaining in the field's allocation.
Tom> That is patch #2. Then, I noticed that the tests did not fail,
Tom> despite the output changing. It turns out the tests weren't working
Tom> correctly, so I wrote patch #1.
Tom> Tested on x86-64 Fedora 29. Let me know what you think.
Tom> Patch #2 includes a doc change.
I'm checking this in now.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-08 16:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 18:31 [PATCH 0/2] two ptype/o changes Tom Tromey
2019-04-29 18:31 ` [PATCH 1/2] Fix ptype/o comment formatting Tom Tromey
2019-04-29 18:31 ` [PATCH 2/2] Change ptype/o to print bit offset Tom Tromey
2019-04-29 18:52 ` Eli Zaretskii
2019-04-29 18:57 ` Tom Tromey
2019-04-29 20:10 ` John Baldwin
2019-05-08 16:14 ` [PATCH 0/2] two ptype/o changes Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).