gdb/mi: fix ^running record with multiple MI interpreters

I stumbled on the mi_proceeded and running_result_record_printed
globals, which are shared by all MI interpreter instances (it's unlikely
that people use multiple MI interpreter instances, but it's possible).
After poking at it, I found this bug:

1. Start GDB in MI mode
2. Add a second MI interpreter with the new-ui command
3. Use -exec-run on the second interpreter

This is the output I get on the first interpreter:

    =thread-group-added,id="i1"
    ~"Reading symbols from a.out...\n"
    ~"New UI allocated\n"
    (gdb)
    =thread-group-started,id="i1",pid="94718"
    =thread-created,id="1",group-id="i1"
    ^running
    *running,thread-id="all"

And this is the output I get on the second intepreter:

    =thread-group-added,id="i1"
    (gdb)
    -exec-run
    =thread-group-started,id="i1",pid="94718"
    =thread-created,id="1",group-id="i1"
    *running,thread-id="all"

The problem here is that the `^running` reply to the -exec-run command
is printed on the wrong UI.  It is printed on the first one, it should
be printed on the second (the one on which we sent the -exec-run).

What happens under the hood is that captured_mi_execute_command, while
executing a command for the second intepreter, clears the
running_result_record_printed and mi_proceeded globals.
mi_about_to_proceed then sets mi_proceeded.  Then, mi_on_resume_1 gets
called for the first intepreter first.  Since the

    !running_result_record_printed && mi_proceeded

condition is true, it prints a ^running, and sets
running_result_record_printed.  When mi_on_resume_1 gets called for the
second interpreter, running_result_record_printed is already set, so
^running is not printed there.

It took me a while to understand the relationship between these two
variables.  I think that in the end, this is what we want to track:

 1. When executing an MI command, take note if that command causes a
    "proceed".  This is done in mi_about_to_proceed.
 2. In mi_on_resume_1, if the command indeed caused a "proceed", we want
    to output a ^running record.  And we want to remember that we did,
    because...
 3. Back in captured_mi_execute_command, if we did not output a
    ^running, we want to output a ^done.

Moving those two variables to the mi_interp struture appears to fix it.
Only for the interpreter doing the -exec-run command does the
running_result_record_printed flag get cleared, and therefore only or
that one does the ^running record get printed.

Add a new test for this, that does pretty much what the reproducer above
shows.  Without the fix, the test fails because
mi_send_resuming_command_raw never sees the ^running record.

Change-Id: I63ea30e6cb61a8e1dd5ef03377e6003381a9209b
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>

Automatic date update in version.in

[readline] Fix double free in _rl_scxt_dispose

Consider the following scenario.  We start gdb in TUI mode:
...
$ gdb -q -tui
...
and type ^R which gives us the reverse-isearch prompt in the cmd window:
...
(reverse-i-search)`':
...
and then type "foo", right-arrow-key, and ^C.

In TUI mode, gdb uses a custom rl_getc_function tui_getc.

When pressing the right-arrow-key, tui_getc:
- attempts to scroll the TUI src window, without any effect, and
- returns 0.

The intention of returning 0 is mentioned here in tui_dispatch_ctrl_char:
...
  /* We intercepted the control character, so return 0 (which readline
     will interpret as a no-op).  */
  return 0;
...

However, after this 0 is returned by the rl_read_key () call in
_rl_search_getchar, _rl_read_mbstring is called, which incorrectly interprets
0 as the first part of an utf-8 multibyte char, and tries to read the next
char.

In this state, the ^C takes effect and we run into a double free because
_rl_isearch_cleanup is called twice.

Both these issues need fixing independently, though after fixing the first we
no longer trigger the second.

The first issue is caused by the subtle difference between:
- a char array containing 0 chars, which is zero-terminated, and
- a char array containing 1 char, which is zero.

In mbrtowc terms, this is the difference between:
...
  mbrtowc (&wc, "", 0, &ps);
...
which returns -2, and:
...
  mbrtowc (&wc, "", 1, &ps);
...
which returns 0.

Note that _rl_read_mbstring calls _rl_get_char_len without passing it an
explicit length parameter, and consequently it cannot distinguish between the
two, and defaults to the "0 chars" choice.

Note that the same problem doesn't exist in _rl_read_mbchar.

Fix this by defaulting to the "1 char" choice in _rl_get_char_len:
...
-  if (_rl_utf8locale && l > 0 && UTF8_SINGLEBYTE(*src))
+  if (_rl_utf8locale && l >= 0 && UTF8_SINGLEBYTE(*src))
...

The second problem happens when the call to _rl_search_getchar in
_rl_isearch_callback returns.  At that point _rl_isearch_cleanup has already
been called from the signal handler, but we proceed regardless, using a cxt
pointer that has been freed.

Fix this by checking for "RL_ISSTATE (RL_STATE_ISEARCH)" after the call to
_rl_search_getchar:
...
   c = _rl_search_getchar (cxt);
+  if (!RL_ISSTATE (RL_STATE_ISEARCH))
+    return 1;
...

Tested on x86_64-linux.

Approved-By: Chet Ramey <chet.ramey@case.edu>
PR tui/30056
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30056

Automatic date update in version.in

[PR ld/22263][PR ld/25694] RISC-V: Avoid dynamic TLS relocs in PIE.

Lots of targets already fixed the TEXTREL problem for TLS in PIE.

* For PR ld/25694,
In the check_reloc, refer to spare and loongarch, they don't need to reserve
any local dynamic reloc for TLS LE in pie/pde, and similar to other targets.
So it seems like riscv was too conservative to estimate the TLS LE before.
Just break and don't goto static_reloc for TLS LE in pie/pde can fix the
TEXTREL problem.

* For PR ld/22263,
The risc-v code for TLS GD/IE in the relocate_section seems same as MIPS port.
So similar to MIPS, pr22570, commits 9143e72c6d4d and 1cb83cac9a89, it seems
also the right way to do the same thing for risc-v.

On risc-v, fixes
FAIL: Build pr22263-1

RISC-V haven't supported the TLS transitions, so will need the same fix (use
bfd_link_dll) in the future.

bfd/
PR ld/22263
PR ld/25694
* elfnn-riscv.c (riscv_elf_check_relocs): Replace bfd_link_pic with
bfd_link_dll for TLS IE.  Don't need to reserve the local dynamic
relocation for TLS LE in pie/pde, and report error in pic just like
before.
(riscv_elf_relocate_section): For TLS GD/IE, use bfd_link_dll rather
than !bfd_link_pic in determining the dynamic symbol index.  Avoid
the index of -1.

Automatic date update in version.in

Enhance objdump's --private option so that it can display the contents of PE format files.

  * od-pe.c: New file: Dumps fields in PE format headers.
  * configure.ac (od_vectors): Add objdump_private_desc_pe for PE format targets. (od_files): Add od-pe for PE format targets.
  * configure: Regenerate.
  * Makefile.am (CFILES): Add od-pe.c (EXTRA_objdump_SOURCE): Likewise.
  * Makefile.in: Generate.
  * NEWS: Mention the new feature.
  * doc/binutils.texi: Document the new support.
  * objdump.c (wide_output): Change from local to global.
  * objdump.h (wide_output): Prototype. (objdump_private_desc_pe): Prototype.
  * testsuite/binutils-all/objdump.exp: Add a test of the new feature.

Remove duplicate definition

* coff/pe.h (IMAGE_FILE_MACHINE_AMD64): Remove duplicate
definition.  Alphabetize.

x86: fix disassembler build after 1a3b4f90bc5f

In commit 1a3b4f90bc5f ("x86: convert two pointers to (indexing)
integers") I neglected the fact that compilers may warn about comparing
ptrdiff_t (signed long) with size_t (unsigned long) values. Since just
before we've checked that the value is positive, simply add a cast
(despite my dislike for casts).

[gdb/testsuite] Add test-case gdb.tui/color-prompt.exp

Add a test-case that sets a prompt with color in TUI.

The line containing the prompt is shown by get_line_with_attrs as follows:
...
<fg:31>(gdb) <fg:default>
...

The 31 means red, but only for foreground colors, for background colors 41
means red.

Make this more readable by using color names for both foreground and
background, such that we have instead:
....
<fg:red>(gdb) <fg:default>
...

Tested on x86_64-linux.

[gdb/testsuite] Add invisible and blinking attributes in tuiterm

I noticed curses using the invisible and blinking attributes.

Add these in tuiterm.

Tested on x86_64-linux.

[gdb/testsuite] Fix reverse attribute in tuiterm

I noticed in proc Term::_csi_m arguments that while parameters 7 and 27 are
supposed to set the reverse attribute to 1 and 0, in fact it's set to 1 in
both cases:
...
      7 {
set _attrs(reverse) 1
    }
  ...
    27 {
set _attrs(reverse) 1
      }
...

Fix this and add a regression test in gdb.tui/tuiterm.exp.

Tested on x86_64-linux.

iamcu: suppress tests which can't possibly work

With neither --32 nor --64 passed to gas, advanced features like AVX
aren't available without explicitly enabling them.

x86-64: improve gas diagnostic when no 32-bit target is configured

Make this similar to --64 and --x32: Check whether a suitable target
exists.

x86-64: conditionalize tests using --32

Using this option doesn't really work when no support for any 32-bit
target was configured in (as is the case for at least cloudabi and
rdos).

x86: split gas testsuite .exp file

The set of 32-bit-only and 64-bit-only tests has grown quite large. In
particular when one's after only the results for the 64-bit set, having
them live in a separate .exp file is easier / faster.

x86: convert two pointers to (indexing) integers

This in particular reduces the number of pointers to non-const that we
have (and that could potentially be used for undue modification of
state). As a result, fetch_code()'s 2nd parameter can then also become
pointer-to-const.

x86: disassembling over-long insns

The present way of dealing with them - misusing MAX_MNEM_SIZE, which has
nothing to do with insn length - leads to inconsistent results. Since we
allow for up to MAX_CODE_LENGTH - 1 prefix bytes (which then could be
followed by another MAX_CODE_LENGTH "normal" insn bytes until we're done
decoding), size the_buffer[] accordingly.

Move struct dis_private down to be able to use MAX_CODE_LENGTH without
moving its #define. While doing this also alter the order to have the
potentially large array last.

x86: use fixed-width type for codep and friends

This first of all removes a dependency on bfd_byte and unsigned char
being the same types. It further eliminates the need to mask by 0xff
when fetching values (which wasn't done fully consistently anyway),
improving code legibility.

While there, where possible add const.

x86: figure braces aren't really part of mnemonics

Instead they're separators for pseudo-prefixes. Don't insert them in
mnemonic_chars[], handling them explicitly in parse_insn() instead. Note
that this eliminates the need for another separator after a pseudo-
prefix. While maybe not overly interesting for a following real
mnemonic, I view this as quite desirable between multiple successive
pseudo-prefixes (bringing things in line with the other use of figure
braces in AVX512's zeroing-masking).

Drop the unused is_mnemonic_char() at this occasion.

x86: de-duplicate operand_special_chars[] wrt extra_symbol_chars[]

Having to add characters to both arrays can easily lead to oversights.
Consuming extra_symbol_chars[] when populating operand_chars[] also
allows to drop two special cases in md_begin().

Constify operand_special_chars[] at this occasion.

sframe/doc: minor improvements for readability

libsframe/
* sframe-spec.texi: Cosmetic fixes.

libsframe: revisit sframe_find_fre API

Inspite of implementing a rather simple functionality, this function was
relatively difficult to follow, and maintain.  Some changes are done now
to address that - refactor the function and use better names to make it
more readable.

The changes to the implementation do not cause any change in the
contract of the API.

libsframe/
        * sframe.c (sframe_fre_get_end_ip_offset): to here...
        (sframe_find_fre): Refactor some bits from...

libsframe: use const char * consistently for immutable FRE buffers

libsframe/
        * sframe.c (sframe_decode_fre): Use const char * datatype when
handling buffer containing the FREs.
(sframe_fre_get_end_ip_offset): Likewise.
(sframe_find_fre): Likewise.
(sframe_decoder_get_fre): Likewise.

libsframe: use uint8_t data type for FRE info related stubs

libsframe/
* sframe.c: Use uint8_t for FRE offset count and FRE offset
size.  Use uint8_t for FRE info word as well.

PR22263 ld test

A number of targets that I test regularly fail the "Build pr22263-1"
test for various reasons.

arm-linux-gnueabi: "undefined reference to `__aeabi_read_tp'"
ia64-linux-gnu: "Explicit stops are ignored in auto mode"
m68k-linux-gnu: "undefined reference to `__m68k_read_tp'"
microblaze-linux-gnu: "undefined reference to `__tls_get_addr'"
nios2-linux-gnu, s390-linux-gnu and sh4-linux-gnu have a tprel reloc in .got
riscv64-linux-gnu has a dynamic relocation in text

So only riscv really fails the pr.  The rest fail due to test issues
or lack of a linker optimisation.  Lack of an optimisation isn't
really a fail, but it's worth keeping the test to ensure those
optimisations don't regress.  The xfail targets may not be an
exhaustive list.  This just tidies test results for those for which I
have cross compilers installed.

PR 22263
* testsuite/ld-elf/tls.exp: Split pr22263 test into two parts,
one to check for -z text errors, the other to check tprel
linker optimisation.  Supply needed symbols and assembler flags.
xfail the linker optimisation on targets known to fail.

Make MI commands const-correct

I've had this patch for a while now and figured I'd update it and send
it.  It changes MI commands to use a "const char * const" for their
argv parameter.

Regression tested on x86-64 Fedora 36.

Acked-By: Simon Marchi <simon.marchi@efficios.com>

Automatic date update in version.in

Fix scoped_value_mark not working with empty value chain

The scoped_value_mark helper class was setting its internal
mark value to NULL to indicate that the value chain had already
been freed to mark.

However, value_mark() also returns NULL if the value chain is
empty at the time of call.

This lead to the situation that if the value chain was empty
at the time the scoped_value_mark was created, the class
would not correctly clean up the state when it was destroyed,
because it believed it had already been freed.

I noticed this because I was setting a watchpoint very early
in my debug session, and it was becoming a software watchpoint
rather than hardware. Running any command that called evaluate()
beforehand (such as 'x 0') would mean that a hardware watchpoint
was correctly used. After some careful examination of the
differences in execution, I noticed that values were being freed
later in the 'bad case', which lead me to notice the issue with
scoped_value_mark.

gdb: remove breakpoint_pointer_iterator

Remove the breakpoint_pointer_iterator layer.  Adjust all users of
all_breakpoints and all_tracepoints to use references instead of
pointers.

Change-Id: I376826f812117cee1e6b199c384a10376973af5d
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdbsupport: make filtered_iterator::operator* return the same thing as underlying iterator

This is the same idea as the previous patch, but for filtered_iterator.
Without this patch, I would see this when applying the patch that
removes reference_to_pointer_iterator from breakpoint_range:

      CXX    breakpoint.o
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c: In function ‘void download_tracepoint_locations()’:
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:11007:41: error: cannot allocate an object of abstract type ‘breakpoint’
    11007 |   for (breakpoint &b : all_tracepoints ())
          |                                         ^
    In file included from /home/smarchi/src/binutils-gdb/gdb/gdbthread.h:26,
                     from /home/smarchi/src/binutils-gdb/gdb/infrun.h:21,
                     from /home/smarchi/src/binutils-gdb/gdb/gdbarch.h:28,
                     from /home/smarchi/src/binutils-gdb/gdb/arch-utils.h:23,
                     from /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:21:
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.h:619:8: note:   because the following virtual functions are pure within ‘breakpoint’:
      619 | struct breakpoint : public intrusive_list_node<breakpoint>
          |        ^~~~~~~~~~
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:250:1: note:     ‘virtual breakpoint::~breakpoint()’
      250 | breakpoint::~breakpoint ()
          | ^~~~~~~~~~

Change-Id: I05285ff27d21cb0ab80cba392ec4e959167e3cd7
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdbsupport: make basic_safe_iterator::operator* return the same thing as underlying iterator

Using the following patch that removes the reference_to_pointer_iterator
from breakpoint_range, I would get:

      CXX    breakpoint.o
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c: In function ‘void breakpoint_program_space_exit(program_space*)’:
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:3030:46: error: cannot allocate an object of abstract type ‘breakpoint’
     3030 |   for (breakpoint &b : all_breakpoints_safe ())
          |                                              ^
    In file included from /home/smarchi/src/binutils-gdb/gdb/gdbthread.h:26,
                     from /home/smarchi/src/binutils-gdb/gdb/infrun.h:21,
                     from /home/smarchi/src/binutils-gdb/gdb/gdbarch.h:28,
                     from /home/smarchi/src/binutils-gdb/gdb/arch-utils.h:23,
                     from /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:21:
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.h:619:8: note:   because the following virtual functions are pure within ‘breakpoint’:
      619 | struct breakpoint : public intrusive_list_node<breakpoint>
          |        ^~~~~~~~~~
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:250:1: note:     ‘virtual breakpoint::~breakpoint()’
      250 | breakpoint::~breakpoint ()
          | ^~~~~~~~~~

This is because the operator* method of the basic_safe_iterator iterator
wrapper returns a value_type.  So, even if the method of the underlying
iterator (breakpoint_iterator, an intrusive_list iterator) returns a
`breakpoint &`, the method of the wrapper returns a `breakpoint`.

I think it would make sense for iterator wrappers such as
basic_safe_iterator to return the exact same thing as the iterator they
wrap.  At least, it fixes my problem.

Change-Id: Ibbcd390ac03d2fb6ae4854923750c8d7c3c04e8a
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: link breakpoints with intrusive_list

Change-Id: I043d8d6f3dd864d80d5088f6ffc2c098337249ea
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: remove bp_location_pointer_iterator

Remove the bp_location_pointer_iterator layer.  Adjust all users of
breakpoint::locations to use references instead of pointers.

Change-Id: Iceed34f5e0f5790a9cf44736aa658be6d1ba1afa
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: use intrusive_list for breakpoint locations

Replace the hand-maintained linked lists of breakpoint locations with
and intrusive list.

 - Remove breakpoint::loc, add breakpoint::m_locations.

 - Add methods for the various manipulations that need to be done on the
   location list, while maintaining reasonably good encapsulation.

 - bp_location currently has a default constructor because of one use
   in hoist_existing_locations.  hoist_existing_locations now returns a
   bp_location_list, and doesn't need the default-constructor
   bp_location anymore, so remove the bp_location default constructor.

 - I needed to add a call to clear_locations in delete_breakpoint to
   avoid a use-after-free.

 - Add a breakpoint::last_loc method, for use in
   set_breakpoint_condition.

bp_location_range uses reference_to_pointer_iterator, so that all
existing callers of breakpoint::locations don't need to change right
now.  It will be removed in the next patch.

The rest of the changes are to adapt the call sites to use the new
methods, of breakpoint::locations, rather than breakpoint::loc directly.

Change-Id: I25f7ee3d66a4e914a0540589ac414b3b820b6e70
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdbsupport: add missing increment/decrement operators to reference_to_pointer_iterator

Using the following patch, I would get this build failure:

      CXX    breakpoint.o
    In file included from /usr/include/c++/13.1.1/bits/stl_algobase.h:66,
                     from /usr/include/c++/13.1.1/bits/hashtable_policy.h:36,
                     from /usr/include/c++/13.1.1/bits/hashtable.h:35,
                     from /usr/include/c++/13.1.1/bits/unordered_map.h:33,
                     from /usr/include/c++/13.1.1/unordered_map:41,
                     from /usr/include/c++/13.1.1/functional:63,
                     from /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/ptid.h:35,
                     from /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/common-defs.h:206,
                     from /home/smarchi/src/binutils-gdb/gdb/defs.h:26,
                     from /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:20:
    /usr/include/c++/13.1.1/bits/stl_iterator_base_funcs.h: In instantiation of ‘constexpr void std::__advance(_BidirectionalIterator&, _Distance, bidirectional_iterator_tag) [with _BidirectionalIterator = reference_to_pointer_iterator<intrusive_list_iterator<bp_location, intrusive_base_node<bp_location> > >; _Distance = long int]’:
    /usr/include/c++/13.1.1/bits/stl_iterator_base_funcs.h:224:21:   required from ‘constexpr void std::advance(_InputIterator&, _Distance) [with _InputIterator = reference_to_pointer_iterator<intrusive_list_iterator<bp_location, intrusive_base_node<bp_location> > >; _Distance = long int]’
    /usr/include/c++/13.1.1/bits/stl_iterator_base_funcs.h:237:19:   required from ‘constexpr _InputIterator std::next(_InputIterator, typename iterator_traits<_Iter>::difference_type) [with _InputIterator = reference_to_pointer_iterator<intrusive_list_iterator<bp_location, intrusive_base_node<bp_location> > >; typename iterator_traits<_Iter>::difference_type = long int]’
    /home/smarchi/src/binutils-gdb/gdb/breakpoint.c:1073:19:   required from here
    /usr/include/c++/13.1.1/bits/stl_iterator_base_funcs.h:179:11: error: no match for ‘operator--’ (operand type is ‘reference_to_pointer_iterator<intrusive_list_iterator<bp_location, intrusive_base_node<bp_location> > >’)
      179 |           --__i;
          |           ^~~~~

This points out that while intrusive_list_iterator has an operator--,
the reference_to_pointer_iterator wrapper does not.  I'm not to sure why
the compiler chooses the overload of __advance that accepts a
_BidirectionalIterator, given that reference_to_pointer_iterator can't
be decremented, but adding those operators seems like the right thing to
do in any case, for completeness.

Change-Id: I8e2044b6734fadf0f21093047cf35bb7080dbdc3
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: add breakpoint::first_loc methods

Add convenience first_loc methods to struct breakpoint (const and
non-const overloads).  A subsequent patch changes the list of locations
to be an intrusive_list and makes the actual list private, so these
spots would need to change from:

    b->loc

to something ugly like:

    *b->locations ().begin ()

That would make the code much heavier and not readable.  There is a
surprisingly big number of places that access the first location of
breakpoints.  Whether this is correct, or these spots fail to consider
the possibility of multi-location breakpoints, I don't know.  But
anyhow, I think that using this instead:

 b->first_loc ()

conveys the intention better than the other two forms.

Change-Id: Ibbefe3e4ca6cdfe570351fe7e2725f2ce11d1e95
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: add breakpoint "has locations" methods

Add three convenience methods to struct breakpoint:

 - has_locations: returns true if the breakpoint has at least one
   location
 - has_single_location: returns true if the breakpoint has exactly one
   location
 - has_multiple_locations: returns true if the breakpoint has more than
   one location

A subsequent patch changes the list of breakpoints to be an
intrusive_list, so all these spots would need to change.  But in any
case, I think that this:

  if (b->has_multiple_locations ())

conveys the intention better than:

  if (b->loc != nullptr && b->loc->next != nullptr)

Change-Id: Ib18c3605fd35d425ef9df82cb7aacff1606c6747
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: constify breakpoint::print_it parameter

The print_it method itself is const.  In a subsequent patch, the
locations that come out of a const breakpoint will be const as well.  It
will therefore be needed to make the last_loc output parameter const as
well.  Make that change now to reduce the size of the following patches.

Change-Id: I7ed962950bc9582646e31e2e42beca2a1c9c5105
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: make some breakpoint methods use `this`

Some implementations of breakpoint::check_status and
breakpoint::print_it do this:

    struct breakpoint *b = bs->breakpoint_at;

bs->breakpoint_at is always the same as `this` (we can get convinced by
looking at the call sites of check_status and print_it), so it would
just be clearer to access fields through `this` instead.

Change-Id: Ic542a64fcd88e31ae2aad6feff1da278c7086891
Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb: get gdbarch from syscall_catchpoint instead of location

I noticed some methods of syscall_catchpoint doing this:

  struct gdbarch *gdbarch = loc->owner->gdbarch;

`loc` is the list of locations of this catchpoint.  Logically, the owner
the locations are this catchpoint.  So this just ends up getting
this->gdbarch.  Remove the unnecessary indirection through the loc.

syscall_catchpoint::print_recreate does something slightly different,
getting its arch from the loc:

  struct gdbarch *gdbarch = loc->gdbarch;

I suppose it's always going to be the same arch, so get it from the
catchpoint there too.

Change-Id: I6f6a6f8e0cd7cfb754cecfb6249e71ec12ba4855
Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

PR29189, dlltool delaylibs corrupt float/double arguments

PR 29189
* dlltool.c (i386_x64_trampoline): Save and restore xmm0-5.  Make
use of parameter save area for integer arg regs.  Comment.

Automatic date update in version.in

gdbsupport: add support for references to checked_static_cast

Add a checked_static_cast overload that works with references.  A bad
dynamic cast with references throws std::bad_cast, it would be possible
to implement the new overload based on that, but it seemed simpler to
just piggy back off the existing function.

I found some potential uses of this new overload in amd-dbgapi-target.c,
update them to illustrate the use of the new overload.  To build
amd-dbgapi-target.c, on needs the amd-dbgapi library, which I don't
expect many people to have.  But I have it, and it builds fine here.  I
did test the new overload by making a purposely bad cast and it did
catch it.

Change-Id: Id6b6a7db09fe3b4aa43cddb60575ff5f46761e96
Reviewed-By: Lancelot SIX <lsix@lancelotsix.com>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>

gdb/testsuite: fix race in gdb.server/multi-ui-errors.exp

After this commit:

  commit ed32754a8c7919feffc6ddb66ff1c532e4a4d1cd
  Date:   Thu Mar 9 10:45:03 2023 +0100

      [gdb/testsuite] Fix gdb.server/multi-ui-errors.exp for remote target

I noticed the occasional failure in gdb.server/multi-ui-errors.exp,
which looked like this:

  (gdb) PASS: gdb.server/multi-ui-errors.exp: interact with GDB's main UI
  interrupt
  (gdb)
  Program received signal SIGINT, Interrupt.
  0x00007ffff7d501e7 in nanosleep () from /lib64/libc.so.6
  FAIL: gdb.server/multi-ui-errors.exp: interrupt (timeout)
  PASS: gdb.server/multi-ui-errors.exp: interrupt arrived
  p server_pid
  $1 = 718174
  (gdb) PASS: gdb.server/multi-ui-errors.exp: p server_pid

This is triggered by this code in gdb.server/multi-ui-errors.exp:

    gdb_test "interrupt"

    gdb_test_multiple "" "interrupt arrived" {
-re "Program received signal SIGINT, Interrupt\\.\r\n" {
    pass $gdb_test_name
}
    }

The problem here is that the first interrupt will trigger the prompt
to be printed, and then, after some time the inferior will be
interrupted.

However the default pattern for gdb_test includes a '$' end anchor.
If expect sees the prompt with nothing following it then everything is
fine, and the test passes.

However, if the interrupt is quick and so what expect sees is this:

  (gdb)
  Program received signal SIGINT, Interrupt.
  0x00007ffff7d501e7 in nanosleep () from /lib64/libc.so.6

In this case the end anchor means that the gdb_test fails to match,
and eventually times out.

Fix this by passing -no-prompt-anchor to gdb_test.

Reviewed-By: Tom de Vries <tdevries@suse.de>

gdb, infcmd: Support jump command with same line in multiple symtabs

If a header file defining a static function is included in multiple source
files, each calling the function, and GDB is asked to jump to a line inside
that function, there would be multiple locations matching the target.  The
solution in this commit is to select the location in the current symtab.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>

Add "args" and "env" parameters to DAP launch request

This patch augments the DAP launch request with some optional new
parameters that let the client control the command-line arguments and
the environment of the inferior.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Add attributes and methods to gdb.Inferior

This adds two new attributes and three new methods to gdb.Inferior.

The attributes let Python code see the command-line arguments and the
name of "main".  Argument setting is also supported.

The methods let Python code manipulate the inferior's environment
variables.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Remove accidentally added file

Don't optimise bfd_seek to same position

It's not worth avoiding an fseek to the same position, and can cause
problems if the linker's output file (which is opened "w+") is read,
because that can result in writing, reading, then writing again.

POSIX.1-2017 (IEEE Std 1003.1) says of fopen:
"When a file is opened with update mode ('+' as the second or third
character in the mode argument), both input and output may be
performed on the associated stream. However, the application shall
ensure that output is not directly followed by input without an
intervening call to fflush() or to a file positioning function
(fseek(), fsetpos(), or rewind()), and input is not directly followed
by output without an intervening call to a file positioning function,
unless the input operation encounters end-of-file."

