| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This started with me running into this comment in symfile.c:
/* FIXME, should use print_sys_errmsg but it's not filtered. */
gdb_printf (_("`%ps' has disappeared; keeping its symbols.\n"),
styled_string (file_name_style.style (), filename));
In this particular case I think I disagree with the comment; I think
the output should be a warning rather than just a message printed to
gdb_stdout, I think when the executable, or some other objfile that is
currently being debugged, disappears from disk, this is likely an
unexpected situation, and worth warning the user about.
So, in theory, I could just call print_sys_errmsg and remove the
comment, but that would mean loosing the filename styling in the
output... so in the end I remove the comment and updated the code to
call warning.
But that got me looking at print_sys_errmsg and how it's used.
Currently the function takes a string and an errno, and prints, to
stderr, the string followed by the result of calling strerror on the
errno.
In some places the string passed to print_sys_errmsg is just a
filename, and this is used when something goes wrong. In these cases,
I think calling warning rather than gdb_printf to gdb_stderr, would be
better, and in fact, in a couple of places we manually print a
"warning" prefix, and then call print_sys_errmsg. And so, for these
users I have added a new function warning_filename_and_errno, which
takes a filename, which is printed with styling, and an errno, which
is passed through strerror and the resulting string printed. This new
function calls warning to print its output. I then updated some of
the print_sys_errmsg users to use this new function.
Some other users of print_sys_errmsg are also emitting what is clearly
a warning, however, the string being passed in is more than just a
filename, so the new warning_filename_and_errno function can't be
used, it would style the whole string. For these users I have
switched to calling warning directly, this allows me to style the
warning message correctly.
Finally, in inflow.c there is one last call to print_sys_errmsg, in
this case I just inlined the definition of print_sys_errmsg. This is
a really weird case, as after printing this message GDB just does a
hard exit. This is pretty old code, dating back to the initial GDB
import, I guess it should be updated to call error() maybe, but I'm
reluctant to make this change as part of this commit, just in case
there's some reason why we can't throw an error at this point.
With that done there are now no users of print_sys_errmsg, and so the
old function can be removed.
While I was doing all of the above I added some additional filename
styling in soure.c, this is in an else block where the if contained
the print_sys_errmsg call, so these felt related.
And finally, while I was updating the uses of print_sys_errmsg in
procfs.c, I noticed that we used a static errmsg buffer to format some
error strings. As the above changes got rid of one of the users of
errmsg I also removed the other two users, and the static buffer.
There were a couple of tests that depended on the existing output
message format that needed updating. In one case we gained an extra
'warning: ' prefix, and in the other 'Warning: ' becomes 'warning: ',
I think in both cases the new output is an improvement.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Fix up another couple of places where we can apply filename styling.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed a comment by an include and remembered that I think these
don't really provide much value -- sometimes they are just editorial,
and sometimes they are obsolete. I think it's better to just remove
them. Tested by rebuilding.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
When using "list" with no arguments, GDB will first print the lines
around where the inferior is stopped, then print the next N lines until
reaching the end of file, at which point it warns the user "Line X out
of range, file Y only has X-1 lines.". This is usually desirable, but
if the user can no longer see the original line, they may have forgotten
the current line or that a list command was used at all, making GDB's
error message look cryptic. It was reported in bugzilla as PR cli/30497.
This commit improves the user experience by changing the behavior of
"list" slightly when a user passes no arguments. It now prints that the
end of the file has been reached and recommends that the user use the
command "list ." instead.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Same idea as previous patches, but for command_param_changed.
Change-Id: I7c2196343423360da05f016f8ffa871c064092bb
|
|
I noticed that select_source_symtab is only ever called with nullptr
as an argument, so this patch removes the parameter and associated
logic.
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
|
|
Replace spaces with tabs in a bunch of places.
Change-Id: If0f87180f1d13028dc178e5a8af7882a067868b0
|
|
forget_cached_source_info_for_objfile does some objfile-specific work
and then calls objfile::forget_cached_source_info. It seems better to
me to just have the method do all the work.
|
|
open_source_file relies on errno to communicate the reason for a missing
source file.
open_source_file may also call debuginfod_find_source. It is possible
for debuginfod_find_source to set errno to a value unrelated to the
reason for a failed download.
This can result in bogus error messages being reported as the reason for
a missing source file. The following error message should instead be
"No such file or directory":
Temporary breakpoint 1, 0x00005555556f4de0 in main ()
(gdb) list
Downloading source file /usr/src/debug/glibc-2.36-8.fc37.x86_64/elf/<built-in>
1 /usr/src/debug/glibc-2.36-8.fc37.x86_64/elf/<built-in>: Directory not empty.
Fix this by having open_source_file return a negative errno if it fails
to open a source file. Use this value to generate the error message
instead of errno.
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29999
|
|
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.
|
|
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
|
|
When a source file's dirname is solely made up of directory separators
we end up trying to dereference the last character of an empty string
with std::string::back, which results in undefined behaviour. A typical
use case where this can happen is when the root directory "/" is used as
a compilation directory.
With libstdc++.so.6.0.28 we get no out-of-bounds checks and the byte
preceding the storage of the empty string is returned. The character
value of this byte depends on heap implementation and usage, but when
this byte happens to hold the value of the directory separator character
we go on to call std::string::pop_back on the empty string which results
in an out_of_range exception which terminates GDB.
Fix this by using path_join. prepare_path_for_appending ensures that the
filename component is relative.
The testsuite has been run before and after the change and no
regressions were found.
|
|
This changes struct objfile to use a gdb_bfd_ref_ptr. In addition to
removing some manual memory management, this fixes a use-after-free
that was introduced by the registry rewrite series. The issue there
was that, in some cases, registry shutdown could refer to memory that
had already been freed. This help fix the bug by delaying the
destruction of the BFD reference (and thus the per-bfd object) until
after the registry has been shut down.
|
|
This rewrites registry.h, removing all the macros and replacing it
with relatively ordinary template classes. The result is less code
than the previous setup. It replaces large macros with a relatively
straightforward C++ class, and now manages its own cleanup.
The existing type-safe "key" class is replaced with the equivalent
template class. This approach ended up requiring relatively few
changes to the users of the registry code in gdb -- code using the key
system just required a small change to the key's declaration.
All existing users of the old C-like API are now converted to use the
type-safe API. This mostly involved changing explicit deletion
functions to be an operator() in a deleter class.
The old "save/free" two-phase process is removed, and replaced with a
single "free" phase. No existing code used both phases.
The old "free" callbacks took a parameter for the enclosing container
object. However, this wasn't truly needed and is removed here as
well.
|
|
This turns symbol_symtab into a method on symbol. It also replaces
symbol_set_symtab with a method.
|
|
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible. Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).
Change-Id: Iced49b8ee2f189744c5072a3b217aab5af17a993
|
|
Same idea as previous patch, but for symtab::pspace.
Change-Id: I1023abe622bea75ef648c6a97a01b53775d4104d
|
|
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
|
|
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
|
|
This patch removes gdb's dbx mode. Regression tested on x86-64 Fedora
34.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
|
|
Change-Id: I83211d5a47efc0564386e5b5ea4a29c00b1fd46a
|
|
Remove the macro, replace with an equivalent method.
Change-Id: I46ec36b91bb734331138eb9cd086b2db01635aed
|
|
Remove the macro, replace with an equivalent method.
Change-Id: Icccc20e7e8ae03ac4dac1c7514c25a12a9a0ac69
|
|
Remove the macro, replace with an equivalent method.
Change-Id: I8f9ecd290ad28502e53c1ceca5006ba78bf042eb
|
|
Add a getter and a setter for a symtab's language. Remove the
corresponding macro and adjust all callers.
Change-Id: I9f4d840b11c19f80f39bac1bce020fdd1739e11f
|
|
Add a getter and a setter for a symtab's compunit_symtab. Remove the
corresponding macro and adjust all callers.
For brevity, I chose the name "compunit" instead of "compunit_symtab"
the the field, getter and setter names. Since we are already in symtab
context, the _symtab suffix seems redundant.
Change-Id: I4b9b731c96e3594f7733e75af1e3d01bc0e4fe92
|
|
Add a getter and a setter for a compunit_symtab's macro table. Remove the
corresponding macro and adjust all callers.
Change-Id: I00615ea72d5ac43d9a865e941cb2de0a979c173a
|
|
Add a getter and a setter for a compunit_symtab's producer. Remove the
corresponding macro and adjust all callers.
Change-Id: Ia1d6d8a0e247a08a21af23819d71e49b37d8931b
|
|
Add a getter and a setter for a compunit_symtab's debugformat. Remove
the corresponding macro and adjust all callers.
Change-Id: I1667b02d5322346f8e23abd9f8a584afbcd75975
|
|
Make compunit_filetabs, used to iterate a compunit_symtab's filetabs, a
method of compunit_symtab. The name filetabs conflicts with the current
name of the field. Rename the field to m_filetabs, since at this point
nothing outside of compunit_symtab uses it, so we should treat it as
private (even though it's not actually private). Rename the
last_filetab field to m_last_filetab as well (it's only used on
compunit_symtab::add_filetab).
Adjust the COMPUNIT_FILETABS macro to keep its current behavior of
returning the first filetab.
Change-Id: I537b553a44451c52d24b18ee1bfa47e23747cfc3
|
|
Remove the macro, update all users to use the getter directly.
Change-Id: I3f0fd6f4455d1c4ebd5da73b561eb18a979ef1f6
|
|
This changes all existing calls to wrap_here to call the method on the
appropriate ui_file instead. The choice of ui_file is determined by
context.
|
|
I think it only really makes sense to call wrap_here with an argument
consisting solely of spaces. Given this, it seemed better to me that
the argument be an int, rather than a string. This patch is the
result. Much of it was written by a script.
|
|
This moves the gdb_regex convenience class to gdbsupport.
|
|
This moves the gdb_argv class to a new header in gdbsupport.
|
|
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
|
|
I happened to notice that one "show" callback was printing to
gdb_stdout rather than to the passed-in ui_file parameter. I went
through all such callbacks and fixed them to consistently use the
ui_file.
Regression tested on x86-64 Fedora 34.
|
|
I found some uses of xfree in the path substitution code in source.c.
C++-ifying struct substitute_path_rule both simplifies the code and
removes manual memory management.
Regression tested on x86-64 Fedora 34.
|
|
My earlier change to source.c ("Remove an xfree from add_path")
introduced a regression. This patch fixes the problem.
|
|
This removes a temporary \0 assignment and an xfree from add_path,
replacing it with a simpler use of std::string.
|
|
String-like settings (var_string, var_filename, var_optional_filename,
var_string_noescape) currently take a pointer to a `char *` storage
variable (typically global) that holds the setting's value. I'd like to
"mordernize" this by changing them to use an std::string for storage.
An obvious reason is that string operations on std::string are often
easier to write than with C strings. And they avoid having to do any
manual memory management.
Another interesting reason is that, with `char *`, nullptr and an empty
string often both have the same meaning of "no value". String settings
are initially nullptr (unless initialized otherwise). But when doing
"set foo" (where `foo` is a string setting), the setting now points to
an empty string. For example, solib_search_path is nullptr at startup,
but points to an empty string after doing "set solib-search-path". This
leads to some code that needs to check for both to check for "no value".
Or some code that converts back and forth between NULL and "" when
getting or setting the value. I find this very error-prone, because it
is very easy to forget one or the other. With std::string, we at least
know that the variable is not "NULL". There is only one way of
representing an empty string setting, that is with an empty string.
I was wondering whether the distinction between NULL and "" would be
important for some setting, but it doesn't seem so. If that ever
happens, it would be more C++-y and self-descriptive to use
optional<string> anyway.
Actually, there's one spot where this distinction mattered, it's in
init_history, for the test gdb.base/gdbinit-history.exp. init_history
sets the history filename to the default ".gdb_history" if it sees that
the setting was never set - if history_filename is nullptr. If
history_filename is an empty string, it means the setting was explicitly
cleared, so it leaves it as-is. With the change to std::string, this
distinction doesn't exist anymore. This can be fixed by moving the code
that chooses a good default value for history_filename to
_initialize_top. This is ran before -ex commands are processed, so an
-ex command can then clear that value if needed (what
gdb.base/gdbinit-history.exp tests).
Another small improvement, in my opinion is that we can now easily
give string parameters initial values, by simply initializing the global
variables, instead of xstrdup-ing it in the _initialize function.
In Python and Guile, when registering a string-like parameter, we
allocate (with new) an std::string that is owned by the param_smob (in
Guile) and the parmpy_object (in Python) objects.
This patch started by changing all relevant add_setshow_* commands to
take an `std::string *` instead of a `char **` and fixing everything
that failed to build. That includes of course all string setting
variable and their uses.
string_option_def now uses an std::string also, because there's a
connection between options and settings (see
add_setshow_cmds_for_options).
The add_path function in source.c is really complex and twisted, I'd
rather not try to change it to work on an std::string right now.
Instead, I added an overload that copies the std:string to a `char *`
and back. This means more copying, but this is not used in a hot path
at all, so I think it is acceptable.
Change-Id: I92c50a1bdd8307141cdbacb388248e4e4fc08c93
Co-authored-by: Lancelot SIX <lsix@lancelotsix.com>
|
|
Make gdb_open_cloexec return a scoped_fd, to encourage using automatic
management of the file descriptor closing. Except in the most trivial
cases, I changed the callers to just release the fd, which retains their
existing behavior. That will allow the transition to using scoped_fd
more to go gradually, one caller at a time.
Change-Id: Ife022b403f96e71d5ebb4f1056ef6251b30fe554
|
|
In some situations it is possible that a user might not want GDB to
try and access source code files, for example, the source code might
be stored on a slow to access network file system.
It is almost certainly possible that using some combination of 'set
directories' and/or 'set substitute-path' a user can trick GDB into
being unable to find the source files, but this feels like a rather
crude way to solve the problem.
In this commit a new option is add that stops GDB from opening and
reading the source files. A user can run with source code reading
disabled if this is required, then re-enable later if they decide
that they now want to view the source code.
|
|
The final bug fix in this series would duplicate the logic in
psymtab_to_fullname, so this patch extracts the body of this function
into a new function.
|
|
I spotted some indentation issues where we had some spaces followed by
tabs at beginning of line, that I wanted to fix. So while at it, I did
a quick grep to find and fix all I could find.
gdb/ChangeLog:
* Fix tab after space indentation issues throughout.
Change-Id: I1acb414dd9c593b474ae2b8667496584df4316fd
|
|
The alias creation functions currently accept a name to specify the
target command. They pass this to add_alias_cmd, which needs to lookup
the target command by name.
Given that:
- We don't support creating an alias for a command before that command
exists.
- We always use add_info_alias just after creating that target command,
and therefore have access to the target command's cmd_list_element.
... change add_com_alias to accept the target command as a
cmd_list_element (other functions are done in subsequent patches). This
ensures we don't create the alias before the target command, because you
need to get the cmd_list_element from somewhere when you call the alias
creation function. And it avoids an unecessary command lookup. So it
seems better to me in every aspect.
gdb/ChangeLog:
* command.h (add_com_alias): Accept target as
cmd_list_element. Update callers.
Change-Id: I24bed7da57221cc77606034de3023fedac015150
|
|
gdb/ChangeLog:
* ui-out.h (class ui_out): Add ui_out::text accepting a constant
reference to a std::string. Fix all callers using
std::string::c_str.
* ui-out.c (ui_out::text): Ditto.
|
|
Address sanitizer pointed out a buglet in source.c:add_path.
In this test, from gdb.base/source-dir.exp:
(gdb) set directories :/foo:/bar
... 'p[-1]' will result in a buffer underflow.
This patch fixes the bug by introducing a new check.
2021-05-17 Tom Tromey <tromey@adacore.com>
* source.c (add_path): Check 'p' before using 'p[-1]'.
|
