[PATCH5,PR,gdb/16959] gdb hangs in infinite recursion

Message ID 1522269884-129860-1-git-send-email-weimin.pan@oracle.com
State New
Headers show
Series
  • [PATCH5,PR,gdb/16959] gdb hangs in infinite recursion
Related show

Commit Message

Weimin Pan March 28, 2018, 8:44 p.m.
The original problem was fixed (see related PR 22242). But using a typedef
as the declared type for a static member variable, as commented in this PR,
is still causing gdb to get into infinite loop when printing the static
member's value. This problem can be reproduced as follows:

% cat t.cc
class A {
    typedef A type;
public:
    bool operator==(const type& other) { return true; }

    static const type INSTANCE;
};

const A A::INSTANCE;

int main() {
    A a;
    if (a == A::INSTANCE) {
        return -1;
    }
    return 0;
}
% g++ -g t.cc
% gdb -ex "start" -ex "p a" a.out

The fix is rather trivial - in cp_print_static_field(), should call
check_typedef() to get the static member's real type and use it to
check whether it's a struct or an array.

As Simon suggested, I've added a new test case to the testsuite
and am passing the original type, not the real type, as argument
to both cp_print_value_fields() and val_print().

Re-tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.
---
 gdb/ChangeLog                                 |    6 ++++
 gdb/cp-valprint.c                             |    6 ++--
 gdb/testsuite/ChangeLog                       |    5 +++
 gdb/testsuite/gdb.cp/static-typedef-print.cc  |   34 +++++++++++++++++++++++
 gdb/testsuite/gdb.cp/static-typedef-print.exp |   37 +++++++++++++++++++++++++
 5 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.cc
 create mode 100644 gdb/testsuite/gdb.cp/static-typedef-print.exp

-- 
1.7.1

Comments

Simon Marchi March 30, 2018, 5:08 a.m. | #1
On 2018-03-28 16:44, Weimin Pan wrote:
> The original problem was fixed (see related PR 22242). But using a 

> typedef

> as the declared type for a static member variable, as commented in this 

> PR,

> is still causing gdb to get into infinite loop when printing the static

> member's value. This problem can be reproduced as follows:

> 

> % cat t.cc

> class A {

>     typedef A type;

> public:

>     bool operator==(const type& other) { return true; }

> 

>     static const type INSTANCE;

> };

> 

> const A A::INSTANCE;

> 

> int main() {

>     A a;

>     if (a == A::INSTANCE) {

>         return -1;

>     }

>     return 0;

> }

> % g++ -g t.cc

> % gdb -ex "start" -ex "p a" a.out

> 

> The fix is rather trivial - in cp_print_static_field(), should call

> check_typedef() to get the static member's real type and use it to

> check whether it's a struct or an array.

> 

> As Simon suggested, I've added a new test case to the testsuite

> and am passing the original type, not the real type, as argument

> to both cp_print_value_fields() and val_print().

> 

> Re-tested on both aarch64-linux-gnu and amd64-linux-gnu. No 

> regressions.


Hi Weimin,

Could you change the title to something more descriptive about what the 
change/fix is, rather than the problem being fixed?  For example, "Fix 
infinite recursion when printing static member with typedef".

It is ok to push with that fixed (feel free to ask if you still need 
some guidance).

Thanks!

Simon
Weimin Pan March 30, 2018, 9:43 p.m. | #2
On 3/29/2018 10:08 PM, Simon Marchi wrote:
> On 2018-03-28 16:44, Weimin Pan wrote:

>> The original problem was fixed (see related PR 22242). But using a 

>> typedef

>> as the declared type for a static member variable, as commented in 

>> this PR,

>> is still causing gdb to get into infinite loop when printing the static

>> member's value. This problem can be reproduced as follows:

>>

>> % cat t.cc

>> class A {

>>     typedef A type;

>> public:

>>     bool operator==(const type& other) { return true; }

>>

>>     static const type INSTANCE;

>> };

>>

>> const A A::INSTANCE;

