Patchwork [RFA,15/15] Move psymtabs to their own obstack

login
register
mail settings
Submitter Tom Tromey
Date May 10, 2018, 10:23 p.m.
Message ID <20180510222357.27332-16-tom@tromey.com>
Download mbox | patch
Permalink /patch/27228/
State New
Headers show

Comments

Tom Tromey - May 10, 2018, 10:23 p.m.
Previously, the psymtab obstack was just a pointer to the objfile
obstack.  This patch changes psymtabs to use their own obstack,
instead.  A gdb::optional is used to avoid unnecessary allocation when
the obstack is not needed.

After this patch, the psymtab code lifetime model is that, in the core
psymtab code, objects allocated on the psymtab obstack may point to
other such objects, or to objects on the per-BFD obstack -- but never
to the objfile obstack.

Note however that this invariant is only obeyed the core psymtab code.
Symbol readers are free to work however they like; and in particular,
even after this patch, in practice all symbol readers violate this
invariant via the read_symtab_private field.

gdb/ChangeLog
2018-05-10  Tom Tromey  <tom@tromey.com>

	* psymtab.h (psymtab_storage::obstack): Update.
	(psymtab_storage::m_obstack): Use gdb::optional.
	(class psymtab_storage): Update comment.
	* psymtab.c (psymtab_storage::psymtab_storage): Update.
---
 gdb/ChangeLog |  7 +++++++
 gdb/psymtab.c |  3 +--
 gdb/psymtab.h | 23 +++++++++++++++++++----
 3 files changed, 27 insertions(+), 6 deletions(-)
Simon Marchi - July 18, 2018, 2:41 p.m.
On 2018-05-10 06:23 PM, Tom Tromey wrote:
> Previously, the psymtab obstack was just a pointer to the objfile
> obstack.  This patch changes psymtabs to use their own obstack,
> instead.  A gdb::optional is used to avoid unnecessary allocation when
> the obstack is not needed.
> 
> After this patch, the psymtab code lifetime model is that, in the core
> psymtab code, objects allocated on the psymtab obstack may point to
> other such objects, or to objects on the per-BFD obstack -- but never
> to the objfile obstack.
> 
> Note however that this invariant is only obeyed the core psymtab code.
> Symbol readers are free to work however they like; and in particular,
> even after this patch, in practice all symbol readers violate this
> invariant via the read_symtab_private field.

Thanks, LGTM.

I think you can remove the objfile parameter in the psymtab_storage constructor,
it is not used (this shows that psymtab_storage no longer relies on the objfile
for allocations).

Simon
Tom Tromey - Sept. 23, 2018, 9:34 p.m.
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> I think you can remove the objfile parameter in the psymtab_storage constructor,
Simon> it is not used (this shows that psymtab_storage no longer relies on the objfile
Simon> for allocations).

I've made this change locally.

Tom

Patch

diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index f109e9f1ee..afdeeafc02 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -68,8 +68,7 @@  static struct compunit_symtab *psymtab_to_symtab (struct objfile *objfile,
 
 
 psymtab_storage::psymtab_storage (struct objfile *objfile)
-  : psymbol_cache (psymbol_bcache_init ()),
-    m_obstack (&objfile->objfile_obstack)
+  : psymbol_cache (psymbol_bcache_init ())
 {
 }
 
diff --git a/gdb/psymtab.h b/gdb/psymtab.h
index c4b6f3b9ad..134edfc540 100644
--- a/gdb/psymtab.h
+++ b/gdb/psymtab.h
@@ -31,7 +31,19 @@  struct partial_symtab;
 struct psymbol_bcache;
 
 /* An instance of this class manages the partial symbol tables and
-   partial symbols for a given objfile.  */
+   partial symbols for a given objfile.
+
+   The core psymtab functions -- those in psymtab.c -- arrange for all
+   psymtab- and psymbol-related allocations to happen either in the
+   psymtab_storage object (either on its obstack or in other memory
+   managed by this class), or on the per-BFD object.  In particular
+   the core psymtab code will not make links from the psymtab_storage
+   object back to the objfile (or objfile_obstack).
+   
+   However, it is up to each symbol reader to maintain this invariant
+   itself, if it wants to reuse psymtabs across multiple objfiles.
+   The main issue here is ensuring that read_symtab_private does not
+   point into objfile_obstack.  */
 
 class psymtab_storage
 {
@@ -58,7 +70,9 @@  public:
 
   struct obstack *obstack ()
   {
-    return m_obstack;
+    if (!m_obstack.has_value ())
+      m_obstack.emplace ();
+    return &*m_obstack;
   }
 
   struct partial_symtab **allocate_dependencies (int number)
@@ -103,9 +117,10 @@  private:
 
   struct partial_symtab *free_psymtabs = nullptr;
 
-  /* The obstack where allocations are made.  */
+  /* The obstack where allocations are made.  This is lazily allocated
+     so that we don't waste memory when there are no psymtabs.  */
 
-  struct obstack *m_obstack;
+  gdb::optional<auto_obstack> m_obstack;
 };