Re: [PATCH] gdb: restore nullptr check in compunit_symtab::find_call_site

[Simon Marchi <simark@simark.ca>]

On 2024-12-03 12:49, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> Commit de2b4ab50de ("Convert dwarf2_cu::call_site_htab to new hash
> Simon> table") removed this nullptr check for no good reason.  This causes a
> Simon> crash if `m_call_site_htab` is not set, as shown in PR 32410.  My guess
> Simon> is that when doing this change, I tried to make `m_call_site_htab` not a
> Simon> pointer, removed this check, then realized it wasn't so obvious, and
> Simon> forgot to re-add the check.
> 
> Yeah, someday maybe symtabs should be allocated the ordinary way.
> 
> On the one hand, obstacks are nice since they reduce free overhead.
> But on the other hand, they mean we can't use destructors... and for
> this case, I think there normally aren't "too many" symtabs, so it's
> probably fine to make the change.

We can still use destructors, we might just need to call it manually.
In fact, I was wondering if `compunit_symtab::finalize()` couldn't just
become `compunit_symtab::~compunit_symtab()` (it would still be called
manually at the same place.

In fact, can't `allocate_on_obstack` help us with that? What if you
have:

    struct compunit_symtab : allocate_on_obstack<compunit_symtab>
    {
      ...
    };

and then use:

    std::unique_ptr<compunit_symtab> uptr;

My understanding (although I haven't tried) is: when `uptr` gets
destroyed, it will call compunit_symtab's destructor, but won't free any
memory because the delete operator of allocate_on_obstack is a noop.  Do
I understand it correctly?

If I find some time, I would still like to try to make compunit_symtab /
symtab allocated normally and measure how expensive it is to free a few
thousands of those.  If the difference isn't significant, I would be
happy to get rid of that complexity.

Simon