binutils-gdb.git
2 years agoPass PKG_CONFIG_PATH down from top-level Makefile
Simon Marchi [Fri, 8 Apr 2022 14:56:41 +0000 (10:56 -0400)]
Pass PKG_CONFIG_PATH down from top-level Makefile

[Sending to binutils, gdb-patches and gcc-patches, since it touches the
top-level Makefile/configure]

I have my debuginfod library installed in a non-standard location
(/opt/debuginfod), which requires me to set
PKG_CONFIG_PATH=/opt/debuginfod/lib/pkg-config.  If I just set it during
configure:

    $ PKG_CONFIG_PATH=/opt/debuginfod/lib/pkg-config ./configure --with-debuginfod
    $ make

or

    $ ./configure --with-debuginfod PKG_CONFIG_PATH=/opt/debuginfod/lib/pkg-config
    $ make

Then PKG_CONFIG_PATH is only present (and ignored) during the top-level
configure.  When running make (which runs gdb's and binutils'
configure), PKG_CONFIG_PATH is not set, which results in their configure
script not finding the library:

    checking for libdebuginfod >= 0.179... no
    configure: error: "--with-debuginfod was given, but libdebuginfod is missing or unusable."

Change the top-level configure/Makefile system to capture the value
passed when configuring the top-level and pass it down to
subdirectories (similar to CFLAGS, LDFLAGS, etc).

I don't know much about the top-level build system, so I really don't
know if I did this correctly.  The changes are:

 - Use AC_SUBST(PKG_CONFIG_PATH) in configure.ac, so that
   @PKG_CONFIG_PATH@ gets replaced with the actual PKG_CONFIG_PATH value
   in config files (i.e. Makefile)
 - Add a PKG_CONFIG_PATH Makefile variable in Makefile.tpl, initialized
   to @PKG_CONFIG_PATH@
 - Add PKG_CONFIG_PATH to HOST_EXPORTS in Makefile.tpl, which are the
   variables set when running the sub-configures

I initially added PKG_CONFIG_PATH to flags_to_pass, in Makefile.def, but
I don't think it's needed.  AFAIU, this defines the flags to pass down
when calling "make" in subdirectories.  We only need PKG_CONFIG_PATH to
be passed down during configure.  After that, it's captured in
gdb/config.status, so even if a "make" causes a re-configure later
(because gdb/configure has changed, for example), the PKG_CONFIG_PATH
value will be remembered.

ChangeLog:

* configure.ac: Add AC_SUBST(PKG_CONFIG_PATH).
* configure: Re-generate.
* Makefile.tpl (HOST_EXPORTS): Pass PKG_CONFIG_PATH.
(PKG_CONFIG_PATH): New.
* Makefile.in: Re-generate.

Change-Id: I91138dfca41c43b05e53e445f62e4b27882536bf

2 years agogdb/testsuite: use nopie in gdb.dwarf2/dw2-inline-param.exp
Simon Marchi [Thu, 7 Apr 2022 19:08:57 +0000 (15:08 -0400)]
gdb/testsuite: use nopie in gdb.dwarf2/dw2-inline-param.exp

I see this failure:

    (gdb) run ^M
    Starting program: /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-inline-param/dw2-inline-param ^M
    Warning:^M
    Cannot insert breakpoint 1.^M
    Cannot access memory at address 0x113b^M
    ^M
    (gdb) FAIL: gdb.dwarf2/dw2-inline-param.exp: runto: run to *0x113b

The test loads the binary in GDB, grabs the address of a symbol, strips
the binary, reloads it in GDB, runs the program, and then tries to place
a breakpoint at that address.  The problem is that the binary is built
as position independent, so the address GDB grabs in the first place
isn't where the code ends up after running.

Fix this by linking the binary as non-position-independent.  The
alternative would be to compute the relocated address where to place the
breakpoint, but that's not very straightforward, unfortunately.

I was confused for a while, I was trying to load the binary in GDB
manually to get the symbol address, but GDB was telling me the symbol
could not be found.  Meanwhile, it clearly worked in gdb.log.  The thing
is that GDB strips the binary in-place, so we don't have access to the
intermediary binary with symbols.  Change the test to output the
stripped binary to a separate file instead.

Change-Id: I66c56293df71b1ff49cf748d6784bd0e935211ba

2 years agogdb maintainer commit rights
Alan Modra [Fri, 8 Apr 2022 11:08:50 +0000 (20:38 +0930)]
gdb maintainer commit rights

Formalise what ought to be obvious.  The top level of the binutils-gdb
repository isn't owned by binutils.

* MAINTAINERS: Spelling fix.  GDB global maintainer rights.

2 years agogdb/fortran: print fortran extended types with ptype
Bernhard Heckel [Tue, 5 Apr 2022 15:44:46 +0000 (17:44 +0200)]
gdb/fortran: print fortran extended types with ptype

Add the print of the base-class of an extended type to the output of
ptype.  This requires the Fortran compiler to emit DW_AT_inheritance
for the extended type.

Co-authored-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
2 years agogdb/fortran: add support for accessing fields of extended types
Bernhard Heckel [Tue, 5 Apr 2022 15:44:46 +0000 (17:44 +0200)]
gdb/fortran: add support for accessing fields of extended types

Fortran 2003 supports type extension.  This patch allows access
to inherited members by using their fully qualified name as described
in the Fortran standard.

In doing so the patch also fixes a bug in GDB when trying to access the
members of a base class in a derived class via the derived class' base
class member.

This patch fixes PR22497 and PR26373 on GDB side.

Using the example Fortran program from PR22497

program mvce
  implicit none

  type :: my_type
     integer :: my_int
  end type my_type

  type, extends(my_type) :: extended_type
  end type extended_type

  type(my_type) :: foo
  type(extended_type) :: bar

  foo%my_int = 0
  bar%my_int = 1

  print*, foo, bar

end program mvce

and running this with GDB and setting a BP at 17:

Before:
(gdb) p bar%my_type
A syntax error in expression, near `my_type'.
(gdb) p bar%my_int
There is no member named my_int.
(gdb) p bar%my_type%my_int
A syntax error in expression, near `my_type%my_int'.
(gdb) p bar
$1 = ( my_type = ( my_int = 1 ) )

After:
(gdb) p bar%my_type
$1 = ( my_int = 1 )
(gdb) p bar%my_int
$2 = 1                 # this line requires DW_TAG_inheritance to work
(gdb) p bar%my_type%my_int
$3 = 1
(gdb) p bar
$4 = ( my_type = ( my_int = 1 ) )

In the above example "p bar%my_int" requires the compiler to emit
information about the inheritance relationship between extended_type
and my_type which gfortran and flang currently do not de.  The
respective issue gcc/49475 has been put as kfail.

Co-authored-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26373
     https://sourceware.org/bugzilla/show_bug.cgi?id=22497

2 years agogdb: add Nils-Christian Kempke to gdb/MAINTAINERS
Nils-Christian Kempke [Fri, 8 Apr 2022 10:03:45 +0000 (12:03 +0200)]
gdb: add Nils-Christian Kempke to gdb/MAINTAINERS

Signed-off-by: Nils-Christian Kempke <nils-christian.kempke@intel.com>
2 years agogdb: change file_file_name to return an std::string
Simon Marchi [Thu, 7 Apr 2022 20:43:05 +0000 (16:43 -0400)]
gdb: change file_file_name to return an std::string

Straightforward change, return an std::string instead of a
gdb::unique_xmalloc_ptr<char>.  No behavior change expected.

Change-Id: Ia5e94c94221c35f978bb1b7bdffbff7209e0520e

2 years agoAutomatic date update in version.in
GDB Administrator [Fri, 8 Apr 2022 00:00:12 +0000 (00:00 +0000)]
Automatic date update in version.in

2 years agogdb/fortran: fix fetching assumed rank array content
Andrew Burgess [Thu, 7 Apr 2022 13:46:18 +0000 (14:46 +0100)]
gdb/fortran: fix fetching assumed rank array content

Commit:

  commit df7a7bdd9766adebc6b117c31bc617d81c1efd43
  Date:   Thu Mar 17 18:56:23 2022 +0000

      gdb: add support for Fortran's ASSUMED RANK arrays

Added support for Fortran assumed rank arrays.  Unfortunately, this
commit contained a bug that means though GDB can correctly calculate
the rank of an assumed rank array, GDB can't fetch the contents of an
assumed rank array.

The history of this patch can be seen on the mailing list here:

  https://sourceware.org/pipermail/gdb-patches/2022-January/185306.html

The patches that were finally committed can be found here:

  https://sourceware.org/pipermail/gdb-patches/2022-March/186906.html

The original patches did support fetching the array contents, it was
only the later series that introduced the regression.

The problem is that when calculating the array rank the result is a
count of the number of ranks, i.e. this is a 1 based result, 1, 2, 3,
etc.

In contrast, when computing the details of any particular rank the
value passed to the DWARF expression evaluator should be a 0 based
rank offset, i.e. a 0 based number, 0, 1, 2, etc.

In the patches that were originally merged, this was not the case, and
we were passing the 1 based rank number to the expression evaluator,
e.g. passing 1 when we should pass 0, 2 when we should pass 1, etc.
As a result the DWARF expression evaluator was reading the
wrong (undefined) memory, and returning garbage results.

In this commit I have extended the test case to cover checking the
array contents, I've then ensured we make use of the correct rank
value, and extended some comments, and added or adjusted some asserts
as appropriate.

2 years agogdb/testsuite: add "macros" option to gdb_compile
Simon Marchi [Thu, 7 Apr 2022 01:51:57 +0000 (21:51 -0400)]
gdb/testsuite: add "macros" option to gdb_compile

Make gdb_compile handle a new "macros" option, which makes it pass the
appropriate flag to make the compiler include macro information in the
debug info.  This will help simplify tests using macros, reduce
redundant code, and make it easier to add support for a new compiler.

Right now it only handles clang specially (using -fdebug-macro) and
falls back to -g3 otherwise (which works for gcc).  Other compilers can
be added as needed.

There are some tests that are currently skipped if the compiler is nor
gcc nor clang.  After this patch, the tests will attempt to run (the -g3
fall back will be used).  That gives a chance to people using other
compilers to notice something is wrong and maybe add support for their
compiler.  If it is needed to support a compiler that doesn't have a way
to include macro information, then we can always introduce a
"skip_macro_tests" that can be used to skip over them.

Change-Id: I50cd6ab1bfbb478c1005486408e214b551364c9b

2 years agogdb: remove subfile::buildsym_compunit field
Simon Marchi [Thu, 7 Apr 2022 13:03:59 +0000 (09:03 -0400)]
gdb: remove subfile::buildsym_compunit field

It is only set, never used.

Change-Id: Ia46ed2f9da243b0ccfc4588c1b57be2a0f3939de

2 years ago[gdb/testsuite] Make gdb.base/annota1.exp more robust
Tom de Vries [Thu, 7 Apr 2022 17:25:26 +0000 (19:25 +0200)]
[gdb/testsuite] Make gdb.base/annota1.exp more robust

On openSUSE Tumbleweed I run into:
...
FAIL: gdb.base/annota1.exp: run until main breakpoint (timeout)
...

The problem is that the libthread_db message occurs at a location where it's
not expected:
...
Starting program: outputs/gdb.base/annota1/annota1 ^M
^M
^Z^Zstarting^M
^M
^Z^Zframes-invalid^M
[Thread debugging using libthread_db enabled]^M
Using host libthread_db library "/lib64/libthread_db.so.1".^M
^M
^Z^Zbreakpoints-invalid^M
^M
...

Fix this by making the matching more robust:
- rewrite the regexp such that each annotation is on a single line,
  starting with \r\n\032\032 and ending with \r\n
- add a regexp variable optional_re, that matches all possible optional
  output, and use it as a separator in the first part of the regexp

Tested on x86_64-linux.

2 years agogdb/testsuite/dwarf: simplify line number program syntax
Simon Marchi [Thu, 7 Apr 2022 01:51:55 +0000 (21:51 -0400)]
gdb/testsuite/dwarf: simplify line number program syntax

By calling `uplevel $body` in the program proc (a pattern we use at many
places), we can get rid of curly braces around each line number program
directive.  That seems like a nice small improvement to me.

Change-Id: Ib327edcbffbd4c23a08614adee56c12ea25ebc0b

