Recent commit in binutils-gdb

Recent commit in binutils-gdb

[Simon Marchi]

Hi Eric,

I was just wondering about the recent commit 84ed7a4725 in the binutils-gdb
git repo, touching dwarf2/read.c, there's no commit message and I haven't seen
it pass by on the mailing list.  Where does this come from, what is it about?

Simon

Re: Recent commit in binutils-gdb

[Eric Christopher]

Hi Simon!

Sterling misplaced his commit keys and asked me to commit on his behalf.
IIRC it was approved a couple months ago and just now had the chance to
commit.

I tried to set the author etc correctly but let me know if I
missed anything.

-eric

On Wed, Apr 29, 2020, 5:50 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:

> Hi Eric,
>
> I was just wondering about the recent commit 84ed7a4725 in the binutils-gdb
> git repo, touching dwarf2/read.c, there's no commit message and I haven't
> seen
> it pass by on the mailing list.  Where does this come from, what is it
> about?
>
> Simon
>

Re: Recent commit in binutils-gdb

[Sterling Augustine]

The commit was proposed here:

https://sourceware.org/pipermail/gdb/2020-March/000012.html

And approved here:

https://sourceware.org/pipermail/gdb/2020-April/048466.html

On Wed, Apr 29, 2020 at 6:57 PM Eric Christopher <echristo@gmail.com> wrote:

> Hi Simon!
>
> Sterling misplaced his commit keys and asked me to commit on his behalf.
> IIRC it was approved a couple months ago and just now had the chance to
> commit.
>
> I tried to set the author etc correctly but let me know if I
> missed anything.
>
> -eric
>
> On Wed, Apr 29, 2020, 5:50 PM Simon Marchi <simon.marchi@polymtl.ca>
> wrote:
>
>> Hi Eric,
>>
>> I was just wondering about the recent commit 84ed7a4725 in the
>> binutils-gdb
>> git repo, touching dwarf2/read.c, there's no commit message and I haven't
>> seen
>> it pass by on the mailing list.  Where does this come from, what is it
>> about?
>>
>> Simon
>>
>

Re: Recent commit in binutils-gdb

[Tom de Vries]

On 30-04-2020 06:10, Sterling Augustine via Gdb-patches wrote:
> The commit was proposed here:
> 
> https://sourceware.org/pipermail/gdb/2020-March/000012.html
> 

With reproducer:
...
$ cat b.cpp
namespace hide { struct Bar {  int y; }; }
struct Foo {  hide::Bar* b; };

inline void x() { hide::Bar b; }

static Foo aFoo;

void * f() {
  return aFoo.b;
}

int main() {
  f();
  return 0;
}
$ g++ -fdebug-types-section b.cpp -g
$ ./build/gdb/gdb -ex "b f" -ex run -ex "ptype hide::Bar" ./a.out
...

I can reproduce this problem before the recent commit 770479f223e "gdb:
Fix toplevel types with -fdebug-types-section", but not after.

Also, when I revert commit 770479f223e, and leave this one in place, it
doesn't fix the problem. That seems to be because the proposed commit:
...
@@ -10954,6 +10954,7 @@ dwarf2_cu::setup_type_unit_groups (struct
die_info *die)
                        compunit_language (cust),
                        0, cust));

