[Bug remote/15256] New: remote.c uses pop_target

[Bug remote/15256] New: remote.c uses pop_target

[jan.kratochvil at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

             Bug #: 15256
           Summary: remote.c uses pop_target
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: remote
        AssignedTo: unassigned@sourceware.org
        ReportedBy: jan.kratochvil@redhat.com
    Classification: Unclassified


remote.c uses pop_target assuming topmost target is 'remote'.
It does not have to be, for example 'record' is higher.
Use explicit unpush_target in remote.c.

Found by Markus Metzger.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug remote/15256] remote.c uses pop_target

[jan.kratochvil at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

Jan Kratochvil <jan.kratochvil at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |jan.kratochvil at redhat
                   |                            |dot com
         AssignedTo|unassigned at sourceware    |jan.kratochvil at redhat
                   |dot org                     |dot com

--- Comment #1 from Jan Kratochvil <jan.kratochvil at redhat dot com> 2013-03-11 17:18:52 UTC ---
discussed in:

RE: Crash of GDB with gdbserver btrace enabled [Re: [patch v9 00/23] branch
tracing support for Atom]
http://sourceware.org/ml/gdb-patches/2013-03/msg00296.html
Message-ID:
<A78C989F6D9628469189715575E55B2307B9C2F6@IRSMSX102.ger.corp.intel.com>
From: "Metzger, Markus T" <markus.t.metzger@intel.com>

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug remote/15256] remote.c uses pop_target

[jan.kratochvil at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

--- Comment #2 from Jan Kratochvil <jan.kratochvil at redhat dot com> 2013-03-11 17:29:37 UTC ---
Patch:
[patch] Fix remote.c incorrectly using pop_target (wrt btrace)
http://sourceware.org/ml/gdb-patches/2013-03/msg00477.html
Message-ID: <20130311172836.GA22575@host2.jankratochvil.net>

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug remote/15256] remote.c uses pop_target

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at redhat dot com

--- Comment #3 from Tom Tromey <tromey at redhat dot com> ---
It seems like we could remove pop_target entirely.
There aren't many calls and it is hard to picture when
it would be correct -- and even in those cases where
it could be correct, it is just as easy to use unpush_target.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/15256] remote.c uses pop_target

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

--- Comment #4 from Tom Tromey <tromey at redhat dot com> ---
(In reply to Tom Tromey from comment #3)
> It seems like we could remove pop_target entirely.
> There aren't many calls and it is hard to picture when
> it would be correct -- and even in those cases where
> it could be correct, it is just as easy to use unpush_target.

I put this on my target hacks branch.
The original bug here seems to be fixed for trunk at least.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/15256] remote.c uses pop_target

[cvs-commit at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
CVSROOT:    /cvs/src
Module name:    src
Changes by:    tromey@sourceware.org    2013-07-25 14:34:51

Modified files:
    gdb            : ChangeLog bfd-target.c monitor.c 
                     remote-m32r-sdi.c remote-mips.c remote-sim.c 
                     target.c target.h tracepoint.c 

Log message:
    remove pop_target

    This patch fixes the target double-close problem (PR remote/15266),
    and in the process removes pop_target entire (PR remote/15256).

    The first issue is that pop_target calls target_close.  However, it
    then calls unpush_target, which also calls target_close.  This means
    targets must be able to be closed twice.  Not only is this strange,
    but it also directly contradicts the contract of to_xclose targets.
    (We currently have just a single such target, and it is never pushed;
    but I plan to add more, and so this latent bug is triggered.)

    The second issue is that it seems to me that calling pop_target is
    often unsafe.  This is what cropped up in 15256, where the remote
    target assumed that it could pop_target -- but there was another
    target higher on the stack, leading to confusion.

    But, it is always just as easy to call unpush_target as it is to call
    pop_target; and it is also safer.  So, removing pop_target seemed like
    an improvement.

    Finally, this adds an assertion to target_close to ensure that no
    currently-pushed target can be closed.

    Built and regtested on x86-64 Fedora 18; both natively and using the
    native-gdbserver board file.

    PR remote/15256, PR remote/15266:
    * bfd-target.c (target_bfd_reopen): Initialize to_magic.
    * monitor.c (monitor_detach): Use unpush_target.
    * remote-m32r-sdi.c (m32r_detach): Use unpush_target.
    * remote-mips.c (mips_detach): Use unpush_target.  Don't
    call mips_close.
    * remote-sim.c (gdbsim_detach): Use unpush_target.
    * target.c (pop_target): Remove.
    (pop_all_targets_above): Don't call target_close.
    (target_close): Assert that the target is unpushed.
    * target.h (pop_target): Don't declare.
    * tracepoint.c (tfile_open): Use unpush_target.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15829&r2=1.15830
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/bfd-target.c.diff?cvsroot=src&r1=1.23&r2=1.24
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/monitor.c.diff?cvsroot=src&r1=1.117&r2=1.118
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/remote-m32r-sdi.c.diff?cvsroot=src&r1=1.73&r2=1.74
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/remote-mips.c.diff?cvsroot=src&r1=1.142&r2=1.143
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/remote-sim.c.diff?cvsroot=src&r1=1.121&r2=1.122
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/target.c.diff?cvsroot=src&r1=1.342&r2=1.343
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/target.h.diff?cvsroot=src&r1=1.269&r2=1.270
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/tracepoint.c.diff?cvsroot=src&r1=1.320&r2=1.321

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug remote/15256] remote.c uses pop_target

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15256

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |7.7

--- Comment #6 from Tom Tromey <tromey at redhat dot com> ---
Fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.