* [PATCH 1/2] Make gdb property batons type-safe
2023-03-01 15:18 [PATCH 0/2] Two minor dynamic property fixes Tom Tromey
@ 2023-03-01 15:18 ` Tom Tromey
2023-03-01 18:44 ` Simon Marchi
2023-03-01 15:18 ` [PATCH 2/2] Use const for dwarf2_property_baton Tom Tromey
2023-03-01 18:45 ` [PATCH 0/2] Two minor dynamic property fixes Simon Marchi
2 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2023-03-01 15:18 UTC (permalink / raw)
To: gdb-patches
gdbtypes treats dynamic property batons as 'void *', but in actuality
the only users all use dwarf2_property_baton. This patch changes this
code to be type-safe. If a new type is needed here, it seems like
that too could be done in a type-safe way.
---
gdb/dwarf2/loc.c | 12 ++++--------
gdb/gdbtypes.h | 11 ++++++-----
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 4727651027b..bf582bcfeff 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -1649,8 +1649,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
{
case PROP_LOCEXPR:
{
- const struct dwarf2_property_baton *baton
- = (const struct dwarf2_property_baton *) prop->baton ();
+ const struct dwarf2_property_baton *baton = prop->baton ();
gdb_assert (baton->property_type != NULL);
bool is_reference = baton->locexpr.is_reference;
@@ -1692,8 +1691,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
case PROP_LOCLIST:
{
- struct dwarf2_property_baton *baton
- = (struct dwarf2_property_baton *) prop->baton ();
+ struct dwarf2_property_baton *baton = prop->baton ();
CORE_ADDR pc;
const gdb_byte *data;
struct value *val;
@@ -1724,8 +1722,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
case PROP_ADDR_OFFSET:
{
- struct dwarf2_property_baton *baton
- = (struct dwarf2_property_baton *) prop->baton ();
+ struct dwarf2_property_baton *baton = prop->baton ();
const struct property_addr_info *pinfo;
struct value *val;
@@ -1775,8 +1772,7 @@ dwarf2_compile_property_to_c (string_file *stream,
CORE_ADDR pc,
struct symbol *sym)
{
- struct dwarf2_property_baton *baton
- = (struct dwarf2_property_baton *) prop->baton ();
+ struct dwarf2_property_baton *baton = prop->baton ();
const gdb_byte *data;
size_t size;
dwarf2_per_cu_data *per_cu;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c2253310666..701a64d457a 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -64,6 +64,7 @@ struct value_print_options;
struct language_defn;
struct dwarf2_per_cu_data;
struct dwarf2_per_objfile;
+struct dwarf2_property_baton;
/* Some macros for char-based bitfields. */
@@ -289,7 +290,7 @@ union dynamic_prop_data
/* Storage for dynamic property. */
- void *baton;
+ dwarf2_property_baton *baton;
/* Storage of variant parts for a type. A type with variant parts
has all its fields "linearized" -- stored in a single field
@@ -339,7 +340,7 @@ struct dynamic_prop
m_data.const_val = const_val;
}
- void *baton () const
+ dwarf2_property_baton *baton () const
{
gdb_assert (m_kind == PROP_LOCEXPR
|| m_kind == PROP_LOCLIST
@@ -348,19 +349,19 @@ struct dynamic_prop
return m_data.baton;
}
- void set_locexpr (void *baton)
+ void set_locexpr (dwarf2_property_baton *baton)
{
m_kind = PROP_LOCEXPR;
m_data.baton = baton;
}
- void set_loclist (void *baton)
+ void set_loclist (dwarf2_property_baton *baton)
{
m_kind = PROP_LOCLIST;
m_data.baton = baton;
}
- void set_addr_offset (void *baton)
+ void set_addr_offset (dwarf2_property_baton *baton)
{
m_kind = PROP_ADDR_OFFSET;
m_data.baton = baton;
--
2.39.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Make gdb property batons type-safe
2023-03-01 15:18 ` [PATCH 1/2] Make gdb property batons type-safe Tom Tromey
@ 2023-03-01 18:44 ` Simon Marchi
2023-03-01 22:28 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2023-03-01 18:44 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 3/1/23 10:18, Tom Tromey via Gdb-patches wrote:
> gdbtypes treats dynamic property batons as 'void *', but in actuality
> the only users all use dwarf2_property_baton. This patch changes this
> code to be type-safe. If a new type is needed here, it seems like
> that too could be done in a type-safe way.
I don't mind doing this, because in practice the DWARF reader is the
only one to use that data pointer. But just wondering, what would be
the "right" way to implement this pattern in a type-safe way, if
multiple debug info readers wanted to use that field?
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Make gdb property batons type-safe
2023-03-01 18:44 ` Simon Marchi
@ 2023-03-01 22:28 ` Tom Tromey
0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2023-03-01 22:28 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> On 3/1/23 10:18, Tom Tromey via Gdb-patches wrote:
>> gdbtypes treats dynamic property batons as 'void *', but in actuality
>> the only users all use dwarf2_property_baton. This patch changes this
>> code to be type-safe. If a new type is needed here, it seems like
>> that too could be done in a type-safe way.
Simon> I don't mind doing this, because in practice the DWARF reader is the
Simon> only one to use that data pointer. But just wondering, what would be
Simon> the "right" way to implement this pattern in a type-safe way, if
Simon> multiple debug info readers wanted to use that field?
There's several possibilities but one would be to just add a new element
to the union and new setters/getters.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Use const for dwarf2_property_baton
2023-03-01 15:18 [PATCH 0/2] Two minor dynamic property fixes Tom Tromey
2023-03-01 15:18 ` [PATCH 1/2] Make gdb property batons type-safe Tom Tromey
@ 2023-03-01 15:18 ` Tom Tromey
2023-03-01 18:45 ` [PATCH 0/2] Two minor dynamic property fixes Simon Marchi
2 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2023-03-01 15:18 UTC (permalink / raw)
To: gdb-patches
Once a baton is stored in a struct type, it doesn't make sense to
modify it. This patch constifies the API.
---
gdb/dwarf2/loc.c | 8 ++++----
gdb/dwarf2/loc.h | 2 +-
gdb/gdbtypes.h | 10 +++++-----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index bf582bcfeff..914e016f085 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -347,7 +347,7 @@ decode_debug_loc_dwo_addresses (dwarf2_per_cu_data *per_cu,
can be more than one in the list. */
const gdb_byte *
-dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
+dwarf2_find_location_expression (const dwarf2_loclist_baton *baton,
size_t *locexpr_length, CORE_ADDR pc)
{
dwarf2_per_objfile *per_objfile = baton->per_objfile;
@@ -1691,7 +1691,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
case PROP_LOCLIST:
{
- struct dwarf2_property_baton *baton = prop->baton ();
+ const dwarf2_property_baton *baton = prop->baton ();
CORE_ADDR pc;
const gdb_byte *data;
struct value *val;
@@ -1722,7 +1722,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
case PROP_ADDR_OFFSET:
{
- struct dwarf2_property_baton *baton = prop->baton ();
+ const dwarf2_property_baton *baton = prop->baton ();
const struct property_addr_info *pinfo;
struct value *val;
@@ -1772,7 +1772,7 @@ dwarf2_compile_property_to_c (string_file *stream,
CORE_ADDR pc,
struct symbol *sym)
{
- struct dwarf2_property_baton *baton = prop->baton ();
+ const dwarf2_property_baton *baton = prop->baton ();
const gdb_byte *data;
size_t size;
dwarf2_per_cu_data *per_cu;
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 37925f48497..ad60177e93c 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -37,7 +37,7 @@ extern unsigned int entry_values_debug;
/* Find a particular location expression from a location list. */
const gdb_byte *dwarf2_find_location_expression
- (struct dwarf2_loclist_baton *baton,
+ (const dwarf2_loclist_baton *baton,
size_t *locexpr_length,
CORE_ADDR pc);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 701a64d457a..c4889a4a05b 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -290,7 +290,7 @@ union dynamic_prop_data
/* Storage for dynamic property. */
- dwarf2_property_baton *baton;
+ const dwarf2_property_baton *baton;
/* Storage of variant parts for a type. A type with variant parts
has all its fields "linearized" -- stored in a single field
@@ -340,7 +340,7 @@ struct dynamic_prop
m_data.const_val = const_val;
}
- dwarf2_property_baton *baton () const
+ const dwarf2_property_baton *baton () const
{
gdb_assert (m_kind == PROP_LOCEXPR
|| m_kind == PROP_LOCLIST
@@ -349,19 +349,19 @@ struct dynamic_prop
return m_data.baton;
}
- void set_locexpr (dwarf2_property_baton *baton)
+ void set_locexpr (const dwarf2_property_baton *baton)
{
m_kind = PROP_LOCEXPR;
m_data.baton = baton;
}
- void set_loclist (dwarf2_property_baton *baton)
+ void set_loclist (const dwarf2_property_baton *baton)
{
m_kind = PROP_LOCLIST;
m_data.baton = baton;
}
- void set_addr_offset (dwarf2_property_baton *baton)
+ void set_addr_offset (const dwarf2_property_baton *baton)
{
m_kind = PROP_ADDR_OFFSET;
m_data.baton = baton;
--
2.39.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] Two minor dynamic property fixes
2023-03-01 15:18 [PATCH 0/2] Two minor dynamic property fixes Tom Tromey
2023-03-01 15:18 ` [PATCH 1/2] Make gdb property batons type-safe Tom Tromey
2023-03-01 15:18 ` [PATCH 2/2] Use const for dwarf2_property_baton Tom Tromey
@ 2023-03-01 18:45 ` Simon Marchi
2 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2023-03-01 18:45 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 3/1/23 10:18, Tom Tromey via Gdb-patches wrote:
> I noticed that dynamic properties were not type-safe or const-correct.
> These two patches fix this. Tested by rebuilding.
>
> Tom
>
> ---
> Tom Tromey (2):
> Make gdb property batons type-safe
> Use const for dwarf2_property_baton
>
> gdb/dwarf2/loc.c | 14 +++++---------
> gdb/dwarf2/loc.h | 2 +-
> gdb/gdbtypes.h | 11 ++++++-----
> 3 files changed, 12 insertions(+), 15 deletions(-)
> ---
> base-commit: 2c29b1ed19711fa2a16558015e5a6b46a09aefeb
> change-id: 20230301-submit-baton-stuff-70d3db0cd50b
>
> Best regards,
> --
> Tom Tromey <tromey@adacore.com>
>
I sent a general design question on patch 1, but otherwise:
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread