Commit Graph

209 Commits

Author SHA1 Message Date
Tom de Vries
b859a3ef48 [gdbsupport] Add task size parameter in parallel_for_each
Add a task_size parameter to parallel_for_each, defaulting to nullptr, and use
the task size to distribute similarly-sized chunks to the threads.

Tested on x86_64-linux.
2022-08-05 16:12:56 +02:00
Pedro Alves
377c3a9c91 Introduce gdb::make_function_view
This adds gdb::make_function_view, which lets you create a function
view from a callable without specifying the function_view's template
parameter.  For example, this:

    auto lambda = [&] (int) { ... };
    auto fv = gdb::make_function_view (lambda);

instead of:

    auto lambda = [&] (int) { ... };
    gdb::function_view<void (int)> fv = lambda;

It is particularly useful if you have a template function with an
optional function_view parameter, whose type depends on the function's
template parameters.  Like:

    template<typename T>
    void my_function (T v, gdb::function_view<void(T)> callback = nullptr);

For such a function, the type of the callback argument you pass must
already be a function_view.  I.e., this wouldn't compile:

    auto lambda = [&] (int) { ... };
    my_function (1, lambda);

With gdb::make_function_view, you can write the call like so:

    auto lambda = [&] (int) { ... };
    my_function (1, gdb::make_function_view (lambda));

Unit tests included.

Tested by building with GCC 9.4, Clang 10, and GCC 4.8.5, on x86_64
GNU/Linux, and running the unit tests.

Change-Id: I5c4b3b4455ed6f0d8878cf1be189bea3ee63f626
2022-08-05 16:12:56 +02:00
Tom de Vries
731d2cc1d5 [gdb] Add debug_{exp,val}
When debugging cc1 I heavily rely on simple one-parameter debug functions
that allow me to inspect a variable of a common type, like:
- debug_generic_expr
- debug_gimple_stmt
- debug_rtx
and I miss similar functions in gdb.

Add functions to dump variables of types 'value' and 'expression':
- debug_exp, and
- debug_val.

Tested on x86_64-linux, by breaking on varobj_create, and doing:
...
(gdb) call debug_exp (var->root->exp.get ())
&"Operation: OP_VAR_VALUE\n"
&" Block symbol:\n"
&"  Symbol: aaa\n"
&"  Block: 0x2d064f0\n"
(gdb)
...
and:
...
(gdb) call debug_val (value)
&"5"
(gdb)
...
2022-08-05 08:09:57 +02:00
Pedro Alves
b669667d07 struct packed: Add fallback byte array implementation
Attribute gcc_struct is not implemented in Clang targeting Windows, so
add a fallback standard-conforming implementation based on arrays.

I ran the testsuite on x86_64 GNU/Linux with this implementation
forced, and saw no regressions.

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

Change-Id: I023315ee03622c59c397bf4affc0b68179c32374
2022-07-25 16:04:05 +01:00
Pedro Alves
e249e6b801 struct packed: Unit tests and more operators
For PR gdb/29373, I wrote an alternative implementation of struct
packed that uses a gdb_byte array for internal representation, needed
for mingw+clang.  While adding that, I wrote some unit tests to make
sure both implementations behave the same.  While at it, I implemented
all relational operators.  This commit adds said unit tests and
relational operators.  The alternative gdb_byte array implementation
will come next.

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

Change-Id: I023315ee03622c59c397bf4affc0b68179c32374
2022-07-25 16:04:05 +01:00
Pedro Alves
4ca26ad7de struct packed: Use gcc_struct on Windows
Building GDB on mingw/gcc hosts is currently broken, due to a static
assertion failure in gdbsupport/packed.h:

  In file included from ../../../../../binutils-gdb/gdb/../gdbsupport/common-defs.h:201,
		   from ../../../../../binutils-gdb/gdb/defs.h:28,
		   from ../../../../../binutils-gdb/gdb/dwarf2/read.c:31:
  ../../../../../binutils-gdb/gdb/../gdbsupport/packed.h: In instantiation of 'packed<T, Bytes>::packed(T) [with T = dwarf_unit_type; long long unsigned int Bytes = 1]':
  ../../../../../binutils-gdb/gdb/dwarf2/read.h:181:74:   required from here
  ../../../../../binutils-gdb/gdb/../gdbsupport/packed.h:41:40: error: static assertion failed
     41 |     gdb_static_assert (sizeof (packed) == Bytes);
	|                        ~~~~~~~~~~~~~~~~^~~~~~~~
  ../../../../../binutils-gdb/gdb/../gdbsupport/gdb_assert.h:27:48: note: in definition of macro 'gdb_static_assert'
     27 | #define gdb_static_assert(expr) static_assert (expr, "")
	|                                                ^~~~
  ../../../../../binutils-gdb/gdb/../gdbsupport/packed.h:41:40: note: the comparison reduces to '(4 == 1)'
     41 |     gdb_static_assert (sizeof (packed) == Bytes);
	|                        ~~~~~~~~~~~~~~~~^~~~~~~~


