diff mbox series

[v2,2/3,gdb/dap] Fix race between dap startup and dap log file

Message ID 20240221132023.15147-2-tdevries@suse.de
State Committed
Headers
Series [v2,1/3,gdb/dap] Factor out thread_log |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom de Vries Feb. 21, 2024, 1:20 p.m. UTC 
  In dap_gdb_start we do:
...
        append GDBFLAGS " -iex \"set debug dap-log-file $logfile\" -q -i=dap"
...

While the dap log file setting comes before the dap interpreter setting,
the order is the other way around:
- first, the dap interpreter is started
- second, the -iex commands are executed and the log file is initialized.

Consequently, there's a race between dap interpreter startup and dap log file
initialization.

This cannot be fixed by using -eiex instead.  Before the interpreter is
started, the "set debug dap-log-file" command is not yet registered.

Fix this by postponing the start of the DAP server until GDB has processed all
command files.

Tested on aarch64-linux.

PR dap/31386
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31386
---
 gdb/python/lib/gdb/dap/__init__.py | 20 +++++++++++++++++++-
 gdb/python/py-dap.c                | 23 ++++++++++++++++++++---
 gdb/testsuite/gdb.dap/eof.exp      |  3 +++
 gdb/testsuite/lib/dap-support.exp  | 10 ++++++++++
 4 files changed, 52 insertions(+), 4 deletions(-)

Comments

Tom Tromey Feb. 21, 2024, 9:09 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Fix this by postponing the start of the DAP server until GDB has processed all
Tom> command files.

Looks good, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
diff mbox series

Patch

diff --git a/gdb/python/lib/gdb/dap/__init__.py b/gdb/python/lib/gdb/dap/__init__.py
index f60b3fda1db..86da9b8ef3c 100644
--- a/gdb/python/lib/gdb/dap/__init__.py
+++ b/gdb/python/lib/gdb/dap/__init__.py
@@ -69,5 +69,23 @@  def run():
     os.close(wfd)
 
     # Note the inferior output is opened in text mode.
+    global server
     server = Server(open(saved_in, "rb"), open(saved_out, "wb"), open(rfd, "r"))
-    startup.start_dap(server.main_loop)
+
+
+# Whether the interactive session has started.
+session_started = False
+
+
+def pre_command_loop():
+    """DAP's pre_command_loop interpreter hook.  This is called by the GDB DAP
+    interpreter."""
+    global session_started
+    if not session_started:
+        # The pre_command_loop interpreter hook can be called several times.
+        # The first time it's called, it means we're starting an interactive
+        # session.
+        session_started = True
+        startup.thread_log("starting DAP server")
+        global server
+        startup.start_dap(server.main_loop)
diff --git a/gdb/python/py-dap.c b/gdb/python/py-dap.c
index 5757c150165..2034105c939 100644
--- a/gdb/python/py-dap.c
+++ b/gdb/python/py-dap.c
@@ -61,13 +61,18 @@  class dap_interp final : public interp
     return m_ui_out.get ();
   }
 
+  void pre_command_loop () override;
+
 private:
 
   std::unique_ptr<ui_out> m_ui_out;
 };
 
-void
-dap_interp::init (bool top_level)
+
+/* Call function FN_NAME from module gdb.dap. */
+
+static void
+call_dap_fn (const char *fn_name)
 {
   gdbpy_enter enter_py;
 
@@ -75,18 +80,30 @@  dap_interp::init (bool top_level)
   if (dap_module == nullptr)
     gdbpy_handle_exception ();
 
-  gdbpy_ref<> func (PyObject_GetAttrString (dap_module.get (), "run"));
+  gdbpy_ref<> func (PyObject_GetAttrString (dap_module.get (), fn_name));
   if (func == nullptr)
     gdbpy_handle_exception ();
 
   gdbpy_ref<> result_obj (PyObject_CallObject (func.get (), nullptr));
   if (result_obj == nullptr)
     gdbpy_handle_exception ();
+}
+
+void
+dap_interp::init (bool top_level)
+{
+  call_dap_fn ("run");
 
   current_ui->input_fd = -1;
   current_ui->m_input_interactive_p = false;
 }
 
+void
+dap_interp::pre_command_loop ()
+{
+  call_dap_fn ("pre_command_loop");
+}
+
 void _initialize_py_interp ();
 void
 _initialize_py_interp ()
diff --git a/gdb/testsuite/gdb.dap/eof.exp b/gdb/testsuite/gdb.dap/eof.exp
index 9c17725c0d0..a84b1d21e04 100644
--- a/gdb/testsuite/gdb.dap/eof.exp
+++ b/gdb/testsuite/gdb.dap/eof.exp
@@ -35,3 +35,6 @@  catch "wait -i $gdb_spawn_id"
 unset gdb_spawn_id
 
 dap_check_log_file
+
+# Check that first log message is present.
+dap_check_log_file_re [string_to_regexp "starting DAP server"]
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index 31f036eddf2..5818d815df3 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -366,6 +366,16 @@  proc dap_check_log_file {} {
     }
 }
 
+# Read the most recent DAP log file and check that regexp RE matches.
+proc dap_check_log_file_re { re } {
+    set fd [open [current_dap_log_file]]
+    set contents [read $fd]
+    close $fd
+
+    set ok [regexp $re $contents]
+    gdb_assert {$ok} "log file matched $re"
+}
+
 # Cleanly shut down gdb.  TERMINATE is passed as the terminateDebuggee
 # parameter to the request.
 proc dap_shutdown {{terminate false}} {