2 years agogdb/testsuite/dwarf: remove two unused variables
Simon Marchi [Thu, 7 Apr 2022 01:51:54 +0000 (21:51 -0400)]
gdb/testsuite/dwarf: remove two unused variables

These variables seem to be unused, remove them.

Change-Id: I7d613d9d35735930ee78b2c348943c73a702afbb

2 years agogdb: remove symtab::pspace
Simon Marchi [Tue, 29 Mar 2022 21:07:52 +0000 (17:07 -0400)]
gdb: remove symtab::pspace

Same idea as previous patch, but for symtab::pspace.

Change-Id: I1023abe622bea75ef648c6a97a01b53775d4104d

2 years agogdb: remove symtab::objfile
Simon Marchi [Tue, 29 Mar 2022 20:14:36 +0000 (16:14 -0400)]
gdb: remove symtab::objfile

Same idea as previous patch, but for symtab::objfile.  I find
it clearer without this wrapper, as it shows that the objfile is
common to all symtabs of a given compunit.  Otherwise, you could think
that each symtab (of a given compunit) can have a specific objfile.

Change-Id: Ifc0dbc7ec31a06eefa2787c921196949d5a6fcc6

2 years agogdb: remove symtab::blockvector
Simon Marchi [Wed, 6 Apr 2022 14:42:03 +0000 (10:42 -0400)]
gdb: remove symtab::blockvector

symtab::blockvector is a wrapper around compunit_symtab::blockvector.
It is a bit misleadnig, as it gives the impression that a symtab has a
blockvector.  Remove it, change all users to fetch the blockvector
through the compunit instead.

Change-Id: Ibd062cd7926112a60d52899dff9224591cbdeebf

2 years agogdb: remove symtab::dirname
Simon Marchi [Wed, 6 Apr 2022 14:42:02 +0000 (10:42 -0400)]
gdb: remove symtab::dirname

I think the symtab::dirname method is bogus, or at least very
misleading.  It makes you think that it returns the directory that was
used to find that symtab's file during compilation (i.e. the directory
the file refers to in the DWARF line header file table), or the
directory part of the symtab's filename maybe.  In fact, it returns the
compilation unit's directory, which is the CWD of the compiler, at
compilation time.  At least for DWARF, if the symtab's filename is
relative, it will be relative to that directory.  But if the symtab's
filename is absolute, then the directory returned by symtab::dirname has
nothing to do with the symtab's filename.

Remove symtab::dirname to avoid this confusion, change all users to
fetch the same information through the compunit.  At least, it will be
clear that this is a compunit property, not a symtab property.

Change-Id: I2894c3bf3789d7359a676db3c58be2c10763f5f0

2 years agogdb/testsuite: make gdb_breakpoint and runto take a linespec
Simon Marchi [Tue, 8 Feb 2022 21:31:09 +0000 (16:31 -0500)]
gdb/testsuite: make gdb_breakpoint and runto take a linespec

Change gdb_breakpoint to accept a linespec, not just a function.  In
fact, no behavior changes are necessary, this only changes the parameter
name and documentation.  Change runto as well, since the two are so
close (runto forwards all its arguments to gdb_breakpoint).

I wrote this for a downstrean GDB port,  but thought it could be
useful upstream, eventually, even though not callers take advantage of
it yet.

Change-Id: I08175fd444d5a60df90fd9985e1b5dfd87c027cc

2 years agogdb: update comments throughout reggroups.{c,h} files
Andrew Burgess [Thu, 31 Mar 2022 17:19:23 +0000 (18:19 +0100)]
gdb: update comments throughout reggroups.{c,h} files

This commit updates the comments in the gdb/reggroups.{c,h} files.
Fill in some missing comments, correct a few comments that were not
clear, and where we had comments duplicated between .c and .h files,
update the .c to reference the .h.

No user visible changes after this commit.

2 years agogdb: move struct reggroup into reggroups.h header
Andrew Burgess [Thu, 31 Mar 2022 17:10:34 +0000 (18:10 +0100)]
gdb: move struct reggroup into reggroups.h header

Move 'struct reggroup' into the reggroups.h header.  Remove the
reggroup_name and reggroup_type accessor functions, and just use the
name/type member functions within 'struct reggroup', update all uses
of these removed functions.

There should be no user visible changes after this commit.

2 years agogdb: convert reggroup to a C++ class with constructor, etc
Andrew Burgess [Thu, 31 Mar 2022 15:44:15 +0000 (16:44 +0100)]
gdb: convert reggroup to a C++ class with constructor, etc

Convert the 'struct reggroup' into a real class, with a constructor
and getter methods.

There should be no user visible changes after this commit.

2 years agogdb: make the pre-defined register groups const
Andrew Burgess [Thu, 31 Mar 2022 15:36:21 +0000 (16:36 +0100)]
gdb: make the pre-defined register groups const

Convert the 7 global, pre-defined, register groups const, and fix the
fall out (a minor tweak required in riscv-tdep.c).

There should be no user visible changes after this commit.

2 years agogdb: more 'const' in gdb/reggroups.{c,h}
Andrew Burgess [Thu, 31 Mar 2022 15:32:50 +0000 (16:32 +0100)]
gdb: more 'const' in gdb/reggroups.{c,h}

Convert the reggroup_new and reggroup_gdbarch_new functions to return
a 'const regggroup *', and fix up all the fallout.

There should be no user visible changes after this commit.

2 years agogdb: remove reggroup_next and reggroup_prev
Andrew Burgess [Thu, 31 Mar 2022 10:43:13 +0000 (11:43 +0100)]
gdb: remove reggroup_next and reggroup_prev

Add a new function gdbarch_reggroups that returns a reference to a
vector containing all the reggroups for an architecture.

Make use of this function throughout GDB instead of the existing
reggroup_next and reggroup_prev functions.

Finally, delete the reggroup_next and reggroup_prev functions.

Most of these changes are pretty straight forward, using range based
for loops instead of the old style look using reggroup_next.  There
are two places where the changes are less straight forward.

In gdb/python/py-registers.c, the register group iterator needed to
change slightly.  As the iterator is tightly coupled to the gdbarch, I
just fetch the register group vector from the gdbarch when needed, and
use an index counter to find the next item from the vector when
needed.

In gdb/tui/tui-regs.c the tui_reg_next and tui_reg_prev functions are
just wrappers around reggroup_next and reggroup_prev respectively.
I've just inlined the logic of the old functions into the tui
functions.  As the tui function had its own special twist (wrap around
behaviour) I think this is OK.

There should be no user visible changes after this commit.

2 years agogdb: convert reggroups to use a std::vector
Andrew Burgess [Thu, 31 Mar 2022 10:15:04 +0000 (11:15 +0100)]
gdb: convert reggroups to use a std::vector

Replace manual linked list with a std::vector.  This commit doesn't
change the reggroup_next and reggroup_prev API, but that will change
in a later commit.

This commit is focused on the minimal changes needed to manage the
reggroups using a std::vector, without changing the API exposed by the
reggroup.c file.

There should be no user visible changes after this commit.

2 years agogdb: always add the default register groups
Andrew Burgess [Thu, 31 Mar 2022 09:10:54 +0000 (10:10 +0100)]
gdb: always add the default register groups

There's a set of 7 default register groups.  If we don't add any
gdbarch specific register groups during gdbarch initialisation, then
when we iterate over the register groups using reggroup_next and
reggroup_prev we will make use of these 7 default groups.  See the use
of default_groups in gdb/reggroups.c for details on this.

However, if the gdbarch adds its own groups during gdbarch
initialisation, then these groups will be used in preference to the
default groups.

A problem arises though if the particular architecture makes use of
the target description mechanism.  If the default target
description(s) (i.e. those internal to GDB that are used when the user
doesn't provide their own) don't mention any additional register
groups then the default register groups will be used.

But if the target description does mention additional groups then the
default groups are not used, and instead, the groups from the target
description are used.

The problem with this is that what usually happens is that the target
description will mention additional groups, e.g. groups for special
registers.  Most architectures that use target descriptions work
around this by adding all (or most) of the default register groups in
all cases.  See i386_add_reggroups, aarch64_add_reggroups,
riscv_add_reggroups, xtensa_add_reggroups, and others.

In this patch, my suggestion is that we should just add the default
register groups for every architecture, always.  This change is in
gdb/reggroups.c.

All the remaining changes are me updating the various architectures to
not add the default groups themselves.

So, where will this change be visible to the user?  I think the
following commands will possibly change:

* info registers / info all-registers:

  The user can provide a register group to these commands.  For example,
  on csky, we previously never added the 'vector' group.  Now, as a
  default group, this will be available, but (presumably) will not
  contain any registers.  I don't think this is necessarily a bad
  thing, there's something to be said for having some consistent
  defaults available.  There are other architectures that didn't add
  all 7 of the defaults, which will now have gained additional groups.

* maint print reggroups

  This prints the set of all available groups.  As a maintenance
  command I'm less concerned with the output changing here.
  Obviously, for the architectures that didn't previously add all the
  defaults, this list just got bigger.

* maint print register-groups

  This prints all the registers, and the groups they are in.  If the
  defaults were not previously being added then a register (obviously)
  can't appear in one of the default groups.  Now the groups are
  available then registers might be in more groups than previously.
  However, this is again a maintenance command, so I'm less concerned
  about this changing.

2 years agogdb/tui: fix 'tui reg next/prev' command when data window is hidden
Andrew Burgess [Thu, 31 Mar 2022 14:17:27 +0000 (15:17 +0100)]
gdb/tui: fix 'tui reg next/prev' command when data window is hidden

Start GDB like:

  $ gdb -q executable
  (gdb) start
  (gdb) layout src
  ... tui windows are now displayed ...
  (gdb) tui reg next

At this point the data (register) window should be displayed, but will
contain the message 'Register Values Unavailable', and at the console
you'll see the message "unknown register group 'next'".

The same happens with 'tui reg prev' (but the error message is
slightly different).

At this point you can continue to use 'tui reg next' and/or 'tui reg
prev' and you'll keep getting the error message.

The problem is that when the data (register) window is first
displayed, it's current register group is nullptr.  As a consequence
tui_reg_next and tui_reg_prev (tui/tui-regs.c) will always just return
nullptr, which triggers an error in tui_reg_command.

In this commit I change tui_reg_next and tui_reg_prev so that they
instead return the first and last register group respectively if the
current register group is nullptr.

So, after this, using 'tui reg next' will (in the above case) show the
first register group, while 'tui reg prev' will display the last
register group.

2 years agogdb/tui: avoid theoretical bug with 'tui reg' command
Andrew Burgess [Thu, 31 Mar 2022 13:44:25 +0000 (14:44 +0100)]
gdb/tui: avoid theoretical bug with 'tui reg' command

While looking at the 'tui reg' command as part of another patch, I
spotted a theoretical bug.

The 'tui reg' command takes the name of a register group, but also
handles partial register group matches, though the partial match has to
be unique.  The current command logic goes:

With the code as currently written, if a target description named a
register group either 'prev' or 'next' then GDB would see this as an
ambiguous register name, and refuse to switch groups.

Naming a register group 'prev' or 'next' seems pretty unlikely, but,
by adding a single else block we can prevent this problem.

Now, if there's a 'prev' or 'next' register group, the user will not
be able to select the group directly, the 'prev' and 'next' names will
always iterate through the available groups instead.  But at least the
user could select their groups by iteration, rather than direct
selection.

2 years agogdb: have reggroup_find return a const
Andrew Burgess [Thu, 31 Mar 2022 15:24:32 +0000 (16:24 +0100)]
gdb: have reggroup_find return a const

Update reggroup_find to return a const reggroup *.

There are other function in gdb/reggroup.{c,h} files that could
benefit from returning const, these will be updated in later commits.

There should be no user visible changes after this commit.

2 years agogdb: use 'const reggroup *' in python/py-registers.c file
Andrew Burgess [Thu, 31 Mar 2022 12:03:41 +0000 (13:03 +0100)]
gdb: use 'const reggroup *' in python/py-registers.c file

Convert uses of 'struct reggroup *' in python/py-registers.c to be
'const'.

There should be no user visible changes after this commit.

2 years agogdb: switch to using 'const reggroup *' in tui-regs.{c,h}
Andrew Burgess [Thu, 31 Mar 2022 11:49:50 +0000 (12:49 +0100)]
gdb: switch to using 'const reggroup *' in tui-regs.{c,h}

Make uses of 'reggroup *' const throughout tui-regs.{c,h}.

There should be no user visible changes after this commit.

2 years agogdb: make gdbarch_register_reggroup_p take a const reggroup *
Andrew Burgess [Thu, 31 Mar 2022 11:36:06 +0000 (12:36 +0100)]
gdb: make gdbarch_register_reggroup_p take a const reggroup *

Change gdbarch_register_reggroup_p to take a 'const struct reggroup *'
argument.  This requires a change to the gdb/gdbarch-components.py
script, regeneration of gdbarch.{c,h}, and then updates to all the
architectures that implement this method.

There should be no user visible changes after this commit.

2 years agogdb: add some const in gdb/reggroups.c
Andrew Burgess [Wed, 30 Mar 2022 14:53:42 +0000 (15:53 +0100)]
gdb: add some const in gdb/reggroups.c

This commit makes the 'struct reggroup *' argument const for the
following functions:

  reggroup_next
  reggroup_prev
  reggroup_name
  reggroup_type

There are other places that could benefit from const in the
reggroup.{c,h} files, but these will be changing in further commits.

There should be no user visible changes after this commit.

2 years agogdb: don't try to use readline before it's initialized
Andrew Burgess [Wed, 30 Mar 2022 13:49:11 +0000 (14:49 +0100)]
gdb: don't try to use readline before it's initialized

While working on a different patch, I triggered an assertion from the
initialize_current_architecture code, specifically from one of
the *_gdbarch_init functions in a *-tdep.c file.  This exposes a
couple of issues with GDB.

This is easy enough to reproduce by adding 'gdb_assert (false)' into a
suitable function.  For example, I added a line into i386_gdbarch_init
and can see the following issue.

I start GDB and immediately hit the assert, the output is as you'd
expect, except for the very last line:

  $ ./gdb/gdb --data-directory ./gdb/data-directory/
  ../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  ... snip ...
  ---------------------
  ../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) ../../src.dev-1/gdb/ser-event.c:212:16: runtime error: member access within null pointer of type 'struct serial'