The issue is that mingw gcc defaults to "-mms-bitfields", which
affects how bitfields are laid out.  We can however tell GCC that we
want the regular GCC layout instead using attribute gcc_struct.

Attribute gcc_struct is not implemented in "clang -target
x86_64-pc-windows-gnu", so that will need a different fix.

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

Change-Id: I023315ee03622c59c397bf4affc0b68179c32374
2022-07-25 16:04:05 +01:00
Andrew Burgess
11da1b13b3 gdbsupport: add checked_static_cast
This commit was inspired by these mailing list posts:

  https://sourceware.org/pipermail/gdb-patches/2022-June/190323.html
  https://sourceware.org/pipermail/gdb-patches/2022-April/188098.html

The idea is to add a new function gdb::checked_static_cast, which can,
in some cases, be used as a drop-in replacement for static_cast.  And
so, if I previously wrote this:

  BaseClass *base = get_base_class_pointer ();
  DerivedClass *derived = static_cast<DerivedClass *> (base);

I can now write:

  BaseClass *base = get_base_class_pointer ();
  DerivedClass *derived = gdb::checked_static_cast<DerivedClass *> (base);

The requirement is that BaseClass and DerivedClass must be
polymorphic.

The benefit of making this change is that, when GDB is built in
developer mode, a run-time check will be made to ensure that `base`
really is of type DerivedClass before the cast is performed.  If
`base` is not of type DerivedClass then GDB will assert.

In a non-developer build gdb::checked_static_cast is equivalent to a
static_cast, and there should be no performance difference.

This commit adds the support function, but does not make use of this
function, a use will be added in the next commit.

Co-Authored-By: Pedro Alves <pedro@palves.net>
Co-Authored-By: Tom Tromey <tom@tromey.com>
2022-07-21 15:19:43 +01:00
Tom de Vries
9b89bf16c3 [gdbsupport] Fix type of parallel_for_each_debug
When I changed the initialization of parallel_for_each_debug from 0 to false,
I forgot to change the type from int to bool.  Fix this.

Tested by rebuilding on x86_64-linux.
2022-07-21 13:34:14 +02:00
Tom de Vries
4319180c81 [gdbsupport] Improve thread scheduling in parallel_for_each
When running a task using parallel_for_each, we get the following
distribution:
...
Parallel for: n_elements: 7271
Parallel for: minimum elements per thread: 10
Parallel for: elts_per_thread: 1817
Parallel for: elements on worker thread 0       : 1817
Parallel for: elements on worker thread 1       : 1817
Parallel for: elements on worker thread 2       : 1817
Parallel for: elements on worker thread 3       : 0
Parallel for: elements on main thread           : 1820
...

Note that there are 4 active threads, and scheduling elts_per_thread on each
of those handles 4 * 1817 == 7268, leaving 3 "left over" elements.

These leftovers are currently handled in the main thread.

That doesn't seem to matter much for this example, but for say 10 threads and
99 elements, you'd have 9 threads handling 9 elements and 1 thread handling 18
elements.

Instead, distribute the left over elements over the worker threads, such that
we have:
...
Parallel for: elements on worker thread 0       : 1818
Parallel for: elements on worker thread 1       : 1818
Parallel for: elements on worker thread 2       : 1818
Parallel for: elements on worker thread 3       : 0
Parallel for: elements on main thread           : 1817
...

Tested on x86_64-linux.
2022-07-18 08:34:06 +02:00
Tom de Vries
53944a3bf5 [gdbsupport] Add parallel_for_each_debug
Add a parallel_for_each_debug variable, set to false by default.

With an a.out compiled from hello world, we get with
parallel_for_each_debug == true:
...
$ gdb -q -batch a.out -ex start
  ...
Parallel for: n_elements: 7271
Parallel for: minimum elements per thread: 10
Parallel for: elts_per_thread: 1817
Parallel for: elements on worker thread 0       : 1817
Parallel for: elements on worker thread 1       : 1817
Parallel for: elements on worker thread 2       : 1817
Parallel for: elements on worker thread 3       : 0
Parallel for: elements on main thread           : 1820

Temporary breakpoint 1, main () at /home/vries/hello.c:6
6         printf ("hello\n");
...

Tested on x86_64-linux.
2022-07-18 05:34:01 +02:00
Tom de Vries
18a5766d09 [gdbsupport] Add sequential_for_each
Add a sequential_for_each alongside the parallel_for_each, which can be used
as a drop-in replacement.

This can be useful when debugging multi-threading behaviour, and you want to
limit multi-threading in a fine-grained way.

Tested on x86_64-linux, by using it instead of the parallel_for_each in
dwarf2_build_psymtabs_hard.
2022-07-14 17:01:52 +02:00
Tom de Vries
2df41bda2f [gdb/build] Fix gdb build with gcc 4.8.5
When building gdb with gcc 4.8.5, we run into:
...
In file included from /usr/include/c++/4.8/future:43:0,
                 from gdbsupport/thread-pool.h:30,
                 from gdb/dwarf2/cooked-index.h:33,
                 from gdb/dwarf2/read.h:26,
                 from gdb/dwarf2/abbrev-cache.c:21:
/usr/include/c++/4.8/atomic: In instantiation of \
  ‘_Tp std::atomic<_Tp>::load(std::memory_order) const [with _Tp = \
  packed<dwarf_unit_type, 1ul>; std::memory_order = std::memory_order]’:
gdb/dwarf2/read.h:332:44:   required from here
/usr/include/c++/4.8/atomic:208:13: error: no matching function for call to \
  ‘packed<dwarf_unit_type, 1ul>::packed()’
         _Tp tmp;
             ^
...

Fix this by adding the default constructor for packed.

Tested on x86_64-linux, with gdb build with gcc 4.8.5.
2022-07-14 10:20:16 +02:00
Tom de Vries
02f0597c46 [gdb/build] Fix build with gcc 4.8.5
When building gdb with gcc 4.8.5, we run into problems due to unconditionally
using:
...
     gdb_static_assert (std::is_trivially_copyable<packed>::value);
...
in gdbsupport/packed.h.

Fix this by guarding the usage with HAVE_IS_TRIVIALLY_COPYABLE.

Tested by doing a full gdb build with gcc 4.8.5.
2022-07-12 13:36:57 +02:00
Pedro Alves
68c0faca76 Introduce struct packed template
When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a few data races.  For example, between:

 ...
   Write of size 1 at 0x7b200000300e by thread T4:
     #0 process_psymtab_comp_unit gdb/dwarf2/read.c:6789 (gdb+0x830720)
 ...

and:

 ...
   Previous read of size 1 at 0x7b200000300e by main thread:
     #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
     abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6164 \
     (gdb+0x82edab)
 ...

In other words, between:

 ...
       this_cu->unit_type = DW_UT_partial;
 ...

and:

 ...
   if (this_cu->reading_dwo_directly)
 ...

The problem is that the written fields are part of the same memory
location as the read fields, so executing a read and write in
different threads is undefined behavour.

Making the written fields separate memory locations, like this:

...
  struct {
    ENUM_BITFIELD (dwarf_unit_type) unit_type : 8;
  };
...

fixes it, however that also increases the size of struct
dwarf2_per_cu_data, because it introduces padding due to alignment of
these new structs, which align on the natural alignment of the
specified type of their fields.  We can fix that with
__attribute__((packed)), like so:

  struct {
    ENUM_BITFIELD (dwarf_unit_type) unit_type : 8 __attribute__((packed));
  };

but to avoid having to write that in several places and add suitable
comments explaining how that concoction works, introduce a new struct
packed template that wraps/hides this.  Instead of the above, we'll be
able to write:

    packed<dwarf_unit_type, 1> unit_type;

Note that we can't change the type of dwarf_unit_type, as that is
defined in include/, and shared with other projects, some of those
written in C.

This patch just adds the struct packed type.  Following patches will
make use of it.  One of those patches will want to wrap a struct
packed in an std::atomic, like:

  std::atomic<std::packed<language, 1>> m_lang;

so the new gdbsupport/packed.h header adds some operators to make
comparisions between that std::atomic and the type that the wrapped
struct packed wraps work, like in:

   if (m_lang == language_c)

It would be possible to implement struct packed without using
__attribute__((packed)), by having it store an array of bytes of the
appropriate size instead, however that would make it less convenient
to debug GDB.  The way it's implemented, printing a struct packed
variable just prints its field using its natural type, which is
particularly useful if the type is an enum.  I believe that
__attribute__((packed)) is supported by all compilers that are able to
build GDB.  Even a few BFD headers use on ATTRIBUTE_PACKED on external
types:

 include/coff/external.h:  } ATTRIBUTE_PACKED
 include/coff/external.h:} ATTRIBUTE_PACKED ;
 include/coff/external.h:} ATTRIBUTE_PACKED ;
 include/coff/pe.h:} ATTRIBUTE_PACKED ;
 include/coff/pe.h:} ATTRIBUTE_PACKED;
 include/elf/external.h:} ATTRIBUTE_PACKED  Elf_External_Versym;

It is not possible to build GDB with MSVC today, but if it could, that
would be one compiler that doesn't support this attribute.  However,
it supports packing via pragmas, so there's a way to cross that bridge
if we ever get to it.  I believe any compiler worth its salt supports
some way of packing.

In any case, the worse that happens without the attribute is that some
types become larger than ideal.  Regardless, I've added a couple
static assertions to catch such compilers in action:

    /* Ensure size and aligment are what we expect.  */
    gdb_static_assert (sizeof (packed) == Bytes);
    gdb_static_assert (alignof (packed) == 1);

Change-Id: Ifa94f0a2cebfae5e8f6ddc73265f05e7fd9e1532
2022-07-12 12:20:13 +02:00
Tom de Vries
6418644b0d [gdb] Block SIGTERM in worker threads
With gdb build with gcc-12 and -fsanitize=thread, and test-case
gdb.base/gdb-sigterm.exp, I run into:
...
WARNING: ThreadSanitizer: data race (pid=9722)^M
  Write of size 4 at 0x00000325bc68 by thread T1:^M
  #0 handle_sigterm(int) src/gdb/event-top.c:1211 (gdb+0x8ec01f)^M
  ...
  Previous read of size 4 at 0x00000325bc68 by main thread:^M
    [failed to restore the stack]^M
