Commit graph

576 commits

Author SHA1 Message Date
Tom Tromey
36ed3d84e0 Introduce symbol_block_ops::get_block_value
This adds a new callback to symbol_block_ops.  This callback lets a
LOC_BLOCK symbol implement its own function to find the underlying
block.
2023-05-12 13:25:28 -06:00
Tom Tromey
bb051d7a30 Define symbol::value_block separately
This moves the definition of symbol::value_block outside of the class.
A subsequent patch will change this method to use SYMBOL_BLOCK_OPS,
and it seemed simplest to move this method out-of-line, and cleaner to
do this as a separate change.
2023-05-12 13:25:28 -06:00
Tom Tromey
93d50cd8f0 Rename "raw" to "unrelocated"
Per an earlier discussion, this patch renames the existing "raw" APIs
to use the word "unrelocated" instead.
2023-03-28 15:12:44 -06:00
Tom Tromey
9675da2535 Use unrelocated_addr in minimal symbols
This changes minimal symbols to use unrelocated_addr.  I believe this
detected a latent bug in add_pe_forwarded_sym.
2023-03-28 15:12:44 -06:00
Tom Tromey
1ee8702db9 Use unrelocated_addr in psymbols
This changes psymbols themselves to use unrelocated_addr.  This
transform is largely mechanical.  I don't think it finds any bugs.
2023-03-28 15:12:44 -06:00
Tom Tromey
257c302c69 Move definition of unrelocated_addr earlier
This moves the definition of unrelocated_addr a bit earlier in
symtab.h, so that it can be used elsewhere in the file.
2023-03-28 15:12:43 -06:00
Tom Tromey
72a8f76323 Change symbol::line to unsigned int
A user here at AdaCore noticed that, when debugging a certain program,
a stack frame reported line 34358, where it should have been line
99894.

After debugging a bit, I discovered:

(top) p (99894 & ~65536)
$60 = 34358

That line, symbol::line is too narrow.

This patch widens the member and changes all the uses that currently
use the narrower type.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-03-27 08:37:14 -06:00
Tom Tromey
48e0f38c30 Fix line table regression
Simon pointed out a line table regression, and after a couple of false
starts, I was able to reproduce it by hand using his instructions.

The bug is that most of the code in do_mixed_source_and_assembly uses
unrelocated addresses, but one spot does:

  pc = low;

... after the text offset has been removed.

This patch fixes the problem by introducing a new type to represent
unrelocated addresses in the line table.  This prevents this sort of
bug to some degree (it's still possible to manipulate a CORE_ADDR in a
bad way, this is unavoidable).

However, this did let the compiler flag a few spots in that function,
and now it's not possible to compare an unrelocated address from a
line table with an ordinary CORE_ADDR.

Regression tested on x86-64 Fedora 36, though note this setup never
reproduced the bug in the first place.  I also tested it by hand on
the disasm-optim test program.
2023-03-17 16:17:43 -06:00
Tom Tromey
ddc6677b17 Change linetable_entry::is_stmt to bool
This changes linetable_entry::is_stmt to type bool, rather than
unsigned.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-03-11 08:48:29 -07:00
Tom Tromey
977a0c161d Constify linetables
Linetables no longer change after they are created.  This patch
applies const to them.

Note there is one hack to cast away const in mdebugread.c.  This code
allocates a linetable using 'malloc', then later copies it to the
obstack.  While this could be cleaned up, I chose not to do so because
I have no way of testing it.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-03-11 08:48:10 -07:00
Tom Tromey
1acc9dca42 Change linetables to be objfile-independent
This changes linetables to not add the text offset to the addresses
they contain.  I did this in a few steps, necessarily combined
together in one patch: I renamed the 'pc' member to 'm_pc', added the
appropriate accessors, and then recompiled.  Then I fixed all the
errors.  Where possible I generally chose to use the raw_pc accessor,
as it is less expensive.