* bfdio.c (bfd_seek): Always call iovec->bseek.

Automatic date update in version.in

Handle DAP evaluate request without a frame ID

DAP specifies that if an evaluate request does not have a frameID
parameter, then the expression is evaluated in the global scope.

Add global_context parameter to gdb.parse_and_eval

This adds a 'global_context' parse_and_eval to gdb.parse_and_eval.
This lets users request a parse that is done at "global scope".

I considered letting callers pass in a block instead, with None
meaning "global" -- but then there didn't seem to be a clean way to
express the default for this parameter.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Add flags to parse_and_eval

This adds a flags parameter to parse_and_eval.

Add PARSER_LEAVE_BLOCK_ALONE flag

This adds a PARSER_LEAVE_BLOCK_ALONE flag, and changes the parse API
to respect it.  This flag lets callers avoid any change to the
passed-in block and expression PC, letting them specify the context
exactly.  In particular, now nullptr can be used to indicate that the
parse should not examine any local variables.

Add PARSER_DEBUG flag

This adds a new PARSER_DEBUG constant and changes the parser code to
use it.  This lets us make the 'parser_debug' global 'static'.

Rearrange parser_state

This patch mildly rearranges parser_state, moving all the bool fields
together.

Boolify parser_state::comma_terminates

parser_state::comma_terminates ought to be boolean, and changing it
does not require any other changes.

