[Bug gdb/30695] New: [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[Bug gdb/30695] New: [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

            Bug ID: 30695
           Summary: [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp:
                    no force memory write: SIGINT does not interrupt
                    background execution
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

With an -fsanitize=thread build, I ran into:
...
FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does
not interrupt background execution
FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: force memory write: SIGINT does not
interrupt background execution
...

The first in more detail:
...
(gdb) PASS: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: break
foo if <condition>
c&^M
Continuing.^M
(gdb) ^M
Breakpoint 2, foo () at
/labs/tdevries/gdb/src/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c:23^M
23        return 0;^M
FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does
not interrupt background execution
...

Instead, we expect to see:
...
(gdb) PASS: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: break
foo if <condition>
c&^M
Continuing.^M
(gdb) Quit^M
(gdb) quit_count=1
Quit^M
(gdb) quit_count=2
Quit^M
(gdb) quit_count=3
Quit^M
(gdb) quit_count=4
Quit^M
(gdb) quit_count=5
Quit^M
(gdb) quit_count=6
Quit^M
(gdb) quit_count=7
Quit^M
(gdb) quit_count=8
Quit^M
(gdb) quit_count=9
Quit^M
(gdb) quit_count=10
^M
Breakpoint 2, foo () at
/data/vries/gdb/src/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c:23^M
23        return 0;^M
PASS: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does
not interrupt background execution
...

Triggering this may be a timing issue, this is on a fast server with lots of
memory.

On my laptop, the test passes with an -fsanitize=thread build.

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #0)
> On my laptop, the test passes with an -fsanitize=thread build.

Scrap that, now also reproduces on my laptop.

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
From https://www.gnu.org/software/libc/manual/html_node/Merged-Signals.html:
...
If multiple signals of the same type are delivered to your process before your
signal handler has a chance to be invoked at all, the handler may only be
invoked once, as if only a single signal had arrived. In effect, the signals
merge into one. This situation can arise when the signal is blocked, or in a
multiprocessing environment where the system is busy running some other
processes while the signals are delivered. This means, for example, that you
cannot reliably use a signal handler to count signals. The only distinction you
can reliably make is whether at least one signal has arrived since a given time
in the past. 
...

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |palves at sourceware dot org

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Pedro, cc-ing you because you wrote the test-case.

To me it looks like the test-case needs fixing because it doesn't take signal
merging into account.  WDYT?

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
After debugging this in more detail, it looks like what happens is that:
- the breakpoint hits
- while evaluating the condition of the breakpoint, 
  $_shell("kill -INT <pid-of-gdb>") is called, handled by run_under_shell
- in run_under_shell, a vfork is issued
- in the vfork child, execl executes the kill command
- in the vfork parent, waitpid is called to wait for the result of the kill
  command
- waitpid returns -1 with errno set to EINTR
- run_under_shell doesn't check the result of waitpid, and returns the
  value of local variable status.  Since waitpid returned -1, status was
  not assigned a value, so it's uninitialized, and happens to be
  non-zero
- the breakpoint condition evaluates to true, because
  $_shell("kill -INT <pid-of-gdb>") != 0
- the breakpoint triggers a stop, which the test-case doesn't expect.

This fixes it:
...
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index ea2e156a00c..6a15e0bbebb 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -921,7 +921,14 @@ run_under_shell (const char *arg, int from_tty)
     }

   if (pid != -1)
-    waitpid (pid, &status, 0);
+    {
+      int ret;
+      do
+       {
+         ret = waitpid (pid, &status, 0);
+       }
+      while (ret == -1 && errno == EINTR);
+    }
   else
     error (_("Fork failed"));
   return status;
...

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #4)
> - run_under_shell doesn't check the result of waitpid, and returns the
>   value of local variable status.  Since waitpid returned -1, status was
>   not assigned a value, so it's uninitialized, and happens to be
>   non-zero

Reproduced without thread sanitizer, using:
...
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index ea2e156a00c..5e517bc8085 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -899,7 +899,7 @@ run_under_shell (const char *arg, int from_tty)
 #endif
   return rc;
 #else /* Can fork.  */
-  int status, pid;
+  int status = 1, pid;

   if ((pid = vfork ()) == 0)
     {
...

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/pipermail/gdb-patches/2024-October/212353.html

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

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

commit 9858f29e6a9ebc02a70e6e31d54404f13fbf4b10
Author: Tom de Vries <tdevries@suse.de>
Date:   Tue Oct 22 08:53:51 2024 +0200

    [gdb] Handle EINTR in run_under_shell

    When building gdb with -O2 -fsanitize=thread and running test-case
    gdb.base/bg-exec-sigint-bp-cond.exp, I run into:
    ...
    (gdb) c&^M
    Continuing.^M
    (gdb) Quit^M
    (gdb) quit_count=1
    ^M
    Breakpoint 2, foo () at bg-exec-sigint-bp-cond.c:23^M
    23        return 0;^M
    FAIL: $exp: no force memory write: \
      SIGINT does not interrupt background execution
    ...

    What happens is that:
    - the breakpoint hits
    - while evaluating the condition of the breakpoint,
      $_shell("kill -INT <pid-of-gdb>") is called, handled by run_under_shell
    - in run_under_shell, a vfork is issued
    - in the vfork child, execl executes the kill command
    - in the vfork parent, waitpid is called to wait for the result of the kill
      command
    - waitpid returns -1 with errno set to EINTR
    - run_under_shell doesn't check the result of waitpid, and returns the
      value of local variable status.  Since waitpid returned -1, status was
      not assigned a value, so it's uninitialized, and happens to be
      non-zero
    - the breakpoint condition evaluates to true, because
      $_shell("kill -INT <pid-of-gdb>") != 0
    - the breakpoint triggers a stop, which the test-case doesn't expect.

    Fix this by using gdb::handle_eintr to call waitpid in run_under_shell.

    Also handle the case that waitpid returns an error other than EINTR, using
    perror_with_name.

    Tested on x86_64-linux.

    Approved-By: Tom Tromey <tom@tromey.com>

    PR gdb/30695
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30695

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

[Bug gdb/30695] [gdb, tsan] FAIL: gdb.base/bg-exec-sigint-bp-cond.exp: no force memory write: SIGINT does not interrupt background execution

[vries at gcc dot gnu.org]

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

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |16.1
             Status|NEW                         |RESOLVED

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed.

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