Skip to content

Commit 86e248f

Browse files
vrieslgaver2
authored andcommitted
[gdb/cli] Use captured per_command_time in worker threads
With test-case gdb.base/maint.exp, I ran into: ... (gdb) file maint^M Reading symbols from maint...^M (gdb) mt set per-command on^M (gdb) Time for "DWARF indexing worker": ...^M Time for "DWARF indexing worker": ...^M Time for "DWARF indexing worker": ...^M Time for "DWARF indexing worker": ...^M Time for "DWARF skeletonless type units": ...^M Time for "DWARF add parent map": ...^M Time for "DWARF finalize worker": ...^M Time for "DWARF finalize worker": ...^M Time for "DWARF finalize worker": ...^M Time for "DWARF finalize worker": ...^M Time for "DWARF finalize worker": ...^M FAIL: $exp: warnings: per-command: mt set per-command on (timeout) mt set per-command off^M 2025-05-31 09:33:44.711 - command started^M (gdb) PASS: $exp: warnings: per-command: mt set per-command off ... I didn't manage to reproduce this by rerunning the test-case, but it's fairly easy to reproduce using a file with more debug info, for instance gdb: ... $ gdb -q -batch -ex "file build/gdb/gdb" -ex "mt set per-command on" ... Due to the default "mt dwarf synchronous" == off, the file command starts building the cooked index in the background, and returns immediately without waiting for the result. The subsequent "mt set per-command on" implies "mt set per-command time on", which switches on displaying of per-command execution time. The "Time for" lines are the result of those two commands, but these lines shouldn't be there because "mt per-command time" == off at the point of issuing the file command. Fix this by capturing the per_command_time variable, and using the captured value instead. Tested on x86_64-linux. Approved-By: Simon Marchi <[email protected]> PR cli/33039 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33039
1 parent 062bbd4 commit 86e248f

File tree

6 files changed

+25
-11
lines changed

6 files changed

+25
-11
lines changed

gdb/dwarf2/cooked-index-worker.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ void
246246
cooked_index_worker::done_reading ()
247247
{
248248
{
249-
scoped_time_it time_it ("DWARF add parent map");
249+
scoped_time_it time_it ("DWARF add parent map", m_per_command_time);
250250

251251
for (auto &one_result : m_results)
252252
m_all_parents_map.add_map (*one_result.get_parent_map ());

gdb/dwarf2/cooked-index-worker.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#include "dwarf2/cooked-index-shard.h"
2626
#include "dwarf2/types.h"
2727
#include "dwarf2/read.h"
28+
#include "maint.h"
29+
#include "run-on-main-thread.h"
2830

2931
#if CXX_STD_THREAD
3032
#include <mutex>
@@ -216,8 +218,12 @@ class cooked_index_worker
216218

217219
explicit cooked_index_worker (dwarf2_per_objfile *per_objfile)
218220
: m_per_objfile (per_objfile),
219-
m_cache_store (global_index_cache, per_objfile->per_bfd)
220-
{ }
221+
m_cache_store (global_index_cache, per_objfile->per_bfd),
222+
m_per_command_time (per_command_time)
223+
{
224+
/* Make sure we capture per_command_time from the main thread. */
225+
gdb_assert (is_main_thread ());
226+
}
221227
virtual ~cooked_index_worker ()
222228
{ }
223229
DISABLE_COPY_AND_ASSIGN (cooked_index_worker);
@@ -307,6 +313,9 @@ class cooked_index_worker
307313
std::optional<gdb_exception> m_failed;
308314
/* An object used to write to the index cache. */
309315
index_cache_store_context m_cache_store;
316+
317+
/* Captured value of per_command_time. */
318+
bool m_per_command_time;
310319
};
311320

312321
using cooked_index_worker_up = std::unique_ptr<cooked_index_worker>;

gdb/dwarf2/cooked-index.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ cooked_index::set_contents ()
104104
const parent_map_map *parent_maps = m_state->get_parent_map_map ();
105105
finalizers.add_task ([=] ()
106106
{
107-
scoped_time_it time_it ("DWARF finalize worker");
107+
scoped_time_it time_it ("DWARF finalize worker",
108+
m_state->m_per_command_time);
108109
this_shard->finalize (parent_maps);
109110
});
110111
}

gdb/dwarf2/read.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3464,7 +3464,7 @@ void
34643464
cooked_index_worker_debug_info::process_skeletonless_type_units
34653465
(dwarf2_per_objfile *per_objfile, cooked_index_worker_result *storage)
34663466
{
3467-
scoped_time_it time_it ("DWARF skeletonless type units");
3467+
scoped_time_it time_it ("DWARF skeletonless type units", m_per_command_time);
34683468

34693469
/* Skeletonless TUs in DWP files without .gdb_index is not supported yet. */
34703470
if (per_objfile->per_bfd->dwp_file == nullptr)
@@ -3573,7 +3573,7 @@ cooked_index_worker_debug_info::do_reading ()
35733573
gdb_assert (iter != last);
35743574
workers.add_task ([this, task_count, iter, last] ()
35753575
{
3576-
scoped_time_it time_it ("DWARF indexing worker");
3576+
scoped_time_it time_it ("DWARF indexing worker", m_per_command_time);
35773577
process_units (task_count, iter, last);
35783578
});
35793579

gdb/maint.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -920,9 +920,9 @@ maintenance_show_worker_threads (struct ui_file *file, int from_tty,
920920
}
921921

922922

923-
/* If true, display time usage both at startup and for each command. */
923+
/* See maint.h. */
924924

925-
static bool per_command_time;
925+
bool per_command_time;
926926

927927
/* If true, display space usage both at startup and for each command. */
928928

@@ -1175,8 +1175,8 @@ per-thread run time information not available on this platform"));
11751175

11761176
/* See maint.h. */
11771177

1178-
scoped_time_it::scoped_time_it (const char *what)
1179-
: m_enabled (per_command_time),
1178+
scoped_time_it::scoped_time_it (const char *what, bool enabled)
1179+
: m_enabled (enabled),
11801180
m_what (what),
11811181
m_start_wall (m_enabled
11821182
? std::chrono::steady_clock::now ()

gdb/maint.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,17 @@ class scoped_command_stats
7070
int m_start_nr_blocks;
7171
};
7272

73+
/* If true, display time usage both at startup and for each command. */
74+
75+
extern bool per_command_time;
76+
7377
/* RAII structure used to measure the time spent by the current thread in a
7478
given scope. */
7579

7680
struct scoped_time_it
7781
{
7882
/* WHAT is the prefix to show when the summary line is printed. */
79-
scoped_time_it (const char *what);
83+
scoped_time_it (const char *what, bool enabled = per_command_time);
8084

8185
DISABLE_COPY_AND_ASSIGN (scoped_time_it);
8286
~scoped_time_it ();

0 commit comments

Comments
 (0)