Simplify parser_state constructor

This simplifies the parser_state constructor by having it accept a
parser_flags parameter.

Introduce and use parser flags

This patch adds a new parser_flags type and changes the parser APIs to
use it rather than a collection of 'int' and 'bool'.  More flags will
be added in subsquent patches.

Move innermost_block_tracker to expression.h

I think parser-defs.h should hold declarations that can be used by
parser implementations, whereas expression.h should hold declarations
that are used by code that wants to call a parser.  Following this
logic, this patch moves innermost_block_tracker to expression.h.

Avoid forward declaration in parse.c

This minorly rearranges parse.c to avoid the need for a forward
declaration.

Implement DAP loadedSources request

This implements the DAP loadedSources request, using gdb.execute_mi to
avoid having to write another custom Python API.

Implement gdb.execute_mi

This adds a new Python function, gdb.execute_mi, that can be used to
invoke an MI command but get the output as a Python object, rather
than a string.  This is done by implementing a new ui_out subclass
that builds a Python object.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=11688
Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Add second mi_parse constructor

This adds a second mi_parse constructor.  This constructor takes a
command name and vector of arguments, and does not do any escape
processing.  This also changes mi_parse::args to handle parse objects
created this new way.

Introduce mi_parse helper methods

This introduces some helper methods for mi_parse that handle some of
the details of parsing.  This approach lets us reuse them later.