Something goes wrong when we try to query the user.  Note, I
configured GDB with --enable-ubsan, I suspect that without this the
above "error" would actually just be a crash.

The backtrace from ser-event.c:212 looks like this:

  (gdb) bt 10
  #0  serial_event_clear (event=0x675c020) at ../../src/gdb/ser-event.c:212
  #1  0x0000000000769456 in invoke_async_signal_handlers () at ../../src/gdb/async-event.c:211
  #2  0x000000000295049b in gdb_do_one_event () at ../../src/gdbsupport/event-loop.cc:194
  #3  0x0000000001f015f8 in gdb_readline_wrapper (
      prompt=0x67135c0 "../../src/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugg"...)
      at ../../src/gdb/top.c:1141
  #4  0x0000000002118b64 in defaulted_query(const char *, char, typedef __va_list_tag __va_list_tag *) (
      ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ", defchar=0 '\000', args=0x7fffffffa6e0)
      at ../../src/gdb/utils.c:934
  #5  0x0000000002118f72 in query (ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ")
      at ../../src/gdb/utils.c:1026
  #6  0x00000000021170f6 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=0x6107bc0 <internal_error_problem>, file=0x2b976c8 "../../src/gdb/i386-tdep.c",
      line=8455, fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:417
  #7  0x00000000021175a0 in internal_verror (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455,
      fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:485
  #8  0x00000000029503b3 in internal_error (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455,
      fmt=0x2b96d7f "%s: Assertion `%s' failed.") at ../../src/gdbsupport/errors.cc:55
  #9  0x000000000122d5b6 in i386_gdbarch_init (info=..., arches=0x0) at ../../src/gdb/i386-tdep.c:8455
  (More stack frames follow...)

It turns out that the problem is that the async event handler
mechanism has been invoked, but this has not yet been initialized.

If we look at gdb_init (in gdb/top.c) we can indeed see the call to
gdb_init_signals is after the call to initialize_current_architecture.

If I reorder the calls, moving gdb_init_signals earlier, then the
initial error is resolved, however, things are still broken.  I now
see the same "Quit this debugging session? (y or n)" prompt, but when
I provide an answer and press return GDB immediately crashes.

So what's going on now?  The next problem is that the call_readline
field within the current_ui structure is not initialized, and this
callback is invoked to process the reply I entered.

The problem is that call_readline is setup as a result of calling
set_top_level_interpreter, which is called from captured_main_1.
Unfortunately, set_top_level_interpreter is called after gdb_init is
called.

I wondered how to solve this problem for a while, however, I don't
know if there's an easy "just reorder some lines" solution here.
Looking through captured_main_1 there seems to be a bunch of
dependencies between printing various things, parsing config files,
and setting up the interpreter.  I'm sure there is a solution hiding
in there somewhere.... I'm just not sure I want to spend any longer
looking for it.

So.

I propose a simpler solution, more of a hack/work-around.  In utils.c
we already have a function filtered_printing_initialized, this is
checked in a few places within internal_vproblem.  In some of these
cases the call gates whether or not GDB will query the user.

My proposal is to add a new readline_initialized function, which
checks if the current_ui has had readline initialized yet.  If this is
not the case then we should not attempt to query the user.

After this change GDB prints the error message, the backtrace, and
then aborts (including dumping core).  This actually seems pretty sane
as, if GDB has not yet made it through the initialization then it
doesn't make much sense to allow the user to say "no, I don't want to
quit the debug session" (I think).

2 years agoRecognize the NT_ARM_SYSTEM_CALL register set
Luis Machado [Mon, 4 Apr 2022 08:38:48 +0000 (09:38 +0100)]
Recognize the NT_ARM_SYSTEM_CALL register set

Update binutils to recognize the NT_ARM_SYSTEM_CALL set that is dumped by
Linux to core files.

2 years agoAdd support for COFF secidx relocations
Mark Harmstone [Thu, 7 Apr 2022 13:47:17 +0000 (14:47 +0100)]
Add support for COFF secidx relocations

bfd * coff-i386.c (in_reloc_p): Add R_SECTION.
(howto_table): Add R_SECTION.
(coff_pe_i386_relocation_section): Add support for R_SECTION.
(coff_i386_reloc_type_lookup): Add support for
BFD_RELOC_16_SECCIDX.
* coff-x86_64.c (in_reloc_p): Add R_SECTION.
(howto_table): Add R_SECTION.
(coff_pe_amd64_relocation_section): Add support for R_SECTION.
(coff_amd64_reloc_type_lookup): Add support for
BFD_RELOC_16_SECCIDX.
* reloc.c: Add BFD_RELOC_16_SECIDX.
* bfd-in2.h: Regenerate.
* libbfd.h: Regenerate.

gas * config/tc-i386.c (pe_directive_secidx): New function.
(md_pseudo_table): Add support for secidx.
(x86_cons_fix_new): Likewise.
(tc_gen_reloc): Likewise.
* expr.c (op_rank): Add O_secidx.
* expr.h (operatorT): Likewise.
* symbols.c (resolve_symbol_value): Add support for O_secidx.
* testsuite/gas/i386/secidx.s: New test source file.
* testsuite/gas/i386/secidx.d: New test driver file.
* testsuite/gas/i386/i386.exp: Run new test.

include * coff/i386.h: Define R_SECTION.
* coff/x86_64.h: Likewise.

ld * testsuite/ld-pe/secidx1.s: New test source file.
* testsuite/ld-pe/secidx2.s: New test source file.
* testsuite/ld-pe/secidx.d: New test driver file.
* testsuite/ld-pe/secidx_64.d: New test driver file.
* testsuite/ld-pe/pe.exp: Add new tests.

2 years agogas/Dwarf: record functions
Jan Beulich [Thu, 7 Apr 2022 06:18:00 +0000 (08:18 +0200)]
gas/Dwarf: record functions

To help tools like addr2line looking up function names, in particular
when dealing with e.g. PE/COFF binaries (linked from ELF objects), where
there's no ELF symbol table to fall back to, emit minimalistic
information for functions marked as such and having their size
specified.

Notes regarding the restriction to (pure) ELF:
- I realize this is a layering violation; I don't see how to deal with
  that in a better way.
- S_GET_SIZE(), when OBJ_MAYBE_ELF is defined, looks wrong: Unlike
  S_SET_SIZE() it does not check whether the hook is NULL.
- symbol_get_obj(), when OBJ_MAYBE_ELF is defined, looks unusable, as
  its return type can only ever be one object format's type (and this
  may then not be ELF's).

The new testcases are limited to x86 because I wanted to include the
case where function size can't be determined yet at the time Dwarf2 info
is generated. As .nops gains support by further targets, they could also
be added here then (with, as necessary, expecations suitably relaxed to
cover for insn size differences).

2 years agoArm64: arrange for line number emission for .inst
Jan Beulich [Thu, 7 Apr 2022 06:16:29 +0000 (08:16 +0200)]
Arm64: arrange for line number emission for .inst

Just like insns encoded the more conventional way these should have line
number info associated with them.

2 years agoArm32: arrange for line number emission for .inst
Jan Beulich [Thu, 7 Apr 2022 06:15:24 +0000 (08:15 +0200)]
Arm32: arrange for line number emission for .inst

Just like insns encoded the more conventional way these should have line
number info associated with them.

2 years agoRISC-V: add testcase to check line number emission for .insn
Jan Beulich [Thu, 7 Apr 2022 06:14:33 +0000 (08:14 +0200)]
RISC-V: add testcase to check line number emission for .insn

Since no such test looks to exist, derive one from insn.s.

2 years agoIBM zSystems: Add support for z16 as CPU name.
Andreas Krebbel [Thu, 7 Apr 2022 05:45:49 +0000 (07:45 +0200)]
IBM zSystems: Add support for z16 as CPU name.

So far z16 was identified as arch14. After the machine has been
announced we can now add the real name.

gas/ChangeLog:

* config/tc-s390.c (s390_parse_cpu): Add z16 as alternate CPU
name.
* doc/as.texi: Add z16 and arch14 to CPU string list.
* doc/c-s390.texi: Add z16 to CPU string list.

opcodes/ChangeLog:

* s390-mkopc.c (main): Enable z16 as CPU string in the opcode
table.

2 years agoAutomatic date update in version.in
GDB Administrator [Thu, 7 Apr 2022 00:00:11 +0000 (00:00 +0000)]
Automatic date update in version.in

2 years agogdb: mips: Fix the handling of complex type of function return value
Youling Tang [Wed, 6 Apr 2022 22:38:21 +0000 (23:38 +0100)]
gdb: mips: Fix the handling of complex type of function return value

$ objdump -d outputs/gdb.base/varargs/varargs
00000001200012e8 <find_max_float_real>:
...
   1200013b8:   c7c10000        lwc1    $f1,0(s8)
   1200013bc:   c7c00004        lwc1    $f0,4(s8)
   1200013c0:   46000886        mov.s   $f2,$f1
   1200013c4:   46000046        mov.s   $f1,$f0
   1200013c8:   46001006        mov.s   $f0,$f2
   1200013cc:   46000886        mov.s   $f2,$f1
   1200013d0:   03c0e825        move    sp,s8
   1200013d4:   dfbe0038        ld      s8,56(sp)
   1200013d8:   67bd0080        daddiu  sp,sp,128
   1200013dc:   03e00008        jr      ra
   1200013e0:   00000000        nop

From the above disassembly, we can see that when the return value of the
function is a complex type and len <= 2 * MIPS64_REGSIZE, the return value
will be passed through $f0 and $f2, so fix the corresponding processing
in mips_n32n64_return_value().

$ make check RUNTESTFLAGS='GDB=../gdb gdb.base/varargs.exp --outdir=test'

Before applying the patch:
 FAIL: gdb.base/varargs.exp: print find_max_float_real(4, fc1, fc2, fc3, fc4)
 FAIL: gdb.base/varargs.exp: print find_max_double_real(4, dc1, dc2, dc3, dc4)

 # of expected passes            9
 # of unexpected failures        2

After applying the patch:
 # of expected passes            11

This also fixes:
 FAIL: gdb.base/callfuncs.exp: call inferior func with struct - returns float _Complex

Signed-off-by: Youling Tang <tangyouling@loongson.cn>
Co-Authored-By: Maciej W. Rozycki <macro@orcam.me.uk>
2 years agoUse new and delete in jit.c
Tom Tromey [Tue, 5 Apr 2022 20:43:30 +0000 (14:43 -0600)]
Use new and delete in jit.c