^M
  Location is global 'sync_quit_force_run' of size 4 at \
  0x00000325bc68 (gdb+0x325bc68)^M
  ...
SUMMARY: ThreadSanitizer: data race gdb/event-top.c:1211 in \
  handle_sigterm(int)^M
...
and 3 more data races involving handle_sigterm and locations:
- active_ext_lang
- quit_flag
- heap block of size 40
  (XNEW (async_signal_handler) in create_async_signal_handler)

This was reported in PR29297.

The testcase executes a "kill -TERM $gdb_pid", which generates a
process-directed signal.

A process-directed signal can be delivered to any thread, and what we see
here is the fallout of the signal being delivered to a worker thread rather
than the main thread.

Fix this by blocking SIGTERM in the worker threads.

[ I have not been able to reproduce this after it occurred for the first time,
so unfortunately I cannot confirm that the patch fixes the problem. ]

Tested on x86_64-linux, with and without -fsanitize=thread.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29297
2022-06-30 13:31:06 +02:00
Pedro Alves
171fba11ab Make GDBserver abort on internal error in development mode
Currently, if GDBserver hits some internal assertion, it exits with
error status, instead of aborting.  This makes it harder to debug
GDBserver, as you can't just debug a core file if GDBserver fails an
assertion.  I've had to hack the code to make GDBserver abort to debug
something several times before.

I believe the reason it exits instead of aborting, is to prevent
potentially littering the filesystem of smaller embedded targets with
core files.  I think I recall Daniel Jacobowitz once saying that many
years ago, but I can't be sure.  Anyhow, that seems reasonable to me.

Since we nowadays have a distinction between development and release
modes, I propose to make GDBserver abort on internal error if in
development mode, while keeping the status quo when in release mode.

Thus, after this patch, in development mode, you get:

 $ ../gdbserver/gdbserver
 ../../src/gdbserver/server.cc:3711: A problem internal to GDBserver has been detected.
 captured_main: Assertion `0' failed.
 Aborted (core dumped)
 $

while in release mode, you'll continue to get:

 $ ../gdbserver/gdbserver
 ../../src/gdbserver/server.cc:3711: A problem internal to GDBserver has been detected.
 captured_main: Assertion `0' failed.
 $ echo $?
 1

I do not think that this requires a separate configure switch.

A "--target_board=native-extended-gdbserver" run on Ubuntu 20.04 ends
up with:

		 === gdb Summary ===

 # of unexpected core files      29
 ...

for me, of which 8 are GDBserver core dumps, 7 more than without this
patch.

Change-Id: I6861e08ad71f65a0332c91ec95ca001d130b0e9d
2022-06-27 13:55:36 +01:00
Tom Tromey
20a26f4e01 Finalize each cooked index separately
After DWARF has been scanned, the cooked index code does a
"finalization" step in a worker thread.  This step combines all the
index entries into a single master list, canonicalizes C++ names, and
splits Ada names to synthesize package names.

While this step is run in the background, gdb will wait for the
results in some situations, and it turns out that this step can be
slow.  This is PR symtab/29105.

This can be sped up by parallelizing, at a small memory cost.  Now
each index is finalized on its own, in a worker thread.  The cost
comes from name canonicalization: if a given non-canonical name is
referred to by multiple indices, there will be N canonical copies (one
per index) rather than just one.

This requires changing the users of the index to iterate over multiple
results.  However, this is easily done by introducing a new "chained
range" class.

When run on gdb itself, the memory cost seems rather low -- on my
current machine, "maint space 1" reports no change due to the patch.

For performance testing, using "maint time 1" and "file" will not show
correct results.  That approach measures "time to next prompt", but
because the patch only affects background work, this shouldn't (and
doesn't) change.  Instead, a simple way to make gdb wait for the
results is to set a breakpoint.

Before:

    $ /bin/time -f%e ~/gdb/install/bin/gdb -nx -q -batch \
        -ex 'break main' /tmp/gdb
    Breakpoint 1 at 0x43ec30: file ../../binutils-gdb/gdb/gdb.c, line 28.
    2.00

After:

    $ /bin/time -f%e ./gdb/gdb -nx -q -batch \
        -ex 'break main' /tmp/gdb
    Breakpoint 1 at 0x43ec30: file ../../binutils-gdb/gdb/gdb.c, line 28.
    0.65

Regression tested on x86-64 Fedora 34.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29105
2022-05-26 07:35:30 -06:00
Tom de Vries
735dfe028c [gdbsupport] Fix UB in print-utils.cc:int_string
When building gdb with -fsanitize=undefined, I run into:
...
(gdb) PASS: gdb.ada/access_to_packed_array.exp: set logging enabled on
maint print symbols^M
print-utils.cc:281:29:runtime error: negation of -9223372036854775808 cannot \
  be represented in type 'long int'; cast to an unsigned type to negate this \
  value to itself
(gdb) FAIL: gdb.ada/access_to_packed_array.exp: maint print symbols
...

