[v3] gdb: CTF support

  * 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.

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(-)
  

On 2019-10-07 8:07 a.m., 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.
> 
> 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.

Thanks guys for catching this.  Just in case you didn't already know,
another way to avoid these problems (if we apply it everywhere) is to
define everything that is local to a source file in an anonymous
namespace.  I don't really have the habit of doing it at the moment,
but maybe I should start to do it consistently.

Simon
  

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);
>