This changes jit.c to use new and delete, rather than XCNEW.  This
simplifies the code a little.  This was useful for another patch I'm
working on, and I thought it would make sense to send it separately.

Regression tested on x86-64 Fedora 34.

2 years agogdb: don't copy entirely optimized out values in value_copy
Simon Marchi [Mon, 4 Apr 2022 21:45:59 +0000 (17:45 -0400)]
gdb: don't copy entirely optimized out values in value_copy

Bug 28980 shows that trying to value_copy an entirely optimized out
value causes an internal error.  The original bug report involves MI and
some Python pretty printer, and is quite difficult to reproduce, but
another easy way to reproduce (that is believed to be equivalent) was
proposed:

    $ ./gdb -q -nx --data-directory=data-directory -ex "py print(gdb.Value(gdb.Value(5).type.optimized_out()))"
    /home/smarchi/src/binutils-gdb/gdb/value.c:1731: internal-error: value_copy: Assertion `arg->contents != nullptr' failed.

This is caused by 5f8ab46bc691 ("gdb: constify parameter of
value_copy").  It added an assertion that the contents buffer is
allocated if the value is not lazy:

  if (!value_lazy (val))
    {
      gdb_assert (arg->contents != nullptr);

This was based on the comment on value::contents, which suggest that
this is the case:

  /* Actual contents of the value.  Target byte-order.  NULL or not
     valid if lazy is nonzero.  */
  gdb::unique_xmalloc_ptr<gdb_byte> contents;

However, it turns out that it can also be nullptr also if the value is
entirely optimized out, for example on exit of
allocate_optimized_out_value.  That function creates a lazy value, marks
the entire value as optimized out, and then clears the lazy flag.  But
contents remains nullptr.

This wasn't a problem for value_copy before, because it was calling
value_contents_all_raw on the input value, which caused contents to be
allocated before doing the copy.  This means that the input value to
value_copy did not have its contents allocated on entry, but had it
allocated on exit.  The result value had it allocated on exit.  And that
we copied bytes for an entirely optimized out value (i.e. meaningless
bytes).

From here I see two choices:

 1. respect the documented invariant that contents is nullptr only and
    only if the value is lazy, which means making
    allocate_optimized_out_value allocate contents
 2. extend the cases where contents can be nullptr to also include
    values that are entirely optimized out (note that you could still
    have some entirely optimized out values that do have contents
    allocated, it depends on how they were created) and adjust
    value_copy accordingly

Choice #1 is safe, but less efficient: it's not very useful to allocate
a buffer for an entirely optimized out value.  It's even a bit less
efficient than what we had initially, because values coming out of
allocate_optimized_out_value would now always get their contents
allocated.

Choice #2 would be more efficient than what we had before: giving an
optimized out value without allocated contents to value_copy would
result in an optimized out value without allocated contents (and the
input value would still be without allocated contents on exit).  But
it's more risky, since it's difficult to ensure that all users of the
contents (through the various_contents* accessors) are all fine with
that new invariant.

In this patch, I opt for choice #2, since I think it is a better
direction than choice #1.  #1 would be a pessimization, and if we go
this way, I doubt that it will ever be revisited, it will just stay that
way forever.

Add a selftest to test this.  I initially started to write it as a
Python test (since the reproducer is in Python), but a selftest is more
straightforward.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28980
Change-Id: I6e2f5c0ea804fafa041fcc4345d47064b5900ed7

2 years agoFix for v850e divq instruction
Jeff Law [Wed, 6 Apr 2022 15:10:40 +0000 (11:10 -0400)]
Fix for v850e divq instruction

This is the last of the correctness fixes I've been carrying around for the
v850.

Like the other recent fixes, this is another case where we haven't been as
careful as we should WRT host vs target types.   For the divq instruction
both operands are 32 bit types.  Yet in the simulator code we convert them
from unsigned int to signed long by assignment.  So 0xfffffffb (aka -5)
turns into 4294967291 and naturally that changes the result of our division.

The fix is simple, insert a cast to int32_t to force interpretation as a
signed value.

Testcase for the simulator is included.  It has a trivial dependency on the
bins patch.

2 years agoFix "bins" simulation for v850e3v5
Jeff Law [Wed, 6 Apr 2022 15:06:53 +0000 (11:06 -0400)]
Fix "bins" simulation for v850e3v5

I've been carrying this for a few years.   One test in the GCC testsuite is
failing due to a bug in the handling of the v850e3v5 instruction "bins".

When the "bins" instruction specifies a 32bit bitfield size, the simulator
exhibits undefined behavior by trying to shift a 32 bit quantity by 32 bits.
In the case of a 32 bit shift, we know what the resultant mask should be.  So
we can just set it.

That seemed better than using 1UL for the constant (on a 32bit host unsigned
long might still just be 32 bits) or needlessly forcing everything to
long long types.

Thankfully the case where this shows up is only bins <src>, 0, 32, <dest>
which would normally be encoded as a simple move.

* testsuite/v850/allinsns.exp: Add v850e3v5.
* testsuite/v850/bins.cgs: New test.
* v850/simops.c (v850_bins): Avoid undefined behavior on left shift.

2 years agogdb: LoongArch: prepend tramp frame unwinder for signal
Tiezhu Yang [Thu, 31 Mar 2022 11:56:51 +0000 (19:56 +0800)]
gdb: LoongArch: prepend tramp frame unwinder for signal

Implement the "init" method of struct tramp_frame to prepend tramp
frame unwinder for signal on LoongArch.

With this patch, the following failed testcases can be fixed:

  FAIL: gdb.base/annota1.exp: backtrace @ signal handler (timeout)
  FAIL: gdb.base/annota3.exp: backtrace @ signal handler (pattern 2)

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
2 years agogdb: make interp_add static
Andrew Burgess [Wed, 24 Nov 2021 17:18:34 +0000 (17:18 +0000)]
gdb: make interp_add static

Since this commit:

  commit 8322445e0584be846f5873b9aab257dc9fbda05d
  Date:   Tue Jun 21 01:11:45 2016 +0100

      Introduce interpreter factories

Interpreters should be registered with GDB, not by calling interp_add,
but with a call to interp_factory_register.  I've checked the insight
source, and it too has moved over to using interp_factory_register.

In this commit I make interp_add static within interps.c.

There should be no user visible change after this commit.

2 years agoAdd code to display the contents of .debug_loclists sections which contain offset...
Nick Clifton [Wed, 6 Apr 2022 13:43:37 +0000 (14:43 +0100)]
Add code to display the contents of .debug_loclists sections which contain offset entry tables.

PR 28981
* dwarf.c (fetch_indexed_value): Rename to fecth_indexed_addr and
return the address, rather than a string.
(fetch_indexed_value): New function - returns a value indexed by a
DW_FORM_loclistx or DW_FORM_rnglistx form.
(read_and_display_attr_value): Add support for DW_FORM_loclistx
and DW_FORM_rnglistx.
(process_debug_info): Load the loclists and rnglists sections.
(display_loclists_list): Add support for DW_LLE_base_addressx,
DW_LLE_startx_endx, DW_LLE_startx_length and
DW_LLE_default_location.
(display_offset_entry_loclists): New function.  Displays a
.debug_loclists section that contains offset entry tables.
(display_debug_loc): Call the new function.
(display_debug_rnglists_list): Add support for
DW_RLE_base_addressx, DW_RLE_startx_endx and DW_RLE_startx_length.
(display_debug_ranges): Display the contents of the section's
header.
* dwarf.h (struct debug_info): Add loclists_base field.
* testsuite/binutils-all/dw5.W: Update expected output.
* testsuite/binutils-all/x86-64/pr26808.dump: Likewise.

2 years agoEnable ARMv8.1-m PACBTI support
Luis Machado [Mon, 1 Nov 2021 20:14:26 +0000 (17:14 -0300)]
Enable ARMv8.1-m PACBTI support

This set of changes enable support for the ARMv8.1-m PACBTI extensions [1].

The goal of the PACBTI extensions is similar in scope to that of a-profile
PAC/BTI (aarch64 only), but the underlying implementation is different.

One important difference is that the pointer authentication code is stored
in a separate register, thus we don't need to mask/unmask the return address
from a function in order to produce a correct backtrace.

The patch introduces the following modifications:

- Extend the prologue analyser for 32-bit ARM to handle some instructions
from ARMv8.1-m PACBTI: pac, aut, pacg, autg and bti. Also keep track of
return address signing/authentication instructions.

- Adds code to identify object file attributes that indicate the presence of
ARMv8.1-m PACBTI (Tag_PAC_extension, Tag_BTI_extension, Tag_PACRET_use and
Tag_BTI_use).

- Adds support for DWARF pseudo-register RA_AUTH_CODE, as described in the
aadwarf32 [2].

- Extends the dwarf unwinder to track the value of RA_AUTH_CODE.

- Decorates backtraces with the "[PAC]" identifier when a frame has signed
the return address.

- Makes GDB aware of a new XML feature "org.gnu.gdb.arm.m-profile-pacbti". This
feature is not included as an XML file on GDB's side because it is only
supported for bare metal targets.

- Additional documentation.

[1] https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension
[2] https://github.com/ARM-software/abi-aa/blob/main/aadwarf32/aadwarf32.rst

2 years agogdb: move gdb_disassembly_flag into a new disasm-flags.h file
Andrew Burgess [Mon, 4 Apr 2022 20:07:54 +0000 (21:07 +0100)]
gdb: move gdb_disassembly_flag into a new disasm-flags.h file

While working on the disassembler I was getting frustrated.  Every
time I touched disasm.h it seemed like every file in GDB would need to
be rebuilt.  Surely the disassembler can't be required by that many
parts of GDB, right?

Turns out that disasm.h is included in target.h, so pretty much every
file was being rebuilt!

The only thing from disasm.h that target.h needed is the
gdb_disassembly_flag enum, as this is part of the target_ops api.

In this commit I move gdb_disassembly_flag into its own file.  This is
then included in target.h and disasm.h, after which, the number of
files that depend on disasm.h is much reduced.

I also audited all the other includes of disasm.h and found that the
includes in mep-tdep.c and python/py-registers.c are no longer needed,
so I've removed these.

Now, after changing disasm.h, GDB rebuilds much quicker.

There should be no user visible changes after this commit.

2 years agoAutomatic date update in version.in
GDB Administrator [Wed, 6 Apr 2022 00:00:09 +0000 (00:00 +0000)]
Automatic date update in version.in

2 years agoIntroduce wrapped_file
Tom Tromey [Tue, 5 Apr 2022 13:44:59 +0000 (07:44 -0600)]
Introduce wrapped_file

Simon pointed out that timestamped_file probably needed to implement a
few more methods.  This patch introduces a new file-wrapping file that
forwards most of its calls, making it simpler to implement new such
files.  It also converts timestamped_file and pager_file to use it.

Regression tested on x86-64 Fedora 34.

2 years agoDon't call init_thread_list in windows-nat.c
Tom Tromey [Tue, 5 Apr 2022 13:25:10 +0000 (07:25 -0600)]
Don't call init_thread_list in windows-nat.c

I don't think there's any need to call init_thread_list in
windows-nat.c.  This patch removes it.  I tested this using the
internal AdaCore test suite on Windows, which FWIW does include some
multi-threaded inferiors.

2 years agogdb/testsuite: fix intermittent failure in gdb.base/vfork-follow-parent.exp
Simon Marchi [Mon, 4 Apr 2022 18:45:36 +0000 (14:45 -0400)]
gdb/testsuite: fix intermittent failure in gdb.base/vfork-follow-parent.exp

Tom de Vries reported some failures in this test:

    continue
    Continuing.
    [New inferior 2 (process 14967)]

    Thread 1.1 "vfork-follow-pa" hit Breakpoint 2, break_parent () at /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/vfork-follow-parent.c:23
    23 }
    (gdb) FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to end of inferior 2
    inferior 1
    [Switching to inferior 1 [process 14961] (/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.base/vfork-follow-parent/vfork-follow-parent)]
    [Switching to thread 1.1 (process 14961)]
    #0  break_parent () at /home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/vfork-follow-parent.c:23
    23 }
    (gdb) PASS: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: inferior 1
    continue
    Continuing.
    [Inferior 2 (process 14967) exited normally]
    (gdb) FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to break_parent (the program exited)

Here, we continue both the vfork parent and child, since
schedule-multiple is on.  The child exits, which un-freezes the parent
and makes an exit event available to GDB.  We expect GDB to consume this
exit event and present it to the user.  Here, we see that GDB shows the
parent hitting a breakpoint before showing the child exit.

Because of the vfork, we know that chronologically, the child exiting
must have happend before the parent hitting a breakpoint.  However,
scheduling being what it is, it is possible for the parent to un-freeze
and exit quickly, such that when GDB pulls events out of the kernel,
exit events for both processes are available.  And then, GDB may chose
at random to return the one for the parent first.  This is what I
imagine what causes the failure shown above.

We could change the test to expect both possible outcomes, but I wanted
to avoid complicating the .exp file that way.  Instead, add a variable
that the parent loops on that we set only after we confirmed the exit of
the child.  That should ensure that the order is always the same.

Note that I wasn't able to reproduce the failure, so I can't tell if
this fix really fixes the problem.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29021
Change-Id: Ibc8e527e0e00dac54b22021fe4d9d8ab0f3b28ad

2 years agogdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp
Simon Marchi [Mon, 4 Apr 2022 16:08:22 +0000 (12:08 -0400)]
gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp

I got failures like this once on a CI:

    frame^M
    &"frame\n"^M
    ~"#0  child_sub_function () at /home/jenkins/workspace/binutils-gdb_master_build/arch/amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:33\n"^M
    ~"33\t    dummy = !dummy; /* thread loop line */\n"^M
    ^done^M
    (gdb) ^M
    FAIL: gdb.mi/mi-cmd-user-context.exp: frame 1 (unexpected output)

The problem is that the test expects the following regexp:

  ".*#0  0x.*"

And that typically works, when the output of the frame command looks
like:

  #0  0x00005555555551bb in child_sub_function () at ...

Note the lack of hexadecimal address in the failing case.  Whether or
not the hexadecimal address is printed (roughly) depends on whether the
current PC is at the beginning of a line.  So depending on where thread
2 was when GDB stopped it (after thread 1 hit its breakpoint), we can
get either output.  Adjust the regexps to not expect an hexadecimal
prefix (0x) but a function name instead (either child_sub_function or
child_function).  That one is always printed, and is also a good check
that we are in the frame we expect.

Note that for test "frame 5", we are showing a pthread frame (on my
system), so the function name is internal to pthread, not something we
can rely on.  In that case, it's almost certain that we are not at the
beginning of a line, or that we don't have debug info, so I think it's
fine to expect the hex prefix.

And for test "frame 6", it's ok to _not_ expect a hex prefix (what the
test currently does), since we are showing thread 1, which has hit a
breakpoint placed at the beginning of a line.

When testing this, Tom de Vries pointed out that the current test code
doesn't ensure that the child threads are in child_sub_function when
they are stopped.  If the scheduler chooses so, it is possible for the
child threads to be still in the pthread_barrier_wait or child_function
functions when they get stopped.  So that would be another racy failure
waiting to happen.

The only way I can think of to ensure the child threads are in the
child_sub_function function when they get stopped is to synchronize the
threads using some variables instead of pthread_barrier_wait.  So,
replace the barrier with an array of flags (one per child thread).  Each
child thread flips its flag in child_sub_function to allow the main
thread to make progress and eventually hit the breakpoint.

I copied user-selected-context-sync.c to a new mi-cmd-user-context.c and
made modifications to that, to avoid interfering with
user-selected-context-sync.exp.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29025
Change-Id: I919673bbf9927158beb0e8b7e9e980b8d65eca90

2 years agoFix qRcmd error code parsing
Luis Machado [Thu, 31 Mar 2022 15:45:53 +0000 (16:45 +0100)]
Fix qRcmd error code parsing

Someone at IRC spotted a bug in qRcmd handling. This looks like an oversight
or it is that way for historical reasons.

The code in gdb/remote.c:remote_target::rcmd uses isdigit instead of
isxdigit. One could argue that we are expecting decimal numbers, but further
below we use fromhex ().

Update the function to use isxdigit instead and also update the documentation.

I see there are lots of other cases of undocumented number format for error
messages, mostly described as NN instead of nn. For now I'll just update
this particular function.

2 years agogdb: resume ongoing step after handling fork or vfork
Simon Marchi [Fri, 14 Jan 2022 21:18:03 +0000 (16:18 -0500)]
gdb: resume ongoing step after handling fork or vfork

The test introduced by this patch would fail in this configuration, with
the native-gdbserver or native-extended-gdbserver boards:

    FAIL: gdb.threads/next-fork-other-thread.exp: fork_func=fork: target-non-stop=auto: non-stop=off: displaced-stepping=auto: i=2: next to for loop

The problem is that the step operation is forgotten when handling the
fork/vfork.  With "debug infrun" and "debug remote", it looks like this
(some lines omitted for brevity).  We do the next:

    [infrun] proceed: enter
      [infrun] proceed: addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT
      [infrun] resume_1: step=1, signal=GDB_SIGNAL_0, trap_expected=0, current thread [4154304.4154304.0] at 0x5555555553bf
      [infrun] do_target_resume: resume_ptid=4154304.0.0, step=1, sig=GDB_SIGNAL_0
      [remote] Sending packet: $vCont;r5555555553bf,5555555553c4:p3f63c0.3f63c0;c:p3f63c0.-1#cd
    [infrun] proceed: exit

We then handle a fork event:

    [infrun] fetch_inferior_event: enter
      [remote] wait: enter
        [remote] Packet received: T05fork:p3f63ee.3f63ee;06:0100000000000000;07:b08e59f6ff7f0000;10:bf60e8f7ff7f0000;thread:p3f63c0.3f63c6;core:17;
      [remote] wait: exit
      [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
      [infrun] print_target_wait_results:   4154304.4154310.0 [Thread 4154304.4154310],
      [infrun] print_target_wait_results:   status->kind = FORKED, child_ptid = 4154350.4154350.0
      [infrun] handle_inferior_event: status->kind = FORKED, child_ptid = 4154350.4154350.0
      [remote] Sending packet: $D;3f63ee#4b
      [infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [4154304.4154310.0] at 0x7ffff7e860bf
      [infrun] do_target_resume: resume_ptid=4154304.0.0, step=0, sig=GDB_SIGNAL_0
      [remote] Sending packet: $vCont;c:p3f63c0.-1#73
    [infrun] fetch_inferior_event: exit

In the first snippet, we resume the stepping thread with the range-stepping (r)
vCont command.  But after handling the fork (detaching the fork child), we
resumed the whole process freely.  The stepping thread, which was paused by
GDBserver while reporting the fork event, was therefore resumed freely, instead
of confined to the addresses of the stepped line.  Note that since this
is a "next", it could be that we have entered a function, installed a
step-resume breakpoint, and it's ok to continue freely the stepping
thread, but that's not the case here.  The two snippets shown above were
next to each other in the logs.

For the fork case, we can resume stepping right after handling the
event.

However, for the vfork case, where we are waiting for the
external child process to exec or exit, we only resume the thread that
called vfork, and keep the others stopped (see patch "gdb: fix handling of
vfork by multi-threaded program" prior in this series).  So we can't
resume the stepping thread right now.  Instead, do it after handling the
vfork-done event.

Change-Id: I92539c970397ce880110e039fe92b87480f816bd

2 years agogdb/remote: remove_new_fork_children don't access target_waitstatus::child_ptid if...
Simon Marchi [Tue, 18 Jan 2022 01:49:07 +0000 (20:49 -0500)]
gdb/remote: remove_new_fork_children don't access target_waitstatus::child_ptid if kind == TARGET_WAITKIND_THREAD_EXITED

Following the previous patch, running
gdb.threads/forking-threads-plus-breakpoints.exp continuously eventually
gives me an internal error.

    gdb/target/waitstatus.h:372: internal-error: child_ptid: Assertion `m_kind == TARGET_WAITKIND_FORKED || m_kind == TARGET_WAITKIND_VFORKED' failed.^M
    FAIL: gdb.threads/forking-threads-plus-breakpoint.exp: cond_bp_target=0: detach_on_fork=on: displaced=off: inferior 1 exited (GDB internal error)

