* [PATCH] Make addrmap const-correct in cooked index @ 2023-01-27 16:26 Tom Tromey 2023-01-27 19:58 ` Simon Marchi 0 siblings, 1 reply; 13+ messages in thread From: Tom Tromey @ 2023-01-27 16:26 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey After the cooked index is created, the addrmaps should be const. This is slightly complicated by the fact that addrmap::foreach is not const -- however, it should be. The underlying splay tree might be rearranged during a foreach, but if so, this is a classic case where 'mutable' semantics make sense. --- gdb/addrmap.c | 4 ++-- gdb/addrmap.h | 6 +++--- gdb/dwarf2/cooked-index.c | 4 ++-- gdb/dwarf2/cooked-index.h | 2 +- gdb/dwarf2/index-write.c | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/gdb/addrmap.c b/gdb/addrmap.c index 0a6b2e5a25a..47ceaca48ad 100644 --- a/gdb/addrmap.c +++ b/gdb/addrmap.c @@ -82,7 +82,7 @@ addrmap_fixed::relocate (CORE_ADDR offset) int -addrmap_fixed::foreach (addrmap_foreach_fn fn) +addrmap_fixed::foreach (addrmap_foreach_fn fn) const { size_t i; @@ -317,7 +317,7 @@ addrmap_mutable_foreach_worker (splay_tree_node node, void *data) int -addrmap_mutable::foreach (addrmap_foreach_fn fn) +addrmap_mutable::foreach (addrmap_foreach_fn fn) const { return splay_tree_foreach (tree, addrmap_mutable_foreach_worker, &fn); } diff --git a/gdb/addrmap.h b/gdb/addrmap.h index a5b784efaf2..659d3c977a8 100644 --- a/gdb/addrmap.h +++ b/gdb/addrmap.h @@ -95,7 +95,7 @@ struct addrmap If FN ever returns a non-zero value, the iteration ceases immediately, and the value is returned. Otherwise, this function returns 0. */ - virtual int foreach (addrmap_foreach_fn fn) = 0; + virtual int foreach (addrmap_foreach_fn fn) const = 0; }; struct addrmap_mutable; @@ -113,7 +113,7 @@ struct addrmap_fixed : public addrmap, void *obj) override; void *find (CORE_ADDR addr) const override; void relocate (CORE_ADDR offset) override; - int foreach (addrmap_foreach_fn fn) override; + int foreach (addrmap_foreach_fn fn) const override; private: @@ -151,7 +151,7 @@ struct addrmap_mutable : public addrmap void *obj) override; void *find (CORE_ADDR addr) const override; void relocate (CORE_ADDR offset) override; - int foreach (addrmap_foreach_fn fn) override; + int foreach (addrmap_foreach_fn fn) const override; private: diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 09b3fd70b26..6f228cdc749 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -401,10 +401,10 @@ cooked_index_vector::lookup (CORE_ADDR addr) /* See cooked-index.h. */ -std::vector<addrmap *> +std::vector<const addrmap *> cooked_index_vector::get_addrmaps () { - std::vector<addrmap *> result; + std::vector<const addrmap *> result; for (const auto &index : m_vector) result.push_back (index->m_addrmap); return result; diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 55eaf9955ab..dc3fe6e262f 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -359,7 +359,7 @@ class cooked_index_vector : public dwarf_scanner_base /* Return a new vector of all the addrmaps used by all the indexes held by this object. */ - std::vector<addrmap *> get_addrmaps (); + std::vector<const addrmap *> get_addrmaps (); /* Return the entry that is believed to represent the program's "main". This will return NULL if no such entry is available. */ diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index ced58eab661..9b4a02bc694 100644 --- a/gdb/dwarf2/index-write.c +++ b/gdb/dwarf2/index-write.c @@ -500,7 +500,7 @@ addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) in the index file. */ static void -write_address_map (struct addrmap *addrmap, data_buf &addr_vec, +write_address_map (const addrmap *addrmap, data_buf &addr_vec, cu_index_map &cu_index_htab) { struct addrmap_index_data addrmap_index_data (addr_vec, cu_index_htab); -- 2.38.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 16:26 [PATCH] Make addrmap const-correct in cooked index Tom Tromey @ 2023-01-27 19:58 ` Simon Marchi 2023-01-27 21:15 ` Tom Tromey 2023-01-27 21:18 ` Tom Tromey 0 siblings, 2 replies; 13+ messages in thread From: Simon Marchi @ 2023-01-27 19:58 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 1/27/23 11:26, Tom Tromey via Gdb-patches wrote: > After the cooked index is created, the addrmaps should be const. This > is slightly complicated by the fact that addrmap::foreach is not const > -- however, it should be. The underlying splay tree might be > rearranged during a foreach, but if so, this is a classic case where > 'mutable' semantics make sense. I tried making the parameter to addrmap_foreach_fn const too (which makes sense I think, if we're going to make foreach const), and came up with a slightly larger patch. It also made the find method return const, but it also makes sense given that it is marked const too. See the patch below, I think we can rebase yours on top of that. > diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c > index 09b3fd70b26..6f228cdc749 100644 > --- a/gdb/dwarf2/cooked-index.c > +++ b/gdb/dwarf2/cooked-index.c > @@ -401,10 +401,10 @@ cooked_index_vector::lookup (CORE_ADDR addr) > > /* See cooked-index.h. */ > > -std::vector<addrmap *> > +std::vector<const addrmap *> > cooked_index_vector::get_addrmaps () You can make the get_addrmaps method const itself. Otherwise, LGTM. Simon ---- From 5ca5ca3ebc138a3474e7708a5696f43a7abff0da Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@polymtl.ca> Date: Fri, 27 Jan 2023 14:46:50 -0500 Subject: [PATCH] gdb: constify addrmap::find and addrmap::foreach Constify the return type of addrmap::find. Since find is a const method, I think it makes sense for it to return a pointer to const, indicating to the callers that they should not modify the returned object. Then, make the foreach method const, and change the parameter to addrmap_foreach_fn to `const void *`. Obviously, the const can be cast away, but if using static_cast instead of C-style casts (like the change I did in addrmap_index_data::operator()), then the compiler won't let you cast the const away. Change-Id: Ia8e69d022564f80d961413658fe6068edc71a094 --- gdb/addrmap.c | 16 ++++++++-------- gdb/addrmap.h | 18 +++++++++--------- gdb/dwarf2/index-write.c | 10 ++++++---- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/gdb/addrmap.c b/gdb/addrmap.c index 0a6b2e5a25a9..43a521f38b23 100644 --- a/gdb/addrmap.c +++ b/gdb/addrmap.c @@ -39,7 +39,7 @@ addrmap_fixed::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, } -void * +const void * addrmap_fixed::find (CORE_ADDR addr) const { const struct addrmap_transition *bottom = &transitions[0]; @@ -82,7 +82,7 @@ addrmap_fixed::relocate (CORE_ADDR offset) int -addrmap_fixed::foreach (addrmap_foreach_fn fn) +addrmap_fixed::foreach (addrmap_foreach_fn fn) const { size_t i; @@ -239,7 +239,7 @@ addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, } -void * +const void * addrmap_mutable::find (CORE_ADDR addr) const { splay_tree_node n = splay_tree_lookup (addr); @@ -265,7 +265,7 @@ addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut) size_t transition_count = 0; /* Count the number of transitions in the tree. */ - mut->foreach ([&] (CORE_ADDR start, void *obj) + mut->foreach ([&] (CORE_ADDR start, const void *obj) { ++transition_count; return 0; @@ -283,7 +283,7 @@ addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut) /* Copy all entries from the splay tree to the array, in order of increasing address. */ - mut->foreach ([&] (CORE_ADDR start, void *obj) + mut->foreach ([&] (CORE_ADDR start, const void *obj) { transitions[num_transitions].addr = start; transitions[num_transitions].value = obj; @@ -317,7 +317,7 @@ addrmap_mutable_foreach_worker (splay_tree_node node, void *data) int -addrmap_mutable::foreach (addrmap_foreach_fn fn) +addrmap_mutable::foreach (addrmap_foreach_fn fn) const { return splay_tree_foreach (tree, addrmap_mutable_foreach_worker, &fn); } @@ -368,7 +368,7 @@ addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload) addrmap entry defines the end of the range). */ bool previous_matched = false; - auto callback = [&] (CORE_ADDR start_addr, void *obj) + auto callback = [&] (CORE_ADDR start_addr, const void *obj) { QUIT; @@ -449,7 +449,7 @@ test_addrmap () CHECK_ADDRMAP_FIND (map2, array, 13, 19, nullptr); /* Iterate over both addrmaps. */ - auto callback = [&] (CORE_ADDR start_addr, void *obj) + auto callback = [&] (CORE_ADDR start_addr, const void *obj) { if (start_addr == core_addr (nullptr)) SELF_CHECK (obj == nullptr); diff --git a/gdb/addrmap.h b/gdb/addrmap.h index a5b784efaf25..008a2dfdd22d 100644 --- a/gdb/addrmap.h +++ b/gdb/addrmap.h @@ -36,8 +36,8 @@ /* The type of a function used to iterate over the map. OBJ is NULL for unmapped regions. */ -typedef gdb::function_view<int (CORE_ADDR start_addr, void *obj)> - addrmap_foreach_fn; +using addrmap_foreach_fn + = gdb::function_view<int (CORE_ADDR start_addr, const void *obj)>; /* The base class for addrmaps. */ struct addrmap @@ -85,7 +85,7 @@ struct addrmap void *obj) = 0; /* Return the object associated with ADDR in MAP. */ - virtual void *find (CORE_ADDR addr) const = 0; + virtual const void *find (CORE_ADDR addr) const = 0; /* Relocate all the addresses in MAP by OFFSET. (This can be applied to either mutable or immutable maps.) */ @@ -95,7 +95,7 @@ struct addrmap If FN ever returns a non-zero value, the iteration ceases immediately, and the value is returned. Otherwise, this function returns 0. */ - virtual int foreach (addrmap_foreach_fn fn) = 0; + virtual int foreach (addrmap_foreach_fn fn) const = 0; }; struct addrmap_mutable; @@ -111,9 +111,9 @@ struct addrmap_fixed : public addrmap, void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, void *obj) override; - void *find (CORE_ADDR addr) const override; + const void *find (CORE_ADDR addr) const override; void relocate (CORE_ADDR offset) override; - int foreach (addrmap_foreach_fn fn) override; + int foreach (addrmap_foreach_fn fn) const override; private: @@ -123,7 +123,7 @@ struct addrmap_fixed : public addrmap, struct addrmap_transition { CORE_ADDR addr; - void *value; + const void *value; }; /* The number of transitions in TRANSITIONS. */ @@ -149,9 +149,9 @@ struct addrmap_mutable : public addrmap void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, void *obj) override; - void *find (CORE_ADDR addr) const override; + const void *find (CORE_ADDR addr) const override; void relocate (CORE_ADDR offset) override; - int foreach (addrmap_foreach_fn fn) override; + int foreach (addrmap_foreach_fn fn) const override; private: diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index ced58eab6612..648e51376eb5 100644 --- a/gdb/dwarf2/index-write.c +++ b/gdb/dwarf2/index-write.c @@ -433,7 +433,8 @@ write_hash_table (mapped_symtab *symtab, data_buf &output, data_buf &cpool) } } -typedef std::unordered_map<dwarf2_per_cu_data *, unsigned int> cu_index_map; +using cu_index_map + = std::unordered_map<const dwarf2_per_cu_data *, unsigned int>; /* Helper struct for building the address table. */ struct addrmap_index_data @@ -446,7 +447,7 @@ struct addrmap_index_data data_buf &addr_vec; cu_index_map &cu_index_htab; - int operator() (CORE_ADDR start_addr, void *obj); + int operator() (CORE_ADDR start_addr, const void *obj); /* True if the previous_* fields are valid. We can't write an entry until we see the next entry (since it is only then @@ -472,9 +473,10 @@ add_address_entry (data_buf &addr_vec, /* Worker function for traversing an addrmap to build the address table. */ int -addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) +addrmap_index_data::operator() (CORE_ADDR start_addr, const void *obj) { - dwarf2_per_cu_data *per_cu = (dwarf2_per_cu_data *) obj; + const dwarf2_per_cu_data *per_cu + = static_cast<const dwarf2_per_cu_data *> (obj); if (previous_valid) add_address_entry (addr_vec, base-commit: 90a7c55706695c0afa8f8393c589cd90bdacfef4 -- 2.39.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 19:58 ` Simon Marchi @ 2023-01-27 21:15 ` Tom Tromey 2023-01-27 21:18 ` Tom Tromey 1 sibling, 0 replies; 13+ messages in thread From: Tom Tromey @ 2023-01-27 21:15 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: Simon> I tried making the parameter to addrmap_foreach_fn const too (which Simon> makes sense I think, if we're going to make foreach const), and came up Simon> with a slightly larger patch. It also made the find method return Simon> const, but it also makes sense given that it is marked const too. Simon> See the patch below, I think we can rebase yours on top of that. Looks good to me. Thank you. >> -std::vector<addrmap *> >> +std::vector<const addrmap *> >> cooked_index_vector::get_addrmaps () Simon> You can make the get_addrmaps method const itself. Will do. Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 19:58 ` Simon Marchi 2023-01-27 21:15 ` Tom Tromey @ 2023-01-27 21:18 ` Tom Tromey 2023-01-27 21:20 ` Simon Marchi 2023-01-27 22:00 ` Simon Marchi 1 sibling, 2 replies; 13+ messages in thread From: Tom Tromey @ 2023-01-27 21:18 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: Simon> -void * Simon> +const void * Simon> addrmap_fixed::find (CORE_ADDR addr) const Simon> { Simon> const struct addrmap_transition *bottom = &transitions[0]; Actually, this is going to rely on casting away const in some spots. For example cooked_index::lookup: dwarf2_per_cu_data *lookup (CORE_ADDR addr) const { return (dwarf2_per_cu_data *) m_addrmap->find (addr); } Probably we can't constify dwarf2_per_cu_data all over. Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 21:18 ` Tom Tromey @ 2023-01-27 21:20 ` Simon Marchi 2023-01-27 21:26 ` Simon Marchi 2023-01-27 22:00 ` Simon Marchi 1 sibling, 1 reply; 13+ messages in thread From: Simon Marchi @ 2023-01-27 21:20 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 1/27/23 16:18, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: > > Simon> -void * > Simon> +const void * > Simon> addrmap_fixed::find (CORE_ADDR addr) const > Simon> { > Simon> const struct addrmap_transition *bottom = &transitions[0]; > > Actually, this is going to rely on casting away const in some spots. > > For example cooked_index::lookup: > > dwarf2_per_cu_data *lookup (CORE_ADDR addr) const > { > return (dwarf2_per_cu_data *) m_addrmap->find (addr); > } > > Probably we can't constify dwarf2_per_cu_data all over. I suppose in this case we can consider that it's the DWARF code that is broken, and that it knows the risks of casting away the const. But at least the addrmap code will be correct. Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 21:20 ` Simon Marchi @ 2023-01-27 21:26 ` Simon Marchi 2023-01-30 14:15 ` Tom Tromey 0 siblings, 1 reply; 13+ messages in thread From: Simon Marchi @ 2023-01-27 21:26 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 1/27/23 16:20, Simon Marchi via Gdb-patches wrote: > > > On 1/27/23 16:18, Tom Tromey wrote: >>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: >> >> Simon> -void * >> Simon> +const void * >> Simon> addrmap_fixed::find (CORE_ADDR addr) const >> Simon> { >> Simon> const struct addrmap_transition *bottom = &transitions[0]; >> >> Actually, this is going to rely on casting away const in some spots. >> >> For example cooked_index::lookup: >> >> dwarf2_per_cu_data *lookup (CORE_ADDR addr) const >> { >> return (dwarf2_per_cu_data *) m_addrmap->find (addr); >> } >> >> Probably we can't constify dwarf2_per_cu_data all over. > > I suppose in this case we can consider that it's the DWARF code that is > broken, and that it knows the risks of casting away the const. But at > least the addrmap code will be correct. Or, if the client code legitimately wants to modify the contained objects, then we provide a non-const version of addrmap::find that returns a non-const pointer. Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 21:26 ` Simon Marchi @ 2023-01-30 14:15 ` Tom Tromey 0 siblings, 0 replies; 13+ messages in thread From: Tom Tromey @ 2023-01-30 14:15 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: >> I suppose in this case we can consider that it's the DWARF code that is >> broken, and that it knows the risks of casting away the const. But at >> least the addrmap code will be correct. Simon> Or, if the client code legitimately wants to modify the contained Simon> objects, then we provide a non-const version of addrmap::find that Simon> returns a non-const pointer. I was thinking about this weekend, and it seems to me that addrmap is just a container and should be able to hold any type -- const or not -- much the way a vector does. The issue on the gdb side is that addrmap isn't templatized and so has to use void* as the type. Anyway from this view, using 'const void *' seems wrong because it moves the type buglet from addrmap to the users of it. Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 21:18 ` Tom Tromey 2023-01-27 21:20 ` Simon Marchi @ 2023-01-27 22:00 ` Simon Marchi 2023-01-30 14:26 ` Tom Tromey 1 sibling, 1 reply; 13+ messages in thread From: Simon Marchi @ 2023-01-27 22:00 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 1/27/23 16:18, Tom Tromey via Gdb-patches wrote: >>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: > > Simon> -void * > Simon> +const void * > Simon> addrmap_fixed::find (CORE_ADDR addr) const > Simon> { > Simon> const struct addrmap_transition *bottom = &transitions[0]; > > Actually, this is going to rely on casting away const in some spots. > > For example cooked_index::lookup: > > dwarf2_per_cu_data *lookup (CORE_ADDR addr) const > { > return (dwarf2_per_cu_data *) m_addrmap->find (addr); > } > > Probably we can't constify dwarf2_per_cu_data all over. I tried constifying dwarf2_per_cu_data to see where it would break. My thinking is that after the quick symbol implementation has produced all those dwarf2_per_cu_data objects, they are not supposed to change. But there is one ligitimate-ish case where we change one after the fact, when we call set_length in cutu_reader::cutu_reader. So, I redesigned my patch to instead provide both const and non-const versions of find and foreach. I think it's typical for containers in C++ anyway. See v2 below: From 030f7e72af736f2f46c2c6c033ed1b74bce6bde8 Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@polymtl.ca> Date: Fri, 27 Jan 2023 14:46:50 -0500 Subject: [PATCH] gdb: provide const-correct versions of addrmap::find and addrmap::foreach Users of addrmap::find and addrmap::foreach that have a const addrmap should ideally receive const pointers to objects, to indicate they should not be modified. However, users that have a non-const addrmap should still receive a non-const pointer. To achieve this, without adding more virtual methods, make the existing find and foreach virtual methods private and prefix them with "do_". Add small non-const and const wrappers for find and foreach. Obviously, the const can be cast away, but if using static_cast instead of C-style casts, then the compiler won't let you cast the const away. I changed all the callers of addrmap::find and addrmap::foreach I could find to make them use static_cast. Change-Id: Ia8e69d022564f80d961413658fe6068edc71a094 --- gdb/addrmap.c | 10 +++++----- gdb/addrmap.h | 34 ++++++++++++++++++++++++++-------- gdb/dwarf2/cooked-index.h | 2 +- gdb/dwarf2/index-write.c | 2 +- gdb/dwarf2/read.c | 13 +++++++------ 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/gdb/addrmap.c b/gdb/addrmap.c index 0a6b2e5a25a9..33dc7762d91c 100644 --- a/gdb/addrmap.c +++ b/gdb/addrmap.c @@ -40,7 +40,7 @@ addrmap_fixed::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, void * -addrmap_fixed::find (CORE_ADDR addr) const +addrmap_fixed::do_find (CORE_ADDR addr) const { const struct addrmap_transition *bottom = &transitions[0]; const struct addrmap_transition *top = &transitions[num_transitions - 1]; @@ -82,7 +82,7 @@ addrmap_fixed::relocate (CORE_ADDR offset) int -addrmap_fixed::foreach (addrmap_foreach_fn fn) +addrmap_fixed::do_foreach (addrmap_foreach_fn fn) const { size_t i; @@ -240,7 +240,7 @@ addrmap_mutable::set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, void * -addrmap_mutable::find (CORE_ADDR addr) const +addrmap_mutable::do_find (CORE_ADDR addr) const { splay_tree_node n = splay_tree_lookup (addr); if (n != nullptr) @@ -317,7 +317,7 @@ addrmap_mutable_foreach_worker (splay_tree_node node, void *data) int -addrmap_mutable::foreach (addrmap_foreach_fn fn) +addrmap_mutable::do_foreach (addrmap_foreach_fn fn) const { return splay_tree_foreach (tree, addrmap_mutable_foreach_worker, &fn); } @@ -368,7 +368,7 @@ addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload) addrmap entry defines the end of the range). */ bool previous_matched = false; - auto callback = [&] (CORE_ADDR start_addr, void *obj) + auto callback = [&] (CORE_ADDR start_addr, const void *obj) { QUIT; diff --git a/gdb/addrmap.h b/gdb/addrmap.h index a5b784efaf25..e00dda6e7115 100644 --- a/gdb/addrmap.h +++ b/gdb/addrmap.h @@ -36,8 +36,10 @@ /* The type of a function used to iterate over the map. OBJ is NULL for unmapped regions. */ -typedef gdb::function_view<int (CORE_ADDR start_addr, void *obj)> - addrmap_foreach_fn; +using addrmap_foreach_fn + = gdb::function_view<int (CORE_ADDR start_addr, void *obj)>; +using addrmap_foreach_const_fn + = gdb::function_view<int (CORE_ADDR start_addr, const void *obj)>; /* The base class for addrmaps. */ struct addrmap @@ -85,7 +87,11 @@ struct addrmap void *obj) = 0; /* Return the object associated with ADDR in MAP. */ - virtual void *find (CORE_ADDR addr) const = 0; + const void *find (CORE_ADDR addr) const + { return this->do_find (addr); } + + void *find (CORE_ADDR addr) + { return this->do_find (addr); } /* Relocate all the addresses in MAP by OFFSET. (This can be applied to either mutable or immutable maps.) */ @@ -95,7 +101,19 @@ struct addrmap If FN ever returns a non-zero value, the iteration ceases immediately, and the value is returned. Otherwise, this function returns 0. */ - virtual int foreach (addrmap_foreach_fn fn) = 0; + int foreach (addrmap_foreach_const_fn fn) const + { return this->do_foreach (fn); } + + int foreach (addrmap_foreach_fn fn) + { return this->do_foreach (fn); } + + +private: + /* Worker for find, implemented by sub-classes. */ + virtual void *do_find (CORE_ADDR addr) const = 0; + + /* Worker for foreach, implemented by sub-classes. */ + virtual int do_foreach (addrmap_foreach_fn fn) const = 0; }; struct addrmap_mutable; @@ -111,11 +129,11 @@ struct addrmap_fixed : public addrmap, void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, void *obj) override; - void *find (CORE_ADDR addr) const override; void relocate (CORE_ADDR offset) override; - int foreach (addrmap_foreach_fn fn) override; private: + void *do_find (CORE_ADDR addr) const override; + int do_foreach (addrmap_foreach_fn fn) const override; /* A transition: a point in an address map where the value changes. The map maps ADDR to VALUE, but if ADDR > 0, it maps ADDR-1 to @@ -149,11 +167,11 @@ struct addrmap_mutable : public addrmap void set_empty (CORE_ADDR start, CORE_ADDR end_inclusive, void *obj) override; - void *find (CORE_ADDR addr) const override; void relocate (CORE_ADDR offset) override; - int foreach (addrmap_foreach_fn fn) override; private: + void *do_find (CORE_ADDR addr) const override; + int do_foreach (addrmap_foreach_fn fn) const override; /* A splay tree, with a node for each transition; there is a transition at address T if T-1 and T map to different objects. diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index e12376cdf982..7a8c18975e5a 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -254,7 +254,7 @@ class cooked_index found. */ dwarf2_per_cu_data *lookup (CORE_ADDR addr) { - return (dwarf2_per_cu_data *) m_addrmap->find (addr); + return static_cast<dwarf2_per_cu_data *> (m_addrmap->find (addr)); } /* Create a new cooked_index_entry and register it with this object. diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index ced58eab6612..7b1b2d6520cb 100644 --- a/gdb/dwarf2/index-write.c +++ b/gdb/dwarf2/index-write.c @@ -474,7 +474,7 @@ add_address_entry (data_buf &addr_vec, int addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) { - dwarf2_per_cu_data *per_cu = (dwarf2_per_cu_data *) obj; + dwarf2_per_cu_data *per_cu = static_cast<dwarf2_per_cu_data *> (obj); if (previous_valid) add_address_entry (addr_vec, diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index cd937f24ee74..9d8952a4eb8d 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -4276,8 +4276,9 @@ dwarf2_base_index_functions::find_per_cu (dwarf2_per_bfd *per_bfd, { if (per_bfd->index_addrmap == nullptr) return nullptr; - return ((struct dwarf2_per_cu_data *) - per_bfd->index_addrmap->find (adjusted_pc)); + + void *obj = per_bfd->index_addrmap->find (adjusted_pc); + return static_cast<dwarf2_per_cu_data *> (obj); } struct compunit_symtab * @@ -18276,8 +18277,8 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu, else if (*parent_entry == nullptr) { CORE_ADDR lookup = form_addr (origin_offset, origin_is_dwz); - *parent_entry - = (cooked_index_entry *) m_die_range_map.find (lookup); + void *obj = m_die_range_map.find (lookup); + *parent_entry = static_cast <cooked_index_entry *> (obj); } unsigned int bytes_read; @@ -18568,8 +18569,8 @@ cooked_indexer::make_index (cutu_reader *reader) for (const auto &entry : m_deferred_entries) { CORE_ADDR key = form_addr (entry.die_offset, m_per_cu->is_dwz); - cooked_index_entry *parent - = (cooked_index_entry *) m_die_range_map.find (key); + void *obj = m_die_range_map.find (key); + cooked_index_entry *parent = static_cast <cooked_index_entry *> (obj); m_index_storage->add (entry.die_offset, entry.tag, entry.flags, entry.name, parent, m_per_cu); } base-commit: 35e1763185224b2bf634e19cdfd06dab0ba914e3 -- 2.39.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-27 22:00 ` Simon Marchi @ 2023-01-30 14:26 ` Tom Tromey 2023-01-30 15:22 ` Simon Marchi 0 siblings, 1 reply; 13+ messages in thread From: Tom Tromey @ 2023-01-30 14:26 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: Simon> So, I redesigned my patch to instead provide both const and non-const Simon> versions of find and foreach. I think it's typical for containers in Simon> C++ anyway. See v2 below: This looks good to me. It addresses my objections. Simon> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c Simon> index ced58eab6612..7b1b2d6520cb 100644 Simon> --- a/gdb/dwarf2/index-write.c Simon> +++ b/gdb/dwarf2/index-write.c Simon> @@ -474,7 +474,7 @@ add_address_entry (data_buf &addr_vec, Simon> int Simon> addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) Simon> { Simon> - dwarf2_per_cu_data *per_cu = (dwarf2_per_cu_data *) obj; Simon> + dwarf2_per_cu_data *per_cu = static_cast<dwarf2_per_cu_data *> (obj); This area could probably be constified at some point. Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make addrmap const-correct in cooked index 2023-01-30 14:26 ` Tom Tromey @ 2023-01-30 15:22 ` Simon Marchi 2023-01-30 15:29 ` [PATCH v2] " Simon Marchi 0 siblings, 1 reply; 13+ messages in thread From: Simon Marchi @ 2023-01-30 15:22 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 1/30/23 09:26, Tom Tromey via Gdb-patches wrote: >>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: > > Simon> So, I redesigned my patch to instead provide both const and non-const > Simon> versions of find and foreach. I think it's typical for containers in > Simon> C++ anyway. See v2 below: > > This looks good to me. It addresses my objections. Thanks, will push. > Simon> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c > Simon> index ced58eab6612..7b1b2d6520cb 100644 > Simon> --- a/gdb/dwarf2/index-write.c > Simon> +++ b/gdb/dwarf2/index-write.c > Simon> @@ -474,7 +474,7 @@ add_address_entry (data_buf &addr_vec, > Simon> int > Simon> addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) > Simon> { > Simon> - dwarf2_per_cu_data *per_cu = (dwarf2_per_cu_data *) obj; > Simon> + dwarf2_per_cu_data *per_cu = static_cast<dwarf2_per_cu_data *> (obj); > > This area could probably be constified at some point. I think it will be, when applying you patch that constifies cooked_index_vector::get_addrmaps. Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] Make addrmap const-correct in cooked index 2023-01-30 15:22 ` Simon Marchi @ 2023-01-30 15:29 ` Simon Marchi 2023-01-30 16:54 ` Tom Tromey 0 siblings, 1 reply; 13+ messages in thread From: Simon Marchi @ 2023-01-30 15:29 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey From: Tom Tromey <tromey@adacore.com> After the cooked index is created, the addrmaps should be const. Change-Id: I8234520ab346ced40a8dd6e478ba21fc438c2ba2 --- gdb/dwarf2/cooked-index.c | 6 +++--- gdb/dwarf2/cooked-index.h | 2 +- gdb/dwarf2/index-write.c | 12 +++++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 1a568e6aae76..646bc76575cd 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -401,10 +401,10 @@ cooked_index_vector::lookup (CORE_ADDR addr) /* See cooked-index.h. */ -std::vector<addrmap *> -cooked_index_vector::get_addrmaps () +std::vector<const addrmap *> +cooked_index_vector::get_addrmaps () const { - std::vector<addrmap *> result; + std::vector<const addrmap *> result; for (const auto &index : m_vector) result.push_back (index->m_addrmap); return result; diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 7a8c18975e5a..7299a4f77b44 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -360,7 +360,7 @@ class cooked_index_vector : public dwarf_scanner_base /* Return a new vector of all the addrmaps used by all the indexes held by this object. */ - std::vector<addrmap *> get_addrmaps (); + std::vector<const addrmap *> get_addrmaps () const; /* Return the entry that is believed to represent the program's "main". This will return NULL if no such entry is available. */ diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index 7b1b2d6520cb..82cd3a8636d0 100644 --- a/gdb/dwarf2/index-write.c +++ b/gdb/dwarf2/index-write.c @@ -433,7 +433,8 @@ write_hash_table (mapped_symtab *symtab, data_buf &output, data_buf &cpool) } } -typedef std::unordered_map<dwarf2_per_cu_data *, unsigned int> cu_index_map; +using cu_index_map + = std::unordered_map<const dwarf2_per_cu_data *, unsigned int>; /* Helper struct for building the address table. */ struct addrmap_index_data @@ -446,7 +447,7 @@ struct addrmap_index_data data_buf &addr_vec; cu_index_map &cu_index_htab; - int operator() (CORE_ADDR start_addr, void *obj); + int operator() (CORE_ADDR start_addr, const void *obj); /* True if the previous_* fields are valid. We can't write an entry until we see the next entry (since it is only then @@ -472,9 +473,10 @@ add_address_entry (data_buf &addr_vec, /* Worker function for traversing an addrmap to build the address table. */ int -addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) +addrmap_index_data::operator() (CORE_ADDR start_addr, const void *obj) { - dwarf2_per_cu_data *per_cu = static_cast<dwarf2_per_cu_data *> (obj); + const dwarf2_per_cu_data *per_cu + = static_cast<const dwarf2_per_cu_data *> (obj); if (previous_valid) add_address_entry (addr_vec, @@ -500,7 +502,7 @@ addrmap_index_data::operator() (CORE_ADDR start_addr, void *obj) in the index file. */ static void -write_address_map (struct addrmap *addrmap, data_buf &addr_vec, +write_address_map (const addrmap *addrmap, data_buf &addr_vec, cu_index_map &cu_index_htab) { struct addrmap_index_data addrmap_index_data (addr_vec, cu_index_htab); -- 2.39.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] Make addrmap const-correct in cooked index 2023-01-30 15:29 ` [PATCH v2] " Simon Marchi @ 2023-01-30 16:54 ` Tom Tromey 2023-01-30 16:55 ` Simon Marchi 0 siblings, 1 reply; 13+ messages in thread From: Tom Tromey @ 2023-01-30 16:54 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches, Tom Tromey >>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes: Simon> From: Tom Tromey <tromey@adacore.com> Simon> After the cooked index is created, the addrmaps should be const. Thanks for updating this. Looks fine to me of course :) Tom ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] Make addrmap const-correct in cooked index 2023-01-30 16:54 ` Tom Tromey @ 2023-01-30 16:55 ` Simon Marchi 0 siblings, 0 replies; 13+ messages in thread From: Simon Marchi @ 2023-01-30 16:55 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 1/30/23 11:54, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes: > > Simon> From: Tom Tromey <tromey@adacore.com> > Simon> After the cooked index is created, the addrmaps should be const. > > Thanks for updating this. > Looks fine to me of course :) > > Tom Thanks, pushed. Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-01-30 16:55 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-27 16:26 [PATCH] Make addrmap const-correct in cooked index Tom Tromey 2023-01-27 19:58 ` Simon Marchi 2023-01-27 21:15 ` Tom Tromey 2023-01-27 21:18 ` Tom Tromey 2023-01-27 21:20 ` Simon Marchi 2023-01-27 21:26 ` Simon Marchi 2023-01-30 14:15 ` Tom Tromey 2023-01-27 22:00 ` Simon Marchi 2023-01-30 14:26 ` Tom Tromey 2023-01-30 15:22 ` Simon Marchi 2023-01-30 15:29 ` [PATCH v2] " Simon Marchi 2023-01-30 16:54 ` Tom Tromey 2023-01-30 16:55 ` Simon Marchi
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).