>>

>> int main() {

>>     A a;

>>     if (a == A::INSTANCE) {

>>         return -1;

>>     }

>>     return 0;

>> }

>> % g++ -g t.cc

>> % gdb -ex "start" -ex "p a" a.out

>>

>> The fix is rather trivial - in cp_print_static_field(), should call

>> check_typedef() to get the static member's real type and use it to

>> check whether it's a struct or an array.

>>

>> As Simon suggested, I've added a new test case to the testsuite

>> and am passing the original type, not the real type, as argument

>> to both cp_print_value_fields() and val_print().

>>

>> Re-tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.

>

> Hi Weimin,

>

> Could you change the title to something more descriptive about what 

> the change/fix is, rather than the problem being fixed?  For example, 

> "Fix infinite recursion when printing static member with typedef".

>

> It is ok to push with that fixed (feel free to ask if you still need 

> some guidance).

>

> Thanks!

>

> Simon

Hi Simon,

I just got started to work on this. Here is what I've done (I followed
your lead to creat a different remote name):

% git add <newfile>
% git commit -a
% git remote add upstream ssh://sourceware.org/git/binutils-gdb.git
% git fetch upstream

I have a few questions:

  * Do I need to do a "git merge" after "git fetch"? Or can I just
    do "git pull" which is equivalent to "git fetch;git merge"?
    (I was a Mercurial(hg) user, its typical workflow is like:
     hg in; do work; hg commit; hg pull; hg rebase(if needed); hg push)

  * In your previous email, you said:

    Make sure you inserted the ChangeLog entries in the actual ChangeLog 
files
    and amended your commit

    It seems the "git commit -a" command will contain all the changes, 
including
    those in ChangeLog files. Why do I have to insert the entries?

  * Changing the commit title to be be more descriptive:

    So I need to use "git commit --amend" to change the title?

Thanks very much for your help.

Weimin
Simon Marchi March 30, 2018, 10:04 p.m. | #3
On 2018-03-30 05:43 PM, Weimin Pan wrote:
> Hi Simon,

> 

> I just got started to work on this. Here is what I've done (I followed

> your lead to creat a different remote name):

> 

> % git add <newfile>

> % git commit -a

> % git remote add upstream ssh://sourceware.org/git/binutils-gdb.git

> % git fetch upstream

> 

> I have a few questions:

> 

>   * Do I need to do a "git merge" after "git fetch"? Or can I just

>     do "git pull" which is equivalent to "git fetch;git merge"?

>     (I was a Mercurial(hg) user, its typical workflow is like:

>      hg in; do work; hg commit; hg pull; hg rebase(if needed); hg push)


There are two different approaches to bringing the commits from upstream into the
branch where you did some work, rebase and merge (they probably exist in mercurial
too, maybe some other name).  Many projects (including us) never use merging,
because it leads to a non-linear history, which is more difficult to follow and
bisect.

To keep it simple, if you have some commits of yours on master and want to "update"
to get the new stuff from the official repo, I suggest doing "git pull --rebase",
which is the same as "git fetch; git rebase".  It will basically try to apply your
commits on top of the "official" master branch.  You may need to handle any conflicts,
I suggest looking on the web, there are plenty of tutorials for that.

>   * In your previous email, you said:

> 

>     Make sure you inserted the ChangeLog entries in the actual ChangeLog 

> files

>     and amended your commit

> 

>     It seems the "git commit -a" command will contain all the changes, 

> including

>     those in ChangeLog files. Why do I have to insert the entries?


As you can see on the mailing list (though there are some variations), we usually
put the ChangeLog entry as part of the commit message when posting the patch to
the list.  I think the historical reason for that is that otherwise, rebasing your
patches always gives some conflicts in the ChangeLog files*.  Therefore, when comes
the time to push the patch to the upstream repo, we must not forget to actually
insert the ChangeLog entry at the top of the right ChangeLog file, and modify the
commit to contain that change.  This can be done with

