Bug 49951 - [4.5/4.6/4.7 Regression] Debug stepping behavior regarding g++ Class destructor has changed for the worse starting at gcc 4.5.0
Summary: [4.5/4.6/4.7 Regression] Debug stepping behavior regarding g++ Class destruct...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: debug (show other bugs)
Version: 4.5.0
: P2 normal
Target Milestone: 4.5.4
Assignee: Dodji Seketeli
URL:
Keywords:
: 55785 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-08-02 20:07 UTC by Peter Thompson
Modified: 2021-04-14 00:20 UTC (History)
9 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.5.3, 4.6.1, 4.7.0
Last reconfirmed: 2011-08-03 09:34:43


Attachments
Test source file (178 bytes, text/x-c++src)
2011-08-02 20:07 UTC, Peter Thompson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Thompson 2011-08-02 20:07:20 UTC
Created attachment 24896 [details]
Test source file

While this was reported to us as a problem with the TotalView debugger, the same behavior is seen with gdb as well.   In short the call to a class destructor no longer seems to take place inline or at the end (fini section) of a routine, but instead jumps back to the declaration of the class instance, before proceeding forward again.  So as one steps through the code, the debugger focus appears to jump around the routine rather than proceeding in a straight forward manner.  

Here is the test code from a user at the European Centre for Mid-Range Weather Forecasting (ECMWF).

---------------------------------
#include <stdio.h>

class MyClass
{
public:
      MyClass() {};  // constructor
     ~MyClass() {};  // destructor
};


int main (int argc, char *argv[])
{
     int i = 1;
     MyClass m;

     if (i == 1)
     {
         printf ("Hello world %d\n", i);
     }
}
----------------------------------------------

Compile 

g++ -g -O0 -o myclass myclass.cxx

Debug

[petert@magny e30025_gcc-517] gdb myclass
GNU gdb (GDB) Red Hat Enterprise Linux (7.1-29.el6)
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /nfs/netapp0/user/home/petert/repro/e30025_gcc/myclass...done.
(gdb) l
4       {
5       public:
6             MyClass() {};  // constructor
7            ~MyClass() {};  // destructor
8       };
9
10
11      int main (int argc, char *argv[])
12      {
13           int i = 1;
(gdb) b 13
Breakpoint 1 at 0x400614: file myclass.cxx, line 13.
(gdb) run
Starting program: /nfs/netapp0/user/home/petert/repro/e30025_gcc/myclass 

Breakpoint 1, main (argc=1, argv=0x7fffffffdb78) at myclass.cxx:13
13           int i = 1;
Missing separate debuginfos, use: debuginfo-install glibc-2.12-1.7.el6.x86_64
(gdb) n
14           MyClass m;
(gdb) n
16           if (i == 1)
(gdb) n
18               printf ("Hello world %d\n", i);
(gdb) n
Hello world 1
14           MyClass m;
(gdb) n
20      }
------------------------------------------------

The question here is why does a 'next' take us from line 18 to line 14, and then back to 20?   Actually the 'why' is easy enough to see in the debug info line number statements:

Line Number Statements:
  Extended opcode 2: set Address to 0x400674
  Special opcode 10: advance Address by 0 to 0x400674 and Line by 5 to 6
  Special opcode 117: advance Address by 8 to 0x40067c and Line by 0 to 6
  Advance PC by 2 to 0x40067e
  Extended opcode 1: End of Sequence

  Extended opcode 2: set Address to 0x40067e
  Special opcode 11: advance Address by 0 to 0x40067e and Line by 6 to 7
  Special opcode 117: advance Address by 8 to 0x400686 and Line by 0 to 7
  Advance PC by 2 to 0x400688
  Extended opcode 1: End of Sequence

  Extended opcode 2: set Address to 0x400604
  Advance Line by 11 to 12
  Copy
  Special opcode 230: advance Address by 16 to 0x400614 and Line by 1 to 13
  Special opcode 104: advance Address by 7 to 0x40061b and Line by 1 to 14
  Special opcode 175: advance Address by 12 to 0x400627 and Line by 2 to 16
  Special opcode 91: advance Address by 6 to 0x40062d and Line by 2 to 18
  Advance PC by constant 17 to 0x40063e
  Special opcode 43: advance Address by 3 to 0x400641 and Line by -4 to 14
  Advance PC by constant 17 to 0x400652
  Special opcode 11: advance Address by 0 to 0x400652 and Line by 6 to 20
  Advance Line by -6 to 14
  Special opcode 145: advance Address by 10 to 0x40065c and Line by 0 to 14
  Advance PC by 23 to 0x400673
  Extended opcode 1: End of Sequence
