RE: [PATCH] Debug support for global alias variable

["Natarajan, Kavitha" <Kavitha.Natarajan@amd.com>]

[AMD Official Use Only]

Sorry for my oversight on the spaces. I fixed it.

Thank you so much @Bruno Larsen for taking time to review the patch. Looking for someone to approve the patch.

Regards,
Kavitha

> -----Original Message-----
> From: Bruno Larsen <blarsen@redhat.com>
> Sent: Tuesday, April 5, 2022 7:03 PM
> To: Natarajan, Kavitha <Kavitha.Natarajan@amd.com>; Natarajan, Kavitha
> via Gdb-patches <gdb-patches@sourceware.org>
> Cc: George, Jini Susan <JiniSusan.George@amd.com>; Parasuraman,
> Hariharan <Hariharan.Parasuraman@amd.com>
> Subject: Re: [PATCH] Debug support for global alias variable
>
> [CAUTION: External Email]
>
> On 3/30/22 09:25, Natarajan, Kavitha wrote:
> > [AMD Official Use Only]
> >
> > Sorry, I missed to send the new patch. Please find it below:
>
> Sorry for the delay on the response.
>
> I only noticed one small nit in a comment. Like I said before, can't approve for
> pushing, but it LGTM.
>
> >
> > ---------------------------------------------------------------------
> > When clang emits the dwarf information of global alias variable as
> > DW_TAG_imported_declaration, gdb does not handle it. GDB reads this
> > tag as C++/fortran imported declaration (type alias, namespace alias
> > and fortran module). Added support to handle this tag as alias
> > variable.
> >
> > This change fixes the failure in gdb.base/symbol-alias.exp testcase.
> > This testcase is also updated to test nested (recursive) alias.
> > ---
> >   gdb/dwarf2/read.c                       | 75 +++++++++++++++++--------
> >   gdb/testsuite/gdb.base/symbol-alias.exp | 63 ++++++++++++++++++++-
> >   gdb/testsuite/gdb.base/symbol-alias2.c  | 16 ++++++
> >   3 files changed, 128 insertions(+), 26 deletions(-)
> >
> > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index
> > f9c942d91d3..12d66378f79 100644
> > --- a/gdb/dwarf2/read.c
> > +++ b/gdb/dwarf2/read.c
> > @@ -1132,6 +1132,10 @@ static void add_partial_subprogram (struct
> partial_die_info *pdi,
> >                                      CORE_ADDR *lowpc, CORE_ADDR *highpc,
> >                                      int need_pc, struct dwarf2_cu
> > *cu);
> >
> > +static void var_decode_location (struct attribute *attr,
> > +                                struct symbol *sym,
> > +                                struct dwarf2_cu *cu);
> > +
> >   static unsigned int peek_abbrev_code (bfd *, const gdb_byte *);
> >
> >   static struct partial_die_info *load_partial_dies @@ -1332,7 +1336,7
> > @@ static struct using_direct **using_directives (struct dwarf2_cu
> > *cu);
> >
> >   static void read_import_statement (struct die_info *die, struct
> > dwarf2_cu *);
> >
> > -static int read_namespace_alias (struct die_info *die, struct
> > dwarf2_cu *cu);
> > +static bool read_alias (struct die_info *die, struct dwarf2_cu *cu);
> >
> >   static struct type *read_module_type (struct die_info *die,
> >                                        struct dwarf2_cu *cu); @@
> > -9741,10 +9745,14 @@ process_die (struct die_info *die, struct dwarf2_cu
> *cu)
> >         read_module (die, cu);
> >         break;
> >       case DW_TAG_imported_declaration:
> > +      /* If the imported declaration is for global variable alias,
> > +         this flag is set to false in read_alias function.  */
> >         cu->processing_has_namespace_info = true;
> > -      if (read_namespace_alias (die, cu))
> > +      if (((cu->per_cu->lang == language_c)
> > +          || (cu->per_cu->lang == language_cplus))
> > +         && read_alias (die, cu))
> >          break;
> > -      /* The declaration is not a global namespace alias.  */
> > +      /* The declaration is not a global namespace or variable alias.
> > + */
> >         /* Fall through.  */
> >       case DW_TAG_imported_module:
> >         cu->processing_has_namespace_info = true; @@ -10212,18
> > +10220,21 @@ dwarf2_physname (const char *name, struct die_info *die,
> struct dwarf2_cu *cu)
> >     return retval;
> >   }
> >
> > -/* Inspect DIE in CU for a namespace alias.  If one exists, record
> > -   a new symbol for it.
> > +#define MAX_NESTED_IMPORTED_DECLARATIONS 100
> >
> > -   Returns 1 if a namespace alias was recorded, 0 otherwise.  */
> > +/* Inspect DIE in CU for a namespace alias or a variable
> > +   with alias attribute. If one exists, record a new symbol
> > +   for it.
> >
> > -static int
> > -read_namespace_alias (struct die_info *die, struct dwarf2_cu *cu)
> > +   Returns "true" if an alias was recorded, "false" otherwise.  */
> > +
> > +static bool
> > +read_alias (struct die_info *die, struct dwarf2_cu *cu)
> >   {
> >     struct attribute *attr;
> >
> > -  /* If the die does not have a name, this is not a namespace
> > -     alias.  */
> > +  /* If the die does not have a name, this is not a namespace alias
> > +     or variable alias.  */
> >     attr = dwarf2_attr (die, DW_AT_name, cu);
> >     if (attr != NULL)
> >       {
> > @@ -10232,13 +10243,12 @@ read_namespace_alias (struct die_info *die,
> struct dwarf2_cu *cu)
> >         struct dwarf2_cu *imported_cu = cu;
> >
> >         /* If the compiler has nested DW_AT_imported_declaration DIEs,
> > -        keep inspecting DIEs until we hit the underlying import.  */
> > -#define MAX_NESTED_IMPORTED_DECLARATIONS 100
> > +         keep inspecting DIEs until we hit the underlying import.  */
>
> This comment still has 8 spaces instead of a tab. You must have missed it on
> your pass.
>
> >         for (num = 0; num  < MAX_NESTED_IMPORTED_DECLARATIONS;
> ++num)
> >          {
> >            attr = dwarf2_attr (d, DW_AT_import, cu);
> >            if (attr == NULL)
> > -           break;
> > +           return false;
> >
> >            d = follow_die_ref (d, attr, &imported_cu);
> >            if (d->tag != DW_TAG_imported_declaration) @@ -10249,26
> > +10259,43 @@ read_namespace_alias (struct die_info *die, struct
> dwarf2_cu *cu)
> >          {
> >            complaint (_("DIE at %s has too many recursively imported "
> >                         "declarations"), sect_offset_str (d->sect_off));
> > -         return 0;
> > +         return false;
> >          }
> >
> >         if (attr != NULL)
> >          {
> >            struct type *type;
> > -         sect_offset sect_off = attr->get_ref_die_offset ();
> > -
> > -         type = get_die_type_at_offset (sect_off, cu->per_cu, cu-
> >per_objfile);
> > -         if (type != NULL && type->code () == TYPE_CODE_NAMESPACE)
> > +         if (d->tag == DW_TAG_variable)
> >              {
> > -             /* This declaration is a global namespace alias.  Add
> > -                a symbol for it whose type is the aliased namespace.  */
> > -             new_symbol (die, type, cu);
> > -             return 1;
> > +             /* This declaration is a global variable alias. Add
> > +                a symbol for it whose type is same as aliased variable.  */
> > +             type = die_type (d, imported_cu);
> > +             struct symbol *sym = new_symbol (die, type, cu);
> > +             attr = dwarf2_attr (d, DW_AT_location, imported_cu);
> > +             sym->set_aclass_index (LOC_UNRESOLVED);
> > +             if (attr != nullptr)
> > +               {
> > +                 var_decode_location (attr, sym, cu);
> > +               }
> > +             /* Reset the flag as it is not a namespace alias.  */
> > +             cu->processing_has_namespace_info = false;
> > +             return true;
> > +           }
> > +         else
> > +           {
> > +             sect_offset sect_off = attr->get_ref_die_offset ();
> > +             type = get_die_type_at_offset (sect_off, cu->per_cu, cu-
> >per_objfile);
> > +             if (type != NULL && type->code () == TYPE_CODE_NAMESPACE)
> > +               {
> > +                 /* This declaration is a global namespace alias. Add
> > +                    a symbol for it whose type is the aliased namespace.  */
> > +                 new_symbol (die, type, cu);
> > +                 return true;
> > +               }
> >              }
> >          }
> >       }
> > -
> > -  return 0;
> > +  return false;
> >   }
> >
> >   /* Return the using directives repository (global or local?) to use
> > in the diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp
> > b/gdb/testsuite/gdb.base/symbol-alias.exp
> > index 2b53cc31053..a9487b25f4d 100644
> > --- a/gdb/testsuite/gdb.base/symbol-alias.exp
> > +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
> > @@ -15,6 +15,9 @@
> >   # along with this program.  If not, see
> <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.gnu.org%2Flicenses%2F&amp;data=04%7C01%7Ckavitha.natarajan%40am
> d.com%7Cee7adda17f644c3ff44c08da1708d555%7C3dd8961fe4884e608e11a8
> 2d994e183d%7C0%7C0%7C637847623963520029%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C3000&amp;sdata=BiRp7rTFWbGY0AMXSmyy02d5uZwi4P52GJEjiRkb
> g0E%3D&amp;reserved=0>.
> >
> >   standard_testfile symbol-alias.c symbol-alias2.c
> > +set using_clang [test_compiler_info clang-*] set old_clang [expr
> > +[test_compiler_info {clang-1[0-3]-*-*}] \
> > +                || [test_compiler_info {clang-[1-9]-*}]]
> >
> >   if { [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile
> $srcfile2]] } {
> >       return -1
> > @@ -31,6 +34,62 @@ foreach f {"func" "func_alias"} {
> >   }
> >
> >   # Variables.
> > -foreach v {"g_var_s" "g_var_s_alias"} {
> > -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
> > +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
> > +foreach v {"g_var_s_alias" "g_var_s_alias2"} {
> > +    gdb_test_multiple "p $v" "p $v" {
> > +       -re  " = {field1 = 1, field2 = 2}.*$gdb_prompt $" {
> > +           pass "print alias of variable $v"
> > +       }
> > +       -re  ".*has unknown type; cast it to its declared type.*$gdb_prompt $"
> {
> > +           if { $old_clang } {
> > +               xfail "print alias variable $v"
> > +           } else {
> > +               fail "print alias variable $v"
> > +           }
> > +       }
> > +    }
> > +}
> > +
> > +# Static Variable.
> > +gdb_test "p g_var" " = 1"
> > +gdb_test_multiple "p g_var_alias" "p g_var_alias" {
> > +    -re  " = 1.*$gdb_prompt $" {
> > +       pass "print alias of static variable"
> > +    }
> > +    -re  ".*has unknown type; cast it to its declared type.*$gdb_prompt $" {
> > +       if { $old_clang } {
> > +         xfail "print alias of static variable"
> > +       } else {
> > +         fail "print alias of static variable"
> > +       }
> > +    }
> > +}
> > +
> > +# Deferred Variable.
> > +gdb_test "p g_def_var" " = 2"
> > +gdb_test_multiple "p g_def_var_alias" "p g_def_var_alias" {
> > +    -re  " = 2.*$gdb_prompt $" {
> > +       pass "print alias of deferred variable"
> > +    }
> > +    -re  ".*has unknown type; cast it to its declared type.*$gdb_prompt $" {
> > +       if { $using_clang } {
> > +         xfail "print alias of deferred variable"
> > +       } else {
> > +         fail "print alias of deferred variable"
> > +       }
> > +    }
> > +}
> > +
> > +# Alias of deferred Variable alias.
> > +gdb_test_multiple "p g_def_var2_alias2" "p g_def_var2_alias2" {
> > +    -re  " = 3.*$gdb_prompt $" {
> > +       pass "print alias of alias of deferred variable"
> > +    }
> > +    -re  ".*has unknown type; cast it to its declared type.*$gdb_prompt $" {
> > +       if { $old_clang } {
> > +         xfail "print alias of alias of deferred variable"
> > +       } else {
> > +         fail "print alias of alias of deferred variable"
> > +       }
> > +    }
> >   }
> > diff --git a/gdb/testsuite/gdb.base/symbol-alias2.c
> > b/gdb/testsuite/gdb.base/symbol-alias2.c
> > index 34f4e121e25..41d4b6a53e5 100644
> > --- a/gdb/testsuite/gdb.base/symbol-alias2.c
> > +++ b/gdb/testsuite/gdb.base/symbol-alias2.c
> > @@ -23,6 +23,8 @@ struct S
> >
> >   struct S g_var_s = { 1, 2 };
> >
> > +static int g_var = 1;
> > +
> >   #ifdef __cplusplus
> >   /* So that the alias attribute below work without having to figure out
> >      this function's mangled name.  */ @@ -38,3 +40,17 @@ func (void)
> >   struct S *func_alias (void) __attribute__ ((alias ("func")));
> >
> >   extern struct S g_var_s_alias __attribute__ ((alias ("g_var_s")));
> > +
> > +extern struct S g_var_s_alias2 __attribute__ ((alias
> > +("g_var_s_alias")));
> > +
> > +extern int g_var_alias __attribute__ ((alias ("g_var")));
> > +
> > +extern int g_def_var_alias __attribute__ ((alias ("g_def_var")));
> > +
> > +int g_def_var = 2;
> > +
> > +extern int g_def_var2_alias __attribute__ ((alias ("g_def_var2")));
> > +
> > +int g_def_var2 = 3;
> > +
> > +extern int g_def_var2_alias2 __attribute__ ((alias
> > +("g_def_var2_alias")));
> > --
> >
> > Regards,
> > Kavitha
>
>
> --
> Cheers!
> Bruno Larsen