public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Improve error messages with incomplete variables
@ 2022-11-04 15:47 Bruno Larsen
  2022-11-04 15:47 ` [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions Bruno Larsen
  2022-11-04 15:47 ` [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution Bruno Larsen
  0 siblings, 2 replies; 11+ messages in thread
From: Bruno Larsen @ 2022-11-04 15:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Bruno Larsen

Currently, if a user attempts to call a C++ fuction by hand using an
incomplete variable, GDB might be unable to find the correct overload,
but the error message in this situation is not intuitive at all. This
series attempts improve those messages with a hint.

To create a reasonably reproducible test, I decided to use the DWARF
assembler, but it needed some fixing to deal with C++ mangled names, so
the first patch happened.

Changes for v2:
 * Used Andrew's suggestion for patch 1
 * Styling changes to code
 * Factored new code into a static function

Bruno Larsen (2):
  gdb/testsuite: allowed for function_range to deal with mangled
    functions
  gdb/c++: Improve error messages in overload resolution

 .../gdb.cp/incomplete-type-overload.cc        |  45 +++++
 .../gdb.cp/incomplete-type-overload.exp       | 183 ++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp                   |   2 +-
 gdb/valops.c                                  |  53 ++++-
 4 files changed, 278 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.cc
 create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.exp

-- 
2.37.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions
  2022-11-04 15:47 [PATCH v2 0/2] Improve error messages with incomplete variables Bruno Larsen
@ 2022-11-04 15:47 ` Bruno Larsen
  2022-11-09 17:15   ` Tom Tromey
  2022-11-04 15:47 ` [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution Bruno Larsen
  1 sibling, 1 reply; 11+ messages in thread
From: Bruno Larsen @ 2022-11-04 15:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Bruno Larsen

When calling get_func_info inside a test case, it would cause failures
if the function was printed using a C++ style mangled name. The current
patch fixes this by allowing for mangled names along with the current
rules.
---
 gdb/testsuite/lib/dwarf.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b843b1acf75..9df8e7f9bdc 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -400,7 +400,7 @@ proc function_range { func src {options {debug}} } {
     if { $func_length != 0 } {
 	set func_pattern "$func_pattern\\+$func_length"
     }
-    set test "x/2i $func+$func_length"
+    set test "with print asm-demangle on -- x/2i $func+$func_length"
     gdb_test_multiple $test $test {
 	-re ".*($hex) <$func_pattern>:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
 	    set start $expect_out(1,string)
-- 
2.37.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution
  2022-11-04 15:47 [PATCH v2 0/2] Improve error messages with incomplete variables Bruno Larsen
  2022-11-04 15:47 ` [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions Bruno Larsen
@ 2022-11-04 15:47 ` Bruno Larsen
  2022-11-09 17:47   ` Tom Tromey
  2022-11-10 15:19   ` Simon Marchi
  1 sibling, 2 replies; 11+ messages in thread
From: Bruno Larsen @ 2022-11-04 15:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Bruno Larsen

When resolving overloaded functions, GDB relies on knowing relationships
between types, i.e. if a type inherits from another. However, some
compilers may not add complete information for given types as a way to
reduce unnecessary debug information. In these cases, GDB would just say
that it couldn't resolve the method or function, with no extra
information.

The problem is that sometimes the user may not know that the type
information is incomplete, and may just assume that there is a bug in
GDB. To improve the user experience, we attempt to detect if the
overload match failed because of an incomplete type, and warn the user
of this.

This commit also adds a testcase confirming that the message is only
triggered in the correct scenario. This test was not developed as an
expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
and porting all of overload.cc seemed unnecessary.
---
 .../gdb.cp/incomplete-type-overload.cc        |  45 +++++
 .../gdb.cp/incomplete-type-overload.exp       | 183 ++++++++++++++++++
 gdb/valops.c                                  |  53 ++++-
 3 files changed, 277 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.cc
 create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.exp

diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.cc b/gdb/testsuite/gdb.cp/incomplete-type-overload.cc
new file mode 100644
index 00000000000..a7b85c92bbe
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.cc
@@ -0,0 +1,45 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+class
+base {
+public:
+  int member;
+};
+
+class complete: public base { };
+
+class incomplete: public base { };
+
+complete comp;
+complete *cp = &comp;
+incomplete *inc;
+int *ip;
+
+int
+foo (base* b)
+{
+    asm ("foo_label: .globl foo_label");
+    return 1;
+}
+
+int
+main (void)
+{
+    asm("main_label: .globl main_label");
+    comp.member = 0;
+    return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.exp b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
new file mode 100644
index 00000000000..96ed25dd5d1
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
@@ -0,0 +1,183 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of GDB's testsuite.
+
+# This test intends to check the error message that GDB emits when unable
+# to determine the correct overloaded function due to incomplete types.
+
+load_lib dwarf.exp
+
+if { [skip_cplus_tests] } { return }
+
+if { ![dwarf2_support] } { return }
+
+standard_testfile .cc .S
+set asm_file [standard_output_file ${srcfile2}]
+
+if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}] {
+    return
+}
+
+if {[test_compiler_info clang-*-*]} {
+    untested "gcc is required for dwarf assembler tests"
+    return
+}
+
+if ![runto_main] {
+    return
+}
+
+# Get important sizes to create fake dwarf for the test
+set int_size [get_sizeof "int" -1]
+set addr_size [get_sizeof "void *" -1]
+set struct_base_size [get_sizeof "base" 4]
+set struct_complete_size [get_sizeof "complete" 4]
+get_func_info foo
+
+# Create fake DWARF for the .cc file.
+# This is the best way to ensure we have an incomplete type.
+Dwarf::assemble ${asm_file} {
+    global srcdir subdir srcfile srcfile2 foo_start foo_end
+    global int_size addr_size struct_base_size struct_complete_size
+    declare_labels L
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {name $srcfile}
+	    {stmt_list $L DW_FORM_sec_offset}
+	} {
+	    declare_labels int_label base_label complete_label incomplete_label
+	    declare_labels ptr_base_label ptr_inc_label ptr_comp_label ptr_int_label
+
+	    int_label: DW_TAG_base_type {
+		{DW_AT_byte_size $int_size DW_FORM_sdata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name "int"}
+	    }
+
+	    base_label: DW_TAG_class_type {
+		{DW_AT_byte_size $struct_base_size DW_FORM_sdata}
+		{DW_AT_name "base"}
+	    } {
+		DW_TAG_member {
+		    {DW_AT_name "member"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location 0 DW_FORM_sdata}
+		}
+	    }
+
+	    complete_label: DW_TAG_class_type {
+		{DW_AT_byte_size $struct_complete_size DW_FORM_sdata}
+		{DW_AT_name "complete"}
+	    } {
+		DW_TAG_inheritance {
+		    {DW_AT_type :$base_label}
+		    {DW_AT_data_member_location 0 DW_FORM_sdata}
+		    {DW_AT_accessibility 1 DW_FORM_data1}
+		}
+	    }
+
+	    incomplete_label: DW_TAG_class_type {
+		{DW_AT_name "incomplete"}
+		{DW_AT_declaration 1 DW_FORM_flag_present}
+	    }
+
+	    ptr_base_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $addr_size DW_FORM_udata}
+		{DW_AT_type :$base_label}
+	    }
+
+	    ptr_inc_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $addr_size DW_FORM_udata}
+		{DW_AT_type :$incomplete_label}
+	    }
+
+	    ptr_comp_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $addr_size DW_FORM_udata}
+		{DW_AT_type :$complete_label}
+	    }
+
+	    ptr_int_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $addr_size DW_FORM_udata}
+		{DW_AT_type :$int_label}
+	    }
+
+	DW_TAG_variable {
+	    {DW_AT_name "comp"}
+	    {DW_AT_type :$complete_label}
+	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "comp"]} SPECIAL_expr}
+	    {DW_AT_external 1 DW_FORM_flag}
+	}
+
+	DW_TAG_variable {
+	    {DW_AT_name "cp"}
+	    {DW_AT_type :$ptr_comp_label}
+	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "cp"]} SPECIAL_expr}
+	    {DW_AT_external 1 DW_FORM_flag}
+	}
+
+	DW_TAG_variable {
+	    {DW_AT_name "inc"}
+	    {DW_AT_type :$ptr_inc_label}
+	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "inc"]} SPECIAL_expr}
+	    {DW_AT_external 1 DW_FORM_flag}
+	}
+
+	DW_TAG_variable {
+	    {DW_AT_name "ip"}
+	    {DW_AT_type :$ptr_int_label}
+	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "ip"]} SPECIAL_expr}
+	    {DW_AT_external 1 DW_FORM_flag}
+	}
+
+	    DW_TAG_subprogram {
+		{MACRO_AT_func {"main"}}
+		{DW_AT_external 1 flag}
+	    }
+	    DW_TAG_subprogram {
+		{MACRO_AT_func {"foo"}}
+		{DW_AT_type :$int_label}
+		{DW_AT_external 1 flag}
+	    } { formal_parameter {
+		    {DW_AT_name "b"}
+		    {DW_AT_type :$ptr_base_label}
+		}
+	    }
+	}
+    }
+
+    lines {version 2} L {
+	include_dir "$srcdir/$subdir"
+	file_name $srcfile 1
+    }
+}
+
+if [prepare_for_testing "failed to prepare" $testfile [list $asm_file $srcfile] {}] {
+    return
+}
+
+if ![runto_main] {
+    return
+}
+
+gdb_test "print foo(cp)" "= 1" "successful invocation"
+gdb_test "print foo(inc)"\
+	 "The type. 'incomplete .' isn't fully known to GDB.*"\
+	 "unsuccessful because declaration"
+gdb_test "print foo(ip)"\
+	 "Cannot resolve function foo to any overloaded instance."\
+	 "unsuccessful because incorrect"
diff --git a/gdb/valops.c b/gdb/valops.c
index ecfceed199a..2b789cd76f4 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -41,6 +41,7 @@
 #include "extension.h"
 #include "gdbtypes.h"
 #include "gdbsupport/byte-vector.h"
+#include "typeprint.h"
 
 /* Local functions.  */
 
@@ -2617,6 +2618,49 @@ value_find_oload_method_list (struct value **argp, const char *method,
 		    basetype, boffset);
 }
 
+/* Helper function for find_overload_match.  If no matches were
+   found, this function may generate a hint for the user that some
+   of the relevant types are incomplete, so GDB can't evaluate
+   type relationships to properly evaluate overloads.
+
+   If no incomplete types are present, an empty string is returned.  */
+static std::string
+incomplete_type_hint (gdb::array_view<value *> args)
+{
+  int incomplete_types = 0;
+  std::string incomplete_arg_names;
+  for (const struct value *arg : args)
+    {
+      struct type *t = value_type (arg);
+      while (t->code () == TYPE_CODE_PTR)
+	t = t->target_type ();
+      if (t->is_stub ())
+	{
+	  string_file buffer;
+	  if (incomplete_types > 0)
+	    incomplete_arg_names += ", ";
+
+	  current_language->print_type (value_type (arg), "", &buffer,
+				       -1, 0, &type_print_raw_options);
+
+	  incomplete_types++;
+	  incomplete_arg_names += buffer.string ();
+	}
+    }
+  std::string hint;
+  if (incomplete_types > 1)
+    hint = string_printf (_("\nThe types: '%s' aren't fully known to GDB."
+			    " Please cast them directly to the desired"
+			    " typed in the function call."),
+			    incomplete_arg_names.c_str ());
+  else if (incomplete_types == 1)
+    hint = string_printf (_("\nThe type: '%s' isn't fully known to GDB."
+			    " Please cast it directly to the desired"
+			    " typed in the function call."),
+			    incomplete_arg_names.c_str ());
+  return hint;
+}
+
 /* Given an array of arguments (ARGS) (which includes an entry for
    "this" in the case of C++ methods), the NAME of a function, and
    whether it's a method or not (METHOD), find the best function that
@@ -2933,14 +2977,15 @@ find_overload_match (gdb::array_view<value *> args,
 
   if (match_quality == INCOMPATIBLE)
     {
+      std::string hint = incomplete_type_hint (args);
       if (method == METHOD)
-	error (_("Cannot resolve method %s%s%s to any overloaded instance"),
+	error (_("Cannot resolve method %s%s%s to any overloaded instance.%s"),
 	       obj_type_name,
 	       (obj_type_name && *obj_type_name) ? "::" : "",
-	       name);
+	       name, hint.c_str ());
       else
-	error (_("Cannot resolve function %s to any overloaded instance"),
-	       func_name);
+	error (_("Cannot resolve function %s to any overloaded instance.%s"),
+	       func_name, hint.c_str ());
     }
   else if (match_quality == NON_STANDARD)
     {
-- 
2.37.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions
  2022-11-04 15:47 ` [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions Bruno Larsen
@ 2022-11-09 17:15   ` Tom Tromey
  2022-11-10 11:41     ` Bruno Larsen
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2022-11-09 17:15 UTC (permalink / raw)
  To: Bruno Larsen via Gdb-patches; +Cc: Bruno Larsen

>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

Bruno> When calling get_func_info inside a test case, it would cause failures
Bruno> if the function was printed using a C++ style mangled name. The current
Bruno> patch fixes this by allowing for mangled names along with the current
Bruno> rules.

Looks good.  Thank you.

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution
  2022-11-04 15:47 ` [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution Bruno Larsen
@ 2022-11-09 17:47   ` Tom Tromey
  2022-11-10 15:19   ` Simon Marchi
  1 sibling, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2022-11-09 17:47 UTC (permalink / raw)
  To: Bruno Larsen via Gdb-patches; +Cc: Bruno Larsen

>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

Bruno> When resolving overloaded functions, GDB relies on knowing relationships
Bruno> between types, i.e. if a type inherits from another. However, some
Bruno> compilers may not add complete information for given types as a way to
Bruno> reduce unnecessary debug information. In these cases, GDB would just say
Bruno> that it couldn't resolve the method or function, with no extra
Bruno> information.

Thanks for the patch.  This looks good to me.

I wonder if this could be the problem underlying
https://sourceware.org/bugzilla/show_bug.cgi?id=28901

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions
  2022-11-09 17:15   ` Tom Tromey
@ 2022-11-10 11:41     ` Bruno Larsen
  2022-11-10 13:44       ` Tom Tromey
  0 siblings, 1 reply; 11+ messages in thread
From: Bruno Larsen @ 2022-11-10 11:41 UTC (permalink / raw)
  To: Tom Tromey, Bruno Larsen via Gdb-patches

On 09/11/2022 18:15, Tom Tromey wrote:
>>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> Bruno> When calling get_func_info inside a test case, it would cause failures
> Bruno> if the function was printed using a C++ style mangled name. The current
> Bruno> patch fixes this by allowing for mangled names along with the current
> Bruno> rules.
>
> Looks good.  Thank you.
Thanks for the review. Which email should I add to the Approved-by tag?

-- 
Cheers,
Bruno

> Tom


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions
  2022-11-10 11:41     ` Bruno Larsen
@ 2022-11-10 13:44       ` Tom Tromey
  2022-11-10 13:52         ` Bruno Larsen
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2022-11-10 13:44 UTC (permalink / raw)
  To: Bruno Larsen via Gdb-patches; +Cc: Tom Tromey, Bruno Larsen

Bruno> Thanks for the review. Which email should I add to the Approved-by tag?

It's not super important to me but I guess as a rule of thumb it seems
safe to use whatever address the approval is sent from.

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions
  2022-11-10 13:44       ` Tom Tromey
@ 2022-11-10 13:52         ` Bruno Larsen
  0 siblings, 0 replies; 11+ messages in thread
From: Bruno Larsen @ 2022-11-10 13:52 UTC (permalink / raw)
  To: Tom Tromey, Bruno Larsen via Gdb-patches

On 10/11/2022 14:44, Tom Tromey wrote:
> Bruno> Thanks for the review. Which email should I add to the Approved-by tag?
>
> It's not super important to me but I guess as a rule of thumb it seems
> safe to use whatever address the approval is sent from.

Thanks, I pushed the whole series!

-- 
Cheers,
Bruno

> Tom
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution
  2022-11-04 15:47 ` [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution Bruno Larsen
  2022-11-09 17:47   ` Tom Tromey
@ 2022-11-10 15:19   ` Simon Marchi
  2022-11-10 15:29     ` Bruno Larsen
  1 sibling, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2022-11-10 15:19 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches



On 11/4/22 11:47, Bruno Larsen via Gdb-patches wrote:
> When resolving overloaded functions, GDB relies on knowing relationships
> between types, i.e. if a type inherits from another. However, some
> compilers may not add complete information for given types as a way to
> reduce unnecessary debug information. In these cases, GDB would just say
> that it couldn't resolve the method or function, with no extra
> information.
> 
> The problem is that sometimes the user may not know that the type
> information is incomplete, and may just assume that there is a bug in
> GDB. To improve the user experience, we attempt to detect if the
> overload match failed because of an incomplete type, and warn the user
> of this.
> 
> This commit also adds a testcase confirming that the message is only
> triggered in the correct scenario. This test was not developed as an
> expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
> and porting all of overload.cc seemed unnecessary.

I get some new failures starting from this patch (tested both on Ubuntu
22.04 and a fairly recent Arch Linux):

FAIL: gdb.cp/converts.exp: p foo3_1 (0, 1)
FAIL: gdb.cp/converts.exp: p foo1_7(ta)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo1_type_check (123)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (0, 1)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (1, 0)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (1, 1)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 0, 1)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 1, 0)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 0, 0)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 1, 1)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 1, 0)
FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 1, 1)
FAIL: gdb.cp/enum-class.exp: print overload2(77)
FAIL: gdb.cp/enum-class.exp: print overload3(E1::THERE)
FAIL: gdb.cp/koenig.exp: p foo(ix)
FAIL: gdb.cp/koenig.exp: p foo (p_union)
FAIL: gdb.cp/local-static.exp: c++: print S::method()
FAIL: gdb.cp/local-static.exp: c++: print S::inline_method()
FAIL: gdb.cp/local-static.exp: c++: print S2<int>::method()
FAIL: gdb.cp/local-static.exp: c++: print S2<int>::inline_method()
FAIL: gdb.cp/operator.exp: namespace alias
FAIL: gdb.cp/operator.exp: imported declaration
FAIL: gdb.cp/pr12028.exp: p D::foo(b)
FAIL: gdb.cp/rvalue-ref-overload.exp: passing lvalue arg to rvalue parameter

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution
  2022-11-10 15:19   ` Simon Marchi
@ 2022-11-10 15:29     ` Bruno Larsen
  2022-11-10 15:45       ` Simon Marchi
  0 siblings, 1 reply; 11+ messages in thread
From: Bruno Larsen @ 2022-11-10 15:29 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 10/11/2022 16:19, Simon Marchi wrote:
>
> On 11/4/22 11:47, Bruno Larsen via Gdb-patches wrote:
>> When resolving overloaded functions, GDB relies on knowing relationships
>> between types, i.e. if a type inherits from another. However, some
>> compilers may not add complete information for given types as a way to
>> reduce unnecessary debug information. In these cases, GDB would just say
>> that it couldn't resolve the method or function, with no extra
>> information.
>>
>> The problem is that sometimes the user may not know that the type
>> information is incomplete, and may just assume that there is a bug in
>> GDB. To improve the user experience, we attempt to detect if the
>> overload match failed because of an incomplete type, and warn the user
>> of this.
>>
>> This commit also adds a testcase confirming that the message is only
>> triggered in the correct scenario. This test was not developed as an
>> expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
>> and porting all of overload.cc seemed unnecessary.
> I get some new failures starting from this patch (tested both on Ubuntu
> 22.04 and a fairly recent Arch Linux):
>
> FAIL: gdb.cp/converts.exp: p foo3_1 (0, 1)
> FAIL: gdb.cp/converts.exp: p foo1_7(ta)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo1_type_check (123)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (0, 1)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (1, 0)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (1, 1)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 0, 1)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 1, 0)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 0, 0)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 1, 1)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 1, 0)
> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 1, 1)

