[PATCH 00/11] Cleanup gdb.cp tests when running with clang

[PATCH 00/11] Cleanup gdb.cp tests when running with clang

[Bruno Larsen]

Following my previous series, where I tried to reduce the amount of
failures when running gdb.base tests using clang, I moved to reducing
errors in gdb.cp.

Actual GDB issues are being addressed in separate patches, and this
series is confined to just changing tests to either work or
XFAIL/disable them, to make it simpler to keep track of issues.

Bruno Larsen (11):
  gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp
  gdb/testsuite: enable running gdb.cp/classes.exp with clang
  gdb/testsuite: account for clang's recursive destructor calls on
    gdb.cp/mb-ctor.exp
  gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang
  gdb/testsuite: allow for clang style destructors on
    gdb.cp/m-static.exp
  gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang
  gdb/testsuite: skip gdb.cp/anon-struct.exp when using clang
  gdb/testsuite: disable gdb.cp/typeid.exp when using clang
  gdb/testsuite: fix gdb.cp/converts.exp to run with clang
  gdb/testsuite: remove XFAIL on gdb.cp/temargs.exp
  gdb/testsuite: disable gdb.cp/call-method-register.exp with clang

 gdb/testsuite/gdb.cp/anon-struct.exp          |  8 +++++
 gdb/testsuite/gdb.cp/call-method-register.exp |  5 +++
 gdb/testsuite/gdb.cp/class2.exp               |  2 +-
 gdb/testsuite/gdb.cp/classes.exp              | 33 ++++++++++++++++-
 gdb/testsuite/gdb.cp/converts.cc              |  2 +-
 gdb/testsuite/gdb.cp/derivation.exp           |  4 +++
 gdb/testsuite/gdb.cp/m-static.exp             | 36 +++++++++++++------
 gdb/testsuite/gdb.cp/mb-ctor.exp              | 16 +++++++++
 gdb/testsuite/gdb.cp/ptype-flags.exp          | 12 +++++++
 gdb/testsuite/gdb.cp/temargs.exp              |  6 ++--
 gdb/testsuite/gdb.cp/typeid.exp               |  5 +++
 11 files changed, 114 insertions(+), 15 deletions(-)

-- 
2.37.3

[PATCH 01/11] gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp

[Bruno Larsen]

When attempting to test gdb.cp/class2.exp using clang, it fails to
prepare with the following error:

Executing on host: clang++    -fdiagnostics-color=never -Wno-unknown-warning-option  -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc    (timeout = 300)
builtin_spawn -ignore SIGHUP clang++ -fdiagnostics-color=never -Wno-unknown-warning-option -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
typedef class {
             ^
              Dbase
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
 public:
 ^~~~~~~
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
} Dbase;
  ^
1 warning generated.
gdb compile failed, /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
typedef class {
             ^
              Dbase
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
 public:
 ^~~~~~~
/home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
} Dbase;
  ^
1 warning generated.
UNTESTED: gdb.cp/class2.exp: failed to prepare

This can be silenced by adding -Wno-non-c-typedef-for-linkage. The test
shows no failures with this change.
---
 gdb/testsuite/gdb.cp/class2.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/class2.exp b/gdb/testsuite/gdb.cp/class2.exp
index 32f9dfc18a5..e1e4ad0fae6 100644
--- a/gdb/testsuite/gdb.cp/class2.exp
+++ b/gdb/testsuite/gdb.cp/class2.exp
@@ -18,7 +18,7 @@ if { [skip_cplus_tests] } { return }
 
 standard_testfile .cc
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {
     return -1
 }
 
-- 
2.37.3

[PATCH 02/11] gdb/testsuite: enable running gdb.cp/classes.exp with clang

[Bruno Larsen]

When attempting to run the gdb.cp/classes.exp test using clang++, the
test fails to prepare with -Wnon-c-typedef-for-linkage like the
previously fixed gdb.cp/class2.exp. Upon fixing this, the test shows 5
unexpected failures. One such failures is:

ptype/r class class_with_public_typedef
type = class class_with_public_typedef {
  private:
    int a;
  public:
    class_with_public_typedef::INT b;

  private:
    typedef int INT;
}
(gdb) FAIL: gdb.cp/classes.exp: ptype class class_with_public_typedef // wrong access specifier for typedef: private

While g++ provided the following output:

ptype/r class class_with_public_typedef
type = class class_with_public_typedef {
  private:
    int a;
  public:
    class_with_public_typedef::INT b;

    typedef int INT;
}
(gdb) PASS: gdb.cp/classes.exp: ptype class class_with_public_typedef

This error happens because clang does not add DW_AT_accessibility to
typedefs inside classes, and without this information GDB defaults to
assuming the typedef is private.  Since there is nothing that GDB can do
about this, these tests have been set as xfails, and a clang bug has
been filed.
---
 gdb/testsuite/gdb.cp/classes.exp | 33 +++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
index 7b8b315ac1f..3f3e587803c 100644
--- a/gdb/testsuite/gdb.cp/classes.exp
+++ b/gdb/testsuite/gdb.cp/classes.exp
@@ -24,14 +24,20 @@ load_lib "cp-support.exp"
 
 standard_testfile .cc
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {
     return -1
 }
 