Note that this patch discounts the possibility that the text section
offset might cause wraparound in the addresses in the line table.
However, this was already discounted -- in particular,
objfile_relocate1 did not re-sort the table in this scenario.  (There
was a bug open about this, but as far as I can tell this has never
happened, it's not even clear what inspired that bug.)

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-03-11 08:47:40 -07:00
Tom Tromey
6e6ac32dde Add operator< and operator== to linetable_entry
This adds a couple of comparison operators to linetable_entry, and
simplifies both the calls to sort and one other spot that checks for
equality.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-03-11 08:39:51 -07:00
Tom Tromey
6fd9b78373 Remove expand_symtab_containing_pc
The function expand_symtab_containing_pc is unused; remove it.
Tested by rebuilding.
2023-02-26 10:46:30 -07:00
Tom de Vries
8908d9c45c [gdb/symtab] Trust epilogue unwind info for unknown producer (-g0 case)
For a -g0 -fasynchronous-unwind-tables exec (without .debug_info but with
.eh_frame section), start using the dwarf2 unwinder instead of the
"amd64 epilogue override" unwinder, by returning true in
compunit_epilogue_unwind_valid for cust == nullptr.

This has effect both on the amd64 and i386 targets, but only add amd64
test-case gdb.base/unwind-on-each-insn-amd64-2.exp.
2023-02-20 12:20:14 +01:00
Tom de Vries
cb911672fb [gdb/symtab] Factor out compunit_epilogue_unwind_valid
Factor out compunit_epilogue_unwind_valid from both
amd64_stack_frame_destroyed_p and i386_stack_frame_destroyed_p.  No functional
changes.

Also add a comment in the new function about the assumption that in absence of
producer information, epilogue unwind info is invalid.

Approved-By: Tom Tromey <tom@tromey.com>
2023-02-20 12:20:14 +01:00
Tom Tromey
49c1de0e72 Merge fixup_section and fixup_symbol_section
fixup_symbol_section delegates all its work to fixup_section, so merge
the two.

Because there is only a single caller to fixup_symbol_section, we can
also remove some of the introductory logic.  For example, this will
never be called with a NULL objfile any more.

The LOC_BLOCK case can be removed, because such symbols are handled by
the buildsym code now.

Finally, a symbol can only appear in a SEC_ALLOC section, so the loop
is modified to skip sections that do not have this flag set.
2023-02-08 08:20:39 -07:00
Tom Tromey
d401e7bf04 Remove compunit_symtab::m_block_line_section
The previous patch hard-coded SECT_OFF_TEXT into the buildsym code.
After this, it's clear that there is only one caller of
compunit_symtab::set_block_line_section, and it always passes
SECT_OFF_TEXT.  So, remove compunit_symtab::m_block_line_section and
use SECT_OFF_TEXT instead.
2023-02-08 08:20:12 -07:00
Tom Tromey
3f01c12b90 Use default section indexes in fixup_symbol_section
If fixup_section does not find a matching section, it arbitrarily
chooses the first one.  However, it seems better to make this default
depend on the type of the symbol -- i.e., default data symbols to
.data and text symbols to .text.

I've also made fixup_section static, as it only has one caller.
2023-02-08 08:20:12 -07:00
Simon Marchi
43e8c9ce20 gdb: move frame_info_ptr to frame.{c,h}
A patch later in this series will make frame_info_ptr access some
fields internal to frame_info, which we don't want to expose outside of
frame.c.  Move the frame_info_ptr class to frame.h, and the definitions
to frame.c.  Remove frame-info.c and frame-info.h.

Change-Id: Ic5949759e6262ea0da6123858702d48fe5673fea
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20 14:48:57 -05:00
Joel Brobecker
213516ef31 Update copyright year range in header of all files managed by GDB
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
2023-01-01 17:01:16 +04:00
Andrew Burgess
2698da268b gdb: add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT
After the previous commit converted symbol-lookup debug to use the new
debug scheme, this commit adds SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT.

The previous commit didn't add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT
because symbol-lookup debug is controlled by an 'unsigned int' rather
than a 'bool' control variable, we use the numeric value to offer
different levels of verbosity for symbol-lookup debug.

The *_SCOPED_DEBUG_ENTER_EXIT mechanism currently relies on capturing
a reference to the bool control variable, and evaluating the variable
both on entry, and at exit, this is done in the scoped_debug_start_end
class (see gdbsupport/common-debug.h).

This commit templates scoped_debug_start_end so that the class can
accept either a 'bool &' or an invokable object, e.g. a lambda
function, or a function pointer.

The existing scoped_debug_start_end and scoped_debug_enter_exit macros
in common-debug.h are updated to support scoped_debug_enter_exit being
templated, however, nothing outside of common-debug.h needs to change.

I've then added SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT in symtab.h, and
added a couple of token uses in symtab.c.  I didn't want to add too
much in this first commit, this is really about updating
common-debug.h to support this new functionality.

Within symtab.h I created a couple of global functions that can be
used to query the status of the symbol_lookup_debug control variable,
these functions are then used within the two existing macros:

  symbol_lookup_debug_printf
  symbol_lookup_debug_printf_v

and also in the new SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT macro.
2022-12-14 10:56:47 +00:00
Andrew Burgess
b1e678d920 gdb: convert 'set debug symbol-lookup' to new debug printing scheme
Convert the implementation of 'set debug symbol-lookup' to the new
debug printing scheme.

In a few places I've updated the debug output to remove places where
the printed debug message included the function name, the new debug
scheme already adds that, but I haven't done all the possible updates.
2022-12-14 10:56:47 +00:00
Tom Tromey
425d5e76e0 Convert compunit_language to a method
This changes compunit_language to be a method on compunit_symtab.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2022-10-28 14:19:22 -06:00
Simon Marchi
9efe17a3a0 gdb: remove spurious spaces after frame_info_ptr
Fix some whitespace issues introduced with the frame_info_ptr patch.

Change-Id: I158d30d8108c97564276c647fc98283ff7b12163
2022-10-25 11:04:58 -04:00
Tom Tromey
bd2b40ac12 Change GDB to use frame_info_ptr
This changes GDB to use frame_info_ptr instead of frame_info *
The substitution was done with multiple sequential `sed` commands:

sed 's/^struct frame_info;/class frame_info_ptr;/'
sed 's/struct frame_info \*/frame_info_ptr /g' - which left some
    issues in a few files, that were manually fixed.
sed 's/\<frame_info \*/frame_info_ptr /g'
sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace
    problems.

The changed files were then manually checked and some 'sed' changes
undone, some constructors and some gets were added, according to what
made sense, and what Tromey originally did

Co-Authored-By: Bruno Larsen <blarsen@redhat.com>
Approved-by: Tom Tomey <tom@tromey.com>
2022-10-10 11:57:10 +02:00
Simon Marchi
f71ad5556c gdb: add "id" fields to identify symtabs and subfiles
Printing macros defined in the main source file doesn't work reliably
using various toolchains, especially when DWARF 5 is used.  For example,
using the binaries produced by either of these commands:

    $ gcc --version
    gcc (GCC) 11.2.0
    $ ld --version
    GNU ld (GNU Binutils) 2.38
    $ gcc test.c -g3 -gdwarf-5

    $ clang --version
    clang version 13.0.1
    $ clang test.c -gdwarf-5 -fdebug-macro

I get:

    $ ./gdb -nx -q --data-directory=data-directory a.out
    (gdb) start
    Temporary breakpoint 1 at 0x111d: file test.c, line 6.
    Starting program: /home/simark/build/binutils-gdb-one-target/gdb/a.out

    Temporary breakpoint 1, main () at test.c:6
    6         return ZERO;
    (gdb) p ZERO
    No symbol "ZERO" in current context.

When starting to investigate this (taking the gcc-compiled binary as an
example), we see that GDB fails to look up the appropriate macro scope
when evaluating the expression.  While stopped in
macro_lookup_inclusion:

    (top-gdb) p name
    $1 = 0x62100011a980 "test.c"
    (top-gdb) p source.filename
    $2 = 0x62100011a9a0 "/home/simark/build/binutils-gdb-one-target/gdb/test.c"

`source` is the macro_source_file that we would expect GDB to find.
`name` comes from the symtab::filename field of the symtab we are
stopped in.  GDB doesn't find the appropriate macro_source_file because
the name of the macro_source_file doesn't match exactly the name of the
symtab.

The name of the main symtab comes from the compilation unit's
DW_AT_name, passed to the buildsym_compunit's constructor:

  4815d6125e/gdb/dwarf2/read.c (L10627-10630)

The contents of DW_AT_name, in this case, is "test.c".  It is typically
(what I witnessed all compilers do) the same string that was passed to
the compiler on the command-line.

The name of the macro_source_file comes from the line number program
header's file table, from the call to the line_header::file_file_name
method:

  4815d6125e/gdb/dwarf2/macro.c (L54-65)

line_header::file_file_name prepends the directory path that the file
entry refers to, in the file table (if the file name is not already
absolute).  In this case, the file name is "test.c", appended to the
directory "/home/simark/build/binutils-gdb-one-target/gdb".

Because the symtab's name is not created the same way as the
macro_source_file's name is created, we get this mismatch.  GDB fails to
find the appropriate macro scope for the symtab, and we can't print
macros when stopped in that symtab.

To make this work, we must ensure that paths produced in these two ways
end up identical.  This can be tricky because of the different ways a
path can be passed to the compiler by the user.

Another thing to consider is that while the main symtab's name (or
subfile, before it becomes a symtab) is created using DW_AT_name, the
main symtab is also referred to using its entry in the line table
header's file table, when processing the line table.  We must therefore
ensure that the same name is produced in both cases, so that a call to
"start_subfile" for the main subfile will correctly find the
already-created subfile, created by buildsym_compunit's constructor.  If
we fail to do that, things still often work, because of a fallback: the
watch_main_source_file_lossage method.  This method determines that if
the main subfile has no symbols but there exists another subfile with
the same basename (e.g. "test.c") that does have symbols, it's probably
because there was some filename mismatch.  So it replaces the main
subfile with that other subfile.  I think that heuristic is useful as a
last effort to work around any bug or bad debug info, but I don't think
we should design things such as to rely on it.  It's a heuristic, it can
get things wrong.  So in my search for a fix, it is important that given
some good debug info, we don't end up relying on that for things to
work.