/me facepalms

I'm very sorry, got tunnel visioned in fixing the issue, forgot to run 
the rest of the tests. Will fix all of these.

> FAIL: gdb.cp/enum-class.exp: print overload2(77)
> FAIL: gdb.cp/enum-class.exp: print overload3(E1::THERE)
> FAIL: gdb.cp/koenig.exp: p foo(ix)
> FAIL: gdb.cp/koenig.exp: p foo (p_union)
> FAIL: gdb.cp/local-static.exp: c++: print S::method()
> FAIL: gdb.cp/local-static.exp: c++: print S::inline_method()
> FAIL: gdb.cp/local-static.exp: c++: print S2<int>::method()
> FAIL: gdb.cp/local-static.exp: c++: print S2<int>::inline_method()
> FAIL: gdb.cp/operator.exp: namespace alias
> FAIL: gdb.cp/operator.exp: imported declaration
> FAIL: gdb.cp/pr12028.exp: p D::foo(b)
> FAIL: gdb.cp/rvalue-ref-overload.exp: passing lvalue arg to rvalue parameter
However, I'm not getting these fails... is gcc only emitting delaration 
DIEs for these tests, or is it something else?

-- 
Cheers,
Bruno

> Simon


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution
  2022-11-10 15:29     ` Bruno Larsen
@ 2022-11-10 15:45       ` Simon Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2022-11-10 15:45 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches



On 11/10/22 10:29, Bruno Larsen via Gdb-patches wrote:
> On 10/11/2022 16:19, Simon Marchi wrote:
>>
>> On 11/4/22 11:47, Bruno Larsen via Gdb-patches wrote:
>>> When resolving overloaded functions, GDB relies on knowing relationships
>>> between types, i.e. if a type inherits from another. However, some
>>> compilers may not add complete information for given types as a way to
>>> reduce unnecessary debug information. In these cases, GDB would just say
>>> that it couldn't resolve the method or function, with no extra
>>> information.
>>>
>>> The problem is that sometimes the user may not know that the type
>>> information is incomplete, and may just assume that there is a bug in
>>> GDB. To improve the user experience, we attempt to detect if the
>>> overload match failed because of an incomplete type, and warn the user
>>> of this.
>>>
>>> This commit also adds a testcase confirming that the message is only
>>> triggered in the correct scenario. This test was not developed as an
>>> expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
>>> and porting all of overload.cc seemed unnecessary.
>> I get some new failures starting from this patch (tested both on Ubuntu
>> 22.04 and a fairly recent Arch Linux):
>>
>> FAIL: gdb.cp/converts.exp: p foo3_1 (0, 1)
>> FAIL: gdb.cp/converts.exp: p foo1_7(ta)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo1_type_check (123)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (0, 1)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (1, 0)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo2_type_check (1, 1)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 0, 1)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 1, 0)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 0, 0)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (0, 1, 1)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 1, 0)
>> FAIL: gdb.cp/converts.exp: strict type checking on: p foo3_type_check (1, 1, 1)
> 
> /me facepalms
> 
> I'm very sorry, got tunnel visioned in fixing the issue, forgot to run the rest of the tests. Will fix all of these.
> 
>> FAIL: gdb.cp/enum-class.exp: print overload2(77)
>> FAIL: gdb.cp/enum-class.exp: print overload3(E1::THERE)
>> FAIL: gdb.cp/koenig.exp: p foo(ix)
>> FAIL: gdb.cp/koenig.exp: p foo (p_union)
>> FAIL: gdb.cp/local-static.exp: c++: print S::method()
>> FAIL: gdb.cp/local-static.exp: c++: print S::inline_method()
>> FAIL: gdb.cp/local-static.exp: c++: print S2<int>::method()
>> FAIL: gdb.cp/local-static.exp: c++: print S2<int>::inline_method()
>> FAIL: gdb.cp/operator.exp: namespace alias
>> FAIL: gdb.cp/operator.exp: imported declaration
>> FAIL: gdb.cp/pr12028.exp: p D::foo(b)
>> FAIL: gdb.cp/rvalue-ref-overload.exp: passing lvalue arg to rvalue parameter
> However, I'm not getting these fails... is gcc only emitting delaration DIEs for these tests, or is it something else?