+
 # Test ptype of class objects.
 
 proc test_ptype_class_objects {} {
 
+    if {[test_compiler_info {clang*}]} {
+	set clang_used true
+    } else {
+	set clang_used false
+    }
     # Simple type.
 
     cp_test_ptype_class \
@@ -319,6 +325,12 @@ proc test_ptype_class_objects {} {
 
     # Classes with typedefs of different access.
 
+    # Clang does not add access information for typedefs in classes.
+    # More information on: https://github.com/llvm/llvm-project/issues/57608
+    if {$clang_used} {
+	setup_xfail "clang 57608" *-*-*
+    }
+
     cp_test_ptype_class \
 	"class class_with_typedefs" "" "class" "class_with_typedefs" \
 	{
@@ -339,6 +351,10 @@ proc test_ptype_class_objects {} {
 	    { typedef private "typedef int private_int;" }
 	}
 
+    if {$clang_used} {
+	setup_xfail "clang 57608" *-*-*
+    }
+
     cp_test_ptype_class \
 	"class class_with_public_typedef" "" "class" \
 	"class_with_public_typedef" {
@@ -346,6 +362,11 @@ proc test_ptype_class_objects {} {
 	    { field public "class_with_public_typedef::INT b;" }
 	    { typedef public "typedef int INT;" }
 	}
+
+    if {$clang_used} {
+	setup_xfail "clang 57608" *-*-*
+    }
+
     cp_test_ptype_class \
 	"class class_with_protected_typedef" "" "class" \
 	"class_with_protected_typedef" {
@@ -353,6 +374,11 @@ proc test_ptype_class_objects {} {
 	    { field protected "class_with_protected_typedef::INT b;" }
 	    { typedef protected "typedef int INT;" }
 	}
+
+    if {$clang_used} {
+	setup_xfail "clang 57608" *-*-*
+    }
+
     cp_test_ptype_class \
 	"struct struct_with_protected_typedef" "" "struct" \
 	"struct_with_protected_typedef" {
@@ -360,6 +386,11 @@ proc test_ptype_class_objects {} {
 	    { field protected "struct_with_protected_typedef::INT b;" }
 	    { typedef protected "typedef int INT;" }
 	}
+
+    if {$clang_used} {
+	setup_xfail "clang 57608" *-*-*
+    }
+
     cp_test_ptype_class \
 	"struct struct_with_private_typedef" "" "struct" \
 	"struct_with_private_typedef" {
-- 
2.37.3

[PATCH 03/11] gdb/testsuite: account for clang's recursive destructor calls on gdb.cp/mb-ctor.exp

[Bruno Larsen]

When compiling virtual classes's destructors, two versions are compiled,
one with a single parameter (this) and the other with 2 parameters (this
and vtt).

GCC's compilation makes it so either the version with 1
parameter or the one with 2 parameters is called, depending on whether
the destructor is being called by the class itself or by an inherited
class. On the test gdb.cp/mb-ctor.exp, this means that the breakpoint
set at the destructor will be hit 4 times.

Clang, on the other hand, makes the single-parameter version call the 2
parameter version, probably in an attempt to reduce the size of the
resulting executable. This means that the gdb.cp/mb-ctor.exp will hit 6
breakpoints before finishing, and is the reason why this test was
failing. To make this test stop failing, a compiler check is added and
another "continue" instruction is issued to account for this difference.
---
 gdb/testsuite/gdb.cp/mb-ctor.exp | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/mb-ctor.exp b/gdb/testsuite/gdb.cp/mb-ctor.exp
index 5a3c0a4a90f..6aadecdd27d 100644
--- a/gdb/testsuite/gdb.cp/mb-ctor.exp
+++ b/gdb/testsuite/gdb.cp/mb-ctor.exp
@@ -25,6 +25,12 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
     return -1
 }
 
+if {[test_compiler_info {clang-*-*}]} {
+    set using_clang true
+} else {
+    set using_clang false
+}
+
 if ![runto_main] then {
     perror "couldn't run to breakpoint"
     return
@@ -71,6 +77,12 @@ gdb_continue_to_breakpoint "set breakpoint here, second dynamic time"\
 
 gdb_test "continue" ".*Breakpoint.*~Derived.*" "Run to dynamic destructor v1"
 
+# Clang makes Derived::~Derived(this) call Derived::~Derives(this, vtt)
+# whereas gcc puts all the logic necessary on both functions.
+if {$using_clang} {
+    gdb_test "continue" ".*Breakpoint.*~Derived.*" "clang's recursive dynamic destructor call"
+}
+
 gdb_test "continue" ".*Breakpoint.*~Derived.*" "Run to dynamic destructor v2"
 
 gdb_test "continue" \
@@ -81,6 +93,10 @@ gdb_test "continue" \
     ".*Breakpoint.*~Derived.*" \
     "run to breakpoint 3 v2"
 
+if {$using_clang} {
+    gdb_test "continue" ".*Breakpoint.*~Derived.*" "clang's recursive destructor call"
+}
+
 gdb_test "continue" \
     ".*exited normally.*" \
     "run to exit"
-- 
2.37.3

[PATCH 4/9] gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang

[Bruno Larsen]

When running gdb.cp/derivation.exp using clang, we get an unexpected
failure when printing the type of a class with an internal typedef. This
happens because clang doesn't add accessibility information for typedefs
inside classes (see https://github.com/llvm/llvm-project/issues/57608
for more info). To help with clang testing, an XFAIL was added to this
test.
---
 gdb/testsuite/gdb.cp/derivation.exp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/derivation.exp b/gdb/testsuite/gdb.cp/derivation.exp
index c95fef004b5..2283e5b3616 100644
--- a/gdb/testsuite/gdb.cp/derivation.exp
+++ b/gdb/testsuite/gdb.cp/derivation.exp
@@ -77,6 +77,10 @@ gdb_test "up" ".*main.*" "up from marker1"
 
 gdb_test "print a_instance" "\\$\[0-9\]+ = \{a = 1, aa = 2\}" "print value of a_instance"
 
+# clang does not add accessibility information for typedefs inside classes
+if {[test_compiler_info {clang*}]} {
+    setup_xfail "clang 57608" *-*-*
+}
 cp_test_ptype_class \
     "a_instance" "" "class" "A" \
     {
-- 
2.37.3

[PATCH 04/11] gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang

[Bruno Larsen]

When running gdb.cp/derivation.exp using clang, we get an unexpected
failure when printing the type of a class with an internal typedef. This
happens because clang doesn't add accessibility information for typedefs
inside classes (see https://github.com/llvm/llvm-project/issues/57608
for more info). To help with clang testing, an XFAIL was added to this
test.
---
 gdb/testsuite/gdb.cp/derivation.exp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/derivation.exp b/gdb/testsuite/gdb.cp/derivation.exp
index c95fef004b5..2283e5b3616 100644
--- a/gdb/testsuite/gdb.cp/derivation.exp
+++ b/gdb/testsuite/gdb.cp/derivation.exp
@@ -77,6 +77,10 @@ gdb_test "up" ".*main.*" "up from marker1"
 
 gdb_test "print a_instance" "\\$\[0-9\]+ = \{a = 1, aa = 2\}" "print value of a_instance"
 
+# clang does not add accessibility information for typedefs inside classes
+if {[test_compiler_info {clang*}]} {
+    setup_xfail "clang 57608" *-*-*
+}
 cp_test_ptype_class \
     "a_instance" "" "class" "A" \
     {
-- 
2.37.3

[PATCH 05/11] gdb/testsuite: allow for clang style destructors on gdb.cp/m-static.exp

[Bruno Larsen]

when running gdb.cp/m-static.exp using clang, we get the following
failures:

    print test1.~gnu_obj_1^M
    $6 = {void (gnu_obj_1 * const)} 0x555555555470 <gnu_obj_1::~gnu_obj_1()>^M
    (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, print destructor
    ptype test1.~gnu_obj_1^M
    type = void (gnu_obj_1 * const)^M
    (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, ptype destructor
    print test1.'~gnu_obj_1'^M
    $7 = {void (gnu_obj_1 * const)} 0x555555555470 <gnu_obj_1::~gnu_obj_1()>^M
    (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, print quoted destructor

This is because the test is expecting an extra integer parameter on the
destructor. Looking at the debuginfo, it seems that there is nothing
actually wrong with this output, so these tests were changed to test
multiple possible regexps.
---
 gdb/testsuite/gdb.cp/m-static.exp | 36 ++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/m-static.exp b/gdb/testsuite/gdb.cp/m-static.exp
index 979de23a44f..332e1cbe1f9 100644
--- a/gdb/testsuite/gdb.cp/m-static.exp
+++ b/gdb/testsuite/gdb.cp/m-static.exp
@@ -102,16 +102,32 @@ if { [is_aarch32_target] } {
 	{type = void \(single_constructor \* const\)} \
 	"simple object class, ptype constructor"
 
-    gdb_test "print test1.~gnu_obj_1" \
-	{ = {void \(gnu_obj_1 \* const, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} \
-	"simple object instance, print destructor"
-    gdb_test "ptype test1.~gnu_obj_1" \
-	{type = void \(gnu_obj_1 \* const, int\)} \
-	"simple object instance, ptype destructor"
-
-    gdb_test "print test1.'~gnu_obj_1'" \
-	{ = {void \(gnu_obj_1 \*( const)?, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} \
-	"simple object instance, print quoted destructor"
+    gdb_test_multiple "print test1.~gnu_obj_1" "simple object instance, print destructor" {
+	-re -wrap { = {void \(gnu_obj_1 \* const, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
+	    pass $gdb_test_name
+	}
+	-re -wrap { = {void \(gnu_obj_1 \* const\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
+	    pass $gdb_test_name
+	}
+    }
+	
+    gdb_test_multiple "ptype test1.~gnu_obj_1" "simple object instance, ptype destructor" {
+	-re -wrap {type = void \(gnu_obj_1 \* const, int\)} {
+	    pass $gdb_test_name
+	}
+	-re -wrap {type = void \(gnu_obj_1 \* const\)} {
+	    pass $gdb_test_name
+	}
+    }
+
+    gdb_test_multiple "print test1.'~gnu_obj_1'" "simple object instance, print quoted destructor" {
+	-re -wrap { = {void \(gnu_obj_1 \*( const)?, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
+	    pass $gdb_test_name
+	}
+	-re -wrap { = {void \(gnu_obj_1 \*( const)?\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
+	    pass $gdb_test_name
+	}
+    }
 
     gdb_test "ptype gnu_obj_1::'~gnu_obj_1'" \
 	{type = void \(gnu_obj_1 \* const\)} \
-- 
2.37.3

[PATCH 06/11] gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang

[Bruno Larsen]

When running gdb.cp/ptype-flags.exp using clang, we get an unexpected
failure when printing the type of a class with an internal typedef. This
happens because clang doesn't add accessibility information for typedefs
inside classes (see https://github.com/llvm/llvm-project/issues/57608
for more info). To help with clang testing, an XFAIL was added to this
test.
---
 gdb/testsuite/gdb.cp/ptype-flags.exp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
index f88c83e03ce..e8d3f285803 100644
--- a/gdb/testsuite/gdb.cp/ptype-flags.exp
+++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
@@ -30,11 +30,18 @@ if ![runto_main] then {
     return
 }
 
+if {[test_compiler_info {clang-*-*}]} {
+    set using_clang true
+} else {
+    set using_clang false
+}
+
 gdb_test_no_output "set language c++" ""
 gdb_test_no_output "set width 0" ""
 
 proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
 		      {raw 0}} {
+    global using_clang
 
     set contents {
 	{ base "public Base<T>" }
@@ -49,6 +56,11 @@ proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
     }
 
     if {$show_typedefs} {
+	# Clang does not add accessibility information for typedefs:
+	# https://github.com/llvm/llvm-project/issues/57608
+	if {$using_clang} {
+	    setup_xfail "clang 57608" *-*-*
+	}
 	lappend contents { typedef public "typedef Simple<Simple<T> > Z;" }
     }
 
-- 
2.37.3

[PATCH 07/11] gdb/testsuite: skip gdb.cp/anon-struct.exp when using clang

[Bruno Larsen]

When clang compiles anonymous structures, it does not add linkage names in
their dwarf representations. This is compounded by clang not adding linkage
names to subprograms of those anonymous structs (for instance, the
constructor).

Since this isn't a bug on clang or GDB, but there is no way to make
anon-struct.exp work when using clang, just mark that test as untested.

Since I was already touching the file, I also added a comment at the top
of the file explaining what it is testing for.
---
 gdb/testsuite/gdb.cp/anon-struct.exp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
index 2c709ab9ecc..774ec882a07 100644
--- a/gdb/testsuite/gdb.cp/anon-struct.exp
+++ b/gdb/testsuite/gdb.cp/anon-struct.exp
@@ -14,12 +14,20 @@
 # 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 test is used to verify GDB's abiility to refer to linkage names
+# for types and functions.
+
 standard_testfile .cc
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug c++}] } {
      return -1
 }
 
+if {[test_compiler_info clang-*-*]} {
+    untested "clang does not use linkage name in this case"
+    return
+}
+
 if { [is_aarch32_target] } {
     gdb_test "ptype t::t" "type = struct t {\r\n    C m;\r\n} \\*\\(t \\* const\\)" \
 	"print type of t::t"
-- 
2.37.3

[PATCH 08/11] gdb/testsuite: disable gdb.cp/typeid.exp when using clang

[Bruno Larsen]

Since clang chooses to not add any debug information from standard
libraries, gdb.cp/typeid.exp will always fail, so this test is skipped
when it clang used.
---
 gdb/testsuite/gdb.cp/typeid.exp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/typeid.exp b/gdb/testsuite/gdb.cp/typeid.exp
index 6411217191a..d110b3e7695 100644
--- a/gdb/testsuite/gdb.cp/typeid.exp
+++ b/gdb/testsuite/gdb.cp/typeid.exp
@@ -23,6 +23,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
     return -1
 }
 
+if {[test_compiler_info clang-*-*]} {
+    untested "clang does not add debuginfo for standard libraries"
+    return
+}
+
 proc do_typeid_tests {started} {
     global hex
 
-- 
2.37.3

[PATCH 09/11] gdb/testsuite: fix gdb.cp/converts.exp to run with clang

[Bruno Larsen]

Clang attempts to minimize the size of the debug-info by not adding
complete information about types that aren't constructed in a given
file.  In specific, this meant that there was minimal information about
class B in the test gdb.cp/converts.exp.  To fix this, we just need to
construct any object of type B in that file.
---
 gdb/testsuite/gdb.cp/converts.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
index ecf806481fd..1224581a879 100644
--- a/gdb/testsuite/gdb.cp/converts.cc
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -55,7 +55,7 @@ int main()
 
   char av = 'a';
   char *a = &av;       // pointer to..
-  B *bp;
+  B *bp = new B();
   foo1_1 (a);          // ..pointer
   foo1_2 (a);          // ..array
   foo1_3 ((int*)a);    // ..pointer of wrong type
-- 
2.37.3

[PATCH 10/11] gdb/testsuite: remove XFAIL on gdb.cp/temargs.exp

[Bruno Larsen]

gdb.cp/temargs.exp last 2 tests always setup an XFAILs, despite checking
for old gcc versions.  However, clang does not fail in this test,
turning into XPASSes and slighty annoying when comparing between
compilers.  To change this, make the xfails only happen if we are not
using clang.
---
 gdb/testsuite/gdb.cp/temargs.exp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/temargs.exp b/gdb/testsuite/gdb.cp/temargs.exp
index 05fd902bb83..135cf316b90 100644
--- a/gdb/testsuite/gdb.cp/temargs.exp
+++ b/gdb/testsuite/gdb.cp/temargs.exp
@@ -46,6 +46,8 @@ if {[test_compiler_info {gcc-[0-3]-*}]
     set have_pr_45024_fixed 0
 }
 
+set using_clang [test_compiler_info clang-*-*]
+
 set line [gdb_get_line_number "Breakpoint 1" $srcfile]
 gdb_test "break $srcfile:$line" "Breakpoint 2.*" \
     "set first breakpoint for temargs"
@@ -190,10 +192,10 @@ gdb_test "print F" "&S::somefunc" "test value of F in k2_m"
 gdb_continue_to_breakpoint "continue to sixth breakpoint for temargs"
 
 if $have_older_template_gcc { setup_xfail "*-*-*"
-} else { setup_xfail gcc/49546 "*-*-*" }
+} elseif !$using_clang { setup_xfail gcc/49546 "*-*-*" }
 gdb_test "ptype F" {type = void \(\*\)\(int\)} "test type of F in k3_m"
 
 if $have_older_template_gcc { setup_xfail "*-*-*"
-} else { setup_xfail gcc/49546 "*-*-*" }
+} elseif !$using_clang { setup_xfail gcc/49546 "*-*-*" }
 gdb_test "print F" { = \(void \(\*\)\(int\)\) 0x[0-9a-f]+ <S3::m\(int\)>} \
 	 "test value of F in k3_m"
-- 
2.37.3

[PATCH 11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang

[Bruno Larsen]

The test gdb.cp/call-method-register.exp assumes that the class will be
placed on a register. However, this keyword has been deprecated since
C++11, and clang does not feel the need to follow it. Since this test is
not usable without this working, this commit marks this test as
untested.
---
 gdb/testsuite/gdb.cp/call-method-register.exp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
index a1e6498d66c..71d1f61f59f 100644
--- a/gdb/testsuite/gdb.cp/call-method-register.exp
+++ b/gdb/testsuite/gdb.cp/call-method-register.exp
@@ -26,6 +26,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
     return -1
 }
 
+if {[test_compiler_info clang-*-*]} {
+    untested "clang does not place the class in the register"
+    return
+}
+
 proc test_call_register_class {} {
     global gdb_prompt
 
-- 
2.37.3

Re: [PATCH 4/9] gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang

[Bruno Larsen]

Please disregard this specific patch. I messed up with git-format-patch 
locally, sorry for the noise

Cheers,
Bruno

On 04/10/2022 19:07, Bruno Larsen wrote:
> When running gdb.cp/derivation.exp using clang, we get an unexpected
> failure when printing the type of a class with an internal typedef. This
> happens because clang doesn't add accessibility information for typedefs
> inside classes (see https://github.com/llvm/llvm-project/issues/57608
> for more info). To help with clang testing, an XFAIL was added to this
> test.
> ---
>   gdb/testsuite/gdb.cp/derivation.exp | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/derivation.exp b/gdb/testsuite/gdb.cp/derivation.exp
> index c95fef004b5..2283e5b3616 100644
> --- a/gdb/testsuite/gdb.cp/derivation.exp
> +++ b/gdb/testsuite/gdb.cp/derivation.exp
> @@ -77,6 +77,10 @@ gdb_test "up" ".*main.*" "up from marker1"
>   
>   gdb_test "print a_instance" "\\$\[0-9\]+ = \{a = 1, aa = 2\}" "print value of a_instance"
>   
> +# clang does not add accessibility information for typedefs inside classes
> +if {[test_compiler_info {clang*}]} {
> +    setup_xfail "clang 57608" *-*-*
> +}
>   cp_test_ptype_class \
>       "a_instance" "" "class" "A" \
>       {

[PING][PATCH 00/11] Cleanup gdb.cp tests when running with clang

[Bruno Larsen]

ping!

Cheers,
Bruno

On 04/10/2022 19:07, Bruno Larsen wrote:
> Following my previous series, where I tried to reduce the amount of
> failures when running gdb.base tests using clang, I moved to reducing
> errors in gdb.cp.
>
> Actual GDB issues are being addressed in separate patches, and this
> series is confined to just changing tests to either work or
> XFAIL/disable them, to make it simpler to keep track of issues.
>
> Bruno Larsen (11):
>    gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp
>    gdb/testsuite: enable running gdb.cp/classes.exp with clang
>    gdb/testsuite: account for clang's recursive destructor calls on
>      gdb.cp/mb-ctor.exp
>    gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang
>    gdb/testsuite: allow for clang style destructors on
>      gdb.cp/m-static.exp
>    gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang
>    gdb/testsuite: skip gdb.cp/anon-struct.exp when using clang
>    gdb/testsuite: disable gdb.cp/typeid.exp when using clang
>    gdb/testsuite: fix gdb.cp/converts.exp to run with clang
>    gdb/testsuite: remove XFAIL on gdb.cp/temargs.exp
>    gdb/testsuite: disable gdb.cp/call-method-register.exp with clang
>
>   gdb/testsuite/gdb.cp/anon-struct.exp          |  8 +++++
>   gdb/testsuite/gdb.cp/call-method-register.exp |  5 +++
>   gdb/testsuite/gdb.cp/class2.exp               |  2 +-
>   gdb/testsuite/gdb.cp/classes.exp              | 33 ++++++++++++++++-
>   gdb/testsuite/gdb.cp/converts.cc              |  2 +-
>   gdb/testsuite/gdb.cp/derivation.exp           |  4 +++
>   gdb/testsuite/gdb.cp/m-static.exp             | 36 +++++++++++++------
>   gdb/testsuite/gdb.cp/mb-ctor.exp              | 16 +++++++++
>   gdb/testsuite/gdb.cp/ptype-flags.exp          | 12 +++++++
>   gdb/testsuite/gdb.cp/temargs.exp              |  6 ++--
>   gdb/testsuite/gdb.cp/typeid.exp               |  5 +++
>   11 files changed, 114 insertions(+), 15 deletions(-)
>

Re: [PATCH 01/11] gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp

[Andrew Burgess]

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

> When attempting to test gdb.cp/class2.exp using clang, it fails to
> prepare with the following error:
>
> Executing on host: clang++    -fdiagnostics-color=never -Wno-unknown-warning-option  -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc    (timeout = 300)
> builtin_spawn -ignore SIGHUP clang++ -fdiagnostics-color=never -Wno-unknown-warning-option -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc
> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
> typedef class {
>              ^
>               Dbase
> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>  public:
>  ^~~~~~~
> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
> } Dbase;
>   ^
> 1 warning generated.
> gdb compile failed, /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
> typedef class {
>              ^
>               Dbase
> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>  public:
>  ^~~~~~~
> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
> } Dbase;
>   ^
> 1 warning generated.
> UNTESTED: gdb.cp/class2.exp: failed to prepare
>
> This can be silenced by adding -Wno-non-c-typedef-for-linkage. The test
> shows no failures with this change.
> ---
>  gdb/testsuite/gdb.cp/class2.exp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.cp/class2.exp b/gdb/testsuite/gdb.cp/class2.exp
> index 32f9dfc18a5..e1e4ad0fae6 100644
> --- a/gdb/testsuite/gdb.cp/class2.exp
> +++ b/gdb/testsuite/gdb.cp/class2.exp
> @@ -18,7 +18,7 @@ if { [skip_cplus_tests] } { return }
>  
>  standard_testfile .cc
>  
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {
>      return -1

I worry about just adding an extra flag like this.  GCC is happy enough
as it ignores unknown arguments, but some other compiler might be
unhappy.

Additionally, with earlier versions of clang++ (I tested 9.0.1) the new
flag isn't recognised, and actually causes the build to fail.

I wonder if we should look to something like gdb.cp/pr9167.exp for how
to solve this?  In that script we do this:

  set flags [list debug c++]
  if { [test_compiler_info gcc-*] && [gcc_major_version] >= 10 } {
      # Work around PR gcc/101452.
      lappend flags additional_flags=-fno-eliminate-unused-debug-types
  }

this might be a better model for how to add the flag.

Thanks,
Andrew

Re: [PATCH 02/11] gdb/testsuite: enable running gdb.cp/classes.exp with clang

[Andrew Burgess]

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

> When attempting to run the gdb.cp/classes.exp test using clang++, the
> test fails to prepare with -Wnon-c-typedef-for-linkage like the
> previously fixed gdb.cp/class2.exp. Upon fixing this, the test shows 5
> unexpected failures. One such failures is:
>
> ptype/r class class_with_public_typedef
> type = class class_with_public_typedef {
>   private:
>     int a;
>   public:
>     class_with_public_typedef::INT b;
>
>   private:
>     typedef int INT;
> }
> (gdb) FAIL: gdb.cp/classes.exp: ptype class class_with_public_typedef // wrong access specifier for typedef: private
>
> While g++ provided the following output:
>
> ptype/r class class_with_public_typedef
> type = class class_with_public_typedef {
>   private:
>     int a;
>   public:
>     class_with_public_typedef::INT b;
>
>     typedef int INT;
> }
> (gdb) PASS: gdb.cp/classes.exp: ptype class class_with_public_typedef
>
> This error happens because clang does not add DW_AT_accessibility to
> typedefs inside classes, and without this information GDB defaults to
> assuming the typedef is private.  Since there is nothing that GDB can do
> about this, these tests have been set as xfails, and a clang bug has
> been filed.

I think it would be useful to include the bug number and URL in the
commit message.  I know you included this info in the comments below,
which I think is also a good thing, but having it here too would be
useful.

> ---
>  gdb/testsuite/gdb.cp/classes.exp | 33 +++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
> index 7b8b315ac1f..3f3e587803c 100644
> --- a/gdb/testsuite/gdb.cp/classes.exp
> +++ b/gdb/testsuite/gdb.cp/classes.exp
> @@ -24,14 +24,20 @@ load_lib "cp-support.exp"
>  
>  standard_testfile .cc
>  
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {

My comments for the previous patch apply here too.

>      return -1
>  }
>  
> +
>  # Test ptype of class objects.

Unexpected extra blank line added here.

>  
>  proc test_ptype_class_objects {} {
>  
> +    if {[test_compiler_info {clang*}]} {

You should pass the correct language through to test_compiler_info, like
this:

  if {[test_compiler_info {clang*} c++]} {

otherwise you are checking the C compiler by default.  When I tested
this patch I only changed the C++ compiler to clang, I left the C
compiler as GCC, and the tests showed as FAIL.

Thanks,
Andrew
> +	set clang_used true
> +    } else {
> +	set clang_used false
> +    }
>      # Simple type.
>  
>      cp_test_ptype_class \
> @@ -319,6 +325,12 @@ proc test_ptype_class_objects {} {
>  
>      # Classes with typedefs of different access.
>  
> +    # Clang does not add access information for typedefs in classes.
> +    # More information on: https://github.com/llvm/llvm-project/issues/57608
> +    if {$clang_used} {
> +	setup_xfail "clang 57608" *-*-*
> +    }
> +
>      cp_test_ptype_class \
>  	"class class_with_typedefs" "" "class" "class_with_typedefs" \
>  	{
> @@ -339,6 +351,10 @@ proc test_ptype_class_objects {} {
>  	    { typedef private "typedef int private_int;" }
>  	}
>  
> +    if {$clang_used} {
> +	setup_xfail "clang 57608" *-*-*
> +    }
> +
>      cp_test_ptype_class \
>  	"class class_with_public_typedef" "" "class" \
>  	"class_with_public_typedef" {
> @@ -346,6 +362,11 @@ proc test_ptype_class_objects {} {
>  	    { field public "class_with_public_typedef::INT b;" }
>  	    { typedef public "typedef int INT;" }
>  	}
> +
> +    if {$clang_used} {
> +	setup_xfail "clang 57608" *-*-*
> +    }
> +
>      cp_test_ptype_class \
>  	"class class_with_protected_typedef" "" "class" \
>  	"class_with_protected_typedef" {
> @@ -353,6 +374,11 @@ proc test_ptype_class_objects {} {
>  	    { field protected "class_with_protected_typedef::INT b;" }
>  	    { typedef protected "typedef int INT;" }
>  	}
> +
> +    if {$clang_used} {
> +	setup_xfail "clang 57608" *-*-*
> +    }
> +
>      cp_test_ptype_class \
>  	"struct struct_with_protected_typedef" "" "struct" \
>  	"struct_with_protected_typedef" {
> @@ -360,6 +386,11 @@ proc test_ptype_class_objects {} {
>  	    { field protected "struct_with_protected_typedef::INT b;" }
>  	    { typedef protected "typedef int INT;" }
>  	}
> +
> +    if {$clang_used} {
> +	setup_xfail "clang 57608" *-*-*
> +    }
> +
>      cp_test_ptype_class \
>  	"struct struct_with_private_typedef" "" "struct" \
>  	"struct_with_private_typedef" {
> -- 
> 2.37.3

Re: [PATCH 03/11] gdb/testsuite: account for clang's recursive destructor calls on gdb.cp/mb-ctor.exp

[Andrew Burgess]

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

> When compiling virtual classes's destructors, two versions are compiled,
> one with a single parameter (this) and the other with 2 parameters (this
> and vtt).
>
> GCC's compilation makes it so either the version with 1
> parameter or the one with 2 parameters is called, depending on whether
> the destructor is being called by the class itself or by an inherited
> class. On the test gdb.cp/mb-ctor.exp, this means that the breakpoint
> set at the destructor will be hit 4 times.
>
> Clang, on the other hand, makes the single-parameter version call the 2
> parameter version, probably in an attempt to reduce the size of the
> resulting executable. This means that the gdb.cp/mb-ctor.exp will hit 6
> breakpoints before finishing, and is the reason why this test was
> failing. To make this test stop failing, a compiler check is added and
> another "continue" instruction is issued to account for this difference.
> ---
>  gdb/testsuite/gdb.cp/mb-ctor.exp | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/mb-ctor.exp b/gdb/testsuite/gdb.cp/mb-ctor.exp
> index 5a3c0a4a90f..6aadecdd27d 100644
> --- a/gdb/testsuite/gdb.cp/mb-ctor.exp
> +++ b/gdb/testsuite/gdb.cp/mb-ctor.exp
> @@ -25,6 +25,12 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>      return -1
>  }
>  
> +if {[test_compiler_info {clang-*-*}]} {

Include c++ language for test_compiler_info call (see previous patch for
details).

> +    set using_clang true
> +} else {
> +    set using_clang false
> +}
> +
>  if ![runto_main] then {
>      perror "couldn't run to breakpoint"
>      return
> @@ -71,6 +77,12 @@ gdb_continue_to_breakpoint "set breakpoint here, second dynamic time"\
>  
>  gdb_test "continue" ".*Breakpoint.*~Derived.*" "Run to dynamic destructor v1"
>  
> +# Clang makes Derived::~Derived(this) call Derived::~Derives(this, vtt)
> +# whereas gcc puts all the logic necessary on both functions.
> +if {$using_clang} {
> +    gdb_test "continue" ".*Breakpoint.*~Derived.*" "clang's recursive dynamic destructor call"

There lines are a little long, and should be wrapped.

Also, I'm not sure I agree with the use of 'recursive' here.  Maybe
'nested' would be better?  When I think recursive, I think of functions
that directly, or indirectly, call themselves.  Though the two
destructors do have the same name, I don't think they are the same
function.

> +}
> +
>  gdb_test "continue" ".*Breakpoint.*~Derived.*" "Run to dynamic destructor v2"
>  
>  gdb_test "continue" \
> @@ -81,6 +93,10 @@ gdb_test "continue" \
>      ".*Breakpoint.*~Derived.*" \
>      "run to breakpoint 3 v2"
>  
> +if {$using_clang} {
> +    gdb_test "continue" ".*Breakpoint.*~Derived.*" "clang's recursive destructor call"
> +}

Same again.

Thanks,
Andrew

> +
>  gdb_test "continue" \
>      ".*exited normally.*" \
>      "run to exit"
> -- 
> 2.37.3

Re: [PATCH 04/11] gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang

[Andrew Burgess]

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

> When running gdb.cp/derivation.exp using clang, we get an unexpected
> failure when printing the type of a class with an internal typedef. This
> happens because clang doesn't add accessibility information for typedefs
> inside classes (see https://github.com/llvm/llvm-project/issues/57608
> for more info). To help with clang testing, an XFAIL was added to this
> test.
> ---
>  gdb/testsuite/gdb.cp/derivation.exp | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/derivation.exp b/gdb/testsuite/gdb.cp/derivation.exp
> index c95fef004b5..2283e5b3616 100644
> --- a/gdb/testsuite/gdb.cp/derivation.exp
> +++ b/gdb/testsuite/gdb.cp/derivation.exp
> @@ -77,6 +77,10 @@ gdb_test "up" ".*main.*" "up from marker1"
>  
>  gdb_test "print a_instance" "\\$\[0-9\]+ = \{a = 1, aa = 2\}" "print value of a_instance"
>  
> +# clang does not add accessibility information for typedefs inside classes

Missing full-stop at the end of the sentence.

> +if {[test_compiler_info {clang*}]} {
> +    setup_xfail "clang 57608" *-*-*
> +}

Add language to test_compiler_info call.

Thanks,
Andrew

>  cp_test_ptype_class \
>      "a_instance" "" "class" "A" \
>      {
> -- 
> 2.37.3

Re: [PATCH 05/11] gdb/testsuite: allow for clang style destructors on gdb.cp/m-static.exp

[Andrew Burgess]

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

> when running gdb.cp/m-static.exp using clang, we get the following
> failures:
>
>     print test1.~gnu_obj_1^M
>     $6 = {void (gnu_obj_1 * const)} 0x555555555470 <gnu_obj_1::~gnu_obj_1()>^M
>     (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, print destructor
>     ptype test1.~gnu_obj_1^M
>     type = void (gnu_obj_1 * const)^M
>     (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, ptype destructor
>     print test1.'~gnu_obj_1'^M
>     $7 = {void (gnu_obj_1 * const)} 0x555555555470 <gnu_obj_1::~gnu_obj_1()>^M
>     (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, print quoted destructor
>
> This is because the test is expecting an extra integer parameter on the
> destructor. Looking at the debuginfo, it seems that there is nothing
> actually wrong with this output, so these tests were changed to test
> multiple possible regexps.
> ---
>  gdb/testsuite/gdb.cp/m-static.exp | 36 ++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.cp/m-static.exp b/gdb/testsuite/gdb.cp/m-static.exp
> index 979de23a44f..332e1cbe1f9 100644
> --- a/gdb/testsuite/gdb.cp/m-static.exp
> +++ b/gdb/testsuite/gdb.cp/m-static.exp
> @@ -102,16 +102,32 @@ if { [is_aarch32_target] } {
>  	{type = void \(single_constructor \* const\)} \
>  	"simple object class, ptype constructor"
>  
> -    gdb_test "print test1.~gnu_obj_1" \
> -	{ = {void \(gnu_obj_1 \* const, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} \
> -	"simple object instance, print destructor"

Rather than switching to gdb_test_multiple, it might be better to just
update the regexp, like:

  { = {void \(gnu_obj_1 \* const(?:, int)?\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>}

We already use something like this (though without the '?:' modifier) in
one of the tests below, so I think there's precedent.

Thanks,
Andrew


> -    gdb_test "ptype test1.~gnu_obj_1" \
> -	{type = void \(gnu_obj_1 \* const, int\)} \
> -	"simple object instance, ptype destructor"
> -
> -    gdb_test "print test1.'~gnu_obj_1'" \
> -	{ = {void \(gnu_obj_1 \*( const)?, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} \
> -	"simple object instance, print quoted destructor"
> +    gdb_test_multiple "print test1.~gnu_obj_1" "simple object instance, print destructor" {
> +	-re -wrap { = {void \(gnu_obj_1 \* const, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap { = {void \(gnu_obj_1 \* const\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +    }
> +	
> +    gdb_test_multiple "ptype test1.~gnu_obj_1" "simple object instance, ptype destructor" {
> +	-re -wrap {type = void \(gnu_obj_1 \* const, int\)} {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap {type = void \(gnu_obj_1 \* const\)} {
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    gdb_test_multiple "print test1.'~gnu_obj_1'" "simple object instance, print quoted destructor" {
> +	-re -wrap { = {void \(gnu_obj_1 \*( const)?, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap { = {void \(gnu_obj_1 \*( const)?\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +    }
>  
>      gdb_test "ptype gnu_obj_1::'~gnu_obj_1'" \
>  	{type = void \(gnu_obj_1 \* const\)} \
> -- 
> 2.37.3

Re: [PATCH 06/11] gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang

[Andrew Burgess]

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

> When running gdb.cp/ptype-flags.exp using clang, we get an unexpected
> failure when printing the type of a class with an internal typedef. This
> happens because clang doesn't add accessibility information for typedefs
> inside classes (see https://github.com/llvm/llvm-project/issues/57608
> for more info). To help with clang testing, an XFAIL was added to this
> test.
> ---
>  gdb/testsuite/gdb.cp/ptype-flags.exp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
> index f88c83e03ce..e8d3f285803 100644
> --- a/gdb/testsuite/gdb.cp/ptype-flags.exp
> +++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
> @@ -30,11 +30,18 @@ if ![runto_main] then {
>      return
>  }
>  
> +if {[test_compiler_info {clang-*-*}]} {
> +    set using_clang true
> +} else {
> +    set using_clang false
> +}

Missing the language for the test_compiler_info call.  Looks good with
that fixed.

Thanks,
Andrew

> +
>  gdb_test_no_output "set language c++" ""
>  gdb_test_no_output "set width 0" ""
>  
>  proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>  		      {raw 0}} {
> +    global using_clang
>  
>      set contents {
>  	{ base "public Base<T>" }
> @@ -49,6 +56,11 @@ proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>      }
>  
>      if {$show_typedefs} {
> +	# Clang does not add accessibility information for typedefs:
> +	# https://github.com/llvm/llvm-project/issues/57608
> +	if {$using_clang} {
> +	    setup_xfail "clang 57608" *-*-*
> +	}
>  	lappend contents { typedef public "typedef Simple<Simple<T> > Z;" }
>      }
>  
> -- 
> 2.37.3

Re: [PATCH 07/11] gdb/testsuite: skip gdb.cp/anon-struct.exp when using clang

[Andrew Burgess]

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

> When clang compiles anonymous structures, it does not add linkage names in
> their dwarf representations. This is compounded by clang not adding linkage
> names to subprograms of those anonymous structs (for instance, the
> constructor).
>
> Since this isn't a bug on clang or GDB, but there is no way to make
> anon-struct.exp work when using clang, just mark that test as untested.
>
> Since I was already touching the file, I also added a comment at the top
> of the file explaining what it is testing for.
> ---
>  gdb/testsuite/gdb.cp/anon-struct.exp | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
> index 2c709ab9ecc..774ec882a07 100644
> --- a/gdb/testsuite/gdb.cp/anon-struct.exp
> +++ b/gdb/testsuite/gdb.cp/anon-struct.exp
> @@ -14,12 +14,20 @@
>  # 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 test is used to verify GDB's abiility to refer to linkage names
> +# for types and functions.

Type: 'ability'.

Also, I wonder if this comment should explicitly say something like:

      "...for types and functions within anonymous structs."

maybe?

> +
>  standard_testfile .cc
>  
>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug c++}] } {
>       return -1
>  }
>  
> +if {[test_compiler_info clang-*-*]} {

Missing the language argument to test_compiler_info.

> +    untested "clang does not use linkage name in this case"
> +    return

You specifically call out the missing linkage name here, but both the
DW_AT_name and DW_AT_linkage name are filled in by GCC, and not with
Clang.  Is it the linkage name in particular that is critical here?

Either way, I wonder if a more extended comment within this if block
might be useful, just touching on the differences in behaviour between
GCC and Clang, and why than means this test will not work with Clang.

Thanks,
Andrew


> +}
> +
>  if { [is_aarch32_target] } {
>      gdb_test "ptype t::t" "type = struct t {\r\n    C m;\r\n} \\*\\(t \\* const\\)" \
>  	"print type of t::t"
> -- 
> 2.37.3

Re: [PATCH 04/11] gdb/testsuite: add XFAIL to gdb.cp/derivation.exp when using clang

[Andrew Burgess]

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

> When running gdb.cp/derivation.exp using clang, we get an unexpected
> failure when printing the type of a class with an internal typedef. This
> happens because clang doesn't add accessibility information for typedefs
> inside classes (see https://github.com/llvm/llvm-project/issues/57608
> for more info). To help with clang testing, an XFAIL was added to this
> test.
> ---
>  gdb/testsuite/gdb.cp/derivation.exp | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/derivation.exp b/gdb/testsuite/gdb.cp/derivation.exp
> index c95fef004b5..2283e5b3616 100644
> --- a/gdb/testsuite/gdb.cp/derivation.exp
> +++ b/gdb/testsuite/gdb.cp/derivation.exp
> @@ -77,6 +77,10 @@ gdb_test "up" ".*main.*" "up from marker1"
>  
>  gdb_test "print a_instance" "\\$\[0-9\]+ = \{a = 1, aa = 2\}" "print value of a_instance"
>  
> +# clang does not add accessibility information for typedefs inside classes

I believe it's 'Clang' with a capital 'C'.

Thanks,
Andrew

> +if {[test_compiler_info {clang*}]} {
> +    setup_xfail "clang 57608" *-*-*
> +}
>  cp_test_ptype_class \
>      "a_instance" "" "class" "A" \
>      {
> -- 
> 2.37.3

Re: [PATCH 05/11] gdb/testsuite: allow for clang style destructors on gdb.cp/m-static.exp

[Andrew Burgess]

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

> when running gdb.cp/m-static.exp using clang, we get the following

s/clang/Clang/.

Thanks,
Andrew

> failures:
>
>     print test1.~gnu_obj_1^M
>     $6 = {void (gnu_obj_1 * const)} 0x555555555470 <gnu_obj_1::~gnu_obj_1()>^M
>     (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, print destructor
>     ptype test1.~gnu_obj_1^M
>     type = void (gnu_obj_1 * const)^M
>     (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, ptype destructor
>     print test1.'~gnu_obj_1'^M
>     $7 = {void (gnu_obj_1 * const)} 0x555555555470 <gnu_obj_1::~gnu_obj_1()>^M
>     (gdb) FAIL: gdb.cp/m-static.exp: simple object instance, print quoted destructor
>
> This is because the test is expecting an extra integer parameter on the
> destructor. Looking at the debuginfo, it seems that there is nothing
> actually wrong with this output, so these tests were changed to test
> multiple possible regexps.
> ---
>  gdb/testsuite/gdb.cp/m-static.exp | 36 ++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.cp/m-static.exp b/gdb/testsuite/gdb.cp/m-static.exp
> index 979de23a44f..332e1cbe1f9 100644
> --- a/gdb/testsuite/gdb.cp/m-static.exp
> +++ b/gdb/testsuite/gdb.cp/m-static.exp
> @@ -102,16 +102,32 @@ if { [is_aarch32_target] } {
>  	{type = void \(single_constructor \* const\)} \
>  	"simple object class, ptype constructor"
>  
> -    gdb_test "print test1.~gnu_obj_1" \
> -	{ = {void \(gnu_obj_1 \* const, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} \
> -	"simple object instance, print destructor"
> -    gdb_test "ptype test1.~gnu_obj_1" \
> -	{type = void \(gnu_obj_1 \* const, int\)} \
> -	"simple object instance, ptype destructor"
> -
> -    gdb_test "print test1.'~gnu_obj_1'" \
> -	{ = {void \(gnu_obj_1 \*( const)?, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} \
> -	"simple object instance, print quoted destructor"
> +    gdb_test_multiple "print test1.~gnu_obj_1" "simple object instance, print destructor" {
> +	-re -wrap { = {void \(gnu_obj_1 \* const, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap { = {void \(gnu_obj_1 \* const\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +    }
> +	
> +    gdb_test_multiple "ptype test1.~gnu_obj_1" "simple object instance, ptype destructor" {
> +	-re -wrap {type = void \(gnu_obj_1 \* const, int\)} {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap {type = void \(gnu_obj_1 \* const\)} {
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    gdb_test_multiple "print test1.'~gnu_obj_1'" "simple object instance, print quoted destructor" {
> +	-re -wrap { = {void \(gnu_obj_1 \*( const)?, int\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +	-re -wrap { = {void \(gnu_obj_1 \*( const)?\)} 0x[0-9a-f]+ <gnu_obj_1::~gnu_obj_1\(\)>} {
> +	    pass $gdb_test_name
> +	}
> +    }
>  
>      gdb_test "ptype gnu_obj_1::'~gnu_obj_1'" \
>  	{type = void \(gnu_obj_1 \* const\)} \
> -- 
> 2.37.3

Re: [PATCH 08/11] gdb/testsuite: disable gdb.cp/typeid.exp when using clang

[Andrew Burgess]

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

> Since clang chooses to not add any debug information from standard
> libraries, gdb.cp/typeid.exp will always fail, so this test is skipped
> when it clang used.

OK, so I see what you're saying.  But I don't think it follows that we
should skip the whole test.  Also, without digging into the test
further, and looking at the generated symbols, I think the above
explanation makes things sound worse than they are.

So, here's some of the output for 'nm -C' for the test binary (built
with Clang):

  0000000000003b70 V typeinfo for VB2
  0000000000003b38 V typeinfo for Base
  0000000000003b98 V typeinfo for Derived
  0000000000003b28 V typeinfo for char [5]
                   U typeinfo for int@@CXXABI_1.3
  0000000000003bd0 V typeinfo for Base*
                   U typeinfo for char*@@CXXABI_1.3
                   U typeinfo for char const*@@CXXABI_1.3
  0000000000002012 V typeinfo name for VB1
  000000000000201d V typeinfo name for VB2
  0000000000002017 V typeinfo name for Base
  0000000000002009 V typeinfo name for Derived
  0000000000002004 V typeinfo name for char [5]
  0000000000002022 V typeinfo name for Base*

So, you're correct, Clang hasn't included the typeinfo for the base
types.  Except, it sort of has included it, it's just not compiled into
the binary.

So, I think, where the test does this:

  foreach simple_var {i cp ccp ca b} {

We should actually do something like:

  set var {ca b}
  if {$started || ![test_compiler_info clang-*-* c++]} {
      # Clang doesn't place type information for the base types in
      # the executable, and relies on this being linked in from the
      # standard library.  As a result, type information for these
      # variables is only available once the inferior is started.
      lappend var i cp ccp
  }
  foreach simple_var $var {

What do you think?

Thanks,
Andrew


> ---
>  gdb/testsuite/gdb.cp/typeid.exp | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/typeid.exp b/gdb/testsuite/gdb.cp/typeid.exp
> index 6411217191a..d110b3e7695 100644
> --- a/gdb/testsuite/gdb.cp/typeid.exp
> +++ b/gdb/testsuite/gdb.cp/typeid.exp
> @@ -23,6 +23,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>      return -1
>  }
>  
> +if {[test_compiler_info clang-*-*]} {
> +    untested "clang does not add debuginfo for standard libraries"
> +    return
> +}
> +
>  proc do_typeid_tests {started} {
>      global hex
>  
> -- 
> 2.37.3

Re: [PATCH 09/11] gdb/testsuite: fix gdb.cp/converts.exp to run with clang

[Andrew Burgess]

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

> Clang attempts to minimize the size of the debug-info by not adding
> complete information about types that aren't constructed in a given
> file.  In specific, this meant that there was minimal information about

s/In specific/Specifically/.

> class B in the test gdb.cp/converts.exp.  To fix this, we just need to
> construct any object of type B in that file.
> ---
>  gdb/testsuite/gdb.cp/converts.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
> index ecf806481fd..1224581a879 100644
> --- a/gdb/testsuite/gdb.cp/converts.cc
> +++ b/gdb/testsuite/gdb.cp/converts.cc
> @@ -55,7 +55,7 @@ int main()
>  
>    char av = 'a';
>    char *a = &av;       // pointer to..
> -  B *bp;
> +  B *bp = new B();

LGTM.

Thanks,
Andrew

>    foo1_1 (a);          // ..pointer
>    foo1_2 (a);          // ..array
>    foo1_3 ((int*)a);    // ..pointer of wrong type
> -- 
> 2.37.3

Re: [PATCH 10/11] gdb/testsuite: remove XFAIL on gdb.cp/temargs.exp

[Andrew Burgess]

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

> gdb.cp/temargs.exp last 2 tests always setup an XFAILs, despite checking
> for old gcc versions.  However, clang does not fail in this test,
> turning into XPASSes and slighty annoying when comparing between
> compilers.  To change this, make the xfails only happen if we are not
> using clang.
> ---
>  gdb/testsuite/gdb.cp/temargs.exp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.cp/temargs.exp b/gdb/testsuite/gdb.cp/temargs.exp
> index 05fd902bb83..135cf316b90 100644
> --- a/gdb/testsuite/gdb.cp/temargs.exp
> +++ b/gdb/testsuite/gdb.cp/temargs.exp
> @@ -46,6 +46,8 @@ if {[test_compiler_info {gcc-[0-3]-*}]
>      set have_pr_45024_fixed 0
>  }
>  
> +set using_clang [test_compiler_info clang-*-*]
> +
>  set line [gdb_get_line_number "Breakpoint 1" $srcfile]
>  gdb_test "break $srcfile:$line" "Breakpoint 2.*" \
>      "set first breakpoint for temargs"
> @@ -190,10 +192,10 @@ gdb_test "print F" "&S::somefunc" "test value of F in k2_m"
>  gdb_continue_to_breakpoint "continue to sixth breakpoint for temargs"
>  
>  if $have_older_template_gcc { setup_xfail "*-*-*"
> -} else { setup_xfail gcc/49546 "*-*-*" }
> +} elseif !$using_clang { setup_xfail gcc/49546 "*-*-*" }

Surely, if these XFAIL are for GCC, then the right thing here is to
check _for_ GCC, instead of checking !Clang?  I'd assume other non-GCC
compilers will also XPASS this test.

Thanks,
Andrew

>  gdb_test "ptype F" {type = void \(\*\)\(int\)} "test type of F in k3_m"
>  
>  if $have_older_template_gcc { setup_xfail "*-*-*"
> -} else { setup_xfail gcc/49546 "*-*-*" }
> +} elseif !$using_clang { setup_xfail gcc/49546 "*-*-*" }
>  gdb_test "print F" { = \(void \(\*\)\(int\)\) 0x[0-9a-f]+ <S3::m\(int\)>} \
>  	 "test value of F in k3_m"
> -- 
> 2.37.3

Re: [PATCH 01/11] gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp

[Bruno Larsen]

On 26/10/2022 14:02, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> When attempting to test gdb.cp/class2.exp using clang, it fails to
>> prepare with the following error:
>>
>> Executing on host: clang++    -fdiagnostics-color=never -Wno-unknown-warning-option  -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc    (timeout = 300)
>> builtin_spawn -ignore SIGHUP clang++ -fdiagnostics-color=never -Wno-unknown-warning-option -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc
>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
>> typedef class {
>>               ^
>>                Dbase
>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>>   public:
>>   ^~~~~~~
>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
>> } Dbase;
>>    ^
>> 1 warning generated.
>> gdb compile failed, /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
>> typedef class {
>>               ^
>>                Dbase
>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>>   public:
>>   ^~~~~~~
>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
>> } Dbase;
>>    ^
>> 1 warning generated.
>> UNTESTED: gdb.cp/class2.exp: failed to prepare
>>
>> This can be silenced by adding -Wno-non-c-typedef-for-linkage. The test
>> shows no failures with this change.
>> ---
>>   gdb/testsuite/gdb.cp/class2.exp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.cp/class2.exp b/gdb/testsuite/gdb.cp/class2.exp
>> index 32f9dfc18a5..e1e4ad0fae6 100644
>> --- a/gdb/testsuite/gdb.cp/class2.exp
>> +++ b/gdb/testsuite/gdb.cp/class2.exp
>> @@ -18,7 +18,7 @@ if { [skip_cplus_tests] } { return }
>>   
>>   standard_testfile .cc
>>   
>> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {
>>       return -1
> I worry about just adding an extra flag like this.  GCC is happy enough
> as it ignores unknown arguments, but some other compiler might be
> unhappy.
>
> Additionally, with earlier versions of clang++ (I tested 9.0.1) the new
> flag isn't recognised, and actually causes the build to fail.
Good call. This flag seems to have been added by clang-11, so I'll check 
for that version.
>
> I wonder if we should look to something like gdb.cp/pr9167.exp for how
> to solve this?  In that script we do this:
>
>    set flags [list debug c++]
>    if { [test_compiler_info gcc-*] && [gcc_major_version] >= 10 } {
>        # Work around PR gcc/101452.
>        lappend flags additional_flags=-fno-eliminate-unused-debug-types
>    }
>
> this might be a better model for how to add the flag.

Looking at gcc_major_version, this looks too verbose. gcc_major_version 
already checks for the given compiler, returning -1 if the compiler is 
not the expected one, so that code from gdb.cp/pr9167.exp could be 
shortened to:

if  { [ gcc_major_version ] >= 10 } {
     ....
}

What do you think?

(I also think the function should be renamed, but that is a patch for 
another day).

Cheers,
Bruno

>
> Thanks,
> Andrew
>

Re: [PATCH 11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang

[Andrew Burgess]

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

> The test gdb.cp/call-method-register.exp assumes that the class will be
> placed on a register. However, this keyword has been deprecated since
> C++11, and clang does not feel the need to follow it. Since this test is
> not usable without this working, this commit marks this test as
> untested.

As I understand it, the combination of register and asm, as used in the
test source is a GCC extension, and so...

> ---
>  gdb/testsuite/gdb.cp/call-method-register.exp | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
> index a1e6498d66c..71d1f61f59f 100644
> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
> @@ -26,6 +26,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>      return -1
>  }
>  
> +if {[test_compiler_info clang-*-*]} {
> +    untested "clang does not place the class in the register"
> +    return
> +}

... I think this would be better written as:

  if {![test_compiler_info gcc-*-* c++]} {
    untested "test relies on a gcc extension"
    return
  }

However, I also noticed that this test is only currently supported for
x86-64, i386, and ppc, as the test source itself needs a particular
register to be selected for each architecture.

I wondered if we could do better using the DWARF assembler.  Below is my
proposal that would replace this patch.  What do you think?

Thanks,
Andrew

---

commit 613e0a042c3220f183e02c9c3cf76c68b4d7e453
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Oct 27 10:15:09 2022 +0100

    gdb/testsuite: port gdb.cp/call-method-register.exp to DWARF assembler
    
    The test gdb.cp/call-method-register.exp relies on a GCC extension,
    that is combining the register keyword with the asm keyword to place a
    variable into a known register.
    
    This test already suffers from requiring each architecture to pick a
    register to use, currently the test only supports x86-64, i386, and
    ppc64.  Plus we know that the test will only ever work with GCC.
    
    In this commit I add a guard to the call-method-register.exp script so
    if the C++ compiler is not g++ then the test will be skipped.
    
    However, the main change in this commit is that I have added a
    complete new test gdb.dwarf2/dw2-call-method-register.exp, this is a
    copy of the original test but rewritten to use the DWARF assembler.
    I've tested the new test on x86-64, aarch64, and ppc64le.
    
    I did consider removing the original test, however, I thought there
    might be some value in retaining it, just so we can have some
    validation that GDB can correctly handle GCC's register extension, the
    test is pretty short, so doesn't take much time to run.

diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
index a1e6498d66c..2662d6b0891 100644
--- a/gdb/testsuite/gdb.cp/call-method-register.exp
+++ b/gdb/testsuite/gdb.cp/call-method-register.exp
@@ -18,6 +18,19 @@
 
 if { [skip_cplus_tests] } { continue }
 
+# This test relies on a GCC extension to place a structure into a
+# named register.  As a result this test will only work with GCC.  But
+# also, only a few architectures have a register named.  Any
+# architecture not explicitly supported in the source file will fail
+# to compile.
+#
+# However, the test gdb.dwarf2/dw2-call-method-register.exp is a
+# reimplementation of this test using the DWARF assembler, and should
+# work for any architecture, with any compiler (that supports the
+# DWARF assembler).  This test is retained mostly so we can track that
+# nothing weird happens w.r.t. how GDB handles GCC's register extension.
+if { ![test_compiler_info {gcc-*-*} c++] } { continue }
+
 load_lib "cp-support.exp"
 
 standard_testfile .cc
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
new file mode 100644
index 00000000000..d5328156c55
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
@@ -0,0 +1,24 @@
+/* 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/>.  */
+
+int
+main ()
+{
+  asm ("main_label: .globl main_label");
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
new file mode 100644
index 00000000000..3b761698076
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
@@ -0,0 +1,127 @@
+# 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/>.
+
+# Test callling a method on a variable that has been put in a register.
+#
+# We use the DWARF assembler to generate DWARF that says the global variable
+# is located in a register.  We don't care which register we use as the
+# value in the register is not important, we only care that the DWARF says
+# the variable is in a register.  In fact, there is no variable in the
+# source program at all!
+
+load_lib dwarf.exp
+
+# Check the DWARF assembler is supported.
+if {![dwarf2_support]} { return }
+
+standard_testfile .c -dw.S
+
+# Compile and start the .c file so we can figure out pointer sizes.
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+
+    # Get information about function main.
+    set main_result \
+	[function_range main ${::srcdir}/${::subdir}/${::srcfile}]
+    set main_start [lindex $main_result 0]
+    set main_length [lindex $main_result 1]
+
+    cu {} {
+	compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {DW_AT_name $::srcfile}
+	    {DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels int_type_label struct_type_label \
+		struct_ptr_type_label
+            set ptr_size [get_sizeof "void *" 96]
+
+	    DW_TAG_subprogram {
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc $main_length data8}
+		{DW_AT_type :$int_type_label}
+	    }
+
+	    int_type_label: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name int}
+	    }
+
+	    struct_type_label: DW_TAG_structure_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_name small}
+	    } {
+		member {
+		    {name xxx}
+		    {type :$int_type_label}
+		    {data_member_location 0 data1}
+		}
+		subprogram {
+		    {name yyy}
+		    {linkage_name _ZN5small3yyyEv}
+		    {external 1 flag_present}
+		    {type :$int_type_label}
+		    {data_member_location 0 data1}
+		} {
+		    formal_parameter {
+			{type :$struct_ptr_type_label}
+			{artificial 1 flag_present}
+		    }
+		}
+	    }
+
+	    struct_ptr_type_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $ptr_size DW_FORM_data1}
+		{type :$struct_type_label}
+	    }
+
+	    # This is where we place the variable into a register.  Just
+	    # use DWARF register 0, whatever that might be.  See the
+	    # comments at the start of the file for why we don't care
+	    # about the choice of register.
+            DW_TAG_variable {
+                {DW_AT_name global_var}
+                {DW_AT_type :$struct_type_label}
+                {DW_AT_location {
+                    DW_OP_reg0
+                } SPECIAL_expr}
+                {external 1 flag}
+            }
+	}
+    }
+}
+
+# Rebuild the test program, this time include the generated debug
+# information.
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Try to call a method on a variable of structure type, however, the
+# variable is located in a register.
+gdb_test "print global_var.yyy ()" \
+    "Address requested for identifier \"global_var\" which is in register .*" \
+    "call method on register local"

Re: [PATCH 06/11] gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang

[Bruno Larsen]

On 26/10/2022 16:08, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> When running gdb.cp/ptype-flags.exp using clang, we get an unexpected
>> failure when printing the type of a class with an internal typedef. This
>> happens because clang doesn't add accessibility information for typedefs
>> inside classes (see https://github.com/llvm/llvm-project/issues/57608
>> for more info). To help with clang testing, an XFAIL was added to this
>> test.
>> ---
>>   gdb/testsuite/gdb.cp/ptype-flags.exp | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
>> index f88c83e03ce..e8d3f285803 100644
>> --- a/gdb/testsuite/gdb.cp/ptype-flags.exp
>> +++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
>> @@ -30,11 +30,18 @@ if ![runto_main] then {
>>       return
>>   }
>>   
>> +if {[test_compiler_info {clang-*-*}]} {
>> +    set using_clang true
>> +} else {
>> +    set using_clang false
>> +}
> Missing the language for the test_compiler_info call.  Looks good with
> that fixed.
Great! Can I add an Approved-by tag to the patch?

Cheers,
Bruno

> Thanks,
> Andrew
>
>> +
>>   gdb_test_no_output "set language c++" ""
>>   gdb_test_no_output "set width 0" ""
>>   
>>   proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>>   		      {raw 0}} {
>> +    global using_clang
>>   
>>       set contents {
>>   	{ base "public Base<T>" }
>> @@ -49,6 +56,11 @@ proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>>       }
>>   
>>       if {$show_typedefs} {
>> +	# Clang does not add accessibility information for typedefs:
>> +	# https://github.com/llvm/llvm-project/issues/57608
>> +	if {$using_clang} {
>> +	    setup_xfail "clang 57608" *-*-*
>> +	}
>>   	lappend contents { typedef public "typedef Simple<Simple<T> > Z;" }
>>       }
>>   
>> -- 
>> 2.37.3

Re: [PATCH 07/11] gdb/testsuite: skip gdb.cp/anon-struct.exp when using clang

[Bruno Larsen]

On 26/10/2022 16:49, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> When clang compiles anonymous structures, it does not add linkage names in
>> their dwarf representations. This is compounded by clang not adding linkage
>> names to subprograms of those anonymous structs (for instance, the
>> constructor).
>>
>> Since this isn't a bug on clang or GDB, but there is no way to make
>> anon-struct.exp work when using clang, just mark that test as untested.
>>
>> Since I was already touching the file, I also added a comment at the top
>> of the file explaining what it is testing for.
>> ---
>>   gdb/testsuite/gdb.cp/anon-struct.exp | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
>> index 2c709ab9ecc..774ec882a07 100644
>> --- a/gdb/testsuite/gdb.cp/anon-struct.exp
>> +++ b/gdb/testsuite/gdb.cp/anon-struct.exp
>> @@ -14,12 +14,20 @@
>>   # 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 test is used to verify GDB's abiility to refer to linkage names
>> +# for types and functions.
> Type: 'ability'.
>
> Also, I wonder if this comment should explicitly say something like:
>
>        "...for types and functions within anonymous structs."
>
> maybe?
Yeah, good call.
>
>> +
>>   standard_testfile .cc
>>   
>>   if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug c++}] } {
>>        return -1
>>   }
>>   
>> +if {[test_compiler_info clang-*-*]} {
> Missing the language argument to test_compiler_info.
>
>> +    untested "clang does not use linkage name in this case"
>> +    return
> You specifically call out the missing linkage name here, but both the
> DW_AT_name and DW_AT_linkage name are filled in by GCC, and not with
> Clang.  Is it the linkage name in particular that is critical here?

Well, I guess I need to workshop my explanation.

The problem is that GCC generates a linkage name (t in this case), and 
uses this name for the constructor, so we can use t::t() to refer to the 
anonymous structure's constructor. Clang doesn't use the linkage name to 
generate the name of the constructor (or give it any name at all) so it 
ends with the constructor t::(), which isn't a valid expression for GDB.

I decided not to fix this because I'm not convinced calling the 
constructor of an anonymous structure makes much sense, but I'm open to 
being convinced.

Cheers,
Bruno

>
> Either way, I wonder if a more extended comment within this if block
> might be useful, just touching on the differences in behaviour between
> GCC and Clang, and why than means this test will not work with Clang.
>
> Thanks,
> Andrew
>
>
>> +}
>> +
>>   if { [is_aarch32_target] } {
>>       gdb_test "ptype t::t" "type = struct t {\r\n    C m;\r\n} \\*\\(t \\* const\\)" \
>>   	"print type of t::t"
>> -- 
>> 2.37.3

Re: [PATCH 01/11] gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp

[Andrew Burgess]

Bruno Larsen <blarsen@redhat.com> writes:

> On 26/10/2022 14:02, Andrew Burgess wrote:
>> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> When attempting to test gdb.cp/class2.exp using clang, it fails to
>>> prepare with the following error:
>>>
>>> Executing on host: clang++    -fdiagnostics-color=never -Wno-unknown-warning-option  -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc    (timeout = 300)
>>> builtin_spawn -ignore SIGHUP clang++ -fdiagnostics-color=never -Wno-unknown-warning-option -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
>>> typedef class {
>>>               ^
>>>                Dbase
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>>>   public:
>>>   ^~~~~~~
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
>>> } Dbase;
>>>    ^
>>> 1 warning generated.
>>> gdb compile failed, /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
>>> typedef class {
>>>               ^
>>>                Dbase
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>>>   public:
>>>   ^~~~~~~
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
>>> } Dbase;
>>>    ^
>>> 1 warning generated.
>>> UNTESTED: gdb.cp/class2.exp: failed to prepare
>>>
>>> This can be silenced by adding -Wno-non-c-typedef-for-linkage. The test
>>> shows no failures with this change.
>>> ---
>>>   gdb/testsuite/gdb.cp/class2.exp | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.cp/class2.exp b/gdb/testsuite/gdb.cp/class2.exp
>>> index 32f9dfc18a5..e1e4ad0fae6 100644
>>> --- a/gdb/testsuite/gdb.cp/class2.exp
>>> +++ b/gdb/testsuite/gdb.cp/class2.exp
>>> @@ -18,7 +18,7 @@ if { [skip_cplus_tests] } { return }
>>>   
>>>   standard_testfile .cc
>>>   
>>> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {
>>>       return -1
>> I worry about just adding an extra flag like this.  GCC is happy enough
>> as it ignores unknown arguments, but some other compiler might be
>> unhappy.
>>
>> Additionally, with earlier versions of clang++ (I tested 9.0.1) the new
>> flag isn't recognised, and actually causes the build to fail.
> Good call. This flag seems to have been added by clang-11, so I'll check 
> for that version.
>>
>> I wonder if we should look to something like gdb.cp/pr9167.exp for how
>> to solve this?  In that script we do this:
>>
>>    set flags [list debug c++]
>>    if { [test_compiler_info gcc-*] && [gcc_major_version] >= 10 } {
>>        # Work around PR gcc/101452.
>>        lappend flags additional_flags=-fno-eliminate-unused-debug-types
>>    }
>>
>> this might be a better model for how to add the flag.
>
> Looking at gcc_major_version, this looks too verbose. gcc_major_version 
> already checks for the given compiler, returning -1 if the compiler is 
> not the expected one, so that code from gdb.cp/pr9167.exp could be 
> shortened to:
>
> if  { [ gcc_major_version ] >= 10 } {
>      ....
> }

I guess you actually need:

 [ gcc_major_version "gcc-*" c++ ]

in order to check the C++ compiler.

Thanks,
Andrew

>
> What do you think?
>
> (I also think the function should be renamed, but that is a patch for 
> another day).
>
> Cheers,
> Bruno
>
>>
>> Thanks,
>> Andrew
>>

Re: [PATCH 06/11] gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang

[Andrew Burgess]

Bruno Larsen <blarsen@redhat.com> writes:

> On 26/10/2022 16:08, Andrew Burgess wrote:
>> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> When running gdb.cp/ptype-flags.exp using clang, we get an unexpected
>>> failure when printing the type of a class with an internal typedef. This
>>> happens because clang doesn't add accessibility information for typedefs
>>> inside classes (see https://github.com/llvm/llvm-project/issues/57608
>>> for more info). To help with clang testing, an XFAIL was added to this
>>> test.
>>> ---
>>>   gdb/testsuite/gdb.cp/ptype-flags.exp | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
>>> index f88c83e03ce..e8d3f285803 100644
>>> --- a/gdb/testsuite/gdb.cp/ptype-flags.exp
>>> +++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
>>> @@ -30,11 +30,18 @@ if ![runto_main] then {
>>>       return
>>>   }
>>>   
>>> +if {[test_compiler_info {clang-*-*}]} {
>>> +    set using_clang true
>>> +} else {
>>> +    set using_clang false
>>> +}
>> Missing the language for the test_compiler_info call.  Looks good with
>> that fixed.
> Great! Can I add an Approved-by tag to the patch?

Sure, with that fix made.

Thanks,
Andrew


>
> Cheers,
> Bruno
>
>> Thanks,
>> Andrew
>>
>>> +
>>>   gdb_test_no_output "set language c++" ""
>>>   gdb_test_no_output "set width 0" ""
>>>   
>>>   proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>>>   		      {raw 0}} {
>>> +    global using_clang
>>>   
>>>       set contents {
>>>   	{ base "public Base<T>" }
>>> @@ -49,6 +56,11 @@ proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>>>       }
>>>   
>>>       if {$show_typedefs} {
>>> +	# Clang does not add accessibility information for typedefs:
>>> +	# https://github.com/llvm/llvm-project/issues/57608
>>> +	if {$using_clang} {
>>> +	    setup_xfail "clang 57608" *-*-*
>>> +	}
>>>   	lappend contents { typedef public "typedef Simple<Simple<T> > Z;" }
>>>       }
>>>   
>>> -- 
>>> 2.37.3

Re: [PATCH 11/11] gdb/testsuite: disable gdb.cp/call-method-register.exp with clang

[Bruno Larsen]

On 27/10/2022 11:49, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> The test gdb.cp/call-method-register.exp assumes that the class will be
>> placed on a register. However, this keyword has been deprecated since
>> C++11, and clang does not feel the need to follow it. Since this test is
>> not usable without this working, this commit marks this test as
>> untested.
> As I understand it, the combination of register and asm, as used in the
> test source is a GCC extension, and so...
>
>> ---
>>   gdb/testsuite/gdb.cp/call-method-register.exp | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
>> index a1e6498d66c..71d1f61f59f 100644
>> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
>> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
>> @@ -26,6 +26,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>>       return -1
>>   }
>>   
>> +if {[test_compiler_info clang-*-*]} {
>> +    untested "clang does not place the class in the register"
>> +    return
>> +}
> ... I think this would be better written as:
>
>    if {![test_compiler_info gcc-*-* c++]} {
>      untested "test relies on a gcc extension"
>      return
>    }
>
> However, I also noticed that this test is only currently supported for
> x86-64, i386, and ppc, as the test source itself needs a particular
> register to be selected for each architecture.
>
> I wondered if we could do better using the DWARF assembler.  Below is my
> proposal that would replace this patch.  What do you think?
I did think about this, but it felt like it wasn't worth it, especially 
given that we can't use the dwarf assembler with clang. That said, I 
just tried your patch and it works just fine, so it is a pretty good 
improvement. I only have a very minor inlined.
>
> Thanks,
> Andrew
>
> ---
>
> commit 613e0a042c3220f183e02c9c3cf76c68b4d7e453
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Thu Oct 27 10:15:09 2022 +0100
>
>      gdb/testsuite: port gdb.cp/call-method-register.exp to DWARF assembler
>      
>      The test gdb.cp/call-method-register.exp relies on a GCC extension,
>      that is combining the register keyword with the asm keyword to place a
>      variable into a known register.
>      
>      This test already suffers from requiring each architecture to pick a
>      register to use, currently the test only supports x86-64, i386, and
>      ppc64.  Plus we know that the test will only ever work with GCC.
>      
>      In this commit I add a guard to the call-method-register.exp script so
>      if the C++ compiler is not g++ then the test will be skipped.
>      
>      However, the main change in this commit is that I have added a
>      complete new test gdb.dwarf2/dw2-call-method-register.exp, this is a
>      copy of the original test but rewritten to use the DWARF assembler.
>      I've tested the new test on x86-64, aarch64, and ppc64le.
>      
>      I did consider removing the original test, however, I thought there
>      might be some value in retaining it, just so we can have some
>      validation that GDB can correctly handle GCC's register extension, the
>      test is pretty short, so doesn't take much time to run.
>
> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
> index a1e6498d66c..2662d6b0891 100644
> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
> @@ -18,6 +18,19 @@
>   
>   if { [skip_cplus_tests] } { continue }
>   
> +# This test relies on a GCC extension to place a structure into a
> +# named register.  As a result this test will only work with GCC.  But
> +# also, only a few architectures have a register named.  Any
> +# architecture not explicitly supported in the source file will fail
> +# to compile.
> +#
> +# However, the test gdb.dwarf2/dw2-call-method-register.exp is a
> +# reimplementation of this test using the DWARF assembler, and should
> +# work for any architecture, with any compiler (that supports the
> +# DWARF assembler).  This test is retained mostly so we can track that
> +# nothing weird happens w.r.t. how GDB handles GCC's register extension.
> +if { ![test_compiler_info {gcc-*-*} c++] } { continue }
> +
>   load_lib "cp-support.exp"
>   
>   standard_testfile .cc
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
> new file mode 100644
> index 00000000000..d5328156c55
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.c
> @@ -0,0 +1,24 @@
> +/* 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/>.  */
> +
> +int
> +main ()
> +{
> +  asm ("main_label: .globl main_label");
> +  return 0;
> +}
> +
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
> new file mode 100644
> index 00000000000..3b761698076
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-call-method-register.exp
> @@ -0,0 +1,127 @@
> +# 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/>.
> +
> +# Test callling a method on a variable that has been put in a register.
> +#
> +# We use the DWARF assembler to generate DWARF that says the global variable
> +# is located in a register.  We don't care which register we use as the
> +# value in the register is not important, we only care that the DWARF says
> +# the variable is in a register.  In fact, there is no variable in the
> +# source program at all!
> +
> +load_lib dwarf.exp
> +
> +# Check the DWARF assembler is supported.
> +if {![dwarf2_support]} { return }
> +
> +standard_testfile .c -dw.S
> +
> +# Compile and start the .c file so we can figure out pointer sizes.
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +
> +    # Get information about function main.
> +    set main_result \
> +	[function_range main ${::srcdir}/${::subdir}/${::srcfile}]
> +    set main_start [lindex $main_result 0]
> +    set main_length [lindex $main_result 1]
> +
> +    cu {} {
> +	compile_unit {
> +	    {DW_AT_language @DW_LANG_C_plus_plus}
> +	    {DW_AT_name $::srcfile}
> +	    {DW_AT_comp_dir /tmp}
> +	} {
> +	    declare_labels int_type_label struct_type_label \
> +		struct_ptr_type_label
> +            set ptr_size [get_sizeof "void *" 96]
> +
> +	    DW_TAG_subprogram {
> +		{name main}
> +		{low_pc $main_start addr}
> +		{high_pc $main_length data8}
> +		{DW_AT_type :$int_type_label}
> +	    }
> +
> +	    int_type_label: DW_TAG_base_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_encoding @DW_ATE_signed}
> +		{DW_AT_name int}
> +	    }
> +
> +	    struct_type_label: DW_TAG_structure_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_name small}
> +	    } {
> +		member {
> +		    {name xxx}
> +		    {type :$int_type_label}
> +		    {data_member_location 0 data1}
> +		}
> +		subprogram {
> +		    {name yyy}
> +		    {linkage_name _ZN5small3yyyEv}
Why did you decide to give the method a linkage name? It doesn't seem 
related to the error message and I think we should only leave important 
things in the tests.

Cheers,
Bruno

> +		    {external 1 flag_present}
> +		    {type :$int_type_label}
> +		    {data_member_location 0 data1}
> +		} {
> +		    formal_parameter {
> +			{type :$struct_ptr_type_label}
> +			{artificial 1 flag_present}
> +		    }
> +		}
> +	    }
> +
> +	    struct_ptr_type_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $ptr_size DW_FORM_data1}
> +		{type :$struct_type_label}
> +	    }
> +
> +	    # This is where we place the variable into a register.  Just
> +	    # use DWARF register 0, whatever that might be.  See the
> +	    # comments at the start of the file for why we don't care
> +	    # about the choice of register.
> +            DW_TAG_variable {
> +                {DW_AT_name global_var}
> +                {DW_AT_type :$struct_type_label}
> +                {DW_AT_location {
> +                    DW_OP_reg0
> +                } SPECIAL_expr}
> +                {external 1 flag}
> +            }
> +	}
> +    }
> +}
> +
> +# Rebuild the test program, this time include the generated debug
> +# information.
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# Try to call a method on a variable of structure type, however, the
> +# variable is located in a register.
> +gdb_test "print global_var.yyy ()" \
> +    "Address requested for identifier \"global_var\" which is in register .*" \
> +    "call method on register local"
>

Re: [PATCH 06/11] gdb/testsuite: add XFAIL to gdb.cp/ptype-flags.exp when using clang

[Bruno Larsen]

On 28/10/2022 13:38, Andrew Burgess wrote:
> Bruno Larsen <blarsen@redhat.com> writes:
>
>> On 26/10/2022 16:08, Andrew Burgess wrote:
>>> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>
>>>> When running gdb.cp/ptype-flags.exp using clang, we get an unexpected
>>>> failure when printing the type of a class with an internal typedef. This
>>>> happens because clang doesn't add accessibility information for typedefs
>>>> inside classes (see https://github.com/llvm/llvm-project/issues/57608
>>>> for more info). To help with clang testing, an XFAIL was added to this
>>>> test.
>>>> ---
>>>>    gdb/testsuite/gdb.cp/ptype-flags.exp | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
>>>> index f88c83e03ce..e8d3f285803 100644
>>>> --- a/gdb/testsuite/gdb.cp/ptype-flags.exp
>>>> +++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
>>>> @@ -30,11 +30,18 @@ if ![runto_main] then {
>>>>        return
>>>>    }
>>>>    
>>>> +if {[test_compiler_info {clang-*-*}]} {
>>>> +    set using_clang true
>>>> +} else {
>>>> +    set using_clang false
>>>> +}
>>> Missing the language for the test_compiler_info call.  Looks good with
>>> that fixed.
>> Great! Can I add an Approved-by tag to the patch?
> Sure, with that fix made.
Thanks! I pushed this patch with the change

Cheers,
Bruno

>
> Thanks,
> Andrew
>
>
>> Cheers,
>> Bruno
>>
>>> Thanks,
>>> Andrew
>>>
>>>> +
>>>>    gdb_test_no_output "set language c++" ""
>>>>    gdb_test_no_output "set width 0" ""
>>>>    
>>>>    proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>>>>    		      {raw 0}} {
>>>> +    global using_clang
>>>>    
>>>>        set contents {
>>>>    	{ base "public Base<T>" }
>>>> @@ -49,6 +56,11 @@ proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
>>>>        }
>>>>    
>>>>        if {$show_typedefs} {
>>>> +	# Clang does not add accessibility information for typedefs:
>>>> +	# https://github.com/llvm/llvm-project/issues/57608
>>>> +	if {$using_clang} {
>>>> +	    setup_xfail "clang 57608" *-*-*
>>>> +	}
>>>>    	lappend contents { typedef public "typedef Simple<Simple<T> > Z;" }
>>>>        }
>>>>    
>>>> -- 
>>>> 2.37.3

Re: [PATCH 09/11] gdb/testsuite: fix gdb.cp/converts.exp to run with clang

[Bruno Larsen]

On 26/10/2022 17:54, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> Clang attempts to minimize the size of the debug-info by not adding
>> complete information about types that aren't constructed in a given
>> file.  In specific, this meant that there was minimal information about
> s/In specific/Specifically/.
>
>> class B in the test gdb.cp/converts.exp.  To fix this, we just need to
>> construct any object of type B in that file.
>> ---
>>   gdb/testsuite/gdb.cp/converts.cc | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
>> index ecf806481fd..1224581a879 100644
>> --- a/gdb/testsuite/gdb.cp/converts.cc
>> +++ b/gdb/testsuite/gdb.cp/converts.cc
>> @@ -55,7 +55,7 @@ int main()
>>   
>>     char av = 'a';
>>     char *a = &av;       // pointer to..
>> -  B *bp;
>> +  B *bp = new B();
> LGTM.
Thanks! I pushed with the change to the commit message.

Cheers,
Bruno

>
> Thanks,
> Andrew
>
>>     foo1_1 (a);          // ..pointer
>>     foo1_2 (a);          // ..array
>>     foo1_3 ((int*)a);    // ..pointer of wrong type
>> -- 
>> 2.37.3