Introduce "static constructor" for mi_parse

Change the mi_parse function to be a static method of mi_parse.  This
lets us remove the 'set_args' setter function.

Change mi_parse_argv to a method

This changes mi_parse_argv to be a method of mi_parse.  This is just a
minor cleanup.

Use accessor for mi_parse::args

This changes mi_parse::args to be a private member, retrieved via
accessor.  It also changes this member to be a std::string.  This
makes it simpler for a subsequent patch to implement different
behavior for argument parsing.

Use member initializers in mi_parse

This changes mi_parse to use member initializers rather than a
constructor.  This is easier to follow.

Use field_signed from Python MI commands

If an MI command written in Python includes a number in its output,
currently that is simply emitted as a string.  However, it's
convenient for a later patch if these are emitted using field_signed.
This does not make a difference to ordinary MI clients.

gdb/cli-out.c: clear_current_line shouldn't trigger pagination prompt

clear_current_line overwrites the current line with chars_per_line
blank spaces.  Printing the final space triggers a condition in
pager_file::puts that causes lines_printed to be incremented.  If
lines_printed becomes greater than or equal to lines_allowed, the
pagination prompt will appear if enabled.

In this case the prompt is unnecessary since after printing the final
space clear_current_line immediately moves the cursor to the beginning
of the line with '\r'.  A new line isn't actually started, so the prompt
ends up being spurious.