------------------------------------------------------------

This behavior was introduced in 4.5.0.  I have not tested all versions, but 4.4.1 stepped from line 18 to line 19 (the closing bracket) and never backed up to line 14.  This is the expected behavior.  I've tested up to gcc 4.6.1 and the new, annoying, behavior is still in place.

The specific version I used was:

 g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/nfs/netapp0/user/home/compilers/gnu/gcc/4.5.1/x86_64-linux/bin/../libexec/gcc/x86_64-unknown-linux-gnu/4.5.1/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../../src/gcc-4.5.1/configure --prefix=/home/compilers/gnu/gcc/4.5.1/x86_64-linux --enable-languages=c,c++,fortran
Thread model: posix
gcc version 4.5.1 (GCC)
Comment 1 Peter Thompson 2011-08-02 20:15:10 UTC
Note that when the Class destructor is commented out

//      ~MyClass() {};  // destructor

then the debugger steps through the code without jumping around.  Go figure!
Comment 2 Richard Biener 2011-08-03 09:34:43 UTC
Confirmed.

The destruction is implemented via a try {} finally where the finally
block seems to get the source location of the construction of the
object rather than the source location of it going out of scope:

int main(int, char**) (int argc, char * * argv)
{
  int D.2623;

  [t.C : 19:5] {
    int i;
    struct MyClass m;

    [t.C : 13:11] i = 1;
    [t.C : 14:11] MyClass::MyClass ([t.C : 14] &m);
    [t.C : 19:5] try
      {
        [t.C : 16:3] if (i == 1) goto <D.2620>; else goto <D.2621>;
        <D.2620>:
        [t.C : 18:37] printf ("Hello world %d\n", i);
        goto <D.2622>;
        <D.2621>:
        <D.2622>:
      }
    finally
      {
        [t.C : 14:11] MyClass::~MyClass ([t.C : 14] &m);
      }
  }
  [t.C : 0:0] D.2623 = 0;
  [t.C : 0:0] return D.2623;
}


With 4.4 we still had

int main(int, char**) (int argc, char * * argv)
{
  int D.2628;

  [t.C : 19] {
    int i;
    struct MyClass m;

    [t.C : 13] i = 1;
    [t.C : 14] __comp_ctor  ([t.C : 14] &m);
    [t.C : 19] try
      {
        [t.C : 16] if (i == 1) goto <D.2625>; else goto <D.2626>;
        <D.2625>:
        [t.C : 18] printf (&"Hello world %d\n"[0], i);
        goto <D.2627>;
        <D.2626>:
        <D.2627>:
      }
    finally
      {
        [t.C : 19] __comp_dtor  ([t.C : 19] &m);
      }
  }
  [t.C : 0] D.2628 = 0;
  [t.C : 0] return D.2628;
}

So the finally block stmts inherit the try/finally location.

Might be a gimplification or frontend issue.
Comment 3 Jakub Jelinek 2011-08-04 08:42:45 UTC
Started with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149722
Comment 4 Richard Biener 2011-10-10 12:04:49 UTC
Aldy?
Comment 5 asmwarrior 2011-10-29 14:06:53 UTC
Hi, all, I think this bug happened about more than one years, I have test in both gcc 4.5 and gcc 4.6.x under Windows.

I have both post an issue in GDB maillist and GCC-help maillist. see:

http://gcc.gnu.org/ml/gcc-help/2011-10/msg00239.html

and

http://sourceware.org/ml/gdb/2011-10/msg00209.html

