[Bug gdb/17130] New: Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[Bug gdb/17130] New: Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

            Bug ID: 17130
           Summary: Quit regression on targets that don't have job_control
                    / terminal switching (expect signal SIGINT when the
                    program is resumed)
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: palves at redhat dot com

Since the target_ops delegation reword, the "current_target.to_terminal_ours ==
NULL" check in "quit" is always false:

void
quit (void)
{
...
  if (job_control
      /* If there is no terminal switching for this target, then we can't
         possibly get screwed by the lack of job control.  */
      || current_target.to_terminal_ours == NULL)
    fatal ("Quit");
  else
    fatal ("Quit (expect signal SIGINT when the program is resumed)");
}

Indeed, I've just tried gdb on Windows, and I got:

(gdb)
Quit (expect signal SIGINT when the program is resumed)
(gdb)

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

[Bug gdb/17130] Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

Tom Tromey <tromey at redhat dot com> changed:

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

--- Comment #1 from Tom Tromey <tromey at redhat dot com> ---
There's a case in sol-thread.c:

  if (beneath->to_find_new_threads != NULL)
    beneath->to_find_new_threads (beneath);

No bug here, just the test can be deleted.

There are also a few to investigate in record:

./record-btrace.c:    if (ops->to_xfer_partial != NULL)
./record-btrace.c:    if (t->to_fetch_registers != NULL)
./record-btrace.c:    if (t->to_store_registers != NULL)
./record-btrace.c:    if (t->to_prepare_to_store != NULL)
./record-btrace.c:    if (ops->to_resume != NULL)
./record-btrace.c:    if (ops->to_wait != NULL)
./record-btrace.c:    if (ops->to_find_new_threads != NULL)
./record-btrace.c:    if (ops->to_thread_alive != NULL)

