Re: [PATCH 03/14] Introduce dwarf2_per_objfile::obstack

Simon Marchi
2020-02-19T04:13:24Z

On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> Currently much of the DWARF-related data is stored on the obfile
> obstack. This prevents sharing this data across objfiles, so this
> patch adds a new obstack to dwarf2_per_objfile.
>
> One way to check whether this is correct is to look at the remaining
> uses of objfile_obstack in the DWARF code and note that they all
> appear in the "full CU" code paths.
>
> The converse -- storing per-objfile data on the shared obstack -- is
> not good, but it is just a memory leak, not a potential
> use-after-free. Double-checking this would also be useful, though.

I looked and nothing wrong stood out, but there are a lot of uses and they
are not all trivial, so I might have very well missed some mistakes.

Kind of unrelated, but while reviewing this, I noticed:

- allocate_dwo_file_hash_table
- allocate_type_unit_groups_table
- allocate_signatured_type_table
- allocate_dwp_loaded_cutus_table
- allocate_dwo_unit_table

All have objfile parameters which are unused and could probably be removed. In at least
one instance, it would allow removing ann objfile local variable.

> @@ -3202,8 +3200,12 @@ dw2_get_real_path (struct objfile *objfile,
> struct quick_file_names *qfn, int index)
> {
> if (qfn->real_names == NULL)
> - qfn->real_names = OBSTACK_CALLOC (&objfile->objfile_obstack,
> - qfn->num_file_names, const char *);
> + {
> + struct dwarf2_per_objfile *dwarf2_per_objfile
> + = get_dwarf2_per_objfile (objfile);
> + qfn->real_names = OBSTACK_CALLOC (&dwarf2_per_objfile->obstack,
> + qfn->num_file_names, const char *);
> + }

The callers of dw2_get_real_path should be able to pass the dwarf2_per_objfile directly,
instead of the objfile, simplifying this.

Simon