+      list_in_scope = get_builder ()->get_file_symbols ();
       auto &file_names = line_header->file_names ();
       for (i = 0; i < file_names.size (); ++i)
        {
...
and the actual commit differ (note the different in context before):
...
@@ -10942,6 +10942,7 @@ dwarf2_cu::setup_type_unit_groups (struct
die_info *die)
        = XOBNEWVEC (&COMPUNIT_OBJFILE (cust)->objfile_obstack,
                     struct symtab *, line_header->file_names_size ());

+      list_in_scope = get_builder ()->get_file_symbols ();
       auto &file_names = line_header->file_names ();
       for (i = 0; i < file_names.size (); ++i)
        {
...

In summary, the problem was fixed by an earlier commit, and this commit
was misapplied. I think we should probably revert this one.

Thanks,
- Tom

Re: Recent commit in binutils-gdb

[Andrew Burgess]

* Tom de Vries <tdevries@suse.de> [2020-04-30 07:49:56 +0200]:

> On 30-04-2020 06:10, Sterling Augustine via Gdb-patches wrote:
> > The commit was proposed here:
> > 
> > https://sourceware.org/pipermail/gdb/2020-March/000012.html
> > 
> 
> With reproducer:
> ...
> $ cat b.cpp
> namespace hide { struct Bar {  int y; }; }
> struct Foo {  hide::Bar* b; };
> 
> inline void x() { hide::Bar b; }
> 
> static Foo aFoo;
> 
> void * f() {
>   return aFoo.b;
> }
> 
> int main() {
>   f();
>   return 0;
> }
> $ g++ -fdebug-types-section b.cpp -g
> $ ./build/gdb/gdb -ex "b f" -ex run -ex "ptype hide::Bar" ./a.out
> ...
> 
> I can reproduce this problem before the recent commit 770479f223e "gdb:
> Fix toplevel types with -fdebug-types-section", but not after.
> 
> Also, when I revert commit 770479f223e, and leave this one in place, it
> doesn't fix the problem. That seems to be because the proposed commit:
> ...
> @@ -10954,6 +10954,7 @@ dwarf2_cu::setup_type_unit_groups (struct
> die_info *die)
>                         compunit_language (cust),
>                         0, cust));
> 
> +      list_in_scope = get_builder ()->get_file_symbols ();
>        auto &file_names = line_header->file_names ();
>        for (i = 0; i < file_names.size (); ++i)
>         {
> ...
> and the actual commit differ (note the different in context before):
> ...
> @@ -10942,6 +10942,7 @@ dwarf2_cu::setup_type_unit_groups (struct
> die_info *die)
>         = XOBNEWVEC (&COMPUNIT_OBJFILE (cust)->objfile_obstack,
>                      struct symtab *, line_header->file_names_size ());
> 
> +      list_in_scope = get_builder ()->get_file_symbols ();
>        auto &file_names = line_header->file_names ();
>        for (i = 0; i < file_names.size (); ++i)
>         {
> ...
> 
> In summary, the problem was fixed by an earlier commit, and this commit
> was misapplied. I think we should probably revert this one.

FWIW I agree.

Thanks,
Andrew

Re: Recent commit in binutils-gdb

[Simon Marchi]

On 2020-04-30 12:10 a.m., Sterling Augustine wrote:
> The commit was proposed here: 
> 
> https://sourceware.org/pipermail/gdb/2020-March/000012.html
> 
> And approved here:
> 
> https://sourceware.org/pipermail/gdb/2020-April/048466.html

Ah, that's because it's on gdb@, not gdb-patches@.  Sorry about that.

Simon

Re: Recent commit in binutils-gdb

[Tom de Vries]

On 30-04-2020 15:24, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2020-04-30 07:49:56 +0200]:
> 
>> On 30-04-2020 06:10, Sterling Augustine via Gdb-patches wrote:
>>> The commit was proposed here:
>>>
>>> https://sourceware.org/pipermail/gdb/2020-March/000012.html
>>>
>>
>> With reproducer:
>> ...
>> $ cat b.cpp
>> namespace hide { struct Bar {  int y; }; }
>> struct Foo {  hide::Bar* b; };
>>
>> inline void x() { hide::Bar b; }
>>
>> static Foo aFoo;
>>
>> void * f() {
>>   return aFoo.b;
>> }
>>
>> int main() {
>>   f();
>>   return 0;
>> }
>> $ g++ -fdebug-types-section b.cpp -g
>> $ ./build/gdb/gdb -ex "b f" -ex run -ex "ptype hide::Bar" ./a.out
>> ...
>>
>> I can reproduce this problem before the recent commit 770479f223e "gdb:
>> Fix toplevel types with -fdebug-types-section", but not after.
>>
>> Also, when I revert commit 770479f223e, and leave this one in place, it
>> doesn't fix the problem. That seems to be because the proposed commit:
>> ...
>> @@ -10954,6 +10954,7 @@ dwarf2_cu::setup_type_unit_groups (struct
>> die_info *die)
>>                         compunit_language (cust),
>>                         0, cust));
>>
>> +      list_in_scope = get_builder ()->get_file_symbols ();
>>        auto &file_names = line_header->file_names ();
>>        for (i = 0; i < file_names.size (); ++i)
>>         {
>> ...
>> and the actual commit differ (note the different in context before):
>> ...
>> @@ -10942,6 +10942,7 @@ dwarf2_cu::setup_type_unit_groups (struct
>> die_info *die)
>>         = XOBNEWVEC (&COMPUNIT_OBJFILE (cust)->objfile_obstack,
>>                      struct symtab *, line_header->file_names_size ());
>>
>> +      list_in_scope = get_builder ()->get_file_symbols ();
>>        auto &file_names = line_header->file_names ();
>>        for (i = 0; i < file_names.size (); ++i)
>>         {
>> ...
>>
>> In summary, the problem was fixed by an earlier commit, and this commit
>> was misapplied. I think we should probably revert this one.
> 
> FWIW I agree.

Andrew, thanks for the input,  I've now reverted the patch.

Guys, my apologies for how this went.  A patch for this problem was
approved in Januari, but lingered around uncommitted, leading to
duplicated work.  We'll try to do better.

Thanks,
- Tom

Re: Recent commit in binutils-gdb

[Eric Christopher]

Thanks for the revert. Sorry for any trouble it caused :)

-eric

On Thu, Apr 30, 2020 at 10:01 AM Tom de Vries <tdevries@suse.de> wrote:

> On 30-04-2020 15:24, Andrew Burgess wrote:
> > * Tom de Vries <tdevries@suse.de> [2020-04-30 07:49:56 +0200]:
> >
> >> On 30-04-2020 06:10, Sterling Augustine via Gdb-patches wrote:
> >>> The commit was proposed here:
> >>>
> >>> https://sourceware.org/pipermail/gdb/2020-March/000012.html
> >>>
> >>
> >> With reproducer:
> >> ...
> >> $ cat b.cpp
> >> namespace hide { struct Bar {  int y; }; }
> >> struct Foo {  hide::Bar* b; };
> >>
> >> inline void x() { hide::Bar b; }
> >>
> >> static Foo aFoo;
> >>
> >> void * f() {
> >>   return aFoo.b;
> >> }
> >>
> >> int main() {
> >>   f();
> >>   return 0;
> >> }
> >> $ g++ -fdebug-types-section b.cpp -g
> >> $ ./build/gdb/gdb -ex "b f" -ex run -ex "ptype hide::Bar" ./a.out
> >> ...
> >>
> >> I can reproduce this problem before the recent commit 770479f223e "gdb:
> >> Fix toplevel types with -fdebug-types-section", but not after.
> >>
> >> Also, when I revert commit 770479f223e, and leave this one in place, it
> >> doesn't fix the problem. That seems to be because the proposed commit:
> >> ...
> >> @@ -10954,6 +10954,7 @@ dwarf2_cu::setup_type_unit_groups (struct
> >> die_info *die)
> >>                         compunit_language (cust),
> >>                         0, cust));
> >>
> >> +      list_in_scope = get_builder ()->get_file_symbols ();
> >>        auto &file_names = line_header->file_names ();
> >>        for (i = 0; i < file_names.size (); ++i)
> >>         {
> >> ...
> >> and the actual commit differ (note the different in context before):
> >> ...
> >> @@ -10942,6 +10942,7 @@ dwarf2_cu::setup_type_unit_groups (struct
> >> die_info *die)
> >>         = XOBNEWVEC (&COMPUNIT_OBJFILE (cust)->objfile_obstack,
> >>                      struct symtab *, line_header->file_names_size ());
> >>
> >> +      list_in_scope = get_builder ()->get_file_symbols ();
> >>        auto &file_names = line_header->file_names ();
> >>        for (i = 0; i < file_names.size (); ++i)
> >>         {
> >> ...
> >>
> >> In summary, the problem was fixed by an earlier commit, and this commit
> >> was misapplied. I think we should probably revert this one.
> >
> > FWIW I agree.
>
> Andrew, thanks for the input,  I've now reverted the patch.
>
> Guys, my apologies for how this went.  A patch for this problem was
> approved in Januari, but lingered around uncommitted, leading to
> duplicated work.  We'll try to do better.
>
> Thanks,
> - Tom
>