The GDB guys thoughts that this is a bug in GCC, and GCC emit wrong debug_line information.

When I use GDB under Codeblocks, it will always do some extra jump to variable declaration, and quite unfriendly.

asmwarrior
ollydbg from codeblocks' forum
Comment 6 Jonathan Wakely 2011-11-29 19:02:30 UTC
Manu, could you look at this?
This is quite an annoying regression for C++
Comment 7 Manuel López-Ibáñez 2011-11-29 22:49:58 UTC
(In reply to comment #6)
> Manu, could you look at this?
> This is quite an annoying regression for C++

After a couple of hours I gave up. I tried greping for "build.*finally", finally, build_destructor, destructor. I tried breaking in gimplify_expr but I think it is already too late. I tried breaking in build_cleanup and build_delete but those functions get called too often, and I didn't see where the finally block is generated.

If someone knows exactly where the finally block is generated in the C++ FE, please let me know, and I may give it another try in the future.
Comment 8 Dodji Seketeli 2011-12-03 18:23:38 UTC
A candidate analysis and patch has been posted to
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00250.html for comments.
Comment 9 Manuel López-Ibáñez 2011-12-03 19:03:54 UTC
(In reply to comment #8)
> A candidate analysis and patch has been posted to
> http://gcc.gnu.org/ml/gcc-patches/2011-12/msg00250.html for comments.

What I don't understand is how, according to your analysis, this worked before revision 149722 and how that patch could possibly change the behavior.
Comment 10 dodji@seketeli.org 2011-12-03 20:55:15 UTC
> What I don't understand is how, according to your analysis, this
> worked before revision 149722 and how that patch could possibly
> change the behavior.

OK, you caught me.  I admit I haven't really looked at that patch
before starting debugging this issue.  So now I might have an
additional theory.  :-)

I think that before the commit r149722, the call expression to the
destructor simply had no location.  And that patch added a location to
it.  So we didn't have the jumpy behaviour because we were ignoring
the presence of the call to the destructor, from a stepping point of
view.

Here is why I am thinking that.

The call to the destructor of "m" ends up being generated by
build_cxx_call, in cp/call.c.  At the time of the patch, that function
started with:

    tree
    build_cxx_call (tree fn, int nargs, tree *argarray)
    {
      tree fndecl;
      fn = build_call_a (fn, nargs, argarray);
      ...
    }

So it was calling build_call_a, that the patch modified as:

    --- a/gcc/cp/call.c
    +++ b/gcc/cp/call.c
    @@ -362,7 +362,8 @@ build_call_a (tree function, int n, tree *argarray)
				    argarray[i], t);
	    }

    -  function = build_call_array (result_type, function, n, argarray);
    +  function = build_call_array_loc (input_location,
    +				   result_type, function, n, argarray);
       TREE_HAS_CONSTRUCTOR (function) = is_constructor;
       TREE_NOTHROW (function) = nothrow;

See how the call to build_call_array got changed into a call
tobuild_call_array_loc.

The change that turned build_call_array into build_call_array_loc in
tree.c was:

     tree
    -build_call_array (tree return_type, tree fn, int nargs, const tree *args)
    +build_call_array_loc (location_t loc, tree return_type, tree fn,
    +		      int nargs, const tree *args)
     {
       tree t;
       int i;
    @@ -8549,6 +8550,7 @@ build_call_array (tree return_type, tree fn, int nargs, const tree *args)
       for (i = 0; i < nargs; i++)
	 CALL_EXPR_ARG (t, i) = args[i];
       process_call_operands (t);
    +  SET_EXPR_LOCATION (t, loc);
       return t;
     }

See how the patch adds a location to the resulting call expression
there.  Without that, the previous build_call_array just had no
location, I think.

The change you had to do on gcc/testsuite/g++.dg/gcov/gcov-2.C doesn't
seem to argue against this theory:

    @@ -20,7 +20,7 @@ private:

     void foo()
     {
    -  C c;					/* count(1) */
    +  C c;					/* count(2) */
       c.seti (1);				/* count(1) */
     }