An example of failure is:

 89 print overload2(77)^M
 90 Cannot resolve function overload2 to any overloaded instance.^M
 91 (gdb) FAIL: gdb.cp/enum-class.exp: print overload2(77)    
 92 print overload3(E1::THERE)^M 
 93 Cannot resolve function overload3 to any overloaded instance.^M
 94 (gdb) FAIL: gdb.cp/enum-class.exp: print overload3(E1::THERE)

I do have these DIEs for overload2:

0x00000658:   DW_TAG_subprogram
                DW_AT_external [DW_FORM_flag_present]   (true)
                DW_AT_name [DW_FORM_strp]       ("overload2")
                DW_AT_decl_file [DW_FORM_implicit_const]        ("/home/simark/src/binutils-gdb/gdb/testsuite/gdb.cp/enum-class.cc")
                DW_AT_decl_line [DW_FORM_data1] (35)
                DW_AT_decl_column [DW_FORM_implicit_const]      (5)
                DW_AT_linkage_name [DW_FORM_strp]       ("_Z9overload22E2")
                DW_AT_type [DW_FORM_ref4]       (0x000005b8 "int")
                DW_AT_low_pc [DW_FORM_addr]     (0x000000000000114d)
                DW_AT_high_pc [DW_FORM_data8]   (0x000000000000000e)
                DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_call_frame_cfa)
                DW_AT_call_all_calls [DW_FORM_flag_present]     (true)
                DW_AT_sibling [DW_FORM_ref4]    (0x00000689)

