This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] gdb: CTF support
On 10/1/2019 7:47 PM, Simon Marchi wrote:
> On 2019-09-30 8:36 p.m., Weimin Pan wrote:
>> Thank you very much for the thorough review. I have read through the
>> modified patch which looks good and will be adopted. In addition, I
>> have made more changes to address the issues you raised. Please see
>> below and the updated patch (attached).
>
> Great thanks!
>
> I noticed at least one case of leading spaces that should be tab in
the attached
> patch, so maybe do:
>
> grep '^ ' ctfread.c
>
> to catch them.
Hi Simon,
Thanks, I think it's now clean after fixing the one below:
1247c1247
< sym = add_stt_func (ccp, i);
---
> sym = add_stt_func (ccp, i);
>
>> Yes, I've made the change in several places where the allocated copy is
>> freed after
>> it's being dup'ed via obstack_strdup.
>
> Ok thanks. Actually, can you please use gdb::unique_xmalloc_pointer to
> manage that memory? We're trying to minimize the manual memory
management
> in GDB. You'd do:
>
> gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
>
> and use `name.get ()` instead of name. It will get free'd automatically
> when exiting scope.
It doesn't seem using gdb::unique_xmalloc_pointer is appropriate in
these cases
because we want to keep these symbol/type names around in the symbol table.
>
>> > Hmm that looks fishy. It looks like this is taking the address of a
>> > local variable for something that will be used after the current
function
>> > will have returned.
>>
>> Sorry, but it's a "static" local variable.
>
> Oh ok, I did miss that.
>
> I think we should avoid this. What happens, for example, in this case:
>
> 1. Read psymtab for objfile 1
> 2. Read psymtab for objfile 2
> 3. Expand psymtab to symtab for objfile 1
>
> From what I understand, step #2 will overwrite the static variable,
such that
> it won't be valid anymore in step #3. I think it would be better to
allocate the
> context structure dynamically and store it as the objfile, like
dwarf2read does.
Yes, you're right. I've made the following changes:
1322c1322
< static ctf_context_t ccx;
---
> ctf_context_t *ccx;
1326,1328c1326,1329
< ccx.fp = cfp;
< ccx.of = objfile;
< pst->read_symtab_private = (void *) &ccx;
---
> ccx = XOBNEW (&objfile->objfile_obstack, ctf_context_t);
> ccx->fp = cfp;
> ccx->of = objfile;
> pst->read_symtab_private = (void *) ccx;
Many thanks,
Weimin