Here we see that before the patch, there was only one call issued on
the line of C c; - and that was probably the call to the constructor.
After, there were two calls on that line, and I think that was the
call to the destructor that added up.  Note that we didn't have any
call on the line of the closing '}', for instance.

Now I am thinking that maybe it would be best to have the call to the
destructor be on the last statement of the block, rather than on the
'}', if we'd have a location for that call at all.
Comment 11 Dodji Seketeli 2011-12-20 13:36:08 UTC
Author: dodji
Date: Tue Dec 20 13:36:04 2011
New Revision: 182532

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182532
Log:
PR debug/49951 - jumpy stepping at end of scope in C++

gcc/cp/

	PR debug/49951
	* decl.c (cxx_maybe_build_cleanup): Don't set location of the call
	to the destructor.

gcc/testsuite/

	PR debug/49951
	* g++.dg/gcov/gcov-2.C: Adjust.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/gcov/gcov-2.C
Comment 12 Dodji Seketeli 2011-12-20 13:40:30 UTC
This should be fixed in trunk (4.7).
Comment 13 Jonathan Wakely 2011-12-20 13:49:28 UTC
Thanks! It would be very helpful to get this into 4.6.3 too if it's safe
Comment 14 asmwarrior 2011-12-21 03:21:07 UTC
Thanks for fix this bug, I hope some devs will backport this patch to 4.5/4.6 branches.
Comment 15 dodji@seketeli.org 2012-01-02 12:01:45 UTC
> It would be very helpful to get this into 4.6.3 too if it's safe

Sure thing.  I am currently testing the patch on 4.6.  Thanks for the
head-up.
Comment 16 Dodji Seketeli 2012-01-02 17:08:50 UTC
Author: dodji
Date: Mon Jan  2 17:08:45 2012
New Revision: 182807

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182807
Log:
PR debug/49951 - jumpy stepping at end of scope in C++

gcc/cp/

	PR debug/49951
	* decl.c (cxx_maybe_build_cleanup): Don't set location of the call
	to the destructor.

gcc/testsuite/

	PR debug/49951
	* g++.dg/gcov/gcov-2.C: Adjust.

Modified:
    branches/gcc-4_6-branch/gcc/cp/ChangeLog
    branches/gcc-4_6-branch/gcc/cp/decl.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_6-branch/gcc/testsuite/g++.dg/gcov/gcov-2.C
Comment 17 Dodji Seketeli 2012-01-02 17:08:52 UTC
Author: dodji
Date: Mon Jan  2 17:08:48 2012
New Revision: 182808

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182808
Log:
PR debug/49951 - jumpy stepping at end of scope in C++

gcc/cp/

	PR debug/49951
	* decl.c (cxx_maybe_build_cleanup): Don't set location of the call
	to the destructor.

gcc/testsuite/

	PR debug/49951
	* g++.dg/gcov/gcov-2.C: Adjust.

Modified:
    branches/gcc-4_5-branch/gcc/cp/ChangeLog
    branches/gcc-4_5-branch/gcc/cp/decl.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/gcov/gcov-2.C
Comment 18 Andrew Pinski 2012-12-22 04:00:07 UTC
*** Bug 55785 has been marked as a duplicate of this bug. ***
Comment 19 asmwarrior 2013-10-28 07:56:22 UTC
Hi, I see this bug happens at least in GCC 4.8.2 again. I'm using MinGW-Build GCC 4.8.2 under Windows, and debug Codeblocks. I have a source code which have something like:

void CompilerGCC::SetupEnvironment()
{
....
    wxString currentPath;
....
}
But When I debug through lines using the step command, I see that the caret still go back the the local variable definition of the line "wxString currentPath".

I tried the simple test code that Peter Thompson gives, but it works fine, So it looks like this bug happens in a larger project not the simple one.

Currently I don't have much way to show you. When I see the disassembler code, I see some call to destructor of wxString.