The backtrace is:

    0x55925b679c85 internal_error(char const*, int, char const*, ...)
     /home/simark/src/binutils-gdb/gdbsupport/errors.cc:55
    0x559258deadd2 target_waitstatus::child_ptid() const
     /home/simark/src/binutils-gdb/gdb/target/waitstatus.h:372
    0x55925a7cbac9 remote_target::remove_new_fork_children(threads_listing_context*)
     /home/simark/src/binutils-gdb/gdb/remote.c:7311
    0x55925a79dfdb remote_target::update_thread_list()
     /home/simark/src/binutils-gdb/gdb/remote.c:3981
    0x55925ad79b83 target_update_thread_list()
     /home/simark/src/binutils-gdb/gdb/target.c:3793
    0x55925addbb15 update_thread_list()
     /home/simark/src/binutils-gdb/gdb/thread.c:2031
    0x559259d64838 stop_all_threads(char const*, inferior*)
     /home/simark/src/binutils-gdb/gdb/infrun.c:5104
    0x559259d88b45 keep_going_pass_signal
     /home/simark/src/binutils-gdb/gdb/infrun.c:8215
    0x559259d8951b keep_going
     /home/simark/src/binutils-gdb/gdb/infrun.c:8251
    0x559259d78835 process_event_stop_test
     /home/simark/src/binutils-gdb/gdb/infrun.c:6858
    0x559259d750e9 handle_signal_stop
     /home/simark/src/binutils-gdb/gdb/infrun.c:6580
    0x559259d6c07b handle_inferior_event
     /home/simark/src/binutils-gdb/gdb/infrun.c:5832
    0x559259d57db8 fetch_inferior_event()
     /home/simark/src/binutils-gdb/gdb/infrun.c:4222

Indeed, the code accesses target_waitstatus::child_ptid when the kind
is TARGET_WAITKIND_THREAD_EXITED, which is not right.  A
TARGET_WAITKIND_THREAD_EXITED event does not have a child_ptid value
associated, it has an exit status, which we are not interested in.  The
intent is to remove from the thread list the thread that has exited.
Its ptid is found in the stop reply event, get it from there.

Change-Id: Icb298cbb80b8779fdf0c660dde9a5314d5591535

2 years agogdbserver: report correct status in thread stop race condition
Simon Marchi [Sat, 15 Jan 2022 15:55:31 +0000 (10:55 -0500)]
gdbserver: report correct status in thread stop race condition

The test introduced by the following patch would sometimes fail in this
configuration:

    FAIL: gdb.threads/next-fork-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=auto: i=14: next to for loop

The test has multiple threads constantly forking or vforking while the
main thread keep doing "next"s.

(After writing the commit message, I realized this also fixes a similar
failure in gdb.threads/forking-threads-plus-breakpoint.exp with the
native-gdbserver and native-extended-gdbserver boards.)

As stop_all_threads is called, because the main thread finished its
"next", it inevitably happens at some point that we ask the remote
target to stop a thread and wait() reports that this thread stopped with
a fork or vfork event, instead of the SIGSTOP we sent to try to stop it.

While running this test, I attached to GDBserver and stopped at
linux-low.cc:3626.  We can see that the status pulled from the kernel
for 2742805 is indeed a vfork event:

    (gdb) p/x w
    $3 = 0x2057f
    (gdb) p WIFSTOPPED(w)
    $4 = true
    (gdb) p WSTOPSIG(w)
    $5 = 5
    (gdb) p/x (w >> 8) & (PTRACE_EVENT_VFORK << 8)
    $6 = 0x200

However, the statement at line 3626 overrides that:

    ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));

OURSTATUS becomes "stopped by a SIGTRAP".  The information about the
fork or vfork is lost.

It's then all downhill from there, stop_all_threads eventually asks for
a thread list update.  That thread list includes the child of that
forgotten fork or vfork, the remote target goes "oh cool, a new process,
let's attach to it!", when in fact that vfork child's destiny was to be
detached.

My reverse-engineered understanding of the code around there is that the
if/else between lines 3562 and 3583 (in the original code) makes sure
OURSTATUS is always initialized (not "ignore").  Either the details are
already in event_child->waitstatus (in the case of fork/vfork, for
example), in which case we just copy event_child->waitstatus to
ourstatus.  Or, if the event is a plain "stopped by a signal" or a
syscall event, OURSTATUS is set to "stopped", but without a signal
number.  Lines 3601 to 3629 (in the original code) serve to fill in that
last bit of information.

