Message ID | 1522269884-129860-1-git-send-email-weimin.pan@oracle.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 26194 invoked by alias); 28 Mar 2018 21:11:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25246 invoked by uid 89); 28 Mar 2018 21:11:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.2 spammy=Junior, check_typedef, commented, sk:value_e X-HELO: userp2130.oracle.com Received: from userp2130.oracle.com (HELO userp2130.oracle.com) (156.151.31.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Mar 2018 21:11:05 +0000 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w2SL4N16188781 for <gdb-patches@sourceware.org>; Wed, 28 Mar 2018 21:11:03 GMT Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp2130.oracle.com with ESMTP id 2h0jak837m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gdb-patches@sourceware.org>; Wed, 28 Mar 2018 21:11:02 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w2SLB22K005657 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gdb-patches@sourceware.org>; Wed, 28 Mar 2018 21:11:02 GMT Received: from abhmp0014.oracle.com (abhmp0014.oracle.com [141.146.116.20]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w2SLB15f032279 for <gdb-patches@sourceware.org>; Wed, 28 Mar 2018 21:11:01 GMT Received: from wmpan.us.oracle.com (/10.147.27.127) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 28 Mar 2018 14:11:01 -0700 From: Weimin Pan <weimin.pan@oracle.com> To: gdb-patches@sourceware.org Subject: [PATCH5 PR gdb/16959] gdb hangs in infinite recursion Date: Wed, 28 Mar 2018 14:44:44 -0600 Message-Id: <1522269884-129860-1-git-send-email-weimin.pan@oracle.com> X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8846 signatures=668695 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=4 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1803280216 |
Commit Message
Weimin Pan
March 28, 2018, 8:44 p.m. UTC
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
Comments
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
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
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
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
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.
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
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
> $ 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.
> > 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.
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. >
> > 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.
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. >
> 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.
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.
> 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.
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
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"