$ ... copy ChangeLog entry to ChangeLog file ...
$ git add ChangeLog
$ git commit --amend

The last command will modify the currently checked out commit with the stage changes.

* You can use this to mitigate the conflicts in ChangeLogs though:
  http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/git-merge-changelog.c

I personally avoid using "git commit -a", as it's easy to add unwanted changes.  You
can try "git add -p" (with or without specifying a filename afteR).  For each modified
hunk, it will ask you if you want to add it to the staged changes (what's about to be
committed).  It's then easier to spot unintended changes.

> 

>   * Changing the commit title to be be more descriptive:

> 

>     So I need to use "git commit --amend" to change the title?


Exactly, "git commit --amend" will allow you to change the commit message, including
the title (the first line).

> 

> Thanks very much for your help.

> 

> Weimin 


You are welcome, thanks for your perseverance!

Simon
Weimin Pan March 30, 2018, 10:38 p.m. | #4
On 3/30/2018 3:04 PM, Simon Marchi wrote:
> On 2018-03-30 05:43 PM, Weimin Pan wrote:

>> Hi Simon,

>>

>> I just got started to work on this. Here is what I've done (I followed

>> your lead to creat a different remote name):

>>

>> % git add <newfile>

>> % git commit -a

>> % git remote add upstream ssh://sourceware.org/git/binutils-gdb.git

>> % git fetch upstream

>>

>> I have a few questions:

>>

>>    * Do I need to do a "git merge" after "git fetch"? Or can I just

>>      do "git pull" which is equivalent to "git fetch;git merge"?

>>      (I was a Mercurial(hg) user, its typical workflow is like:

>>       hg in; do work; hg commit; hg pull; hg rebase(if needed); hg push)

> There are two different approaches to bringing the commits from upstream into the

> branch where you did some work, rebase and merge (they probably exist in mercurial

> too, maybe some other name).  Many projects (including us) never use merging,

> because it leads to a non-linear history, which is more difficult to follow and

> bisect.

>

> To keep it simple, if you have some commits of yours on master and want to "update"

> to get the new stuff from the official repo, I suggest doing "git pull --rebase",

> which is the same as "git fetch; git rebase".  It will basically try to apply your

> commits on top of the "official" master branch.  You may need to handle any conflicts,

> I suggest looking on the web, there are plenty of tutorials for that.


Yes, there are indeed many tutorials out there and I've been reading 
some of them to learn more
about git :)

>>    * In your previous email, you said:

>>

>>      Make sure you inserted the ChangeLog entries in the actual ChangeLog

>> files

>>      and amended your commit

>>

>>      It seems the "git commit -a" command will contain all the changes,

>> including

>>      those in ChangeLog files. Why do I have to insert the entries?

> As you can see on the mailing list (though there are some variations), we usually

> put the ChangeLog entry as part of the commit message when posting the patch to

> the list.  I think the historical reason for that is that otherwise, rebasing your

> patches always gives some conflicts in the ChangeLog files*.  Therefore, when comes

> the time to push the patch to the upstream repo, we must not forget to actually

> insert the ChangeLog entry at the top of the right ChangeLog file, and modify the

> commit to contain that change.  This can be done with

>

> $ ... copy ChangeLog entry to ChangeLog file ...

> $ git add ChangeLog

> $ git commit --amend

>

> The last command will modify the currently checked out commit with the stage changes.


Thanks for the tips.  But I checked the ChangeLog files which seem to 
contains the entries.

> * You can use this to mitigate the conflicts in ChangeLogs though:

>    http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/git-merge-changelog.c

>

> I personally avoid using "git commit -a", as it's easy to add unwanted changes.  You

> can try "git add -p" (with or without specifying a filename afteR).  For each modified

> hunk, it will ask you if you want to add it to the staged changes (what's about to be

> committed).  It's then easier to spot unintended changes.


Yes, it looks like "git add -p" is better than "commit -a" which our 
group was told to use.

>>    * Changing the commit title to be be more descriptive:

>>

>>      So I need to use "git commit --amend" to change the title?

> Exactly, "git commit --amend" will allow you to change the commit message, including

> the title (the first line).


I just did my first patch:

$ git push upstream fixes
Enter passphrase for key '/home/wepan/.ssh/id_rsa':
Counting objects: 17, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 2.43 KiB, done.
Total 10 (delta 8), reused 0 (delta 0)
To ssh://sourceware.org/git/binutils-gdb.git
    0f59d5f..c9cf730  fixes -> fixes

and hope I did it correctly.

Thanks again for your help. I really appreciate it.

Weimin

> You are welcome, thanks for your perseverance!

>

> Simon
Joel Brobecker March 30, 2018, 10:52 p.m. | #5
I just emailed Weimin personally when I saw the branch creation,
but now I understand a little better:

> I just did my first patch:

> 

> $ git push upstream fixes

> Enter passphrase for key '/home/wepan/.ssh/id_rsa':

> Counting objects: 17, done.

> Delta compression using up to 8 threads.

> Compressing objects: 100% (10/10), done.

> Writing objects: 100% (10/10), 2.43 KiB, done.

> Total 10 (delta 8), reused 0 (delta 0)

> To ssh://sourceware.org/git/binutils-gdb.git

>    0f59d5f..c9cf730  fixes -> fixes

> 

> and hope I did it correctly.


Actually, no :).

What you did is you pushed your branch called locally "fixes"
to the repository corresponding to "upstream".  So, when you did
the "push" command above, what happened is that it created
the "fixes" branch on the upstream repository. This is not
what you want, because (1) it creates a branch on the remote
where you fix is (and pollutes the already existing branches),
and (2) does not change the "master" branch, and so your fix
is not really applied to the current development branch either.

With your permission, I will start by fixing the mistake, which
was to create the "fixes" branch on the upstream repository.

On your end, I think the simplest solution is for you to
push your current "fixes" branch to upstream's master:

    $ git push upstream fixes:master

**HOWEVER**, BEFORE YOU DO SO, please do the following:

    $ git log upstream/master..fixes

and verify that the list only shows the one path you are trying
to push. From your push of the "fixes" branch, I think that's correct,
but it's not a bad thing to be doing systematically, to make sure
you are pushing exactly what you think you are pushing.

I would like to recomment a book called "Pro Git" if you'd like
to learn about git. This is the book that allowed me to finally
break through git, and understand it.

-- 
Joel
Weimin Pan March 30, 2018, 11:04 p.m. | #6
Hi Joel,

On 3/30/2018 3:52 PM, Joel Brobecker wrote:
> I just emailed Weimin personally when I saw the branch creation,

> but now I understand a little better:

>

>> I just did my first patch:

>>

>> $ git push upstream fixes

>> Enter passphrase for key '/home/wepan/.ssh/id_rsa':

>> Counting objects: 17, done.

>> Delta compression using up to 8 threads.

>> Compressing objects: 100% (10/10), done.

>> Writing objects: 100% (10/10), 2.43 KiB, done.

>> Total 10 (delta 8), reused 0 (delta 0)

>> To ssh://sourceware.org/git/binutils-gdb.git

>>     0f59d5f..c9cf730  fixes -> fixes

>>

>> and hope I did it correctly.

> Actually, no :).

>

> What you did is you pushed your branch called locally "fixes"

> to the repository corresponding to "upstream".  So, when you did

> the "push" command above, what happened is that it created

> the "fixes" branch on the upstream repository. This is not

> what you want, because (1) it creates a branch on the remote

> where you fix is (and pollutes the already existing branches),

> and (2) does not change the "master" branch, and so your fix

> is not really applied to the current development branch either.

>

> With your permission, I will start by fixing the mistake, which

> was to create the "fixes" branch on the upstream repository.


Yes, please go ahead. Thanks.

>

> On your end, I think the simplest solution is for you to

> push your current "fixes" branch to upstream's master:

>