0x0000067c:     DW_TAG_formal_parameter
                  DW_AT_name [DW_FORM_string]   ("v")
                  DW_AT_decl_file [DW_FORM_implicit_const]      ("/home/simark/src/binutils-gdb/gdb/testsuite/gdb.cp/enum-class.cc")
                  DW_AT_decl_line [DW_FORM_data1]       (35)
                  DW_AT_decl_column [DW_FORM_data1]     (0x13)
                  DW_AT_type [DW_FORM_ref4]     (0x000005bf "E2")
                  DW_AT_location [DW_FORM_exprloc]      (DW_OP_fbreg -20)

0x00000688:     NULL

0x00000689:   DW_TAG_subprogram
                DW_AT_external [DW_FORM_flag_present]   (true)
                DW_AT_name [DW_FORM_strp]       ("overload2")
                DW_AT_decl_file [DW_FORM_implicit_const]        ("/home/simark/src/binutils-gdb/gdb/testsuite/gdb.cp/enum-class.cc")
                DW_AT_decl_line [DW_FORM_data1] (34)
                DW_AT_decl_column [DW_FORM_implicit_const]      (5)
                DW_AT_linkage_name [DW_FORM_strp]       ("_Z9overload22E1")
                DW_AT_type [DW_FORM_ref4]       (0x000005b8 "int")
                DW_AT_low_pc [DW_FORM_addr]     (0x0000000000001141)
                DW_AT_high_pc [DW_FORM_data8]   (0x000000000000000c)
                DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_call_frame_cfa)
                DW_AT_call_all_calls [DW_FORM_flag_present]     (true)
                DW_AT_sibling [DW_FORM_ref4]    (0x000006ba)