Additionally it's possible for gdb to crash during this pagination prompt.
Answering the prompt with 'q' throws an exception intended to bring gdb
back to the main event loop.  But since commit 0fea10f32746,
clear_current_line may be called under the progress_update destructor.
The exception will try to propagate through the destructor, causing an abort.

To fix this, pagination is disabled for the duration for clear_current_line.
clear_current_line is also renamed to clear_progress_notify to help
indicate that it is a special purpose function intended for use with
do_progress_notify.

Acked-by: Eli Zaretskii <eliz@gnu.org>

PR30437 aarch64: make RELA relocs idempotent

normally RELA relocs in BFD should not consider the contents of the
relocated place.  The aarch64 psABI is even stricter, it specifies
(section 5.7.16) that all RELA relocs _must_ be idempotent.

Since the inception of the aarch64 BFD backend all the relocs have a
non-zero src_mask, and hence break this invariant.  It's normally not
a very visible problem as one can see it only when the relocated place
already contains a non-zero value, which usually only happens sometimes
when using 'ld -r' (or as in the testcase when jumping through hoops to
generate the relocations).  Or with alternative toolchains that do encode
stuff in the relocated places with the assumption that a relocation
to that place ignores whatever is there (as they can according to
the psABI).

Golang is such a toolchain and https://github.com/golang/go/issues/39927
is ultimately caused by this problem: the testcase testGCData failing
is caused by the garbage collection data-structure to describe a type
containing pointers to be wrong.  It's wrong because a field that's
supposed to contain a file-relative offset (to some gcbits) has a
relocation applied and that relocation has an addend which also is
already part of the go-produced object file (so the addend is
implicitely applied twice).

bfd/
PR ld/30437
* elfnn-aarch64.c (elfNN_aarch64_howto_table): Clear src_mask
if all relocation descriptors.

ld/
* testsuite/ld-aarch64/rela-idempotent.s: New testcase.
* testsuite/ld-aarch64/rela-idempotent.d: New.
* testsuite/ld-aarch64/aarch64-elf.exp: Run it.

Updated Swedish translation for the opcodes directory

gdb/testsuite: change hardcoded assembly in gdb.arch/disp-step-insn-reloc.exp

When testing gdb.arch/disp-step-insn-reloc.exp with clang in an x86_64
machine, the compiled test case would segfault when returning from
the function can_relocate_call, with a suggestion of a broken stack.
The example assembly in the commment was the following:

   f:
     MOV $1, %[ok]
     JMP end
   set_point0:
     CALL f ; tracepoint here.
   end:

And the segmentation fault happening at the final "ret" instruction of
can_relocate_call.  Looking at the disassembled version of the later
half of the important function, we see:

Clang version (f starting at 11a4):
  00000000000011ae <set_point0>:
      11ae:       e8 f1 ff ff ff          callq  11a4 <can_relocate_call+0x14>
      11b3:       89 45 fc                mov    %eax,-0x4(%rbp)
      11b6:       83 7d fc 01             cmpl   $0x1,-0x4(%rbp)
      11ba:       0f 85 0a 00 00 00       jne    11ca <set_point0+0x1c>
      11c0:       e8 5b 00 00 00          callq  1220 <pass>
      11c5:       e9 05 00 00 00          jmpq   11cf <set_point0+0x21>
      11ca:       e8 61 00 00 00          callq  1230 <fail>
      11cf:       48 83 c4 10             add    $0x10,%rsp
      11d3:       5d                      pop    %rbp
      11d4:       c3                      retq
      11d5:       66 66 2e 0f 1f 84 00    data16 nopw %cs:0x0(%rax,%rax,1)
      11dc:       00 00 00 00

gcc version (f starting at 401125):
  000000000040112c <set_point0>:
    40112c:       e8 f4 ff ff ff          callq  401125 <can_relocate_call+0x11>
    401131:       89 45 fc                mov    %eax,-0x4(%rbp)
    401134:       83 7d fc 01             cmpl   $0x1,-0x4(%rbp)
    401138:       75 07                   jne    401141 <set_point0+0x15>
    40113a:       e8 c7 ff ff ff          callq  401106 <pass>
    40113f:       eb 05                   jmp    401146 <set_point0+0x1a>
    401141:       e8 c7 ff ff ff          callq  40110d <fail>
    401146:       90                      nop
    401147:       c9                      leaveq
    401148:       c3                      retq

The epilogue of set_point0 (11cf for clang, 401146 for gcc) is the main
difference: GCC's version uses the leaveq instruction, which resets rsp
based on rbp, while clang adds the same constant to rsp that it
subtracted in the prologue.  Clang fails because the return address that
is added by the "call f" instruction isn't accounted for.

This commit fixes that by adding a return instruction to f, which leaves
the rsp as the compilers would expect.

Approved-By: Andrew Burgess <aburgess@redhat.com>

x86/Intel: address quoted-symbol related FIXMEs

If in a "word ptr <address>" or alike construct the "ptr" part is
double-quoted, it shouldn't be recognized as the specific keyword we're
looking for (just like we don't recognize double-quoted operator or
register names anymore). Be careful though to tell closing from opening
double-quotes, as a quoted symbol may follow right afterwards.

x86: don't recognize quoted symbol names as registers or operators

The concept of quoted symbols names was introduced pretty late. Utilize
it to allow access to symbols with names matching that of a register (or,
in Intel syntax, also an identifier-like operator).

This is primarily to aid gcc when generating Intel syntax output; see
their bug target/53929.

Support Intel FRED LKGS

gas/ChangeLog:

* NEWS: Support Intel FRED LKGS.
* config/tc-i386.c: Add fred lkgs
* doc/c-i386.texi: Document .fred, .lkgs.
* testsuite/gas/i386/i386.exp: Add FRED LKGS tests
* testsuite/gas/i386/x86-64-fred-intel.d: Ditto.
* testsuite/gas/i386/x86-64-fred.d: Ditto.
* testsuite/gas/i386/x86-64-fred.s: Ditto.
* testsuite/gas/i386/x86-64-lkgs-intel.d: Ditto.
* testsuite/gas/i386/x86-64-lkgs-inval.l: Ditto.
* testsuite/gas/i386/x86-64-lkgs-inval.s: Ditto.
* testsuite/gas/i386/x86-64-lkgs.d: Ditto.
* testsuite/gas/i386/x86-64-lkgs.s: Ditto.

opcodes/ChangeLog:

* i386-dis.c: New entry for fred, lkgs.
* i386-gen.c: Add CPU_FRED CPU_LKGS.
* i386-init.h : Regenerated.
* i386-mnem.h : Regenerated.
* i386-opc.h: Add fred, lkgs.
* i386-opc.tbl: Add FRED, LKGS instructions.
* i386-tbl.h: Regenerated.

Revert "Support Intel FRED LKGS"

This reverts commit e5a497fe38e0ab19e16bdd9e4b4ed5e4d0056478.

Support Intel FRED LKGS

gas/ChangeLog:

        * NEWS: Support Intel FRED LKGS.
        * config/tc-i386.c: Add fred lkgs
        * doc/c-i386.texi: Document .fred, .lkgs.
        * testsuite/gas/i386/i386.exp: Add FRED LKGS tests
        * testsuite/gas/i386/x86-64-fred-intel.d: Ditto.
        * testsuite/gas/i386/x86-64-fred.d: Ditto.
        * testsuite/gas/i386/x86-64-fred.s: Ditto.
        * testsuite/gas/i386/x86-64-lkgs-intel.d: Ditto.
        * testsuite/gas/i386/x86-64-lkgs-inval.l: Ditto.
        * testsuite/gas/i386/x86-64-lkgs-inval.s: Ditto.
        * testsuite/gas/i386/x86-64-lkgs.d: Ditto.
        * testsuite/gas/i386/x86-64-lkgs.s: Ditto.

opcodes/ChangeLog:

        * i386-dis.c: New entry for fred, lkgs.
        * i386-gen.c: Add CPU_FRED CPU_LKGS.
        * i386-init.h : Regenerated.
        * i386-mnem.h : Regenerated.
        * i386-opc.h: Add fred, lkgs.
        * i386-opc.tbl: Add FRED, LKGS instructions.
        * i386-tbl.h: Regenerated.

Automatic date update in version.in

[gdb/tui] Fix buglet in tui_update_variables