>      $ git push upstream fixes:master

>

> **HOWEVER**, BEFORE YOU DO SO, please do the following:

>

>      $ git log upstream/master..fixes


Yes, the log command does show the correct patch. It was actually my fault
by following:

   git push  <REMOTENAME> <BRANCHNAME>

with "fixes" as the branch name, as opposed to Simon's suggestion of 
"master:master".

>

> and verify that the list only shows the one path you are trying

> to push. From your push of the "fixes" branch, I think that's correct,

> but it's not a bad thing to be doing systematically, to make sure

> you are pushing exactly what you think you are pushing.

>

> I would like to recomment a book called "Pro Git" if you'd like

> to learn about git. This is the book that allowed me to finally

> break through git, and understand it.

>


OK, will see if Amazon carries it. Thanks.

Weimin
Weimin Pan March 30, 2018, 11:11 p.m. | #7
On 3/30/2018 3:52 PM, Joel Brobecker wrote:
> I just emailed Weimin personally when I saw the branch creation,

> but now I understand a little better:

>

>> I just did my first patch:

>>

>> $ git push upstream fixes

>> Enter passphrase for key '/home/wepan/.ssh/id_rsa':

>> Counting objects: 17, done.

>> Delta compression using up to 8 threads.

>> Compressing objects: 100% (10/10), done.

>> Writing objects: 100% (10/10), 2.43 KiB, done.

>> Total 10 (delta 8), reused 0 (delta 0)

>> To ssh://sourceware.org/git/binutils-gdb.git

>>     0f59d5f..c9cf730  fixes -> fixes

>>

>> and hope I did it correctly.

> Actually, no :).

>

> What you did is you pushed your branch called locally "fixes"

> to the repository corresponding to "upstream".  So, when you did

> the "push" command above, what happened is that it created

> the "fixes" branch on the upstream repository. This is not

> what you want, because (1) it creates a branch on the remote

> where you fix is (and pollutes the already existing branches),

> and (2) does not change the "master" branch, and so your fix

> is not really applied to the current development branch either.

>

> With your permission, I will start by fixing the mistake, which

> was to create the "fixes" branch on the upstream repository.

>

> On your end, I think the simplest solution is for you to

> push your current "fixes" branch to upstream's master:

>

>      $ git push upstream fixes:master



$ git push upstream fixes:master
Enter passphrase for key '/home/wepan/.ssh/id_rsa':
To ssh://sourceware.org/git/binutils-gdb.git
  ! [rejected]        fixes -> master (non-fast-forward)
error: failed to push some refs to 
'ssh://sourceware.org/git/binutils-gdb.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes before pushing again.  See the 'Note about
fast-forwards' section of 'git push --help' for details.

Does it mean that I  need to do a "git merge"?

Thanks,
Weimin
Joel Brobecker March 30, 2018, 11:41 p.m. | #8
> $ git push upstream fixes:master

> Enter passphrase for key '/home/wepan/.ssh/id_rsa':

> To ssh://sourceware.org/git/binutils-gdb.git

>  ! [rejected]        fixes -> master (non-fast-forward)

> error: failed to push some refs to

> 'ssh://sourceware.org/git/binutils-gdb.git'

> To prevent you from losing history, non-fast-forward updates were rejected

> Merge the remote changes before pushing again.  See the 'Note about

> fast-forwards' section of 'git push --help' for details.

> 

> Does it mean that I  need to do a "git merge"?


Not quite. It is telling you that your "fixes" branch is behind
upstream's "master".  You need to do a "rebase" your "fixes" branch
instead (while having the "fixes" being the current branch):

    $ git rebase upstream/master

You may have some conflicts to resolve, particularly around
ChangeLog files.

Once that's done, do a "git show" to make sure your commit looks
exactly the way you think it should look (in particular, that
the "diff" contains exactly the changes you mean to push).
And then, once done, try the push command again.

-- 
Joel
Joel Brobecker March 30, 2018, 11:45 p.m. | #9
> > With your permission, I will start by fixing the mistake, which

