[5/7] jit: make gdb_object::symtabs a vector of unique_ptr
Commit Message
This makes the gdb_object hold everything. When it gets destroyed, all
contained symtabs and contained blocks get destroyed.
gdb/ChangeLog:
* jit.c (struct gdb_object) <symtabs>: Change type to
std::vector<std::unique_ptr<gdb_symtab>>.
(jit_symtab_open_impl): Adjust.
(jit_object_close_impl): Adjust.
---
gdb/jit.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
Comments
On 12/13/19 6:03 AM, Simon Marchi wrote:
> struct gdb_object
> {
> - std::vector<gdb_symtab *> symtabs;
> + std::vector<std::unique_ptr<gdb_symtab>> symtabs;
> };
Could this be a vector or objects instead of a vector or pointers?
Like:
std::vector<gdb_symtab> symtabs;
> + object->symtabs.emplace_back (new gdb_symtab (file_name));
> + return object->symtabs.back ().get ();
> }
and:
object->symtabs.emplace_back (file_name);
return &object->symtabs.back ();
Thanks,
Pedro Alves
On 2019-12-13 12:54 p.m., Pedro Alves wrote:
> On 12/13/19 6:03 AM, Simon Marchi wrote:
>> struct gdb_object
>> {
>> - std::vector<gdb_symtab *> symtabs;
>> + std::vector<std::unique_ptr<gdb_symtab>> symtabs;
>> };
>
> Could this be a vector or objects instead of a vector or pointers?
>
> Like:
>
> std::vector<gdb_symtab> symtabs;
I don't think so. We return these pointers to the JIT debug readers, which then pass
it back to the block_open and symtab_close callbacks. It's important that the objects
don't move during their lifetime. If we had a vector of objects, the pointers we return
to the user would get invalidated the moment the vector is resized.
>
>> + object->symtabs.emplace_back (new gdb_symtab (file_name));
>> + return object->symtabs.back ().get ();
>> }
>
> and:
>
> object->symtabs.emplace_back (file_name);
> return &object->symtabs.back ();
>
> Thanks,
> Pedro Alves
>
On 2019-12-13 1:45 p.m., Simon Marchi wrote:
> I don't think so. We return these pointers to the JIT debug readers, which then pass
> it back to the block_open and symtab_close callbacks. It's important that the objects
> don't move during their lifetime. If we had a vector of objects, the pointers we return
> to the user would get invalidated the moment the vector is resized.
Actually, it would be good to document this, so we don't change it to a vector of
objects, out of good intention. I would add this to document the gdb_object::symtabs
field:
/* Symtabs of this object.
This is a vector of pointers, rather than a vector of objects, because the
pointers are returned to the user's debug info reader, so it's important
that the objects don't change location during their lifetime (which would
happen with a vector of objects getting resized. */
I would add a similar comment later in the series to document gdb_symtab::blocks.
Simon
On 12/13/19 6:51 PM, Simon Marchi wrote:
> On 2019-12-13 1:45 p.m., Simon Marchi wrote:
>> I don't think so. We return these pointers to the JIT debug readers, which then pass
>> it back to the block_open and symtab_close callbacks. It's important that the objects
>> don't move during their lifetime. If we had a vector of objects, the pointers we return
>> to the user would get invalidated the moment the vector is resized.
>
> Actually, it would be good to document this, so we don't change it to a vector of
> objects, out of good intention. I would add this to document the gdb_object::symtabs
> field:
>
> /* Symtabs of this object.
>
> This is a vector of pointers, rather than a vector of objects, because the
> pointers are returned to the user's debug info reader, so it's important
> that the objects don't change location during their lifetime (which would
> happen with a vector of objects getting resized. */
>
> I would add a similar comment later in the series to document gdb_symtab::blocks.
That looks good to me.
Thanks,
Pedro Alves
@@ -476,7 +476,7 @@ struct gdb_symtab
struct gdb_object
{
- std::vector<gdb_symtab *> symtabs;
+ std::vector<std::unique_ptr<gdb_symtab>> symtabs;
};
/* The type of the `private' data passed around by the callback
@@ -521,9 +521,8 @@ jit_symtab_open_impl (struct gdb_symbol_callbacks *cb,
{
/* CB stays unused. See comment in jit_object_open_impl. */
- gdb_symtab *symtab = new gdb_symtab (file_name);
- object->symtabs.push_back (symtab);
- return symtab;
+ object->symtabs.emplace_back (new gdb_symtab (file_name));
+ return object->symtabs.back ().get ();
}
/* Called by readers to open a new gdb_block. This function also
@@ -722,8 +721,6 @@ finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile)
BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
}
}
-
- delete stab;
}
/* Called when closing a gdb_objfile. Converts OBJ to a proper
@@ -742,8 +739,8 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
OBJF_NOT_FILENAME);
objfile->per_bfd->gdbarch = target_gdbarch ();
- for (gdb_symtab *symtab : obj->symtabs)
- finalize_symtab (symtab, objfile);
+ for (const std::unique_ptr<gdb_symtab> &symtab : obj->symtabs)
+ finalize_symtab (symtab.get (), objfile);
add_objfile_entry (objfile, *priv_data);