By running in a debug session, we find that this happens during printing of:
...
   typedef system.storage_elements.storage_offset: \
     range -9223372036854775808 .. 9223372036854775807;
...
Possibly, an ada test-case could be created that exercises this in isolation.

The problem is here in int_string, where we negate a val with type LONGEST:
...
         return decimal2str ("-", -val, width);
...

Fix this by, as recommend, using "-(ULONGEST)val" instead.

Tested on x86_64-linux.
2022-05-23 14:50:02 +02:00
Simon Marchi
02646f1960 gdbsupport: fix path_join crash with -std=c++17 and -D_GLIBCXX_DEBUG
When building GDB with -std=c++17 and -D_GLIBCXX_DEBUG=1, I get:

  $ ./gdb -nx --data-directory=data-directory -q -ex "maint selftest path_join"
  /usr/include/c++/11.2.0/string_view:233: constexpr const value_type& std::basic_string_view<_CharT, _Traits>::operator[](std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>; std::basic_string_view<_CharT, _Traits>::const_reference = const char&; std::basic_string_view<_CharT, _Traits>::size_type = long unsigned int]: Assertion  '__pos < this->_M_len' failed.

The problem is that we're passing an empty string_view to
IS_ABSOLUTE_PATH.  IS_ABSOLUTE_PATH accesses [0] on that string_view,
which is out-of-bounds.

The reason this is not seen with -std less than c++17 is that our local
copy of string_view (used with C++ < 17) does not have the assert in
operator[], as that wouldn't work in a constexpr method:

  5890af36e5/gdbsupport/gdb_string_view.h (L180)

IS_ABSOLUTE_PATH is normally used with null-terminated string.  It's
fine to pass an empty null-terminated string to IS_ABSOLUTE_PATH,
because index 0 in such a string is valid.  But not with an empty
string_view.

Fix that by avoiding the "call" to IS_ABSOLUTE_PATH if the string_view
is empty.

Change-Id: Idf4df961b63f513b3389235e93814c02b89ea32e
2022-05-19 08:04:13 -04:00
Pedro Alves
187075ebbc Reindent gdbsupport/event-loop.cc:handle_file_event
The handle_file_event function has a few unnecessary {} lexical
blocks, presumably because they were originally if blocks, and the
conditions were removed, or something along those lines.

Remove the unnecessary blocks, and reindent.

Change-Id: Iaecbe5c9f4940a80b81dbbc42e51ce506f6aafb2
2022-05-16 19:58:08 +01:00
Pedro Alves
36a5b37053 gdbsupport/event-loop.cc: simplify !HAVE_POLL paths
gdbsupport/event-loop.cc throughout handles the case of use_poll being
true on a system where HAVE_POLL is not defined, by calling
internal_error if that situation ever happens.

Simplify this by moving the "use_poll" global itself under HAVE_POLL,
so that it's way more unlikely to ever end up in such a situation.
Then, move the code that checks the value of use_poll under HAVE_POLL
too, and remove the internal_error calls.  Like, from:

    if (use_poll)
      {
  #ifdef HAVE_POLL
        // poll code
  #else
      internal_error (....);
  #endif /* HAVE_POLL */
      }
    else
      {
	// select code
      }

to

  #ifdef HAVE_POLL
    if (use_poll)
      {
        // poll code
      }
    else
  #endif /* HAVE_POLL */
      {
	// select code
      }

While at it, make use_poll be a bool.  The current code is using
unsigned char most probably to save space, but I don't think it really
matters here.

Co-Authored-By: Youling Tang <tangyouling@loongson.cn>
Change-Id: I0dd74fdd4d393ccd057906df4cd75e8e83c1cdb4
2022-05-16 19:54:20 +01:00
Tom Tromey
20c4eb4226 Fix --disable-threading build
PR build/29110 points out that GDB fails to build on mingw when the
"win32" thread model is in use.  It turns out that the Fedora cross
tools using the "posix" thread model, which somehow manages to support
std::future, whereas the win32 model does not.

While looking into this, I found that the configuring with
--disable-threading will also cause a build failure.

This patch fixes this build by introducing a compatibility wrapper for
std::future.

