[PATCH 1/1] gdb, linespec: also error when explicit line BP is outside source file range
Guinevere Larsen
guinevere@redhat.com
Mon Nov 11 17:58:41 GMT 2024
On 11/11/24 9:38 AM, Klaus Gerlicher wrote:
> From: "Gerlicher, Klaus" <klaus.gerlicher@intel.com>
>
> Setting a breakpoint on an explicit source line outside
> of the source files line range informs the user that
> there is no compiled code at this line.
>
> An error will be printed that "No compiled code for
> line" exists. This is misleading if requesting a line
> beyond the source file's line range.
>
> (gdb) b 1
> No compiled code for line 1 in the current file.
> (gdb) b 403432
> No compiled code for line 403432 in the current file.
>
> Informing the user that the line requested is outside
> the source file's line range could improve the user
> experience.
>
> This patch will then yield the following output:
>
> (gdb) b 1
> No compiled code for line 1 in the current file.
> (gdb) b 534435345
> No source for line 534435345 in the current file. File has 92 lines.
> (gdb) b file.cpp:32432
> No source for line 32432 in file "file.cpp". File has 92 lines.
>
> Note that depending on the GDB setting for breakpoint pending
> (set breakpoint pending on|off|auto), set to auto or off
> this will not create a pending breakpoint. For breakpoint
> pending set to on a pending breakpoint will be created
> regardless of the line being in source file line range.
Thanks for the changes! I am always happy to see error message
improvements for users!
I have 2 comments on this patch
> ---
> gdb/linespec.c | 49 +++++++++++++++++++---
> gdb/testsuite/gdb.base/break.exp | 2 +-
> gdb/testsuite/gdb.base/hbreak2.exp | 2 +-
> gdb/testsuite/gdb.base/sepdebug.exp | 3 +-
> gdb/testsuite/gdb.linespec/ls-errs.exp | 8 +++-
> gdb/testsuite/gdb.python/py-breakpoint.exp | 4 +-
> gdb/testsuite/gdb.trace/tfind.exp | 4 +-
> gdb/testsuite/gdb.trace/tracecmd.exp | 2 +-
> 8 files changed, 58 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 528abc46b89..3aa34678991 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -45,6 +45,7 @@
> #include "gdbsupport/def-vector.h"
> #include <algorithm>
> #include "inferior.h"
> +#include "source-cache.h"
>
> /* An enumeration of the various things a user might attempt to
> complete for a linespec location. */
> @@ -2163,14 +2164,52 @@ create_sals_line_offset (struct linespec_state *self,
>
> if (values.empty ())
> {
> + std::string lines;
> +
> + /* Try to find source code associated with the explicitly
> + specified line. If the line is out of source file range
> + error out the same way as for no compiled code but with
> + a different message for clarity. */
> + bool line_found = false;
> + std::string max_line_msg;
> + for (const auto &symtab : ls->file_symtabs)
> + {
> + if (!g_source_cache.get_source_lines (symtab, val.line, val.line,
> + &lines))
> + {
> + const std::vector<off_t> *offsets = nullptr;
> + g_source_cache.get_line_charpos (symtab, &offsets);
> +
> + if (offsets == nullptr)
> + continue;
> +
> + max_line_msg = " File has "
> + + std::to_string (offsets->size ())
> + + " lines.";
> + }
> + else
> + {
> + line_found = true;
> + break;
> + }
> + }
> +
> + std::string filename_qualifier;
> if (ls->explicit_loc.source_filename)
> - throw_error (NOT_FOUND_ERROR,
> - _("No compiled code for line %d in file \"%s\"."),
> - val.line, ls->explicit_loc.source_filename.get ());
> + filename_qualifier
> + = std::string (" in file \"") + ls->explicit_loc.source_filename
> + + "\"";
> else
> + filename_qualifier = " in the current file";
> +
> + if (line_found)
> throw_error (NOT_FOUND_ERROR,
> - _("No compiled code for line %d in the current file."),
> - val.line);
> + _("No compiled code for line %d%s."), val.line,
> + filename_qualifier.c_str ());
> + else
> + throw_error (NOT_FOUND_ERROR, _("No source for line %d%s.%s"),
> + val.line, filename_qualifier.c_str (),
> + max_line_msg.c_str ());
Building the error piecemeal like this makes it impossible to translate
the outputs. There is no call to gettext with the string "in current
file" for example.
If it better to do something like the following:
if (ls->explicit_loc.source_filename)
{
if (line_found)
throw_error (NOT_FOUND_ERROR, _("No compiled code for line %d in
file \"%s\"."), ...);
else
throw_error (NOT_FOUND_ERROR, _("No source for line %d in file
\"%s\"."), ...);
}...
this is rather verbose, but it is the best way to ensure translation.
> }
>
> return values;
> diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
> index 34ac21982ea..4a9e27bd69f 100644
> --- a/gdb/testsuite/gdb.base/break.exp
> +++ b/gdb/testsuite/gdb.base/break.exp
> @@ -487,7 +487,7 @@ proc_with_prefix test_break_nonexistent_line {} {
> # breakpoint on a nonexistent source line.
> gdb_test_no_output "set breakpoint pending off"
> gdb_test "break 999" \
> - "^No compiled code for line 999 in the current file\\." \
> + "No source for line 999 in the current file. File has \[0-9\]+ lines\\." \
You removed the ^ character, which anchored the regexp on the start of
the line. Was this on purpose? If you haven't run into failures, I think
it would be better if the anchor was kept.
This is true of several other tests as well.
--
Cheers,
Guinevere Larsen
She/Her/Hers
> "break on non-existent source line"
> }
>
> diff --git a/gdb/testsuite/gdb.base/hbreak2.exp b/gdb/testsuite/gdb.base/hbreak2.exp
> index 37c001aa4b5..c18e43943b7 100644
> --- a/gdb/testsuite/gdb.base/hbreak2.exp
> +++ b/gdb/testsuite/gdb.base/hbreak2.exp
> @@ -296,7 +296,7 @@ if {![runto_main]} {
> #
> gdb_test_no_output "set breakpoint pending off"
> gdb_test "hbreak 999" \
> - "^No compiled code for line 999 in the current file\\." \
> + "No source for line 999 in the current file. File has \[0-9\]+ lines\\." \
> "hardware break on non-existent source line"
>
> # Run to the desired default location. If not positioned here, the
> diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
> index eb3515b84ae..77438bbb865 100644
> --- a/gdb/testsuite/gdb.base/sepdebug.exp
> +++ b/gdb/testsuite/gdb.base/sepdebug.exp
> @@ -296,7 +296,8 @@ gdb_test "catch exec" "Catchpoint \[0-9\]+ \\(exec\\)" \
> #
>
> gdb_test_no_output "set breakpoint pending off"
> -gdb_test "break 999" "^No compiled code for line 999 in the current file\\." \
> +gdb_test "break 999" \
> + "No source for line 999 in the current file. File has \[0-9\]+ lines\\." \
> "break on non-existent source line"
>
> # Run to the desired default location. If not positioned here, the
> diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp
> index 58125f3626c..7222b7c9a19 100644
> --- a/gdb/testsuite/gdb.linespec/ls-errs.exp
> +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp
> @@ -71,8 +71,8 @@ proc do_test {lang} {
> "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
> invalid_label "No label \"%s\" defined in function \"%s\"."
> invalid_parm "invalid linespec argument, \"%s\""
> - invalid_offset "No compiled code for line %d in the current file."
> - invalid_offset_f "No compiled code for line %d in file \"%s\"."
> + invalid_offset "No source for line %d in the current file. File has 46 lines."
> + invalid_offset_f "No source for line %d in file \"%s\". File has 46 lines."
> malformed_line_offset "malformed line offset: \"%s\""
> source_incomplete \
> "Source filename requires function, label, or line offset."
> @@ -80,6 +80,7 @@ proc do_test {lang} {
> unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
> unmatched_quote "unmatched quote"
> garbage "Garbage '%s' at end of command"
> + no_compiled_code "No compiled code for line %d in the current file."
> }
>
> # We intentionally do not use gdb_breakpoint for these tests.
> @@ -126,6 +127,9 @@ proc do_test {lang} {
> test_break "-line $x" invalid_offset $offset
> }
>
> + # Test that No compiled code is also a possible error.
> + test_break "1" no_compiled_code "1"
> +
> # Test offsets with trailing tokens w/ and w/o spaces.
> foreach x $spaces {
> test_break "3$x" unexpected "colon"
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index 934690db2a1..79c3119b409 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -743,9 +743,7 @@ proc_with_prefix test_bkpt_explicit_loc {} {
> "No source file named foo.*" \
> "set invalid explicit breakpoint by missing source and line"
> gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\", line=\"900\")" \
> - [multi_line \
> - "^No compiled code for line 900 in file \"$srcfile\"\\." \
> - "Breakpoint $::decimal \[^\r\n\]+ pending\\."] \
> + "No source for line 900 in file \"$srcfile\".*" \
> "set invalid explicit breakpoint by source and invalid line"
> gdb_test "python bp1 = gdb.Breakpoint (function=\"blah\")" \
> "Function \"blah\" not defined.*" \
> diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
> index c6ac80bfdb5..0f3b1b83d6d 100644
> --- a/gdb/testsuite/gdb.trace/tfind.exp
> +++ b/gdb/testsuite/gdb.trace/tfind.exp
> @@ -342,10 +342,10 @@ gdb_test "disassemble gdb_c_test" \
> "8.36: trace disassembly"
>
> gdb_test "tfind line 0" \
> - "out of range.*|failed to find.*|No compiled code for line 0 in .*" \
> + "out of range.*|failed to find.*|No source for line 0 in .*" \
> "8.18: tfind line 0"
> gdb_test "tfind line 32767" \
> - "out of range.*|failed to find.*|No compiled code for line 32767 in .*" \
> + "out of range.*|failed to find.*|No source for line 32767 in .*" \
> "8.27: tfind line 32767"
> gdb_test "tfind line NoSuChFiLe.c:$baseline" \
> "No source file named.*" \
> diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
> index 688980c78f7..b6337f24b2e 100644
> --- a/gdb/testsuite/gdb.trace/tracecmd.exp
> +++ b/gdb/testsuite/gdb.trace/tracecmd.exp
> @@ -65,7 +65,7 @@ gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
> gdb_delete_tracepoints
> gdb_test_no_output "set breakpoint pending off"
> gdb_test "trace $srcfile:99999" \
> - "No compiled code for line 99999 in file \".*$srcfile\"\\." \
> + "No source for line 99999 in file \".*$srcfile\"\\. File has 157 lines\\." \
> "1.2a: trace invalid line in sourcefile"
> gdb_test "info trace" "No tracepoints.*" \
> "1.2b: reject invalid line in srcfile"
More information about the Gdb-patches
mailing list