Some of those are probably not delegated (I didn't check yet),
but some are.

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

[Bug gdb/17130] Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

--- Comment #2 from Pedro Alves <palves at redhat dot com> ---
I think all of these can just call the beneath target unconditionally.

Similarly, 'grep beneath * | grep if' finds these that can be cleaned up:

corelow.c:      if (ops->beneath != NULL)
procfs.c:      if (ops->beneath != NULL)
spu-multiarch.c:  if (ops_beneath)
spu-multiarch.c:  if (!ops_beneath || SPUADDR_SPU (start_addr) >= 0)
windows-nat.c:      if (ops->beneath != NULL)

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

[Bug gdb/17130] Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |tromey at redhat dot com

--- Comment #3 from Tom Tromey <tromey at redhat dot com> ---
Mine.

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

[Bug gdb/17130] Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  44e89118cb33b71a1eaf6514ea2b686aac1974dd (commit)
       via  b0ed115fa5895ccb20d73e26d89a3b8430fe0f0a (commit)
       via  e75fdfcad1c868eae5396a95be9dd18010406306 (commit)
      from  83814951ff9b8f73dcf7c860ad7b4f34a31f2be6 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=44e89118cb33b71a1eaf6514ea2b686aac1974dd

commit 44e89118cb33b71a1eaf6514ea2b686aac1974dd
Author: Tom Tromey <tromey@redhat.com>
Date:   Fri Jul 11 09:39:12 2014 -0600

    fix calls to find_target_beneath

    A long time ago Pedro pointed out that there are some calls to
    find_target_beneath that pass in an explicit target_ops; but which
    should instead use the ops provided to the method in question.  See:

        https://sourceware.org/ml/gdb-patches/2014-01/msg00429.html

    This patch is just a minor cleanup to fix all such calls.  There were
    only three.

    2014-07-18  Tom Tromey  <tromey@redhat.com>

        * spu-multiarch.c (spu_region_ok_for_hw_watchpoint): Pass "self"
        to find_target_beneath.
        * ravenscar-thread.c (ravenscar_prepare_to_store): Pass "ops" to
        find_target_beneath.
        (ravenscar_mourn_inferior): Pass "self" to find_target_beneath.

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b0ed115fa5895ccb20d73e26d89a3b8430fe0f0a

commit b0ed115fa5895ccb20d73e26d89a3b8430fe0f0a
Author: Tom Tromey <tromey@redhat.com>
Date:   Fri Jul 11 08:30:34 2014 -0600

    fix PR gdb/17130

    This fixes PR gdb/17130.

    The bug is that some code in utils.c was not updated during the target
    delegation change:

      if (job_control
          /* If there is no terminal switching for this target, then we can't
             possibly get screwed by the lack of job control.  */
          || current_target.to_terminal_ours == NULL)
        fatal ("Quit");
      else
        fatal ("Quit (expect signal SIGINT when the program is resumed)");

    After the delegation change, to_terminal_ours will never be NULL.

    I think this bug can be seen before the target delegation change by
    enabling target debugging -- this would also cause to_terminal_ours to
    be non-NULL.

    The fix is to introduce a new target_supports_terminal_ours function,
    that properly checks the target stack.  This is not perhaps ideal, but
    I think is a reasonable-enough approach, and in keeping with some
    other existing code of the same form.

    This patch also fixes a similar bug in target_supports_delete_record.

    2014-07-18  Tom Tromey  <tromey@redhat.com>

        PR gdb/17130:
        * utils.c (quit): Use target_supports_terminal_ours.
        * target.h (target_supports_terminal_ours): Declare.
        * target.c (target_supports_delete_record): Don't check
        to_delete_record against NULL.
        (target_supports_terminal_ours): New function.

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e75fdfcad1c868eae5396a95be9dd18010406306

commit e75fdfcad1c868eae5396a95be9dd18010406306
Author: Tom Tromey <tromey@redhat.com>
Date:   Fri Jul 11 08:02:03 2014 -0600

    clean up some target delegation cases

    This patch cleans up some minor inconsistencies in target delegation.
    It's primary purpose is to avoid confusion in the code.  A few spots
    were checking the "beneath" target; however this can only be NULL for
    the dummy target, so such tests are not needed.  Some other spots were
    iterating over the beneath targets, looking for a method
    implementation.  This is not needed for methods handled by
    make-target-delegates, as there is always an implementation.

    2014-07-18  Tom Tromey  <tromey@redhat.com>

        PR gdb/17130:
        * spu-multiarch.c (spu_region_ok_for_hw_watchpoint)
        (spu_fetch_registers, spu_store_registers, spu_xfer_partial)
        (spu_search_memory, spu_mourn_inferior): Simplify delegation.
        * linux-thread-db.c (thread_db_pid_to_str): Always delegate.
        * windows-nat.c (windows_xfer_partial): Always delegate.
        * record-btrace.c (record_btrace_xfer_partial): Simplify
        delegation.
        (record_btrace_fetch_registers, record_btrace_store_registers)
        (record_btrace_prepare_to_store, record_btrace_resume)
        (record_btrace_wait, record_btrace_find_new_threads)
        (record_btrace_thread_alive): Likewise.
        * procfs.c (procfs_xfer_partial): Always delegate.
        * corelow.c (core_xfer_partial): Always delegate.
        * sol-thread.c (sol_find_new_threads): Simplify delegation.

-----------------------------------------------------------------------

Summary of changes:
 gdb/ChangeLog          |   35 +++++++++++++++++++++++++
 gdb/corelow.c          |   10 +++----
 gdb/linux-thread-db.c  |    5 +---
 gdb/procfs.c           |    8 ++---
 gdb/ravenscar-thread.c |    4 +-
 gdb/record-btrace.c    |   66 ++++++++++++-----------------------------------
 gdb/sol-thread.c       |    3 +-
 gdb/spu-multiarch.c    |   31 +++-------------------
 gdb/target.c           |   20 ++++++++++++++-
 gdb/target.h           |    5 +++
 gdb/utils.c            |    2 +-
 gdb/windows-nat.c      |    8 ++---
 12 files changed, 95 insertions(+), 102 deletions(-)

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

[Bug gdb/17130] Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[tromey at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Tom Tromey <tromey at redhat dot com> ---
Fixed on trunk.

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

[Bug gdb/17130] Quit regression on targets that don't have job_control / terminal switching (expect signal SIGINT when the program is resumed)

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17130

--- Comment #6 from Pedro Alves <palves at redhat dot com> ---
I think 7.8 needs the fix as well?

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