A first attempt at fixing this was to try to prepend the compilation
directory here or not prepend it there.  In practice, because of all the
possible combinations of debug info the compilers produce, it was not
possible to get something that would produce reliable, consistent paths.

Another attempt at fixing this was to make both macro_source_file
objects and symtab objects use the most complete form of path possible.
That means to prepend directories at least until we get an absolute
path.  In theory, we should end up with the same path in all cases.
This generally worked, but because it changed the symtab names, it
resulted in user-visible changes (for example, paths to source files in
Breakpoint hit messages becoming always absolute).  I didn't find this
very good, first because there is a "set filename-display" setting that
lets the user control how they want the paths to be displayed, and that
would suddenly make this setting completely ineffective (although even
today, it is a bit dependent on the debug info).  Second, it would
require a good amount of testsuite tweaks to make tests accept these
suddenly absolute paths.

This new patch is a slight variation of that: it adds a new field called
"filename_for_id" in struct symtab and struct subfile, next to the
existing filename field. The goal is to separate the internal ids used
for finding objects from the names used for presentation.  This field is
used for identifying subfiles, symtabs and macro_source_files
internally.  For DWARF symtabs, this new field is meant to contain the
"most complete possible" path, as discussed above.  So for a given file,
it must always be in the same form, everywhere.  The existing
symtab::filename field remains the one used for printing to the user, so
there shouldn't be any change in how paths are printed.