The problem is that when `w` holds the vfork status, the code wrongfully
takes this branch, because WSTOPSIG(w) returns SIGTRAP:

  else if (current_thread->last_resume_kind == resume_stop
       && WSTOPSIG (w) != SIGSTOP)

The intent of this branch is, for example, when we sent SIGSTOP to try
to stop a thread, but wait() reports that it stopped with another signal
(that it must have received from somewhere else simultaneously), say
SIGWINCH.  In that case, we want to report the SIGWINCH.  But in our
fork/vfork case, we don't want to take this branch, as the thread didn't
really stop because it received a signal.  For the non "stopped by a
signal" and non "syscall signal" cases, we would ideally skip over all
that snippet that fills in the signal or syscall number.

The fix I propose is to move this snipppet of the else branch of the
if/else above.  In addition to moving the code, the last two "else if"
branches:

  else if (current_thread->last_resume_kind == resume_stop
   && WSTOPSIG (w) != SIGSTOP)
    {
      /* A thread that has been requested to stop by GDB with vCont;t,
 but, it stopped for other reasons.  */
      ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));
    }
  else if (ourstatus->kind () == TARGET_WAITKIND_STOPPED)
    ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));

are changed into a single else:

  else
    ourstatus->set_stopped (gdb_signal_from_host (WSTOPSIG (w)));

This is the default path we take if:

 - W is not a syscall status
 - W does not represent a SIGSTOP that have sent to stop the thread and
   therefore want to suppress it

Change-Id: If2dc1f0537a549c293f7fa3c53efd00e3e194e79

2 years agogdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach...
Simon Marchi [Fri, 14 Jan 2022 20:40:59 +0000 (15:40 -0500)]
gdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach-on-fork=on)

There is a problem with how GDB handles a vfork happening in a
multi-threaded program.  This problem was reported to me by somebody not
using vfork directly, but using system(3) in a multi-threaded program,
which may be implemented using vfork.

This patch only deals about the follow-fork-mode=parent,
detach-on-fork=on case, because it would be too much to chew at once to
fix the bugs in the other cases as well (I tried).

The problem
-----------

When a program vforks, the parent thread is suspended by the kernel
until the child process exits or execs.  Specifically, in a
multi-threaded program, only the thread that called vfork is suspended,
other threads keep running freely. This is documented in the vfork(2)
man page ("Caveats" section).

Let's suppose GDB is handling a vfork and the user's desire is to detach
from the child. Before detaching the child, GDB must remove the software
breakpoints inserted in the shared parent/child address space, in case
there's a breakpoint in the path the child is going to take before
exec'ing or exit'ing (unlikely, but possible). Otherwise the child could
hit a breakpoint instruction while running outside the control of GDB,
which would make it crash.  GDB must also avoid re-inserting breakpoints
in the parent as long as it didn't receive the "vfork done" event (that
is, when the child has exited or execed): since the address space is
shared with the child, that would re-insert breakpoints in the child
process also. So what GDB does is:

  1. Receive "vfork" event for the parent
  2. Remove breakpoints from the (shared) address space and set
     program_space::breakpoints_not_allowed to avoid re-inserting them
  3. Detach from the child thread
  4. Resume the parent
  5. Wait for and receive "vfork done" event for the parent
  6. Clean program_space::breakpoints_not_allowed and re-insert
     breakpoints
  7. Resume the parent

Resuming the parent at step 4 is necessary in order for the kernel to
report the "vfork done" event.  The kernel won't report a ptrace event
for a thread that is ptrace-stopped.  But the theory behind this is that
between steps 4 and 5, the parent won't actually do any progress even
though it is ptrace-resumed, because the kernel keeps it suspended,
waiting for the child to exec or exit.  So it doesn't matter for that
thread if breakpoints are not inserted.

The problem is when the program is multi-threaded.  In step 4, GDB
resumes all threads of the parent. The thread that did the vfork stays
suspended by the kernel, so that's fine. But other threads are running
freely while breakpoints are removed, which is a problem because they
could miss a breakpoint that they should have hit.

The problem is present with all-stop and non-stop targets.  The only
difference is that with an all-stop targets, the other threads are
stopped by the target when it reports the vfork event and are resumed by
the target when GDB resumes the parent.  With a non-stop target, the
other threads are simply never stopped.

The fix
-------

There many combinations of settings to consider (all-stop/non-stop,
target-non-stop on/off, follow-fork-mode parent/child, detach-on-fork
on/off, schedule-multiple on/off), but for this patch I restrict the
scope to follow-fork-mode=parent, detach-on-fork=on.  That's the
"default" case, where we detach the child and keep debugging the
parent.  I tried to fix them all, but it's just too much to do at once.
The code paths and behaviors for when we don't detach the child are
completely different.

The guiding principle for this patch is that all threads of the vforking
inferior should be stopped as long as breakpoints are removed.  This is
similar to handling in-line step-overs, in a way.

For non-stop targets (the default on Linux native), this is what
happens:

 - In follow_fork, we call stop_all_threads to stop all threads of the
   inferior
 - In follow_fork_inferior, we record the vfork parent thread in
   inferior::thread_waiting_for_vfork_done
 - Back in handle_inferior_event, we call keep_going, which resumes only
   the event thread (this is already the case, with a non-stop target).
   This is the thread that will be waiting for vfork-done.
 - When we get the vfork-done event, we go in the (new) handle_vfork_done
   function to restart the previously stopped threads.

In the same scenario, but with an all-stop target:

 - In follow_fork, no need to stop all threads of the inferior, the
   target has stopped all threads of all its inferiors before returning
   the event.
 - In follow_fork_inferior, we record the vfork parent thread in
   inferior::thread_waiting_for_vfork_done.
 - Back in handle_inferior_event, we also call keep_going.  However, we
   only want to resume the event thread here, not all inferior threads.
   In internal_resume_ptid (called by resume_1), we therefore now check
   whether one of the inferiors we are about to resume has
   thread_waiting_for_vfork_done set.  If so, we only resume that
   thread.

   Note that when resuming multiple inferiors, one vforking and one not
   non-vforking, we could resume the vforking thread from the vforking
   inferior plus all threads from the non-vforking inferior.  However,
   this is not implemented, it would require more work.
 - When we get the vfork-done event, the existing call to keep_going
   naturally resumes all threads.

Testing-wise, add a test that tries to make the main thread hit a
breakpoint while a secondary thread calls vfork.  Without the fix, the
main thread keeps going while breakpoints are removed, resulting in a
missed breakpoint and the program exiting.

Change-Id: I20eb78e17ca91f93c19c2b89a7e12c382ee814a1

2 years agogdb/infrun: add logging statement to do_target_resume
Simon Marchi [Thu, 13 Jan 2022 19:32:27 +0000 (14:32 -0500)]
gdb/infrun: add logging statement to do_target_resume

This helped me, it shows which ptid we actually call target_resume with.

Change-Id: I2dfd771e83df8c25f39371a13e3e91dc7882b73d

2 years agogdb/infrun: add inferior parameters to stop_all_threads and restart_threads
Simon Marchi [Thu, 13 Jan 2022 19:15:45 +0000 (14:15 -0500)]
gdb/infrun: add inferior parameters to stop_all_threads and restart_threads

A following patch will want to stop all threads of a given inferior (as
opposed to all threads of all inferiors) while handling a vfork, and
restart them after.  To help with this, add inferior parameters to
stop_all_threads and restart_threads.  This is done as a separate patch
to make sure this doesn't cause regressions on its own, and to keep the
following patches more concise.

No visible changes are expected here, since all calls sites pass
nullptr, which should keep the existing behavior.

Change-Id: I4d9ba886ce842042075b4e346cfa64bbe2580dbf

2 years agogdb: replace inferior::waiting_for_vfork_done with inferior::thread_waiting_for_vfork...
Simon Marchi [Wed, 12 Jan 2022 02:40:34 +0000 (21:40 -0500)]
gdb: replace inferior::waiting_for_vfork_done with inferior::thread_waiting_for_vfork_done

The inferior::waiting_for_vfork_done flag indicates that some thread in
that inferior is waiting for a vfork-done event.  Subsequent patches
will need to know which thread precisely is waiting for that event.

Replace the boolean flag (waiting_for_vfork_done) with a thread_info
pointer (thread_waiting_for_vfork_done).

I think there is a latent buglet in that waiting_for_vfork_done is
currently not reset on inferior exec or exit.  I could imagine that if a
thread in the parent process calls exec or exit while another thread of
the parent process is waiting for its vfork child to exec or exit, we
could end up with inferior::waiting_for_vfork_done without a thread
actually waiting for a vfork-done event anymore.  And since that flag is
checked in resume_1, things could misbehave there.

Since the new field points to a thread_info object, and those are
destroyed on exec or exit, it could be worse now since we could try to
access freed memory, if thread_waiting_for_vfork_done were to point to a
stale thread_info.  To avoid this, clear the field in
infrun_inferior_exit and infrun_inferior_execd.

Change-Id: I31b847278613a49ba03fc4915f74d9ceb228fdce

2 years agogdb: make timestamped_file implement write_async_safe
Simon Marchi [Tue, 5 Apr 2022 00:53:52 +0000 (20:53 -0400)]
gdb: make timestamped_file implement write_async_safe

Trying to use "set debug linux-nat 1", I get an internal error:

    /home/smarchi/src/binutils-gdb/gdb/ui-file.h:70: internal-error: write_async_safe: write_async_safe

The problem is that timestamped_file doesn't implement write_async_safe,
which linux-nat's sigchld_handler uses.  Implement it.

Change-Id: I830981010c6119f13ae673605ed015cced0f5ee8

2 years agoAutomatic date update in version.in
GDB Administrator [Tue, 5 Apr 2022 00:00:06 +0000 (00:00 +0000)]
Automatic date update in version.in

2 years agogdb/testsuite: fix timeout in server-pipe.exp test
Andrew Burgess [Thu, 31 Mar 2022 18:42:17 +0000 (19:42 +0100)]
gdb/testsuite: fix timeout in server-pipe.exp test

I noticed that the gdb.server/server-pipe.exp test would sometimes
timeout when my machine was more heavily loaded.  Turns out the test
is reading all the shared libraries over GDB's remote protocol, which
can be slow.

We avoid this in other tests by setting the sysroot in GDBFLAGS,
something which is missing from the gdb.server/server-pipe.exp test.

Fix the timeouts by setting sysroot in GDBFLAGS, after this the shared
libraries are no longer copied over the remote protocol, and I no
longer see the test timeout.

2 years agoHandle TLS variable lookups when using separate debug files.
John Baldwin [Mon, 4 Apr 2022 22:08:15 +0000 (15:08 -0700)]
Handle TLS variable lookups when using separate debug files.

Commit df22c1e5d53c38f38bce6072bb46de240f9e0e2b handled the case that
a separate debug file was passed as the objfile for a shared library
to svr4_fetch_objfile_link_map.  However, a separate debug file can
also be passed for TLS variables in the main executable.  In addition,
frv_fetch_objfile_link_map also expects to be passed the original
objfile rather than a separate debug file, so pull the code to resolve
a separate debug file to the main objfile up into
target_translate_tls_address.

2 years agogdb: Add maint set ignore-prologue-end-flag
Lancelot SIX [Fri, 1 Apr 2022 10:59:29 +0000 (11:59 +0100)]
gdb: Add maint set ignore-prologue-end-flag

The previous patch added support for the DWARF prologue-end flag in line
table. This flag can be used by DWARF producers to indicate where to
place breakpoints past a function prologue.  However, this takes
precedence over prologue analyzers. So if we have to debug a program
with erroneous debug information, the overall debugging experience will
be degraded.

This commit proposes to add a maintenance command to instruct GDB to
ignore the prologue_end flag.

Tested on x86_64-gnu-linux.

Change-Id: Idda6d1b96ba887f4af555b43d9923261b9cc6f82

2 years agogdb: Add support for DW_LNS_set_prologue_end in line-table
Lancelot SIX [Fri, 25 Feb 2022 23:41:47 +0000 (23:41 +0000)]
gdb: Add support for DW_LNS_set_prologue_end in line-table

Add support for DW_LNS_set_prologue_end when building line-tables.  This
attribute can be set by the compiler to indicate that an instruction is
an adequate place to set a breakpoint just after the prologue of a
function.

The compiler might set multiple prologue_end, but considering how
current skip_prologue_using_sal works, this commit modifies it to accept
the first instruction with this marker (if any) to be the place where a
breakpoint should be placed to be at the end of the prologue.

The need for this support came from a problematic usecase generated by
hipcc (i.e. clang).  The problem is as follows:  There's a function
(lets call it foo) which covers PC from 0xa800 to 0xa950.  The body of
foo begins with a call to an inlined function, covering from 0xa800 to
0xa94c.   The issue is that when placing a breakpoint at 'foo', GDB
inserts the breakpoint at 0xa818.  The 0x18 offset is what GDB thinks is
foo's first address past the prologue.

Later, when hitting the breakpoint, GDB reports the stop within the
inlined function because the PC falls in its range while the user
expects to stop in FOO.

Looking at the line-table for this location, we have:

    INDEX  LINE   ADDRESS            IS-STMT
    [...]
    14     293    0x000000000000a66c Y
    15     END    0x000000000000a6e0 Y
    16     287    0x000000000000a800 Y
    17     END    0x000000000000a818 Y
    18     287    0x000000000000a824 Y
    [...]

For comparison, let's look at llvm-dwarfdump's output for this CU:

    Address            Line   Column File   ISA Discriminator Flags
    ------------------ ------ ------ ------ --- ------------- -------------
    [...]
    0x000000000000a66c    293     12      2   0             0  is_stmt
    0x000000000000a6e0     96     43     82   0             0  is_stmt
    0x000000000000a6f8    102     18     82   0             0  is_stmt
    0x000000000000a70c    102     24     82   0             0
    0x000000000000a710    102     18     82   0             0
    0x000000000000a72c    101     16     82   0             0  is_stmt
    0x000000000000a73c   2915     50     83   0             0  is_stmt
    0x000000000000a74c    110      1      1   0             0  is_stmt
    0x000000000000a750    110      1      1   0             0  is_stmt end_sequence
    0x000000000000a800    107      0      1   0             0  is_stmt
    0x000000000000a800    287     12      2   0             0  is_stmt prologue_end
    0x000000000000a818    114     59     81   0             0  is_stmt
    0x000000000000a824    287     12      2   0             0  is_stmt
    0x000000000000a828    100     58     82   0             0  is_stmt
    [...]

The main difference we are interested in here is that llvm-dwarfdump's
output tells us that 0xa800 is an adequate place to place a breakpoint
past a function prologue.  Since we know that foo covers from 0xa800 to
0xa94c, 0xa800 is the address at which the breakpoint should be placed
if the user wants to break in foo.

This commit proposes to add support for the prologue_end flag in the
line-program processing.

The processing of this prologue_end flag is made in skip_prologue_sal,
before it calls gdbarch_skip_prologue_noexcept.  The intent is that if
the compiler gave information on where the prologue ends, we should use
this information and not try to rely on architecture dependent logic to
guess it.

The testsuite have been executed using this patch on GNU/Linux x86_64.
Testcases have been compiled with both gcc/g++ (verison 9.4.0) and
clang/clang++ (version 10.0.0) since at the time of writing GCC does not
set the prologue_end marker.  Tests done with GCC 11.2.0 (not over the
entire testsuite) show that it does not emit this flag either.

No regression have been observed with GCC or Clang.  Note that when
using Clang, this patch fixes a failure in
gdb.opt/inline-small-func.exp.

Change-Id: I720449a8a9b2e1fb45b54c6095d3b1e9da9152f8

2 years agogdb/buildsym: Line record use a record flag
Lancelot SIX [Mon, 7 Mar 2022 17:36:53 +0000 (17:36 +0000)]
gdb/buildsym: Line record use a record flag

Currently when recording a line entry (with
buildsym_compunit::record_line), a boolean argument argument is used to
indicate that the is_stmt flag should be set for this particular record.
As a later commit will add support for new flags, instead of adding a
parameter to record_line for each possible flag, transform the current
is_stmt parameter into a enum flag.  This flags parameter will allow
greater flexibility in future commits.

This enum flags type is not propagated into the linetable_entry type as
this would require a lot of changes across the codebase for no practical
gain (it currently uses a bitfield where each interesting flag only
occupy 1 bit in the structure).

Tested on x86_64-linux, no regression observed.

Change-Id: I5d061fa67bdb34918742505ff983d37453839d6a

2 years agogdb: make timestamped_file implement can_emit_style_escape
Simon Marchi [Thu, 31 Mar 2022 17:32:20 +0000 (13:32 -0400)]
gdb: make timestamped_file implement can_emit_style_escape

In our AMDGPU downstream port, we use styling in some logging output.
We noticed it stopped working after the gdb_printf changes.  Making
timestamped_file implement can_emit_style_escape (returning the value of
the stream it wraps) fixes it.  To show that it works, modify some
logging statements in auto-load.c to output style filenames.  You can
see it in action by setting "set debug auto-load 1" and running a
program.  We can incrementally add styling to other debug statements
throughout GDB, as needed.

Change-Id: I78a2fd1e078f80f2263251cf6bc53b3a9de9c17a

2 years agogdb: remove assertion in psymbol_functions::expand_symtabs_matching
Simon Marchi [Thu, 31 Mar 2022 20:38:33 +0000 (16:38 -0400)]
gdb: remove assertion in psymbol_functions::expand_symtabs_matching

psymtab_to_symtab is documented as possibly returning nullptr, if the
primary symtab of the partial symtab has no symbols.  However,
psymbol_functions::expand_symtabs_matching asserts that the result of
psymtab_to_symtab as non-nullptr.

I caught this assert by trying the CTF symbol reader on a library I
built with -gctf:

    $ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
    ...
    Reading symbols from /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0...
    (gdb) maintenance expand-symtabs
    /home/simark/src/binutils-gdb/gdb/psymtab.c:1142: internal-error: expand_symtabs_matching: Assertion `symtab != nullptr' failed.

The "symtab" in question is:

    $  readelf --ctf=.ctf /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
    ...
    CTF archive member: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c:

      Header:
        Magic number: 0xdff2
        Version: 4 (CTF_VERSION_3)
        Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
        Parent name: .ctf
        Compilation unit name: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c
        Type section:       0x0 -- 0x13 (0x14 bytes)
        String section:     0x14 -- 0x5f (0x4c bytes)

      Labels:

      Data objects:

      Function objects:

      Variables:

      Types:
        0x80000001: (kind 5) bt_bool (*) (const bt_value *) (aligned at 0x8)

      Strings:
        0x0:
        0x1: .ctf
        0x6: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c

It contains a single type, and it is skipped by ctf_add_type_cb, because
an identical type was already seen earlier in this objfile.  As a
result, no compunit_symtab is created.

Change psymbol_functions::expand_symtabs_matching to expect that
psymtab_to_symtab can return nullptr.

Another possibility would be to make the CTF symbol reader always create
a compunit_symtab, even if there are no symbols in it (like the DWARF
parser does), but so far I don't see any advantage in doing so.

Change-Id: Ic43c38202c838a5eb87630ed1fd61d33528164f4

2 years agosim: fixes for libopcodes styled disassembler
Andrew Burgess [Mon, 4 Apr 2022 21:38:04 +0000 (22:38 +0100)]
sim: fixes for libopcodes styled disassembler

In commit:

  commit 60a3da00bd5407f07d64dff82a4dae98230dfaac
  Date:   Sat Jan 22 11:38:18 2022 +0000

      objdump/opcodes: add syntax highlighting to disassembler output

I broke several sim/ targets by forgetting to update their uses of the
libopcodes disassembler to take account of the new styled printing.

These should all be fixed by this commit.

I've not tried to add actual styled output to the simulator traces,
instead, the styled print routines just ignore the style and print the
output unstyled.

2 years agoRemove some globals from nat/windows-nat.c
Tom Tromey [Thu, 31 Mar 2022 19:41:02 +0000 (13:41 -0600)]
Remove some globals from nat/windows-nat.c

nat/windows-nat.c has a number of globals that it uses to communicate
with its clients (gdb and gdbserver).  However, if we ever want the
Windows ports to be multi-inferior, globals won't work.

This patch takes a step toward that by moving most nat/windows-nat.c
globals into a new struct windows_process_info.  Many functions are
converted to be methods on this object.

A couple of globals remain, as they are needed to truly be global due
to the way that the Windows debugging APIs work.

The clients still have a global for the current process.  That is,
this patch is a step toward the end goal, but doesn't implement the
goal itself.

2 years agoRemove windows_thread_info destructor
Tom Tromey [Thu, 31 Mar 2022 16:58:55 +0000 (10:58 -0600)]
Remove windows_thread_info destructor

windows_thread_info declares and defines a destructor, but this
doesn't need to be explicit.

2 years agoUse unique_ptr in the Windows thread list
Tom Tromey [Thu, 31 Mar 2022 16:08:45 +0000 (10:08 -0600)]
Use unique_ptr in the Windows thread list

windows-nat.c uses some manual memory management when manipulating the
thread_list global.  Changing this to use unique_ptr simplifies the
code, in particular windows_init_thread_list.  (Note that, while I
think the the call to init_thread_list in there is wrong, I haven't
removed it in this patch.)

2 years agoUse auto_obstack in windows-nat.c
Tom Tromey [Thu, 31 Mar 2022 15:45:23 +0000 (09:45 -0600)]
Use auto_obstack in windows-nat.c

One spot in windows-nat.c can use auto_obstack, removing some manual
memory management.

2 years agoSimplify windows-nat.c solib handling
Tom Tromey [Thu, 31 Mar 2022 15:43:49 +0000 (09:43 -0600)]
Simplify windows-nat.c solib handling

Currently windows-nat.c uses struct so_list to record its local idea
of which shared libraries have been loaded.  However, many fields in
this are not needed, and furthermore I found this quite confusing at
first -- Windows actually uses solib-target and so the use of so_list
here is weird.

This patch simplifies this code by changing it to use a std::vector
and a new type that holds exactly what's needed for the Windows code.

2 years agoAvoid undefined behavior in gdbscm_make_breakpoint
Pedro Alves [Fri, 1 Apr 2022 13:31:18 +0000 (14:31 +0100)]
Avoid undefined behavior in gdbscm_make_breakpoint

Running gdb.guile/scm-breakpoint.exp against an --enable-ubsan build,
we see:

 UNRESOLVED: gdb.guile/scm-breakpoint.exp: test_watchpoints: create a breakpoint with an invalid type number
 ...
 guile (define wp2 (make-breakpoint "result" #:wp-class WP_WRITE #:type 999))
 ../../src/gdb/guile/scm-breakpoint.c:377:11: runtime error: load of value 999, which is not a valid value for type 'bptype'
 ERROR: GDB process no longer exists

Fix this by parsing the user/guile input as plain int, and cast to
internal type only after we know we have a number that would be valid.

Change-Id: I03578d07db00be01b610a8f5ce72e5521aea6a4b

2 years agoAdd context-sensitive field name completion to Ada parser
Tom Tromey [Tue, 22 Feb 2022 20:12:02 +0000 (13:12 -0700)]
Add context-sensitive field name completion to Ada parser

This updates the Ada expression parser to implement context-sensitive
field name completion.  This is PR ada/28727.

This is somewhat complicated due to some choices in the Ada lexer --
it chooses to represent a sequence of "."-separated identifiers as a
single token, so the parser must partially recreate the completer's
logic to find the completion word boundaries.

Despite the minor warts in this patch, though, it is a decent
improvement.  It's possible that the DWARF reader rewrite will help
fix the package completion problem pointed out in this patch as well.

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

2 years agoConsolidate single-char tokens in ada-lex.l
Tom Tromey [Wed, 23 Feb 2022 15:48:40 +0000 (08:48 -0700)]
Consolidate single-char tokens in ada-lex.l

There are two rules in ada-lex.l that match single-character tokens.
This merges them.

Also, this removes '.' from the list of such tokens.  '.' is not used
in any production in ada-exp.y, and removing it here helps the
subsequent completion patches.

2 years agoRemove the Ada DOT_ALL token
Tom Tromey [Tue, 22 Feb 2022 19:56:09 +0000 (12:56 -0700)]
Remove the Ada DOT_ALL token

The Ada parser has a DOT_ALL token to represent ".all", and another
token to represent other ".<identifier>" forms.  However, for
completion it is a bit more convenient to unify these cases, so this
patch removes DOT_ALL.

2 years agoRefactor ada-lex.l:processId
Tom Tromey [Tue, 22 Feb 2022 19:53:14 +0000 (12:53 -0700)]
Refactor ada-lex.l:processId

processId in ada-lex.l is a bit funny -- it uses an "if" and a
"switch", and a nested loop.  This patch cleans it up a bit, changing
it to use a boolean flag and a simpler "if".

2 years agoImplement completion for Ada attributes
Tom Tromey [Tue, 22 Feb 2022 18:18:01 +0000 (11:18 -0700)]
Implement completion for Ada attributes

This adds a completer for Ada attributes.  Some work in the lexer is
required in order to match end-of-input correctly, as flex does not
have a general-purpose way of doing this.  (The approach taken here is
recommended in the flex manual.)

2 years agoRefactor expression completion
Tom Tromey [Tue, 22 Feb 2022 16:48:25 +0000 (09:48 -0700)]
Refactor expression completion

This refactors the gdb expression completion code to make it easier to
add more types of completers.

In the old approach, just two kinds of completers were supported:
field names for some sub-expression, or tag names (like "enum
something").  The data for each kind was combined in single structure,
"expr_completion_state", and handled explicitly by
complete_expression.

In the new approach, the parser state just holds an object that is
responsible for implementing completion.  This way, new completion
types can be added by subclassing this base object.

The structop completer is moved into structop_base_operation, and new
objects are defined for use by the completion code.  This moves much
of the logic of expression completion out of completer.c as well.

2 years agoEnable "set debug parser" for Ada
Tom Tromey [Tue, 22 Feb 2022 18:41:24 +0000 (11:41 -0700)]
Enable "set debug parser" for Ada

I noticed that "set debug parser 1" did not affect Ada parsing.  This
patch fixes the problem.

Because this is rarely useful, and pretty much only for maintainers, I
didn't write a test case.

2 years agoFix bug in Ada attributes lexing
Tom Tromey [Tue, 22 Feb 2022 19:02:10 +0000 (12:02 -0700)]
Fix bug in Ada attributes lexing

The Ada lexer allows whitespace between the apostrophe and the
attribute text, but processAttribute does not handle this.  This patch
fixes the problem and introduces a regression test.

2 years agoRemove null sentinel from 'attributes'
Tom Tromey [Tue, 22 Feb 2022 18:05:41 +0000 (11:05 -0700)]
Remove null sentinel from 'attributes'

In a subsequent patch, it's handy if the 'attributes' array in
ada-lex.l does not have a NULL sentinel at the end.  In C++, this is
easy to avoid.

2 years agoHandle ghost entities in symbol lookup
Tom Tromey [Tue, 15 Mar 2022 15:03:14 +0000 (09:03 -0600)]
Handle ghost entities in symbol lookup

Normally, SPARK ghost entities are removed from the executable.
However, with -gnata, they will be preserved.  In this situation, it's
handy to be able to inspect them.  This patch allows this by removing
the "___ghost_" prefix in the appropriate places.

2 years agogdb: rename start_symtab/end_symtab to start_compunit_symtab/end_compunit_symtab
Simon Marchi [Mon, 28 Mar 2022 22:17:17 +0000 (18:17 -0400)]
gdb: rename start_symtab/end_symtab to start_compunit_symtab/end_compunit_symtab

It's a bit confusing because we have both "compunit_symtab" and "symtab"
types, and many methods and functions containing "start_symtab" or
"end_symtab", which actually deal with compunit_symtabs.  I believe this
comes from the time before compunit_symtab was introduced, where
symtab did the job of both.

Rename everything I found containing start_symtab or end_symtab to use
start_compunit_symtab or end_compunit_symtab.

Change-Id: If3849b156f6433640173085ad479b6a0b085ade2

2 years agogdb: remove some unused buildsym-legacy functions
Simon Marchi [Mon, 28 Mar 2022 22:17:16 +0000 (18:17 -0400)]
gdb: remove some unused buildsym-legacy functions

Pretty much self-explanatory.

Change-Id: I5b658d017cd891ecdd1df61075eacb0f44316935

2 years agogas: copy st_size only if unset
Fangrui Song [Mon, 4 Apr 2022 15:43:50 +0000 (08:43 -0700)]
gas: copy st_size only if unset

For
```
.size foo1, 1
foo1:

.set bar1, foo1
.size bar1, 2
.size bar2, 2
.set bar2, foo1

.set bar3, foo2
.size bar3, 2
.size bar4, 2
.set bar4, foo2

.size foo2, 1
foo2:
```

bar1's size is 2 while bar2, bar3, bar4's is 1. The behavior of bar1 makes sense
(generally directives on the new symbol should win) and is relied upon by glibc
stdio-common/errlist.c:

```
        .hidden _sys_errlist_internal
        .globl  _sys_errlist_internal
        .type   _sys_errlist_internal, @object
        .size   _sys_errlist_internal, 1072
_sys_errlist_internal:

        .globl __GLIBC_2_1_sys_errlist
        .set __GLIBC_2_1_sys_errlist, _sys_errlist_internal
        .type __GLIBC_2_1_sys_errlist, %object
        .size __GLIBC_2_1_sys_errlist, 125 * (64 / 8)

// glibc expects that .size __GLIBC_2_1_sys_errlist, 125 * (64 / 8) wins.
```

The behavior of bar2/bar3/bar4 seems brittle. To avoid the reordering of the two
code blocks which will result in the bar3 situation, glibc compiles errlist.c
with gcc -fno-toplevel-reorder (previously -fno-unit-at-a-time).

To fix the inconsistency and improve robustness, make bar2/bar3/bar4 match bar1,
removing the directive order sensitivity.

There is a pity that `.size dest, 0` is indistinguishable from the case where
dest is unset, but the compromise seems fine.

    PR gas/29012
    * config/obj-elf.c (elf_copy_symbol_attributes): don't copy if src's size
      has been set.
    * testsuite/gas/elf/elf.exp: New test.
    * testsuite/gas/elf/size.d: New file.
    * testsuite/gas/elf/size.s: Likewise.

2 years agogdb: remove use of vfprintf_filtered
Andrew Burgess [Mon, 4 Apr 2022 12:41:49 +0000 (13:41 +0100)]
gdb: remove use of vfprintf_filtered

Commit:

  commit 60a3da00bd5407f07d64dff82a4dae98230dfaac
  Date:   Sat Jan 22 11:38:18 2022 +0000

      objdump/opcodes: add syntax highlighting to disassembler output

Introduced a new use of vfprintf_filtered, which has been deprecated.
This commit replaces this with gdb_vprintf instead.

2 years agoopcodes/i386: partially implement disassembler style support
Andrew Burgess [Sat, 5 Feb 2022 11:25:14 +0000 (11:25 +0000)]
opcodes/i386: partially implement disassembler style support

This commit adds partial support for disassembler styling in the i386
disassembler.

The i386 disassembler collects the instruction arguments into an array
of strings, and then loops over the array printing the arguments out
later on.  The problem is that by the time we print the arguments out
it's not obvious what the type of each argument is.

Obviously this can be fixed, but I'd like to not do that as part of
this commit, rather, I'd prefer to keep this commit as small as
possible to get the basic infrastructure in place, then we can improve
on this, to add additional styling, in later commits.

For now then, I think this commit should correctly style mnemonics,
some immediates, and comments.  Everything else will be printed as
plain text, which will include most instruction arguments, unless the
argument is printed as a symbol, by calling the print_address_func
callback.

Ignoring colours, there should be no other user visible changes in the
output of the disassembler in either objdump or gdb.

opcodes/ChangeLog:

* disassembler.c (disassemble_init_for_target): Set
created_styled_output for i386 based targets.
* i386-dis.c: Changed throughout to use fprintf_styled_func
instead of fprintf_func.

2 years agoopcodes/riscv: implement style support in the disassembler
Andrew Burgess [Sat, 5 Feb 2022 11:25:24 +0000 (11:25 +0000)]
opcodes/riscv: implement style support in the disassembler

Update the RISC-V disassembler to supply style information.  This
allows objdump to apply syntax highlighting to the disassembler
output (when the appropriate command line flag is used).

Ignoring colours, there should be no other user visible changes in the
output of the disassembler in either objdump or gdb.

opcodes/ChangeLog:

* disassembler.c (disassemble_init_for_target): Set
created_styled_output for riscv.
* riscv-dis.c: Changed throughout to use fprintf_styled_func
instead of fprintf_func.

2 years agoobjdump/opcodes: add syntax highlighting to disassembler output
Andrew Burgess [Sat, 22 Jan 2022 11:38:18 +0000 (11:38 +0000)]
objdump/opcodes: add syntax highlighting to disassembler output

This commit adds the _option_ of having disassembler output syntax
highlighted in objdump.  This option is _off_ by default.  The new
command line options are:

  --disassembler-color=off # The default.
  --disassembler-color=color
  --disassembler-color=extended-color

I have implemented two colour modes, using the same option names as we
use of --visualize-jumps, a basic 8-color mode ("color"), and an
extended 8bit color mode ("extended-color").

The syntax highlighting requires that each targets disassembler be
updated; each time the disassembler produces some output we now pass
through an additional parameter indicating what style should be
applied to the text.

As updating all target disassemblers is a large task, the old API is
maintained.  And so, a user of the disassembler (i.e. objdump, gdb)
must provide two functions, the current non-styled print function, and
a new, styled print function.

I don't currently have a plan for converting every single target
disassembler, my hope is that interested folk will update the
disassemblers they are interested in.  But it is possible some might
never get updated.

In this initial series I intend to convert the RISC-V disassembler
completely, and also do a partial conversion of the x86 disassembler.
Hopefully having the x86 disassembler at least partial converted will
allow more people to try this out easily and provide feedback.

In this commit I have focused on objdump.  The changes to GDB at this
point are the bare minimum required to get things compiling, GDB makes
no use of the styling information to provide any colors, that will
come later, if this commit is accepted.

This first commit in the series doesn't convert any target
disassemblers at all (the next two commits will update some targets),
so after this commit, the only color you will see in the disassembler
output, is that produced from objdump itself, e.g. from
objdump_print_addr_with_sym, where we print an address and a symbol
name, these are now printed with styling information, and so will have
colors applied (if the option is on).

Finally, my ability to pick "good" colors is ... well, terrible.  I'm
in no way committed to the colors I've picked here, so I encourage
people to suggest new colors, or wait for this commit to land, and
then patch the choice of colors.

I do have an idea about using possibly an environment variable to
allow the objdump colors to be customised, but I haven't done anything
like that in this commit, the color choices are just fixed in the code
for now.

binutils/ChangeLog:

* NEWS: Mention new feature.
* doc/binutils.texi (objdump): Describe --disassembler-color
option.
* objdump.c (disassembler_color): New global.
(disassembler_extended_color): Likewise.
(disassembler_in_comment): Likewise.
(usage): Mention --disassembler-color option.
(long_options): Add --disassembler-color option.
(objdump_print_value): Use fprintf_styled_func instead of
fprintf_func.
(objdump_print_symname): Likewise.
(objdump_print_addr_with_sym): Likewise.
(objdump_color_for_disassembler_style): New function.
(objdump_styled_sprintf): New function.
(fprintf_styled): New function.
(disassemble_jumps): Use disassemble_set_printf, and reset
disassembler_in_comment.
(null_styled_print): New function.
(disassemble_bytes): Use disassemble_set_printf, and reset
disassembler_in_comment.
(disassemble_data): Update init_disassemble_info call.
(main): Handle --disassembler-color option.

include/ChangeLog:

* dis-asm.h (enum disassembler_style): New enum.
(struct disassemble_info): Add fprintf_styled_func field, and
created_styled_output field.
(disassemble_set_printf): Declare.
(init_disassemble_info): Add additional parameter.
(INIT_DISASSEMBLE_INFO): Add additional parameter.

opcodes/ChangeLog:

* dis-init.c (init_disassemble_info): Take extra parameter,
initialize the new fprintf_styled_func and created_styled_output
fields.
* disassembler.c (disassemble_set_printf): New function definition.

2 years agoRemove more Python 2 code
Tom Tromey [Fri, 1 Apr 2022 20:29:35 +0000 (14:29 -0600)]
Remove more Python 2 code

I found another more place that still had a workaround for Python 2.
This patch removes it.