> > was to create the "fixes" branch on the upstream repository.

> 

> Yes, please go ahead. Thanks.


FTR, done. This is how I did it:

    $ git push origin :fixes
    remote: fatal: Invalid revision range c9cf7302418ad14b7f72949ba0421b9df3cd1127..0000000000000000000000000000000000000000
    To ssh://sourceware.org/git/binutils-gdb.git
     - [deleted]               fixes

The error is a limitation of one the the scripts that get run
when we create or delete a new branch or a new tag, but is not
fatal. The output confirms the branch we deleted.

-- 
Joel
Weimin Pan March 31, 2018, 12:03 a.m. | #10
On 3/30/2018 4:41 PM, Joel Brobecker wrote:
>> $ git push upstream fixes:master

>> Enter passphrase for key '/home/wepan/.ssh/id_rsa':

>> To ssh://sourceware.org/git/binutils-gdb.git

>>   ! [rejected]        fixes -> master (non-fast-forward)

>> error: failed to push some refs to

>> 'ssh://sourceware.org/git/binutils-gdb.git'

>> To prevent you from losing history, non-fast-forward updates were rejected

>> Merge the remote changes before pushing again.  See the 'Note about

>> fast-forwards' section of 'git push --help' for details.

>>

>> Does it mean that I  need to do a "git merge"?

> Not quite. It is telling you that your "fixes" branch is behind

> upstream's "master".  You need to do a "rebase" your "fixes" branch

> instead (while having the "fixes" being the current branch):

>

>      $ git rebase upstream/master

>

> You may have some conflicts to resolve, particularly around

> ChangeLog files.


After the "git rebase" command, it no longer points to my branch:

$ git show
commit d8611974cf819e5f8cb9eb36907251f3e2d721c6
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Fri Mar 30 17:18:56 2018 -0400
....

$ git branch
* (no branch)
   fixes
   master

> Once that's done, do a "git show" to make sure your commit looks

> exactly the way you think it should look (in particular, that

> the "diff" contains exactly the changes you mean to push).

> And then, once done, try the push command again.

>
Joel Brobecker March 31, 2018, 1:52 a.m. | #11
> > Not quite. It is telling you that your "fixes" branch is behind

> > upstream's "master".  You need to do a "rebase" your "fixes" branch

> > instead (while having the "fixes" being the current branch):

> > 

> >      $ git rebase upstream/master

> > 

> > You may have some conflicts to resolve, particularly around

> > ChangeLog files.

> 

> After the "git rebase" command, it no longer points to my branch:


That tells me the rebase did not complete successfully (the merge
conflict I was talking about). What output did you get when you
rebased? Did you resolve the merge conflict? And after you did so,
did you do a "git rebase --continue"?

Looking around, I found the following documentation which seems to
provide some information on how to handle merge conflicts after
a git rebase.

https://help.github.com/articles/resolving-merge-conflicts-after-a-git-rebase/
https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/
https://help.github.com/articles/about-merge-conflicts/

If you're not sure where you are, right now, try the following:

    $ git rebase --abort

That should abort the rebase operation, and get you back where you
started; and in particular, the current branch should be back to
the "fixes" branch.

-- 
Joel
Weimin Pan March 31, 2018, 3:11 a.m. | #12
On 3/30/2018 6:52 PM, Joel Brobecker wrote:
>>> Not quite. It is telling you that your "fixes" branch is behind

>>> upstream's "master".  You need to do a "rebase" your "fixes" branch

>>> instead (while having the "fixes" being the current branch):

>>>

>>>       $ git rebase upstream/master

>>>

>>> You may have some conflicts to resolve, particularly around

>>> ChangeLog files.

>> After the "git rebase" command, it no longer points to my branch:

> That tells me the rebase did not complete successfully (the merge

> conflict I was talking about). What output did you get when you

> rebased? Did you resolve the merge conflict? And after you did so,

> did you do a "git rebase --continue"?