0x000006ad:     DW_TAG_formal_parameter
                  DW_AT_name [DW_FORM_string]   ("v")
                  DW_AT_decl_file [DW_FORM_implicit_const]      ("/home/simark/src/binutils-gdb/gdb/testsuite/gdb.cp/enum-class.cc")
                  DW_AT_decl_line [DW_FORM_data1]       (34)
                  DW_AT_decl_column [DW_FORM_data1]     (0x13)
                  DW_AT_type [DW_FORM_ref4]     (0x0000059f "E1")
                  DW_AT_location [DW_FORM_exprloc]      (DW_OP_fbreg -20)

0x000006b9:     NULL

It's gcc 11.3 for Ubuntu 22.04, gcc 12.2 for Arch Linux.

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-11-10 15:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 15:47 [PATCH v2 0/2] Improve error messages with incomplete variables Bruno Larsen
2022-11-04 15:47 ` [PATCH v2 1/2] gdb/testsuite: allowed for function_range to deal with mangled functions Bruno Larsen
2022-11-09 17:15   ` Tom Tromey
2022-11-10 11:41     ` Bruno Larsen
2022-11-10 13:44       ` Tom Tromey
2022-11-10 13:52         ` Bruno Larsen
2022-11-04 15:47 ` [PATCH v2 2/2] gdb/c++: Improve error messages in overload resolution Bruno Larsen
2022-11-09 17:47   ` Tom Tromey
2022-11-10 15:19   ` Simon Marchi
2022-11-10 15:29     ` Bruno Larsen
2022-11-10 15:45       ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).