I am not able to test the win32 thread model build, but I'm going to
ask the reporter to try this patch.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29110
2022-05-10 08:15:40 -06:00
Pedro Alves
62b33fde9c Move non-dependent gdb::observers::observable::visit_state outside template
The other day, while looking at the symbols that end up in a GDB
index, I noticed that the gdb::observers::observable::visit_state enum
class appears a number of times:

 $ grep VISIT gdb-index-symbol-names.txt
 gdb::observers::observable<bpstat*, int>::visit_state::NOT_VISITED
 gdb::observers::observable<bpstat*, int>::visit_state::VISITED
 gdb::observers::observable<bpstat*, int>::visit_state::VISITING
 gdb::observers::observable<breakpoint*>::visit_state::NOT_VISITED
 gdb::observers::observable<breakpoint*>::visit_state::VISITED
 gdb::observers::observable<breakpoint*>::visit_state::VISITING
 gdb::observers::observable<char const*, char const*>::visit_state::NOT_VISITED
 gdb::observers::observable<char const*, char const*>::visit_state::VISITED
 gdb::observers::observable<char const*, char const*>::visit_state::VISITING
 gdb::observers::observable<char const*>::visit_state::NOT_VISITED
 gdb::observers::observable<char const*>::visit_state::VISITED
 gdb::observers::observable<char const*>::visit_state::VISITING
 gdb::observers::observable<enum_flags<user_selected_what_flag> >::visit_state::NOT_VISITED
 gdb::observers::observable<enum_flags<user_selected_what_flag> >::visit_state::VISITED
 gdb::observers::observable<enum_flags<user_selected_what_flag> >::visit_state::VISITING
 gdb::observers::observable<frame_info*, int>::visit_state::NOT_VISITED
 gdb::observers::observable<frame_info*, int>::visit_state::VISITED
 gdb::observers::observable<frame_info*, int>::visit_state::VISITING
 gdb::observers::observable<gdbarch*>::visit_state::NOT_VISITED
 gdb::observers::observable<gdbarch*>::visit_state::VISITED
 gdb::observers::observable<gdbarch*>::visit_state::VISITING
 gdb::observers::observable<gdb_signal>::visit_state::NOT_VISITED
 gdb::observers::observable<gdb_signal>::visit_state::VISITED
 gdb::observers::observable<gdb_signal>::visit_state::VISITING
 [... snip ...]

 $ grep VISIT gdb-index-symbol-names.txt | wc -l
 72

enum class visit_state is defined inside the class template
observable, but it doesn't have to be, as it does not depend on the
template parameters.  This commit moves it out, so that only one such
type exists.  This reduces the size of a -O0 -g3 build for me by
around 0.6%, like so:

 $ du -b gdb.before gdb.after
 164685280       gdb.before
 163707424       gdb.fixed

and codesize by some 0.5%.

Change-Id: I405f4ef27b8358fdd22158245b145d849b45658e
2022-05-10 13:40:57 +01:00
John Baldwin
3f0423f913 gdbsupport/pathstuff.h: #include <array> explicitly for std::array<>
This fixes build breakage using clang with libc++ on FreeBSD where
std::array<> is not yet declared when used by the path_join variadic
function template.
2022-04-25 17:06:09 -07:00
Simon Marchi
ffaebc199e gdbsupport: add path_join function
In this review [1], Eli pointed out that we should be careful when
concatenating file names to avoid duplicated slashes.  On Windows, a
double slash at the beginning of a file path has a special meaning.  So
naively concatenating "/"  and "foo/bar" would give "//foo/bar", which
would not give the desired results.  We already have a few spots doing:

  if (first_path ends with a slash)
    path = first_path + second_path
  else
    path = first_path + slash + second_path

In general, I think it's nice to avoid superfluous slashes in file
paths, since they might end up visible to the user and look a bit
unprofessional.

Introduce the path_join function that can be used to join multiple path
components together (along with unit tests).

I initially wanted to make it possible to join two absolute paths, to
support the use case of prepending a sysroot path to a target file path,
or the prepending the debug-file-directory to a target file path.  But
the code in solib_find_1 shows that it is more complex than this anyway
(for example, when the right hand side is a Windows path with a drive
letter).  So I don't think we need to support that case in path_join.
That also keeps the implementation simpler.

Change a few spots to use path_join to show how it can be used.  I
believe that all the spots I changed are guarded by some checks that
ensure the right hand side operand is not an absolute path.

Regression-tested on Ubuntu 18.04.  Built-tested on Windows, and I also
ran the new unit-test there.

[1] https://sourceware.org/pipermail/gdb-patches/2022-April/187559.html

Change-Id: I0df889f7e3f644e045f42ff429277b732eb6c752
2022-04-21 11:11:21 -04:00
Lancelot SIX
9a0f7f634e gdbsupport/selftest: Allow lazy registration
This patch adds a way to delay the registration of tests until the
latest possible moment.  This is intended for situations where GDB needs
to be fully initialized in order to decide if a particular selftest can
be executed or not.

This mechanism will be used in the next patch.

Change-Id: I7f6b061f4c0a6832226c7080ab4e3a2523e1b0b0
2022-04-19 09:12:42 +01:00
Lancelot SIX
c57207c15c gdbsupport/selftest: Replace for_each_selftest with an iterator_range
Remove the callback-based selftests::for_each_selftest function and use
an iterator_range instead.

Also use this iterator range in run_tests so all iterations over the
selftests are done in a consistent way.  This will become useful in a
later commit.

Change-Id: I0b3a5349a7987fbcb0071f11c394e353df986583
2022-04-19 09:12:42 +01:00
Simon Marchi
56325e2ba6 gdb: use gdb_tilde_expand instead of gdb_tilde_expand_up in source_script_with_search
Since this is the latest use of gdb_tilde_expand_up, remove it.

Change-Id: I964c812ce55fe087876abf91e7a3577ad79c0425
2022-04-18 15:48:03 -04:00
Simon Marchi
5f2491c300 gdbsupport: make gdb_realpath_keepfile return an std::string
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible.  Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).

