* [PATCH 0/3] Make allocate_on_obstack a bit safer
@ 2024-02-27 21:42 Tom Tromey
2024-02-27 21:42 ` [PATCH 1/3] Use addrmap_fixed in a few spots Tom Tromey
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Tom Tromey @ 2024-02-27 21:42 UTC (permalink / raw)
To: gdb-patches
This series makes the allocate_on_obstack mixin a little safer by
having it enforce the rule that an object allocated on an obstack must
have a trivial destructor.
---
Tom Tromey (3):
Use addrmap_fixed in a few spots
Don't use virtual destructor in addrmap
Require trivial destructor in allocate_on_obstack
gdb/addrmap.h | 7 ++++---
gdb/block.c | 2 +-
gdb/block.h | 12 ++++++------
gdb/dwarf2/cooked-index.h | 4 ++--
gdb/dwarf2/read.h | 2 +-
gdb/gdbtypes.h | 4 ++--
gdb/symtab.h | 2 +-
gdbsupport/gdb_obstack.h | 6 +++++-
8 files changed, 22 insertions(+), 17 deletions(-)
---
base-commit: 407ca654547b100903f7eab44d078a2440736f13
change-id: 20240227-obstac-alloc-deabe0f935c3
Best regards,
--
Tom Tromey <tromey@adacore.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] Use addrmap_fixed in a few spots
2024-02-27 21:42 [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
@ 2024-02-27 21:42 ` Tom Tromey
2024-02-28 12:27 ` Alexandra Petlanova Hajkova
2024-02-27 21:42 ` [PATCH 2/3] Don't use virtual destructor in addrmap Tom Tromey
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2024-02-27 21:42 UTC (permalink / raw)
To: gdb-patches
There are a few spots in the tree that use 'addrmap' where only an
addrmap_fixed will ever really be seen. This patch changes this code
to use the more specific type.
---
gdb/block.h | 10 +++++-----
gdb/dwarf2/cooked-index.h | 2 +-
gdb/dwarf2/read.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/gdb/block.h b/gdb/block.h
index 00630123120..4c29f6599ef 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -30,7 +30,7 @@ struct compunit_symtab;
struct block_namespace_info;
struct using_direct;
struct obstack;
-struct addrmap;
+struct addrmap_fixed;
/* Blocks can occupy non-contiguous address ranges. When this occurs,
startaddr and endaddr within struct block (still) specify the lowest
@@ -410,22 +410,22 @@ struct blockvector
{ return this->block (STATIC_BLOCK); }
/* Return the address -> block map of this blockvector. */
- addrmap *map ()
+ addrmap_fixed *map ()
{ return m_map; }
/* Const version of the above. */
- const addrmap *map () const
+ const addrmap_fixed *map () const
{ return m_map; }
/* Set this blockvector's address -> block map. */
- void set_map (addrmap *map)
+ void set_map (addrmap_fixed *map)
{ m_map = map; }
private:
/* An address map mapping addresses to blocks in this blockvector.
This pointer is zero if the blocks' start and end addresses are
enough. */
- struct addrmap *m_map;
+ addrmap_fixed *m_map;
/* Number of blocks in the list. */
int m_num_blocks;
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 629a5b6b9ee..928c4ef655c 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -362,7 +362,7 @@ class cooked_index_shard
cooked_index_entry *m_main = nullptr;
/* The addrmap. This maps address ranges to dwarf2_per_cu_data
objects. */
- addrmap *m_addrmap = nullptr;
+ addrmap_fixed *m_addrmap = nullptr;
/* Storage for canonical names. */
std::vector<gdb::unique_xmalloc_ptr<char>> m_names;
};
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 5ee7e09a743..73def88c4c0 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -536,7 +536,7 @@ struct dwarf2_per_bfd
abstract_to_concrete;
/* The address map that is used by the DWARF index code. */
- struct addrmap *index_addrmap = nullptr;
+ addrmap_fixed *index_addrmap = nullptr;
};
/* An iterator for all_units that is based on index. This
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] Don't use virtual destructor in addrmap
2024-02-27 21:42 [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
2024-02-27 21:42 ` [PATCH 1/3] Use addrmap_fixed in a few spots Tom Tromey
@ 2024-02-27 21:42 ` Tom Tromey
2024-02-28 15:16 ` Alexandra Petlanova Hajkova
2024-02-27 21:42 ` [PATCH 3/3] Require trivial destructor in allocate_on_obstack Tom Tromey
2024-03-21 18:20 ` [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
3 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2024-02-27 21:42 UTC (permalink / raw)
To: gdb-patches
The addrmap polymorphism is sort of "phony" in that there isn't really
code in the tree that can be presented with either type. I haven't
tried to fix this (though perhaps I may); but meanwhile it's handy for
the next patch if addrmap_fixed has a trivial destructor. This patch
achieves this by making the addrmap destructor non-virtual, and also
making it protected so that objects of any of these types cannot be
destroyed when only the base class is known.
---
gdb/addrmap.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index ba83607ad8c..0d61046cebd 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -44,8 +44,6 @@ using addrmap_foreach_const_fn
/* The base class for addrmaps. */
struct addrmap
{
- virtual ~addrmap () = default;
-
/* Return the object associated with ADDR in MAP. */
const void *find (CORE_ADDR addr) const
{ return this->do_find (addr); }
@@ -68,6 +66,9 @@ struct addrmap
{ return this->do_foreach (fn); }
+protected:
+ ~addrmap () = default;
+
private:
/* Worker for find, implemented by sub-classes. */
virtual void *do_find (CORE_ADDR addr) const = 0;
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] Require trivial destructor in allocate_on_obstack
2024-02-27 21:42 [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
2024-02-27 21:42 ` [PATCH 1/3] Use addrmap_fixed in a few spots Tom Tromey
2024-02-27 21:42 ` [PATCH 2/3] Don't use virtual destructor in addrmap Tom Tromey
@ 2024-02-27 21:42 ` Tom Tromey
2024-02-28 16:55 ` Alexandra Petlanova Hajkova
2024-03-21 18:20 ` [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
3 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2024-02-27 21:42 UTC (permalink / raw)
To: gdb-patches
This patch makes allocate_on_obstack a little bit safer, by enforcing
the rule that objects allocated on an obstack must have a trivial
destructor.
The static assert is done in a method -- doing it inside the class
itself won't work because the class is incomplete at that point.
---
gdb/addrmap.h | 2 +-
gdb/block.c | 2 +-
gdb/block.h | 2 +-
gdb/dwarf2/cooked-index.h | 2 +-
gdb/gdbtypes.h | 4 ++--
gdb/symtab.h | 2 +-
gdbsupport/gdb_obstack.h | 6 +++++-
7 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index 0d61046cebd..55dea36b877 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -81,7 +81,7 @@ struct addrmap_mutable;
/* Fixed address maps. */
struct addrmap_fixed : public addrmap,
- public allocate_on_obstack
+ public allocate_on_obstack<addrmap_fixed>
{
public:
diff --git a/gdb/block.c b/gdb/block.c
index 079053cd79e..6d0d33fa85e 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -31,7 +31,7 @@
C++ files, namely using declarations and the current namespace in
scope. */
-struct block_namespace_info : public allocate_on_obstack
+struct block_namespace_info : public allocate_on_obstack<block_namespace_info>
{
const char *scope = nullptr;
struct using_direct *using_decl = nullptr;
diff --git a/gdb/block.h b/gdb/block.h
index 4c29f6599ef..ae676c4ba67 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -105,7 +105,7 @@ struct blockranges
This implies that within the body of one function
the blocks appear in the order of a depth-first tree walk. */
-struct block : public allocate_on_obstack
+struct block : public allocate_on_obstack<block>
{
/* Return this block's start address. */
CORE_ADDR start () const
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 928c4ef655c..0d4fcb8538e 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -104,7 +104,7 @@ extern bool language_requires_canonicalization (enum language lang);
This is an "open" class and the members are all directly
accessible. It is read-only after the index has been fully read
and processed. */
-struct cooked_index_entry : public allocate_on_obstack
+struct cooked_index_entry : public allocate_on_obstack<cooked_index_entry>
{
cooked_index_entry (sect_offset die_offset_, enum dwarf_tag tag_,
cooked_index_flag flags_,
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 435a040544a..f80bd7e071a 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -211,7 +211,7 @@ struct variant_part;
control other variant parts as well. This struct corresponds to
DW_TAG_variant in DWARF. */
-struct variant : allocate_on_obstack
+struct variant : allocate_on_obstack<variant>
{
/* * The discriminant ranges for this variant. */
gdb::array_view<discriminant_range> discriminants;
@@ -243,7 +243,7 @@ struct variant : allocate_on_obstack
and holds an array of variants. This struct corresponds to
DW_TAG_variant_part in DWARF. */
-struct variant_part : allocate_on_obstack
+struct variant_part : allocate_on_obstack<variant_part>
{
/* * The index of the discriminant field in the outer type. This is
an index into the type's array of fields. If this is -1, there
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 5bd63979132..d49b7066e3a 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1244,7 +1244,7 @@ extern gdb::array_view<const struct symbol_impl> symbol_impls;
/* This structure is space critical. See space comments at the top. */
-struct symbol : public general_symbol_info, public allocate_on_obstack
+struct symbol : public general_symbol_info, public allocate_on_obstack<symbol>
{
symbol ()
/* Class-initialization of bitfields is only allowed in C++20. */
diff --git a/gdbsupport/gdb_obstack.h b/gdbsupport/gdb_obstack.h
index 20f888660f9..7b3bb05bc00 100644
--- a/gdbsupport/gdb_obstack.h
+++ b/gdbsupport/gdb_obstack.h
@@ -133,14 +133,18 @@ struct auto_obstack : obstack
{ obstack_free (this, obstack_base (this)); }
};
-/* Objects are allocated on obstack instead of heap. */
+/* Objects are allocated on obstack instead of heap. This is a mixin
+ that uses CRTP to ensure that the type in question is trivially
+ destructible. */
+template<typename T>
struct allocate_on_obstack
{
allocate_on_obstack () = default;
void* operator new (size_t size, struct obstack *obstack)
{
+ static_assert (IsFreeable<T>::value);
return obstack_alloc (obstack, size);
}
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Use addrmap_fixed in a few spots
2024-02-27 21:42 ` [PATCH 1/3] Use addrmap_fixed in a few spots Tom Tromey
@ 2024-02-28 12:27 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 8+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-02-28 12:27 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 561 bytes --]
On Tue, Feb 27, 2024 at 10:42 PM Tom Tromey <tromey@adacore.com> wrote:
> There are a few spots in the tree that use 'addrmap' where only an
> addrmap_fixed will ever really be seen. This patch changes this code
> to use the more specific type.
> ---
> gdb/block.h | 10 +++++-----
> gdb/dwarf2/cooked-index.h | 2 +-
> gdb/dwarf2/read.h | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
>
>
A reasonable change. I can confirm this change does not introduce any
regressions on aarch64 with Fedora Rawhide.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] Don't use virtual destructor in addrmap
2024-02-27 21:42 ` [PATCH 2/3] Don't use virtual destructor in addrmap Tom Tromey
@ 2024-02-28 15:16 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 8+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-02-28 15:16 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 702 bytes --]
On Tue, Feb 27, 2024 at 10:43 PM Tom Tromey <tromey@adacore.com> wrote:
> The addrmap polymorphism is sort of "phony" in that there isn't really
> code in the tree that can be presented with either type. I haven't
> tried to fix this (though perhaps I may); but meanwhile it's handy for
> the next patch if addrmap_fixed has a trivial destructor. This patch
> achieves this by making the addrmap destructor non-virtual, and also
> making it protected so that objects of any of these types cannot be
> destroyed when only the base class is known.
> ---
>
I think it's a reasonable change and I can confirm this change does not
introduce any regressions on aarch64 with Fedora Rawhide.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Require trivial destructor in allocate_on_obstack
2024-02-27 21:42 ` [PATCH 3/3] Require trivial destructor in allocate_on_obstack Tom Tromey
@ 2024-02-28 16:55 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 8+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-02-28 16:55 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 487 bytes --]
On Tue, Feb 27, 2024 at 10:43 PM Tom Tromey <tromey@adacore.com> wrote:
> This patch makes allocate_on_obstack a little bit safer, by enforcing
> the rule that objects allocated on an obstack must have a trivial
> destructor.
>
> The static assert is done in a method -- doing it inside the class
> itself won't work because the class is incomplete at that point.
> ---
>
> Looks great. I can confirm this change does not cause any regressions for
aarch64 Fedora Rawhide.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Make allocate_on_obstack a bit safer
2024-02-27 21:42 [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
` (2 preceding siblings ...)
2024-02-27 21:42 ` [PATCH 3/3] Require trivial destructor in allocate_on_obstack Tom Tromey
@ 2024-03-21 18:20 ` Tom Tromey
3 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2024-03-21 18:20 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> This series makes the allocate_on_obstack mixin a little safer by
Tom> having it enforce the rule that an object allocated on an obstack must
Tom> have a trivial destructor.
I'm checking this in.
Tom
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-03-21 18:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-27 21:42 [PATCH 0/3] Make allocate_on_obstack a bit safer Tom Tromey
2024-02-27 21:42 ` [PATCH 1/3] Use addrmap_fixed in a few spots Tom Tromey
2024-02-28 12:27 ` Alexandra Petlanova Hajkova
2024-02-27 21:42 ` [PATCH 2/3] Don't use virtual destructor in addrmap Tom Tromey
2024-02-28 15:16 ` Alexandra Petlanova Hajkova
2024-02-27 21:42 ` [PATCH 3/3] Require trivial destructor in allocate_on_obstack Tom Tromey
2024-02-28 16:55 ` Alexandra Petlanova Hajkova
2024-03-21 18:20 ` [PATCH 0/3] Make allocate_on_obstack a bit safer 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).