Changes in the core symtab files are:

 - Add "name_for_id" and "filename_for_id" fields to "struct subfile"
   and "struct symtab", next to existing "name" and "filename" fields.
 - Make buildsym_compunit::buildsym_compunit and
   buildsym_compunit::start_subfile accept a "name_for_id" parameter
   next to the existing "name" ones.
 - Make buildsym_compunit::start_subfile use "name_for_id" for looking
   up existing subfiles.  This is the key thing for making calls
   to start_subfile for the main source file look up the existing
   subfile successfully, and avoid relying on
   watch_main_source_file_lossage.
 - Make sal_macro_scope pass "filename_for_id", rather than "filename",
   to macro_lookup_inclusion.  This is the key thing to making the
   lookup work and macro printing work.

Changes in the DWARF files are:

 - Make line_header::file_file_name return the "most complete possible"
   name.  The only pre-existing user of this method is the macro code,
   to give the macro_source_file objects their name.  And we now want
   them to have this "most complete possible" name, which will match the
   corresponding symtab's "filename_for_id".
 - Make dwarf2_cu::start_compunit_symtab pass the "most complete
   possible" name for the main symtab's "filename_for_id".  In this
   context, where the info comes from the compilation unit's DW_AT_name
   / DW_AT_comp_dir, it means prepending DW_AT_comp_dir to DW_AT_name if
   DW_AT_name is not already absolute.
 - Change dwarf2_start_subfile to build a name_for_id for the subfile
   being started.  The simplest way is to re-use
   line_header::file_file_name, since the callers always have a
   file_entry handy.  This ensures that it will get the exact same path
   representation as the macro code does, for the same file (since it
   also uses line_header::file_file_name).
 - Update calls to allocate_symtab to pass the "name_for_id" from the
   subfile.