I noticed a buglet in tui_update_variables:
...
   entry = translate (tui_border_kind, tui_border_kind_translate_lrcorner);
   if (tui_border_lrcorner != (chtype) entry->value)
    {
      tui_border_lrcorner = (entry->value < 0) ? ACS_LRCORNER : entry->value;
...

When assigning the new value to tui_border_lrcorner, an entry->value of -1 is
taken into account, but not when comparing to the current value of
tui_border_lrcorner.

Fix this by introducing:
...
  int val = (entry->value < 0) ? ACS_LRCORNER : entry->value;
...
and using this in both comparison and assignment.

Tested on x86_64-linux.

Remove some FIXME comments from DAP

I recently added a 'dap' component to bugzilla, and I filed a few bugs
there.  This patch removes the corresponding FIXME comments.

A few such comments still exist.  In at least one case, I have a fix
I'll be submitting eventually; in others I think I need to do a bit of
investigation to properly file a bug report.

gdb: add Richard Bunt to gdb/MAINTAINERS

[gdb/testsuite] Add Term::get_line_with_attrs

Add a new proc Term::get_line_with_attrs, similar to Term::get_line, that
annotates a tuiterm line with the active attributes.

For instance, the line representing the TUI status window with attribute mode
standout looks like this with Term::get_line:
...
exec No process In: ... L??   PC: ??
...
but like this with Term::get_line_with_attrs:
...
<reverse:1>exec No process In: ... L??   PC: ?? <reverse:0>
...

Also add Term::dump_screen_with_attrs, a Term::dump_screen variant that uses
Term::get_line_with_attrs instead of Term::get_line.

Tested by re-running the TUI test-cases (gdb.tui/*.exp and gdb.python/tui*.exp)
on x86_64-linux.

[gdb/testsuite] Factor out Term::_reset_attrs

Factor out new proc Term::_reset_attrs.

Tested by re-running the TUI test-cases (gdb.tui/*.exp and gdb.python/tui*.exp)
on x86_64-linux.

Re: readelf: Support SHT_RELR/DT_RELR for -r

Revert value of DT_ENCODING to as it was before commit a7fd118627, and
adjust readelf.

include/
* elf/common.h (DT_ENCODING): Set back to 32.
binutils/
* readelf.c (struct filedata): Don't size dynamic_info array
using DT_ENCODING.

PowerPC64 report number of stub iterations

As a developer it is sometimes useful to know how many times stubs
have been resized.  Report the count for users too, in ld --stats.

Automatic date update in version.in

Automatic date update in version.in

Re: Bug 23686, two segment faults in nm

The fix for pr23686 had a hole in the reloc address sanity check,
the calculation could overflow.  Note that stabsize is known to be a
non-zero multiple of 12 so stabsize - 4 can't underflow.

PR 23686
* syms.c (_bfd_stab_section_find_nearest_line): Correct
r->address sanity check.

coffcode.h handle_COMDAT tidy

I started down the path of attempting to fix
https://sourceware.org/pipermail/binutils/2023-April/127263.html but
decided after a while that I didn't want to mess with this code..

This patch is a just a few things that I thought worth doing, the main
one being reporting of errors up the call chain.  The while loop to
for loop change is shamelessly stolen from Oleg.

* coffcode.h (handle_COMDAT): Return bool.  Make sec_flags a
flagword*, and adjust to suit.  Replace while loop with for
loop.  Check isym.n_numaux before reading aux entries.  Alloc
coff_comdat_info and name in one call to bfd_alloc.  Remove
goto breakloop.
(styp_to_sec_flags): Adjust handle_COMDAT call.

tic54x set_arch_mach

The tic54x backend provides its own coff_set_arch_mach, but wants to
use the standard coff_set_section_contents.  BFD_JUMP_TABLE_WRITE
defines both of these functions, so the code also provides a wrapper
for coff_set_section_contents.  This is all quite OK, but I was on a
mission to remove unnecessary declarations in coffcode.h, and on
deleting the one for coff_set_arch_mach ran into a warning about the
function being unused.  I could have kept that declaration with its
ATTRIBUTE_UNUSED or written "static bool ATTRIBUTE_UNUSED" on the
definition but the latter is not usual and looks odd to me.  So I
had a closer look at tic54x_set_arch_mach and decided the function is
very likely wrong to allow bfd_arch_unknown.  Thus the backend should
be using the standard coff_set_arch_mach.

* coff-tic54x.c: Use BFD_JUMP_TABLE_WRITE (coff) in target vecs.
(tic54x_coff_set_arch_mach): Delete.
(tic54x_set_section_contents): Delete.
* coffcode.h: Delete unnecessary forward declarations.

Automatic date update in version.in

Update documentation for Python Frame.older and Frame.newer

I noticed that Frame.older and Frame.newer don't document that they
return None at the ends of the stack.  This patch updates the
documentation, and also fixes a somewhat related typo in a comment
that I noticed while digging into this.

Approved-By: Eli Zaretskii <eliz@gnu.org>

ld: drop stray blank from ld.texi

At least older makeinfo complains about it. Also fix an apparent typo
while touching that line.

gdb: fix post-hook execution for remote targets

Commit b5661ff2 ("gdb: fix possible use-after-free when
executing commands") attempted to fix possible use-after-free
in case command redefines itself.

Commit 37e5833d ("gdb: fix command lookup in execute_command ()")
updated the previous fix to handle subcommands as well by using the
original command string to lookup the command again after its execution.

This fixed the test in gdb.base/define.exp but it turned out that it
does not work (at least) for "target remote" and "target extended-remote".

The problem is that the command buffer P passed to execute_command ()
gets overwritten in dont_repeat () while executing "target remote"
command itself:

#0  dont_repeat () at top.c:822
#1  0x000055555730982a in target_preopen (from_tty=1) at target.c:2483
#2  0x000055555711e911 in remote_target::open_1 (name=0x55555881c7fe ":1234", from_tty=1, extended_p=0)
    at remote.c:5946
#3  0x000055555711d577 in remote_target::open (name=0x55555881c7fe ":1234", from_tty=1) at remote.c:5272
#4  0x00005555573062f2 in open_target (args=0x55555881c7fe ":1234", from_tty=1, command=0x5555589d0490)
    at target.c:853
#5  0x0000555556ad22fa in cmd_func (cmd=0x5555589d0490, args=0x55555881c7fe ":1234", from_tty=1)
    at cli/cli-decode.c:2737
#6  0x00005555573487fd in execute_command (p=0x55555881c802 "4", from_tty=1) at top.c:688

Therefore the second call to lookup_cmd () at line 697 fails to find
command because the original command string is gone.

This commit addresses this particular problem by creating a *copy* of
original command string for the sole purpose of using it after command
execution to lookup the command again. It may not be the most efficient
way but it's safer given that command buffer is shared and overwritten
in hard-to-foresee situations.

Tested on x86_64-linux.

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

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

gdb: Remove redundant frame switching

547ce8f00b fixed an issue where dynamic types were not being resolved
correctly prior to printing a value. The same issue was discovered when
printing the value using mi-mode, which was not covered by the fix.
Porting the fix to the mi-mode code path resolved the issue.

However, it was discovered that a later patch series, ending
2fc3b8a4cb8, independently fixed the issue in both the cli- and mi-mode
code paths, making the original fix unneeded.

This commit removes this extra frame switch and adds test coverage for
the mi-mode scenario to protect against any future divergence in this
area.

GDB built with GCC 11.

No test suite regressions detected. Compilers: GCC 12.1.0, ACfL 22.1,
Intel 22.1; Platforms: x86_64, aarch64.

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

gdb: safety checks in skip_prologue_using_sal

While working on the previous patch I reverted this commit:

  commit e86e87f77fd5d8afb3e714f1d9e09e0ff5b4e6ff
  Date:   Tue Nov 28 16:23:32 2006 +0000

          * symtab.c (find_pc_sect_line): Do not return a line before
            the start of a symtab.

When I re-ran the testsuite I saw some GDB crashes in the tests:

  gdb.dwarf2/dw2-line-number-zero.exp
  gdb.dwarf2/dw2-lines.exp
  gdb.dwarf2/dw2-vendor-extended-opcode.exp

GDB was reading beyond the end of an array in the function
skip_prologue_using_sal.

Now, without the above commit reverted I don't believe that this
should ever happen.  Reverting the above commit effectively breaks
GDB's symtab_and_line lookup, we try to find a result for an address,
and return the wrong symtab and line-table.  In
skip_prologue_using_sal we then walk the line table looking for an
appropriate entry, except we never find one, and GDB just keeps going,
wandering off the end of the array.

However, I think adding extra protection to prevent walking off the
end of the array is pretty cheap, and if something does go wrong in
the future then this should prevent a random crash.

Obviously, I have no reproducer for this, as I said, I don't think
this should impact GDB at all, this is just adding a little extra
caution.

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

gdb/testsuite: test for a function with no line table

This commit adds a test for the following commit:

  commit e86e87f77fd5d8afb3e714f1d9e09e0ff5b4e6ff
  Date:   Tue Nov 28 16:23:32 2006 +0000

              * symtab.c (find_pc_sect_line): Do not return a line before
              the start of a symtab.

We have been carrying a test for that commit in the Fedora GDB tree
since that commit was added to GDB.  I don't know why the test wasn't
added along with the original commit, but as was written, the test is
pretty gross, it uses objcopy to pull the .text section from an object
file, which was then injected into another source file within a .asm
statement...

... these days we can just make use of the DWARF assembler to achieve
the same results, so I've rewritten the test and think it is worth
adding this to upstream GDB.

The original patch was about about how we find the best symtab and
line table entry, and what to do when GDB can't find a good match.

The new test creates a CU with two functions, only one of which is
covered by the line table.  With the above patch reverted GDB returns
an invalid address.

With the above patch reverted I did run the testsuite to see what
other tests might already be exercising this functionality, and I
found two tests:

  gdb.dwarf2/dw2-step-out-of-function-no-stmt.exp
  gdb.dwarf2/dw2-vendor-extended-opcode.exp

These are pretty similar, they either create minimal, or no line table
for one of the functions in the source file, and as a consequence GDB
returns an unexpected address at some point during the test.

However, both of those tests are really focused on other issues, so I
think this new test does add some value.  Plus the new test is not
large, so it's not a huge cost to also run this new test.

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