* [RFA 1/2] Conditionally drop the discriminant field in quirk_rust_enum
2018-04-12 19:08 [RFA 0/2] Minor Rust-related DWARF reader fixes Tom Tromey
@ 2018-04-12 19:08 ` Tom Tromey
2018-04-13 7:36 ` Joel Brobecker
2018-04-12 19:12 ` [RFA 2/2] Set TYPE_NAME for Rust unions Tom Tromey
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2018-04-12 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
While debugging the crash that Jan reported, I noticed that in some
situations we could end up with a situation where one branch of a Rust
enum type ended up with a field count of -1.
The fix is simple: only conditionally drop the discriminant field when
rewriting the enum variants.
I couldn't find a way to test this; I only noticed it while debugging
the DWARF reader.
2018-04-12 Tom Tromey <tom@tromey.com>
* dwarf2read.c (quirk_rust_enum): Conditionally drop the
discriminant field.
---
gdb/ChangeLog | 5 +++++
gdb/dwarf2read.c | 9 ++++++---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 649c9cea97..ceaa5f92ab 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-04-12 Tom Tromey <tom@tromey.com>
+
+ * dwarf2read.c (quirk_rust_enum): Conditionally drop the
+ discriminant field.
+
2018-03-29 Tom Tromey <tom@tromey.com>
* dwarf2read.c (quirk_rust_enum): Handle unions correctly.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0d3af00c46..4207e4c531 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10079,10 +10079,13 @@ quirk_rust_enum (struct type *type, struct objfile *objfile)
if (iter != discriminant_map.end ())
disc->discriminants[i] = iter->second;
- /* Remove the discriminant field. */
+ /* Remove the discriminant field, if it exists. */
struct type *sub_type = TYPE_FIELD_TYPE (union_type, i);
- --TYPE_NFIELDS (sub_type);
- ++TYPE_FIELDS (sub_type);
+ if (TYPE_NFIELDS (sub_type) > 0)
+ {
+ --TYPE_NFIELDS (sub_type);
+ ++TYPE_FIELDS (sub_type);
+ }
TYPE_FIELD_NAME (union_type, i) = variant_name;
TYPE_NAME (sub_type)
= rust_fully_qualify (&objfile->objfile_obstack,
--
2.13.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 0/2] Minor Rust-related DWARF reader fixes
@ 2018-04-12 19:08 Tom Tromey
2018-04-12 19:08 ` [RFA 1/2] Conditionally drop the discriminant field in quirk_rust_enum Tom Tromey
2018-04-12 19:12 ` [RFA 2/2] Set TYPE_NAME for Rust unions Tom Tromey
0 siblings, 2 replies; 6+ messages in thread
From: Tom Tromey @ 2018-04-12 19:08 UTC (permalink / raw)
To: gdb-patches
These patches fix some minor, but not fatal, issues in the Rust code
in dwarf2read.c.
I tested these locally using rustc 1.23 on x86-64 Fedora 26.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 2/2] Set TYPE_NAME for Rust unions
2018-04-12 19:08 [RFA 0/2] Minor Rust-related DWARF reader fixes Tom Tromey
2018-04-12 19:08 ` [RFA 1/2] Conditionally drop the discriminant field in quirk_rust_enum Tom Tromey
@ 2018-04-12 19:12 ` Tom Tromey
2018-04-13 7:41 ` Joel Brobecker
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2018-04-12 19:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed that read_structure_type sets TYPE_NAME to TYPE_TAG_NAME for
some things, but not for unions. For Rust, I think we want to do this
for all types, so this patch adds the needed code.
I think that TYPE_TAG_NAME is confusing and should probably be
removed. I am working on a patch to do this, but in the meantime this
seemed like a reasonable change to apply. I suspect this change would
be ok for C++ as well, but I do not know about D.
2018-04-12 Tom Tromey <tom@tromey.com>
* dwarf2read.c (read_structure_type): Set TYPE_NAME for rust
unions.
---
gdb/ChangeLog | 5 +++++
gdb/dwarf2read.c | 4 +++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ceaa5f92ab..f2bb3d0af3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2018-04-12 Tom Tromey <tom@tromey.com>
+ * dwarf2read.c (read_structure_type): Set TYPE_NAME for rust
+ unions.
+
+2018-04-12 Tom Tromey <tom@tromey.com>
+
* dwarf2read.c (quirk_rust_enum): Conditionally drop the
discriminant field.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4207e4c531..e3f544a6a4 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -15631,7 +15631,9 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
TYPE_TAG_NAME (type) = full_name;
if (die->tag == DW_TAG_structure_type
- || die->tag == DW_TAG_class_type)
+ || die->tag == DW_TAG_class_type
+ || (cu->language == language_rust
+ && die->tag == DW_TAG_union_type))
TYPE_NAME (type) = TYPE_TAG_NAME (type);
}
else
--
2.13.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 1/2] Conditionally drop the discriminant field in quirk_rust_enum
2018-04-12 19:08 ` [RFA 1/2] Conditionally drop the discriminant field in quirk_rust_enum Tom Tromey
@ 2018-04-13 7:36 ` Joel Brobecker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Brobecker @ 2018-04-13 7:36 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi Tom,
> 2018-04-12 Tom Tromey <tom@tromey.com>
>
> * dwarf2read.c (quirk_rust_enum): Conditionally drop the
> discriminant field.
Looks good!
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Set TYPE_NAME for Rust unions
2018-04-12 19:12 ` [RFA 2/2] Set TYPE_NAME for Rust unions Tom Tromey
@ 2018-04-13 7:41 ` Joel Brobecker
2018-04-17 19:37 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2018-04-13 7:41 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> I think that TYPE_TAG_NAME is confusing and should probably be
> removed. I am working on a patch to do this, but in the meantime this
> seemed like a reasonable change to apply. I suspect this change would
> be ok for C++ as well, but I do not know about D.
I agree that TYPE_TAG_NAME is confusing, and I would welcome it being
removed.
> 2018-04-12 Tom Tromey <tom@tromey.com>
>
> * dwarf2read.c (read_structure_type): Set TYPE_NAME for rust
> unions.
The patch looks good to me, but I'd like to see if others have comments
about C++. I'm trying to remember if C++ enums might have different
rules compared to C enums, and I'm not remembering any... If there
weren't any difference, and things were OK with C enums, one might
think that we're OK for C++ enums as well...
No way to test this either?
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Set TYPE_NAME for Rust unions
2018-04-13 7:41 ` Joel Brobecker
@ 2018-04-17 19:37 ` Tom Tromey
0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2018-04-17 19:37 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>> 2018-04-12 Tom Tromey <tom@tromey.com>
>>
>> * dwarf2read.c (read_structure_type): Set TYPE_NAME for rust
>> unions.
Joel> The patch looks good to me, but I'd like to see if others have comments
Joel> about C++. I'm trying to remember if C++ enums might have different
Joel> rules compared to C enums, and I'm not remembering any... If there
Joel> weren't any difference, and things were OK with C enums, one might
Joel> think that we're OK for C++ enums as well...
Joel> No way to test this either?
I am not sure. However, my TYPE_TAG_NAME deletion patch is ready now
(I'll send it momentarily), so I think we can just drop this one,
because that patch obsoletes it.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-17 19:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12 19:08 [RFA 0/2] Minor Rust-related DWARF reader fixes Tom Tromey
2018-04-12 19:08 ` [RFA 1/2] Conditionally drop the discriminant field in quirk_rust_enum Tom Tromey
2018-04-13 7:36 ` Joel Brobecker
2018-04-12 19:12 ` [RFA 2/2] Set TYPE_NAME for Rust unions Tom Tromey
2018-04-13 7:41 ` Joel Brobecker
2018-04-17 19:37 ` 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).