Change-Id: I793aab17baaef8345488f4c40b9094e2695425bc
2022-04-18 15:48:03 -04:00
Simon Marchi
7ab2607f97 gdbsupport: make gdb_abspath return an std::string
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible.  Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).

Change-Id: Iced49b8ee2f189744c5072a3b217aab5af17a993
2022-04-18 15:48:03 -04:00
Tom Tromey
1ea519ec19 Set the worker thread name on Windows
This patch is a bit different from the rest of the series, in that it
is a change to gdb's behavior on the host.  It changes gdb's thread
pool to try to set the thread name on Windows, if SetThreadDescription
is available.

This is part of PR win32/29050.

This patch isn't likely to be useful to many people in the short term,
because the Windows port of the libstdc++ thread code is not upstream.
(AdaCore uses it, and sent it upstream, but it did not land, I don't
know why.)  However, if that patch does ever go in, or presumably if
you build using some other C++ runtime library, then this will be
useful.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29050
2022-04-14 12:12:34 -06:00
Tom Tromey
c560a5fbae Let std::thread check pass even without pthreads
Currently, the configure check for std::thread relies on pthreads
existing.  However, this means that if std::thread is implemented for
a non-pthreads host, then the check will yield the wrong answer.  This
happened in AdaCore internal builds.  Here, we have this GCC patch:

    https://gcc.gnu.org/legacy-ml/gcc-patches/2019-06/msg01840.html

... which adds mingw support to GCC's gthreads implementation, and
also to std::thread.

This configure change fixes this problem and enables threading for
gdb.
2022-04-14 09:28:56 -06:00
Tiezhu Yang
11d7dd3357 gdb: fix build errors in gdbsupport/thread-pool.h used with old gcc
When I build gdb with gcc 8.3, there exist the following build errors,
rename the typedef to task_t to fix them.

  CXX      thread-pool.o
In file included from /home/loongson/gdb.git/gdbsupport/thread-pool.cc:21:
/home/loongson/gdb.git/gdbsupport/../gdbsupport/thread-pool.h: In member function ‘std::future<void> gdb::thread_pool::post_task(std::function<void()>&&)’:
/home/loongson/gdb.git/gdbsupport/../gdbsupport/thread-pool.h:69:44: error: declaration of ‘task’ shadows a previous local [-Werror=shadow=local]
     std::packaged_task<void ()> task (std::move (func));
                                            ^~~~
/home/loongson/gdb.git/gdbsupport/../gdbsupport/thread-pool.h:102:39: note: shadowed declaration is here
   typedef std::packaged_task<void ()> task;
                                       ^~~~
/home/loongson/gdb.git/gdbsupport/../gdbsupport/thread-pool.h: In member function ‘std::future<_Res> gdb::thread_pool::post_task(std::function<T()>&&)’:
/home/loongson/gdb.git/gdbsupport/../gdbsupport/thread-pool.h:80:41: error: declaration of ‘task’ shadows a previous local [-Werror=shadow=local]
     std::packaged_task<T ()> task (std::move (func));
                                         ^~~~
/home/loongson/gdb.git/gdbsupport/../gdbsupport/thread-pool.h:102:39: note: shadowed declaration is here
   typedef std::packaged_task<void ()> task;
                                       ^~~~

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
2022-04-14 23:05:00 +08:00
Pedro Alves
50b032ebc0 Make intrusive_list_node's next/prev private
Tromey noticed that intrusive_list_node leaves its data members
public, which seems sub-optimal.

This commit makes intrusive_list_node's data fields private.
intrusive_list_iterator, intrusive_list_reverse_iterator, and
intrusive_list do need to access the fields, so they are made friends.

Change-Id: Ia8b306b40344cc218d423c8dfb8355207a612ac5
2022-04-13 10:24:38 +01:00
Simon Marchi
a09520cdd9 gdbsupport: use result_of_t instead of result_of in parallel-for.h
When building with -std=c++11, I get:

In file included from /home/smarchi/src/binutils-gdb/gdb/unittests/parallel-for-selftests.c:22:                                                                             /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/parallel-for.h:134:10: error: ‘result_of_t’ is not a member of ‘std’; did you mean ‘result_of’?
  134 |     std::result_of_t<RangeFunction (RandomIt, RandomIt)>
      |          ^~~~~~~~~~~
      |          result_of

This is because result_of_t has been introduced in C++14.  Use the
equivalent result_of<...>::type instead.

result_of and result_of_t have been removed in C++20 though, so I think
we'll need some patches eventually to make the code use invoke_result
instead, depending on the C++ version.