0x64B0CF81	call   0x64b4f094 <InfoWindow::Display(wxString const&, wxString const&, unsigned int, unsigned int)>
0x64B0CF86	lea    eax,[ebp-0x34]
0x64B0CF89	mov    ecx,eax
0x64B0CF8B	call   0x64b5c090 <wxString::~wxString()>
0x64B0CF90	lea    eax,[ebp-0x38]
0x64B0CF93	mov    ecx,eax
0x64B0CF95	call   0x64b5c090 <wxString::~wxString()>
0x64B0CF9A	lea    eax,[ebp-0x30]
0x64B0CF9D	mov    DWORD PTR [esp],0x64b6bc70
0x64B0CFA4	mov    ecx,eax

When I run 
> info line *0x64B0CFDB

[debug]Line 801 of "F:\cb_sf_git\trunk\src\plugins\compilergcc\compilergcc.cpp" starts at address 0x64b0cf9a <CompilerGCC::SetupEnvironment()+3258> and ends at 0x64b0cfe0 <CompilerGCC::SetupEnvironment()+3328>.
[debug]F:\cb_sf_git\trunk\src\plugins\compilergcc\compilergcc.cpp:801:32074:beg:0x64b0cf9a

But it looks like this is not enough information I can show you, any suggest how to see the incorrect line-code map? Thanks.
Comment 20 asmwarrior 2013-10-28 13:15:32 UTC
Hi, all. After the conversation on GDB IRC with GDB developer Jan Kratochvil, a simple test code supplied by Jan in his bug report on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58123#c0 can easily demonstrate this issue in either GCC 4.8.2 or GCC 4.8.1.
class C {
public:
  C() {}
  ~C() {}
  int m() { return 0; }
};
/*  7 */ int main() {
/*  8 */   C a;
/*  9 */   C b;
/* 10 */   C c;
/* 11 */   return a.m() + b.m() + c.m();
/* 12 */ }

You will run "next" command back to line 10, and line 9 until go to the end of the function main().

Note: the bug #58123 is another issue, Jan expected that the caret should go back to line 8, but it is not.
Comment 21 asmwarrior 2015-02-03 01:04:40 UTC
I just test the sample code in comment 20 with GCC 4.9.2, and I see this "jumpy" behaviour still exists, do we need to reopen this bug? Thanks.
Comment 22 CVS Commits 2021-04-14 00:20:10 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:006783f4b165dff25aae3697920fcf54754dddd4

commit r11-8164-g006783f4b165dff25aae3697920fcf54754dddd4
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Apr 13 16:18:54 2021 -0400

    c++: debug location of variable cleanups [PR88742]
    
    PR49951 complained about the debugger jumping back to the declaration of a
    local variable when we run its destructor.  That was fixed in 4.7, but broke
    again in 4.8.  PR58123 fixed an inconsistency in the behavior, but not the
    jumping around.  This patch addresses the issue by setting EXPR_LOCATION on
    a cleanup destructor call to the location of the closing brace of the
    compound-statement, or whatever token ends the scope of the variable.
    
    The change to cp_parser_compound_statement is so input_location is the }
    rather than the ; of the last substatement.
    
    gcc/cp/ChangeLog:
    
            PR c++/88742
            PR c++/49951
            PR c++/58123
            * semantics.c (set_cleanup_locs): New.
            (do_poplevel): Call it.
            * parser.c (cp_parser_compound_statement): Consume the }
            before finish_compound_stmt.
    
    gcc/testsuite/ChangeLog:
    
            PR c++/88742
            * g++.dg/debug/cleanup1.C: New test.
            * c-c++-common/Wimplicit-fallthrough-6.c: Adjust diagnostic line.
            * c-c++-common/Wimplicit-fallthrough-7.c: Likewise.
            * g++.dg/cpp2a/constexpr-dtor3.C: Likewise.
            * g++.dg/ext/constexpr-attr-cleanup1.C: Likewise.
            * g++.dg/tm/inherit2.C: Likewise.
            * g++.dg/tm/unsafe1.C: Likewise.
            * g++.dg/warn/Wimplicit-fallthrough-1.C: Likewise.
            * g++.dg/gcov/gcov-2.C: Adjust coverage counts.