Sorry, I lost the output. But I did resolve the merge conflicts by 
modifying the
ChangeLog files.  I just did a "git rebase  --continue" which I didn't 
do before
and it points to my branch and shows my patch:

%  git add ChangeLog
%  git add testsuite/ChangeLog
%  git rebase --continue
%  git commit --amend
%  git show

But when I tried to push again, I got the same "non-fast-forward" error 
as if no merges were ever done.

> Looking around, I found the following documentation which seems to

> provide some information on how to handle merge conflicts after

> a git rebase.

>

> https://help.github.com/articles/resolving-merge-conflicts-after-a-git-rebase/

> https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/

> https://help.github.com/articles/about-merge-conflicts/


Will read the above documentation for clue. Thanks.

> If you're not sure where you are, right now, try the following:

>

>      $ git rebase --abort

>

> That should abort the rebase operation, and get you back where you

> started; and in particular, the current branch should be back to

> the "fixes" branch.

>
Joel Brobecker April 2, 2018, 1:44 p.m. | #13
> Sorry, I lost the output. But I did resolve the merge conflicts by modifying

> the

> ChangeLog files.  I just did a "git rebase  --continue" which I didn't do

> before

> and it points to my branch and shows my patch:

> 

> %  git add ChangeLog

> %  git add testsuite/ChangeLog

> %  git rebase --continue

> %  git commit --amend

> %  git show

> 

> But when I tried to push again, I got the same "non-fast-forward" error as

> if no merges were ever done.


This is most likely because someone pushed a change between the moment
you fetched + rebased, and the moment you tried to push the rebased
result. You're going to have to redo it again, I'm afraid. Once you
learn to handle those merge failures quickly, this will almost never
happen.

-- 
Joel
Weimin Pan April 2, 2018, 5:26 p.m. | #14
On 4/2/2018 6:44 AM, Joel Brobecker wrote:
>> Sorry, I lost the output. But I did resolve the merge conflicts by modifying

>> the

>> ChangeLog files.  I just did a "git rebase  --continue" which I didn't do

>> before

>> and it points to my branch and shows my patch:

>>

>> %  git add ChangeLog

>> %  git add testsuite/ChangeLog

>> %  git rebase --continue

>> %  git commit --amend

>> %  git show

>>

>> But when I tried to push again, I got the same "non-fast-forward" error as

>> if no merges were ever done.

> This is most likely because someone pushed a change between the moment

> you fetched + rebased, and the moment you tried to push the rebased

> result. You're going to have to redo it again, I'm afraid. Once you

> learn to handle those merge failures quickly, this will almost never

> happen.

>


Hi Joel,

I tried it differently by merging the "fixes" branch to my "master" 
branch but got
the same "non-fast-forward"  error when pushing:

$ git checkout master
$ git merge fixes
$ git fetch upstream
$ git push upstream master:master
To ssh://sourceware.org/git/binutils-gdb.git
  ! [rejected]        master -> master (non-fast-forward)
error: failed to push some refs to 
'ssh://sourceware.org/git/binutils-gdb.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes before pushing again.  See the 'Note about
fast-forwards' section of 'git push --help' for details.

Maybe output from the "status" command sheds some light?

$ git status
# On branch master
# Your branch is ahead of 'origin/master' by 19 commits.
#
nothing to commit (working directory clean)

Thanks.
Joel Brobecker April 2, 2018, 5:53 p.m. | #15
> I tried it differently by merging the "fixes" branch to my "master" branch

> but got

> the same "non-fast-forward"  error when pushing:


We're going to take it off-list, because I doubt everything is
really all that interested in following that thread.

-- 
Joel
Weimin Pan April 2, 2018, 7:28 p.m. | #16
This patch has been pushed.

Thanks.

On 3/29/2018 10:08 PM, Simon Marchi wrote:
> On 2018-03-28 16:44, Weimin Pan wrote:

>> The original problem was fixed (see related PR 22242). But using a 

>> typedef

>> as the declared type for a static member variable, as commented in 

