Re: [PATCH v2] gdb: CTF support

[Wei-min Pan <weimin.pan@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]


On 10/3/2019 11:53 AM, Wei-min Pan wrote:
>
> On 10/3/2019 11:31 AM, Simon Marchi wrote:
>> On 2019-10-03 2:21 p.m., Wei-min Pan wrote:
>>> Let's use an example (checking omitted):
>>>
>>> We're replacing:
>>>    name = ctf_type_aname_raw (fp, tid);
>>>    TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>>>    free (name);
>>>
>>> with
>>>    gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
>>>    TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ()); 
>>>
>>>
>>> The allocated copy from ctf_type_aname_raw is not freed. Or did I 
>>> miss something?
>>>
>>> Weimin
>> Yes, the second snippet frees the copy returned by ctf_type_aname_raw.
>>
>> It might just be that you are not familiar with the concept of 
>> std::unique_ptr in C++:
>>
>>    https://en.cppreference.com/w/cpp/memory/unique_ptr
>>
>> It's a wrapper around a simple pointer that automatically calls a 
>> deleter function
>> when it goes out of scope.  gdb::unique_xmalloc_ptr is a 
>> specialization of std::unique_ptr
>> that uses xfree as the deleter function.
>>
>> Here's a similar use:
>>
>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/fbsd-tdep.c;h=9422e3c1a7e1a65c76b088f42bb5986bd13a089f;hb=HEAD#l1530
>>
>> `fbsd_core_vnode_path` return a pointer to an allocated C String, 
>> which `cwd` wraps.  When
>> it goes out of scope, `cwd` automatically calls `xfree` with the 
>> pointer.
>
> Oh, thanks for the explanation, reference and example. Will switch to 
> using gdb::unique_xmalloc_ptr<> instead.

Hi Simon,

I've switched to using gdb::unique_xmalloc_ptr, please see the attached 
patch, and ran
the CTF test suite successfully. Will submit a revised patch upstream 
shortly.

Thanks again for your reviews and help.

Weimin


[-- Attachment #2: 0001-updated2.patch --]
[-- Type: text/plain, Size: 3789 bytes --]

405c410
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
---
>   char *name = ctf_type_aname_raw (fp, tid);
412c417
<       SYMBOL_SET_NAMES (sym, name.get (), strlen (name.get ()), 1, objfile);
---
>       SYMBOL_SET_NAMES (sym, name, strlen (name), 1, objfile);
414a420
>       free (name);
478,479c484,485
<   gdb::unique_xmalloc_ptr<char> copied_name (ctf_type_aname_raw (fp, tid));
<   if (copied_name == NULL || strlen (copied_name.get ()) == 0)
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name == NULL || strlen (name) == 0)
487c493,497
<     name = obstack_strdup (&of->objfile_obstack, copied_name.get ());
---
>     {
>       char *dup_name = name;
>       name = obstack_strdup (&of->objfile_obstack, name);
>       free (dup_name);
>     }
561a572
>   char *name;
566,568c577,582
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
<   if (name != NULL && strlen (name.get() ) != 0)
<     TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name != NULL && strlen (name) != 0)
>     {
>       TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>       free (name);
>     }
617c631
<   struct objfile *of = ccp->of;
---
>   struct objfile *objfile = ccp->of;
620a635
>   char *name;
622c637
<   type = alloc_type (of);
---
>   type = alloc_type (objfile);
624,626c639,644
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
<   if (name != NULL && strlen (name.get ()) != 0)
<     TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name != NULL && strlen (name) != 0)
>     {
>       TYPE_NAME (type) = obstack_strdup (&objfile->objfile_obstack, name);
>       free (name);
>     }
630c648
<   rettype = get_tid_type (of, cfi.ctc_return);
---
>   rettype = get_tid_type (objfile, cfi.ctc_return);
634c652
<   return set_tid_type (of, tid, type);
---
>   return set_tid_type (objfile, tid, type);
646a665
>   char *name;
650,652c669,674
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
<   if (name != NULL && strlen (name.get ()) != 0)
<     TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name != NULL && strlen (name) != 0)
>     {
>       TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>       free (name);
>     }
880a903
>   char *name;
903,907c926,929
< 	{ 
< 	  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
< 	  btid = ctf_type_reference (fp, tid);
< 	  type = read_typedef_type (ccp, tid, btid, name.get ());
< 	} 
---
> 	name = ctf_type_aname_raw (fp, tid);
> 	btid = ctf_type_reference (fp, tid);
> 	type = read_typedef_type (ccp, tid, btid, name);
> 	free (name);
1088a1111
>   char *name;
1103c1126
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
---
>   name = ctf_type_aname_raw (ccp->fp, idx);
1105a1129
>   free (name);
1321a1346
>   char *name;
1326,1327c1351,1352
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, tid));
<   if (name == NULL || strlen (name.get ()) == 0)
---
>   name = ctf_type_aname_raw (ccp->fp, tid);
>   if (name == NULL || strlen (name) == 0)
1368c1393
<     add_psymbol_to_list (name.get (), strlen (name.get ()), true,
---
>     add_psymbol_to_list (name, strlen (name), true,
1372a1398
>   free (name);
1425c1451
<       gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_raw (cfp, tid));
---
>       char *tname = ctf_type_aname_raw (cfp, tid);
1448c1474
<       add_psymbol_to_list (tname.get (), strlen (tname.get ()), true,
---
>       add_psymbol_to_list (tname, strlen (tname), true,
1451a1478
>       free (tname);