Change-Id: I4817f361c0ebcdd4b32976898fc368bb302b61b9
2022-04-12 14:09:49 -04:00
Tom Tromey
85098eeb4c Specialize std::hash for gdb_exception
This adds a std::hash specialization for gdb_exception.  This lets us
store these objects in a hash table, which is used later in this
series to de-duplicate the exception output from multiple threads.
2022-04-12 09:31:16 -06:00
Tom Tromey
f4565e4c99 Return vector of results from parallel_for_each
This changes gdb::parallel_for_each to return a vector of the results.
However, if the passed-in function returns void, the return type
remains 'void'.  This functionality is used later, to parallelize the
new indexer.
2022-04-12 09:31:16 -06:00
Tom Tromey
82d734f7a3 Add batching parameter to parallel_for_each
parallel_for_each currently requires each thread to process at least
10 elements.  However, when indexing, it's fine for a thread to handle
just a single CU.  This patch parameterizes this, and updates the one
user.
2022-04-12 09:31:16 -06:00
Tom Tromey
0981fe1017 Allow thread-pool.h to work without threads
thread-pool.h requires CXX_STD_THREAD in order to even be included.
However, there's no deep reason for this, and during review we found
that one patch in the new DWARF indexer series unconditionally
requires the thread pool.

Because the thread pool already allows a task to be run in the calling
thread (for example if it is configured to have no threads in the
pool), it seemed straightforward to make this code ok to use when host
threads aren't available at all.

This patch implements this idea.  I built it on a thread-less host
(mingw, before my recent configure patch) and verified that the result
builds.

After the thread-pool change, parallel-for.h no longer needs any
CXX_STD_THREAD checks at all, so this patch removes these as well.
2022-04-12 09:31:15 -06:00
Tom Tromey
c50e54825b Consolidate definition of current_directory
I noticed that both gdbserver and gdb define current_directory.
However, as it is referenced by gdbsupport, it seemed better to define
it there as well.  This patch also moves the declaration to
pathstuff.h.  Tested by rebuilding.
2022-03-30 09:08:48 -06:00
Tom Tromey
c8b76e1ec3 Let phex and phex_nz handle sizeof_l==1
Currently, neither phex nor phex_nz handle sizeof_l==1 -- they let
this case fall through to the default case.  However, a subsequent
patch in this series needs this case to work correctly.

I looked at all calls to these functions that pass a 1 for the
sizeof_l parameter.  The only such case seems to be correct with this
change.
2022-03-07 07:52:59 -07:00
Roland McGrath
26caf9aca8 Fix typo in last change. 2022-03-03 13:06:50 -08:00
Roland McGrath
8674f082e3 Avoid conflict with gnulib open/close macros.
On some systems, the gnulib configuration will decide to define open
and/or close as macros to replace the POSIX C functions.  This
interferes with using those names in C++ class or namespace scopes.

gdbsupport/
	* event-pipe.cc (event_pipe::open): Renamed to ...
	(event_pipe::open_pipe): ... this.
	(event_pipe::close): Renamed to ...
	(event_pipe::close_pipe): ... this.
	* event-pipe.h (class event_pipe): Updated.
gdb/
	* inf-ptrace.h (async_file_open, async_file_close): Updated.
gdbserver/
	* gdbserver/linux-low.cc (linux_process_target::async): Likewise.
2022-03-03 11:21:36 -08:00
Andrew Burgess
820ed8af6a gdb: add operator+= and operator+ overload for std::string
This commit adds operator+= and operator+ overloads for adding
gdb::unique_xmalloc_ptr<char> to a std::string.  I could only find 3
places in GDB where this was useful right now, and these all make use
of operator+=.

I've also added a self test for gdb::unique_xmalloc_ptr<char>, which
makes use of both operator+= and operator+, so they are both getting
used/tested.

There should be no user visible changes after this commit, except when
running 'maint selftest', where the new self test is visible.
2022-02-25 17:50:22 +00:00
John Baldwin
ea3e7446dc gdbsupport: Add an event-pipe class.
This pulls out the implementation of an event pipe used to implement
target async support in both linux-low.cc (gdbserver) and linux-nat.c
(gdb).

This will be used to replace the existing event pipe in linux-low.cc
and linux-nat.c in future commits.

Co-Authored-By: Lancelot SIX <lsix@lancelotsix.com>
2022-02-22 11:22:14 -08:00
Simon Marchi
7bf5587873 gdbsupport/gdb_regex.cc: replace defs.h include with common-defs.h
This was forgotten when gdb_regex was moved from gdb to gdbsupport.

Change-Id: I73b446f71861cabbf7afdb7408ef9d59fa64b804
2022-01-20 22:58:21 -05:00
Tom Tromey
d322d6d69d Move gdb_regex to gdbsupport
This moves the gdb_regex convenience class to gdbsupport.
2022-01-18 10:14:43 -07:00
Tom Tromey
0589ca4e7b Introduce gdb-hashtab module in gdbsupport
gdb has some extensions and helpers for working with the libiberty
hash table.  This patch consolidates these and moves them to
gdbsupport.
2022-01-18 10:14:43 -07:00
Tom Tromey
bf31fd38f0 Move gdb obstack code to gdbsupport
This moves the gdb-specific obstack code -- both extensions like
obconcat and obstack_strdup, and things like auto_obstack -- to
gdbsupport.
2022-01-18 10:14:42 -07:00
Tom Tromey
7904e9613e Move gdb_argv to gdbsupport
This moves the gdb_argv class to a new header in gdbsupport.
2022-01-18 10:14:42 -07:00