Tests exercising all this are added by the following patch.

Of all the cases I tried, the only one I found that ends up relying on
watch_main_source_file_lossage is the following one:

    $ clang --version
    clang version 13.0.1
    Target: x86_64-pc-linux-gnu
    Thread model: posix
    InstalledDir: /usr/bin
    $ clang  ./test.c -g3 -O0 -gdwarf-4
    $ ./gdb -nx --data-directory=data-directory -q -readnow -iex "set debug symtab-create 1"  a.out
    ...
    [symtab-create] start_subfile: name = test.c, name_for_id = /home/simark/build/binutils-gdb-one-target/gdb/test.c
    [symtab-create] start_subfile: name = ./test.c, name_for_id = /home/simark/build/binutils-gdb-one-target/gdb/./test.c
    [symtab-create] start_subfile: name = ./test.c, name_for_id = /home/simark/build/binutils-gdb-one-target/gdb/./test.c
    [symtab-create] start_subfile: found existing symtab with name_for_id /home/simark/build/binutils-gdb-one-target/gdb/./test.c (/home/simark/build/binutils-gdb-one-target/gdb/./test.c)
    [symtab-create] watch_main_source_file_lossage: using subfile ./test.c as the main subfile

As we can see, there are two forms used for "test.c", one with a "." and
one without.  This comes from the fact that the compilation unit DIE
contains:

    DW_AT_name ("test.c")
    DW_AT_comp_dir ("/home/simark/build/binutils-gdb-one-target/gdb")

without a ".", and the line table for that file contains:

    include_directories[  1] = "."
    file_names[  1]:
               name: "test.c"
          dir_index: 1

When assembling the filename from that entry, we get a ".".

It is a bit unexpected that the main filename resulting from the line
table header does not match exactly the name in the compilation unit.
For instance, gcc uses "./test.c" for the DW_AT_name, which gives
identical paths in the compilation unit and in the line table header.

Similarly, with DWARF 5:

    $ clang  ./test.c -g3 -O0 -gdwarf-5

clang create two entries that refer to the same file but are of in a different
form.

    include_directories[  0] = "/home/simark/build/binutils-gdb-one-target/gdb"
    include_directories[  1] = "."
    file_names[  0]:
               name: "test.c"
          dir_index: 0
    file_names[  1]:
               name: "test.c"
          dir_index: 1

The first file name produces a path without a "." while the second does.
This is not caught by watch_main_source_file_lossage, because of
dwarf_decode_lines that creates a symtab for each file entry in the line
table.  It therefore appears as "non-empty" to
watch_main_source_file_lossage.  This results in two symtabs:

    (gdb) maintenance info symtabs
    { objfile /home/simark/build/binutils-gdb-one-target/gdb/a.out ((struct objfile *) 0x613000005d00)
      { ((struct compunit_symtab *) 0x62100011aca0)
        debugformat DWARF 5
        producer clang version 13.0.1
        name test.c
        dirname /home/simark/build/binutils-gdb-one-target/gdb
        blockvector ((struct blockvector *) 0x621000129ec0)
        user ((struct compunit_symtab *) (null))
            { symtab test.c ((struct symtab *) 0x62100011ad20)
              fullname (null)
              linetable ((struct linetable *) 0x0)
            }
            { symtab ./test.c ((struct symtab *) 0x62100011ad60)
              fullname (null)
              linetable ((struct linetable *) 0x621000129ef0)
            }
      }
    }

I am not sure what is the consequence of this, but this is also what
happens before my patch, so I think its acceptable to leave it as-is.

To handle these two cases nicely, I think we will need a function that
removes the unnecessary "." from path names, something that can be done
later.

Finally, I made a change in find_file_and_directory is necessary to
avoid breaking test

    gdb.dwarf2/dw2-compdir-oldgcc.exp: info source gcc42

Without that change, we would get:

    (gdb) info source
    Current source file is /dir/d/dw2-compdir-oldgcc42.S
    Compilation directory is /dir/d

whereas the expected result is:

    (gdb) info source
    Current source file is dw2-compdir-oldgcc42.S
    Compilation directory is /dir/d

This test was added here:

  https://sourceware.org/pipermail/gdb-patches/2012-November/098144.html