>> this PR,

>> is still causing gdb to get into infinite loop when printing the static

>> member's value. This problem can be reproduced as follows:

>>

>> % cat t.cc

>> class A {

>>     typedef A type;

>> public:

>>     bool operator==(const type& other) { return true; }

>>

>>     static const type INSTANCE;

>> };

>>

>> const A A::INSTANCE;

>>

>> int main() {

>>     A a;

>>     if (a == A::INSTANCE) {

>>         return -1;

>>     }

>>     return 0;

>> }

>> % g++ -g t.cc

>> % gdb -ex "start" -ex "p a" a.out

>>

>> The fix is rather trivial - in cp_print_static_field(), should call

>> check_typedef() to get the static member's real type and use it to

>> check whether it's a struct or an array.

>>

>> As Simon suggested, I've added a new test case to the testsuite

>> and am passing the original type, not the real type, as argument

>> to both cp_print_value_fields() and val_print().

>>

>> Re-tested on both aarch64-linux-gnu and amd64-linux-gnu. No regressions.

>

> Hi Weimin,

>

> Could you change the title to something more descriptive about what 

> the change/fix is, rather than the problem being fixed?  For example, 

> "Fix infinite recursion when printing static member with typedef".

>

> It is ok to push with that fixed (feel free to ask if you still need 

> some guidance).

>

> Thanks!

>

> Simon

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d0a8dfd..9534225 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2018-02-07  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR gdb/16959
+	* cp-valprint.c: (cp_print_static_field) Fix infinite recursion when
+	printing static type.
+
 2018-01-24  Pedro Alves  <palves@redhat.com>
 
 	GCC PR libstdc++/83906
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 486653f..e019a60 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -633,7 +633,8 @@  cp_print_static_field (struct type *type,
       return;
     }
 
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
+  struct type *real_type = check_typedef (type);
+  if (TYPE_CODE (real_type) == TYPE_CODE_STRUCT)
     {
       CORE_ADDR *first_dont_print;
       CORE_ADDR addr;
@@ -658,7 +659,6 @@  cp_print_static_field (struct type *type,
       addr = value_address (val);
       obstack_grow (&dont_print_statmem_obstack, (char *) &addr,
 		    sizeof (CORE_ADDR));
-      type = check_typedef (type);
       cp_print_value_fields (type, value_enclosing_type (val),
 			     value_embedded_offset (val), addr,
 			     stream, recurse, val,
@@ -666,7 +666,7 @@  cp_print_static_field (struct type *type,
       return;
     }
 
-  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+  if (TYPE_CODE (real_type) == TYPE_CODE_ARRAY)
     {
       struct type **first_dont_print;
       int i;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0f02f4a..6849d5a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-03-20  Weimin Pan  <weimin.pan@oracle.com>
+
+	* gdb.cp/static-typedef-print.exp: New file.
+	* gdb.cp/static-typedef-print.cc: New file.
+
 2018-01-22  Pedro Alves  <palves@redhat.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 
diff --git a/gdb/testsuite/gdb.cp/static-typedef-print.cc b/gdb/testsuite/gdb.cp/static-typedef-print.cc
new file mode 100644
index 0000000..5faff3e
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/static-typedef-print.cc
@@ -0,0 +1,34 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+class A {
+    typedef A type;
+public:
+    bool operator==(const type& other) { return true; }
+
+    static const type INSTANCE;
+};
+
+const A A::INSTANCE = {};
+
+int main() {
+    A a;
+    if (a == A::INSTANCE) {
+        return -1;
+    }
+    return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/static-typedef-print.exp b/gdb/testsuite/gdb.cp/static-typedef-print.exp
new file mode 100644
index 0000000..ff6756e
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/static-typedef-print.exp
@@ -0,0 +1,37 @@ 
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if [get_compiler_info "c++"] {
+    return -1
+}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_test "print a" \
+         "static INSTANCE = <same as static member of an already seen type>}}.*" \
+         "print static member"