Re: [PATCH v3] gdb: CTF support

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

On 10/7/2019 5:07 AM, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-10-07 11:33:12 +0200]:
>
>> On 04-10-19 00:56, Weimin Pan wrote:
>>> +/* The routines that read and process fields/members of a C struct, union,
>>> +   or enumeration, pass lists of data member fields in an instance of a
>>> +   field_info structure. It is derived from dwarf2read.c.  */
>>> +
>>> +struct nextfield
>>> +{
>>> +  struct field field {};
>>> +};
>>> +
>>> +struct field_info
>> Hi,
>>
>> not only is field_info derived from dwarf2read.c, it uses the same name
>> for the type.  This is a C++ One-Definition-Rule violation, which causes
>> most of the test-suite to start failing for me.
>>
>> What happens is that here:
>> ...
>>    if (die->child != NULL && ! die_is_declaration (die, cu))
>>      {
>>        struct field_info fi;
>>        std::vector<struct symbol *> template_args;
>> ...
>> the constructor for field_info is called, but it calls the constructor
>> for field_info defined in ctfread.c rather than dwarf2read.c.
> Tom,
>
> Thanks for tracking this down.  I had just run into the same issue.
> I've pushed the patch below which I believe fixes this issue.
>
> Weimin,
>
> Hopefully you're happy with this fix, I guess if you'd rather see an
> alternative solution then feel free to propose one.

Hi Andrew,

Your fix looks good. Thank you for taking care of it.

Weimin

>
> Thanks,
> Andrew
>
> ---
>
>  From b2caee6aaa78106d7ae3c46dda3a84a325e43a1d Mon Sep 17 00:00:00 2001
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Mon, 7 Oct 2019 12:34:51 +0100
> Subject: [PATCH] gdb: Rename structures within ctfread.c
>
> Commit:
>
>    commit 30d1f0184953478d14641c495261afd06ebfabac
>    Date:   Mon Oct 7 00:46:52 2019 +0000
>
>        gdb: CTF support
>
> Introduces some structures with names that are already in use within
> GBB, this violates C++'s one-definition rule.  Specifically the
> structures 'nextfield' and 'field_info' are now defined in
> dwarf2read.c and ctfread.c.
>
> This commit renames the new structures (in ctfread.c), adding a 'ctf_'
> prefix.  Maybe we should consider renaming the DWARF versions too in
> the future to avoid accidental conflicts.
>
> gdb/ChangeLog:
>
> 	* ctfread.c (struct nextfield): Renamed to ...
> 	(struct ctf_nextfield): ... this.
> 	(struct field_info): Renamed to ...
> 	(strut ctf_field_info): ... this.
> 	(attach_fields_to_type): Update for renamed structures.
> 	(ctf_add_member_cb): Likewise.
> 	(ctf_add_enum_member_cb): Likewise.
> 	(process_struct_members): Likewise.
> 	(process_enum_type): Likewise.
> ---
>   gdb/ChangeLog | 12 ++++++++++++
>   gdb/ctfread.c | 28 ++++++++++++++--------------
>   2 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/ctfread.c b/gdb/ctfread.c
> index 3e3bd89d5f1..44ccff62ae3 100644
> --- a/gdb/ctfread.c
> +++ b/gdb/ctfread.c
> @@ -98,17 +98,17 @@ typedef struct ctf_context
>   
>   /* The routines that read and process fields/members of a C struct, union,
>      or enumeration, pass lists of data member fields in an instance of a
> -   field_info structure. It is derived from dwarf2read.c.  */
> +   ctf_field_info structure. It is derived from dwarf2read.c.  */
>   
> -struct nextfield
> +struct ctf_nextfield
>   {
>     struct field field {};
>   };
>   
> -struct field_info
> +struct ctf_field_info
>   {
>     /* List of data member fields.  */
> -  std::vector<struct nextfield> fields;
> +  std::vector<struct ctf_nextfield> fields;
>   
>     /* Context.  */
>     ctf_context_t *cur_context;
> @@ -269,7 +269,7 @@ set_symbol_address (struct objfile *of, struct symbol *sym, const char *name)
>   /* Create the vector of fields, and attach it to TYPE.  */
>   
>   static void
> -attach_fields_to_type (struct field_info *fip, struct type *type)
> +attach_fields_to_type (struct ctf_field_info *fip, struct type *type)
>   {
>     int nfields = fip->fields.size ();
>   
> @@ -284,7 +284,7 @@ attach_fields_to_type (struct field_info *fip, struct type *type)
>     /* Copy the saved-up fields into the field vector.  */
>     for (int i = 0; i < nfields; ++i)
>       {
> -      struct nextfield &field = fip->fields[i];
> +      struct ctf_nextfield &field = fip->fields[i];
>         TYPE_FIELD (type, i) = field.field;
>       }
>   }
> @@ -314,7 +314,7 @@ ctf_init_float_type (struct objfile *objfile,
>   
>   /* Callback to add member NAME to a struct/union type. TID is the type
>      of struct/union member, OFFSET is the offset of member in bits,
> -   and ARG contains the field_info.  */
> +   and ARG contains the ctf_field_info.  */
>   
>   static int
>   ctf_add_member_cb (const char *name,
> @@ -322,9 +322,9 @@ ctf_add_member_cb (const char *name,
>   		   unsigned long offset,
>   		   void *arg)
>   {
> -  struct field_info *fip = (struct field_info *) arg;
> +  struct ctf_field_info *fip = (struct ctf_field_info *) arg;
>     ctf_context_t *ccp = fip->cur_context;
> -  struct nextfield new_field;
> +  struct ctf_nextfield new_field;
>     struct field *fp;
>     struct type *t;
>     uint32_t kind;
> @@ -358,13 +358,13 @@ ctf_add_member_cb (const char *name,
>   }
>   
>   /* Callback to add member NAME of EVAL to an enumeration type.
> -   ARG contains the field_info.  */
> +   ARG contains the ctf_field_info.  */
>   
>   static int
>   ctf_add_enum_member_cb (const char *name, int enum_value, void *arg)
>   {
> -  struct field_info *fip = (struct field_info *) arg;
> -  struct nextfield new_field;
> +  struct ctf_field_info *fip = (struct ctf_field_info *) arg;
> +  struct ctf_nextfield new_field;
>     struct field *fp;
>     ctf_context_t *ccp = fip->cur_context;
>   
> @@ -587,7 +587,7 @@ process_struct_members (ctf_context_t *ccp,
>   			ctf_id_t tid,
>   			struct type *type)
>   {
> -  struct field_info fi;
> +  struct ctf_field_info fi;
>   
>     fi.cur_context = ccp;
>     if (ctf_member_iter (ccp->fp, tid, ctf_add_member_cb, &fi) == CTF_ERR)
> @@ -665,7 +665,7 @@ static void
>   process_enum_type (ctf_context_t *ccp, ctf_id_t tid)
>   {
>     struct type *type;
> -  struct field_info fi;
> +  struct ctf_field_info fi;
>   
>     type = read_enum_type (ccp, tid);
>