Long story short, GCC <= 4.2 apparently had a bug where it would
generate a DW_AT_name with a full path ("/dir/d/dw2-compdir-oldgcc42.S")
and no DW_AT_comp_dir.  The line table has one entry with filename
"dw2-compdir-oldgcc42.S", which refers to directory 0.  Directory 0
normally refers to the compilation unit's comp dir, but it is
non-existent in this case.

This caused some symtab lookup problems, and to work around them, some
workaround was added, which today reads as:

    if (res.get_comp_dir () == nullptr
        && producer_is_gcc_lt_4_3 (cu)
        && res.get_name () != nullptr
        && IS_ABSOLUTE_PATH (res.get_name ()))
      res.set_comp_dir (ldirname (res.get_name ()));

Source: 6577f365eb/gdb/dwarf2/read.c (L9428-9432)

It extracts an artificial DW_AT_comp_dir from DW_AT_name, if there is no
DW_AT_comp_dir and DW_AT_name is absolute.

Prior to my patch, a subfile would get created with filename
"/dir/d/dw2-compdir-oldgcc42.S", from DW_AT_name, and another would get
created with filename "dw2-compdir-oldgcc42.S" from the line table's
file table.  Then watch_main_source_file_lossage would kick in and merge
them, keeping only the "dw2-compdir-oldgcc42.S" one:

    [symtab-create] start_subfile: name = /dir/d/dw2-compdir-oldgcc42.S
    [symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S
    [symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S
    [symtab-create] start_subfile: found existing symtab with name dw2-compdir-oldgcc42.S (dw2-compdir-oldgcc42.S)
    [symtab-create] watch_main_source_file_lossage: using subfile dw2-compdir-oldgcc42.S as the main subfile

And so "info source" would show "dw2-compdir-oldgcc42.S" as the
filename.

With my patch applied, but without the change in
find_file_and_directory, both DW_AT_name and the line table would try to
start a subfile with the same filename_for_id, and there was no need for
watch_main_source_file_lossage - which is what we want:

[symtab-create] start_subfile: name = /dir/d/dw2-compdir-oldgcc42.S, name_for_id = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S, name_for_id = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: found existing symtab with name_for_id /dir/d/dw2-compdir-oldgcc42.S (/dir/d/dw2-compdir-oldgcc42.S)
[symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S, name_for_id = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: found existing symtab with name_for_id /dir/d/dw2-compdir-oldgcc42.S (/dir/d/dw2-compdir-oldgcc42.S)

But since the one with name == "/dir/d/dw2-compdir-oldgcc42.S", coming
from DW_AT_name, gets created first, it wins, and the symtab ends up
with "/dir/d/dw2-compdir-oldgcc42.S" as the name, "info source" shows
"/dir/d/dw2-compdir-oldgcc42.S" and the test breaks.

This is not wrong per-se, after all DW_AT_name is
"/dir/d/dw2-compdir-oldgcc42.S", so it wouldn't be wrong to report the
current source file as "/dir/d/dw2-compdir-oldgcc42.S".  If you compile
a file passing "/an/absolute/path.c", DW_AT_name typically contains (at
least with GCC) "/an/absolute/path.c" and GDB tells you that the source
file is "/an/absolute/path.c".  But we can also keep the existing
behavior fairly easily with a little change in find_file_and_directory.
When extracting an artificial DW_AT_comp_dir from DW_AT_name, we now
modify the name to just keep the file part.  The result is coherent with
what compilers do when you compile a file by just passing its filename
("gcc path.c -g"):

      DW_AT_name        ("path.c")
      DW_AT_comp_dir    ("/home/simark/build/binutils-gdb-one-target/gdb")

With this change, filename_for_id is still the full name,
"/dir/d/dw2-compdir-oldgcc42.S", but the filename of the subfile /
symtab (what ends up shown by "info source") is just
"dw2-compdir-oldgcc42.S", and that makes the test happy.

Change-Id: I8b5cc4bb3052afdb172ee815c051187290566307
2022-07-29 20:54:49 -04:00
Simon Marchi
2ab317fb82 gdb: introduce symtab_create_debug_printf
Introduce symtab_create_debug_printf and symtab_create_debug_printf_v,
to print the debug messages enabled by "set debug symtab-create".

Change-Id: I442500903f72d4635c2dd9eaef770111f317dc04
2022-07-29 20:54:48 -04:00
Pedro Alves
74421c0bc8 Rename base_breakpoint -> code_breakpoint
Even after the previous patches reworking the inheritance of several
breakpoint types, the present breakpoint hierarchy looks a bit
surprising, as we have "breakpoint" as the superclass, and then
"base_breakpoint" inherits from "breakpoint".  Like so, simplified:

   breakpoint
       base_breakpoint
          ordinary_breakpoint
	  internal_breakpoint
	  momentary_breakpoint
	  ada_catchpoint
	  exception_catchpoint
       tracepoint
       watchpoint
       catchpoint
	  exec_catchpoint
	  ...

The surprising part to me is having "base_breakpoint" being a subclass
of "breakpoint".  I'm just refering to naming here -- I mean, you'd
expect that it would be the top level baseclass that would be called
"base".

Just flipping the names of breakpoint and base_breakpoint around
wouldn't be super great for us, IMO, given we think of every type of
*point as a breakpoint at the user visible level.  E.g., "info
breakpoints" shows watchpoints, tracepoints, etc.  So it makes to call
the top level class breakpoint.

Instead, I propose renaming base_breakpoint to code_breakpoint.  The
previous patches made sure that all code breakpoints inherit from
base_breakpoint, so it's fitting.  Also, "code breakpoint" contrasts
nicely with a watchpoint also being typically known as a "data
breakpoint".

After this commit, the resulting hierarchy looks like:

   breakpoint
       code_breakpoint
          ordinary_breakpoint
	  internal_breakpoint
	  momentary_breakpoint
	  ada_catchpoint
	  exception_catchpoint
       tracepoint
       watchpoint
       catchpoint
	  exec_catchpoint
	  ...

... which makes a lot more sense to me.

I've left this patch as last in the series in case people want to
bikeshed on the naming.

"code" has a nice property that it's exactly as many letters as
"base", so this patch didn't require any reindentation.  :-)

Change-Id: Id8dc06683a69fad80d88e674f65e826d6a4e3f66
2022-05-20 20:41:02 +01:00
Pedro Alves
960bc2bd14 Move add_location(sal) to base_breakpoint
After the previous patches, only base_breakpoint subclasses use
add_location(sal), so we can move it to base_breakpoint (a.k.a. base
class for code breakpoints).

This requires a few casts here and there, but always at spots where
you can see from context what the breakpoint's type actually is.

I inlined new_single_step_breakpoint into its only caller exactly for
this reason.

I did try to propagate more use of base_breakpoint to avoid casts, but
that turned out unwieldy for this patch.

Change-Id: I49d959322b0fdce5a88a216bb44730fc5dd7c6f8
2022-05-20 20:41:01 +01:00
Tom Tromey
0d1703b8fb Remove "typedef enum ..."
I noticed a few spots in GDB that use "typedef enum".  However, in C++
this isn't as useful, as the tag is automatically entered as a
typedef.  This patch removes most uses of "typedef enum" -- the
exceptions being in some nat-* code I can't compile, and
glibc_thread_db.h, which I think is more or less a copy of some C code
from elsewhere.

Tested by rebuilding.
2022-04-28 09:31:15 -06:00
Simon Marchi
63d609debb gdb: remove BLOCKVECTOR_BLOCK and BLOCKVECTOR_NBLOCKS macros
Replace with calls to blockvector::blocks, and the appropriate method
call on the returned array_view.

Change-Id: I04d1f39603e4d4c21c96822421431d9a029d8ddd
2022-04-27 22:05:03 -04:00
Tom Tromey
4206d69e96 Replace symbol_symtab with symbol::symtab
This turns symbol_symtab into a method on symbol.  It also replaces
symbol_set_symtab with a method.
2022-04-20 09:28:40 -06:00
Tom Tromey
bcd6845e2b Replace symbol_arch with symbol::arch
This turns symbol_arch into a method on symbol.
2022-04-20 09:28:17 -06:00
Tom Tromey
e19b2d9465 Replace symbol_objfile with symbol::objfile
This turns symbol_objfile into a method on symbol.
2022-04-20 09:28:16 -06:00
Tom Tromey
f66b536343 Remove symbol::aclass_index
Symbols have an aclass_index method, but this isn't needed, because
the aclass index isn't useful outside of the symbol implementation.
2022-04-20 09:28:16 -06:00
Tom Tromey
6bc3c5b438 Use array_view for symbol_impls
It seemed to me that using array_view for symbol_impls would give a
bit more error checking, at least when gdb is built in libstdc++ debug
mode.
2022-04-20 09:28:16 -06:00
Tom Tromey
496feb16f8 Add accessors for symbol's artificial field
For a series I'm experimenting with, it was handy to hide a symbol's
"artificial" field behind accessors.  This patch is the result.
2022-04-20 09:28:12 -06:00
Tom Tromey
b2bc564fe8 Add name splitting
The new DWARF index code works by keeping names pre-split.  That is,
rather than storing a symbol name like "a:🅱️:c", the names "a", "b",
and "c" will be stored separately.

This patch introduces some helper code to split a full name into its
components.
2022-04-12 09:31:15 -06:00
Simon Marchi
e165fcef1e gdb: remove MSYMBOL_TARGET_FLAG_{1,2} macros
Replace with equivalent getter/setter macros.

Change-Id: I1042564dd47347337374762bd64ec31b5c573ee2
2022-04-11 10:47:54 -04:00
Simon Marchi
5bbfd12dda gdb: remove minimal symbol size macros
Remove MSYMBOL_HAS_SIZE, MSYMBOL_SIZE and SET_MSYMBOL_SIZE, replace them
with equivalent methods.

Change-Id: I6ee1cf82df37e58dff52ea6568ceb4649c7d7538
2022-04-11 10:46:52 -04:00
Simon Marchi
60f62e2b83 gdb: remove MSYMBOL_TYPE macro
Add a getter and a setter for a minimal symbol's type.  Remove the
corresponding macro and adjust all callers.

Change-Id: I89900df5ffa5687133fe1a16b2e0d4684e67a77d
2022-04-11 10:46:07 -04:00
Simon Marchi
4aeddc50d7 gdb: remove symbol value macros
Remove all macros related to getting and setting some symbol value:

    #define SYMBOL_VALUE(symbol)           (symbol)->value.ivalue
    #define SYMBOL_VALUE_ADDRESS(symbol)                         \
    #define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value)    \
    #define SYMBOL_VALUE_BYTES(symbol)     (symbol)->value.bytes
    #define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->value.common_block
    #define SYMBOL_BLOCK_VALUE(symbol)     (symbol)->value.block
    #define SYMBOL_VALUE_CHAIN(symbol)     (symbol)->value.chain
    #define MSYMBOL_VALUE(symbol)          (symbol)->value.ivalue
    #define MSYMBOL_VALUE_RAW_ADDRESS(symbol) ((symbol)->value.address + 0)
    #define MSYMBOL_VALUE_ADDRESS(objfile, symbol)                         \
    #define BMSYMBOL_VALUE_ADDRESS(symbol) \
    #define SET_MSYMBOL_VALUE_ADDRESS(symbol, new_value)   \
    #define MSYMBOL_VALUE_BYTES(symbol)    (symbol)->value.bytes
    #define MSYMBOL_BLOCK_VALUE(symbol)    (symbol)->value.block

Replace them with equivalent methods on the appropriate objects.

Change-Id: Iafdab3b8eefc6dc2fd895aa955bf64fafc59ed50
2022-04-11 10:45:36 -04:00
Tom Tromey
bd1c798f0a Remove MSYMBOL_VALUE_CHAIN
I noticed that MSYMBOL_VALUE_CHAIN is unused, so this patch removes it.
2022-04-09 08:33:11 -06:00
Simon Marchi
7678d1304a gdb: remove symtab::pspace
Same idea as previous patch, but for symtab::pspace.

Change-Id: I1023abe622bea75ef648c6a97a01b53775d4104d
2022-04-07 13:05:29 -04:00
Simon Marchi
3c86fae3d9 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
2022-04-07 13:05:22 -04:00
Simon Marchi
44281e6c08 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
2022-04-07 13:04:53 -04:00
Simon Marchi
e473032828 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
2022-04-07 13:04:48 -04:00
Lancelot SIX
cc96ae7f88 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
2022-04-04 23:03:32 +01:00
Simon Marchi
5d0027b9ba gdb: remove SYMBOL_LINE macro
Add a getter and a setter for a symbol's line.  Remove the corresponding macro
and adjust all callers.

Change-Id: I229f2b8fcf938c07975f641361313a8761fad9a5
2022-02-06 16:03:47 -05:00
Simon Marchi
5f9c5a63ce gdb: remove SYMBOL_TYPE macro
Add a getter and a setter for a symbol's type.  Remove the corresponding
macro and adjust all callers.

Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
2022-02-06 16:03:47 -05:00