[PATCH v2 0/3] clang inspired testsuite fixups.

[PATCH v2 0/3] clang inspired testsuite fixups.

[Guinevere Larsen]

I decided to check out the 95 XPASSes in the clang buildbot and this is
the series that resulted.  The first 2 patches turn 86 XPASSes into
regular passes.

The third patch adds a generic way to request that a test be compiled
with DWARF5 debug information.

Changes for v2:
* rebased on master

Guinevere Larsen (3):
  gdb/testsuite: fix XPASSes when testing with clang
  gdb/testsuite: ask for DWARF5 in gdb.cp/pass-by-ref.exp
  gdb/testsuite: introduce dwarf5 option to gdb_compile

 .../amd64-entry-value-param-dwarf5.exp        |  2 +-
 gdb/testsuite/gdb.cp/classes.exp              | 16 +++---
 gdb/testsuite/gdb.cp/pass-by-ref.exp          |  6 ++-
 gdb/testsuite/gdb.cp/ptype-flags.exp          | 11 ++--
 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp |  3 +-
 .../gdb.dwarf2/gdb-index-types-dwarf5.exp     |  2 +-
 gdb/testsuite/gdb.fortran/assumedrank.exp     |  2 +-
 gdb/testsuite/lib/gdb.exp                     | 51 +++++++++++++++++++
 8 files changed, 74 insertions(+), 19 deletions(-)

-- 
2.47.0

[PATCH v2 1/3] gdb/testsuite: fix XPASSes when testing with clang

[Guinevere Larsen]

From: Guinevere Larsen <blarsen@redhat.com>

Clang used to not add access specifiers for typedefs, and the tests
gdb.cp/ptype-flags.exp and gdb.cp/classes.exp both set XFAILs if clang
is being used to test. This was fixed on the clang 16.0.0 release,
generating many XPASSes. This current commit checks the clang version
used for testing, and only emits XFAIL on when a bad clang version is
used.

The testsuite didn't have a good way to check if a compiler is newer
than a given release, so this commit also adds a naive function that
does that, by assuming that major numbers trump the middle, which
trump the minor number.
---
 gdb/testsuite/gdb.cp/classes.exp     | 16 ++++++-----
 gdb/testsuite/gdb.cp/ptype-flags.exp | 11 ++++---
 gdb/testsuite/lib/gdb.exp            | 43 ++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
index 4d29bf5ab5c..0d6500f7362 100644
--- a/gdb/testsuite/gdb.cp/classes.exp
+++ b/gdb/testsuite/gdb.cp/classes.exp
@@ -25,9 +25,11 @@ load_lib "cp-support.exp"
 standard_testfile .cc
 
 set flags [list debug c++]
-set clang_used false
+set using_broken_clang false
 if { [test_compiler_info "clang-*" "c++"] } {
-    set clang_used true
+    if { [compiler_version_lower_than "16-0-0" "clang" "c++"] } {
+	set using_broken_clang true
+    }
     if { [gcc_major_version "clang-*" "c++"] >= 11} {
 	lappend flags additional_flags=-Wno-non-c-typedef-for-linkage
     }
@@ -330,7 +332,7 @@ proc test_ptype_class_objects {} {
 
     # Clang does not add access information for typedefs in classes.
     # More information on: https://github.com/llvm/llvm-project/issues/57608
-    if {$::clang_used} {
+    if {$::using_broken_clang} {
 	setup_xfail "clang 57608" *-*-*
     }
 
@@ -354,7 +356,7 @@ proc test_ptype_class_objects {} {
 	    { typedef private "typedef int private_int;" }
 	}
 
-    if {$::clang_used} {
+    if {$::using_broken_clang} {
 	setup_xfail "clang 57608" *-*-*
     }
 
@@ -366,7 +368,7 @@ proc test_ptype_class_objects {} {
 	    { typedef public "typedef int INT;" }
 	}
 
-    if {$::clang_used} {
+    if {$::using_broken_clang} {
 	setup_xfail "clang 57608" *-*-*
     }
 
@@ -378,7 +380,7 @@ proc test_ptype_class_objects {} {
 	    { typedef protected "typedef int INT;" }
 	}
 
-    if {$::clang_used} {
+    if {$::using_broken_clang} {
 	setup_xfail "clang 57608" *-*-*
     }
 
@@ -390,7 +392,7 @@ proc test_ptype_class_objects {} {
 	    { typedef protected "typedef int INT;" }
 	}
 
-    if {$::clang_used} {
+    if {$::using_broken_clang} {
 	setup_xfail "clang 57608" *-*-*
     }
 
diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
index bb92da6122a..8ac8e371fb2 100644
--- a/gdb/testsuite/gdb.cp/ptype-flags.exp
+++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
@@ -29,10 +29,9 @@ if {![runto_main]} {
     return
 }
 
-if {[test_compiler_info {clang-*-*} c++]} {
-    set using_clang true
-} else {
-    set using_clang false
+set using_broken_clang false
+if {[compiler_version_lower_than "16-0-0" "clang" "c++"]} {
+    set using_broken_clang true
 }
 
 gdb_test_no_output "set language c++" ""
@@ -40,7 +39,7 @@ gdb_test_no_output "set width 0" ""
 
 proc do_check_holder {name {flags ""} {show_typedefs 1} {show_methods 1}
 		      {raw 0}} {
-    global using_clang
+    global using_broken_clang
 
     set contents {
 	{ base "public Base<T>" }
@@ -57,7 +56,7 @@ 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} {
+	if {$using_broken_clang} {
 	    setup_xfail "clang 57608" *-*-*
 	}
 	lappend contents { typedef public "typedef Simple<Simple<T> > Z;" }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f0a89394b87..a73a84dfd97 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5392,6 +5392,49 @@ proc gcc_major_version { {compiler "gcc-*"} {language "c"} } {
     return $major.$minor
 }
 
+# Calculate if the compiler version is higer than VERSION.
+# If the proc can't tell (for instance, the provided compiler is clang
+# but the testssuite is running with gcc), it returns false.
+# VERSION must be in the form "0-0-0", with all 3 numbers.
+
+proc compiler_version_lower_than { version {compiler "gcc"} \
+				      {language "c"} } {
+    global decimal
+    
+    set res [regexp $compiler-($decimal)-($decimal)-($decimal) \
+		[test_compiler_info "" $language] dummy major mid minor]
+
+    regexp ($decimal)-($decimal)-($decimal) $version dummy \
+	prov_major prov_mid prov_minor
+    
+    # We either can't determine the compiler, or the compiler is
+    # different than provided
+    if { $res != 1} {
+	return false
+    }
+
+    if {$major != $prov_major} {
+	if {$major < $prov_major} {
+	    return true
+	} else {
+	    return false
+	}
+    }
+
+    if {$mid != $prov_mid} {
+	if {$mid < $prov_mid} {
+	    return true
+	} else {
+	    return false
+	}
+    }
+
+    if {$minor < $prov_mid} {
+	return true
+    }
+    return false
+}
+
 proc current_target_name { } {
     global target_info
     if [info exists target_info(target,name)] {
-- 
2.47.0

Re: [PATCH v2 1/3] gdb/testsuite: fix XPASSes when testing with clang

[Keith Seitz]

On 10/15/24 7:38 AM, Guinevere Larsen wrote:
> From: Guinevere Larsen <blarsen@redhat.com>
> 
> Clang used to not add access specifiers for typedefs, and the tests
> gdb.cp/ptype-flags.exp and gdb.cp/classes.exp both set XFAILs if clang
> is being used to test. This was fixed on the clang 16.0.0 release,
> generating many XPASSes. This current commit checks the clang version
> used for testing, and only emits XFAIL on when a bad clang version is
> used.
> 
> The testsuite didn't have a good way to check if a compiler is newer
> than a given release, so this commit also adds a naive function that
> does that, by assuming that major numbers trump the middle, which
> trump the minor number.

Just a quick/naive question here: Does version_compare (in
lib/gdb-utils.exp) not work in this case? Could it be modified to?

[NOTE: Tom Tromey has a recent change to this function to make it a bit
more robust:
https://inbox.sourceware.org/gdb-patches/20240930164451.418588-1-tromey@adacore.com/]

Keith

Re: [PATCH v2 1/3] gdb/testsuite: fix XPASSes when testing with clang

[Guinevere Larsen]

On 10/15/24 11:50 AM, Keith Seitz wrote:
> On 10/15/24 7:38 AM, Guinevere Larsen wrote:
>> From: Guinevere Larsen <blarsen@redhat.com>
>>
>> Clang used to not add access specifiers for typedefs, and the tests
>> gdb.cp/ptype-flags.exp and gdb.cp/classes.exp both set XFAILs if clang
>> is being used to test. This was fixed on the clang 16.0.0 release,
>> generating many XPASSes. This current commit checks the clang version
>> used for testing, and only emits XFAIL on when a bad clang version is
>> used.
>>
>> The testsuite didn't have a good way to check if a compiler is newer
>> than a given release, so this commit also adds a naive function that
>> does that, by assuming that major numbers trump the middle, which
>> trump the minor number.
>
> Just a quick/naive question here: Does version_compare (in
> lib/gdb-utils.exp) not work in this case? Could it be modified to?
>
Huh. I think I based this on the gcc_major_version, and didn't even 
notice that version_compare existed.

I've updated compiler_version_lower_than to use it, and the 
documentation. Thanks for the suggestion!

> [NOTE: Tom Tromey has a recent change to this function to make it a bit
> more robust:
> https://inbox.sourceware.org/gdb-patches/20240930164451.418588-1-tromey@adacore.com/] 
>
>
> Keith
>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

[PATCH v2 2/3] gdb/testsuite: ask for DWARF5 in gdb.cp/pass-by-ref.exp

[Guinevere Larsen]

From: Guinevere Larsen <blarsen@redhat.com>

The test gdb.cp/pass-by-ref.exp relies on some DWARF attributes that
were only added in version 5. Some compilers (notably clang on fedoras
older than 40) default to using DWARF4, which is why there are many
XFAILs in the output.

However, clang is able to emit dwarf5, and will start doing so as the
default in fedora 40 (which is what the buildbot is using) causing 80
XPASSes to be emitted.

This patch identifies which debug version is being produced by the
compiler, and only sets up the XFAIL if DWARF5 is not used.
---
 gdb/testsuite/gdb.cp/pass-by-ref.exp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/pass-by-ref.exp b/gdb/testsuite/gdb.cp/pass-by-ref.exp
index a1f1df4f3e7..b38098c916c 100644
--- a/gdb/testsuite/gdb.cp/pass-by-ref.exp
+++ b/gdb/testsuite/gdb.cp/pass-by-ref.exp
@@ -343,6 +343,9 @@ if {![runto_main]} {
     return -1
 }
 
+get_debug_format
+set using_dwarf5 [test_debug_format "DWARF 5"]
+
 set bp_location [gdb_get_line_number "stop here"]
 gdb_breakpoint $bp_location
 gdb_continue_to_breakpoint "end of main" ".*return .*;"
@@ -412,7 +415,8 @@ proc test_for_class { prefix states cbvfun data_field length} {
 		    "destructor should be called"
 	    }
 	} else {
-	    if {$cctor == "deleted" && ($is_gcc_6_or_older || $is_clang)} {
+	    if {$cctor == "deleted"
+		&& ($is_gcc_6_or_older || !$::using_dwarf5)} {
 		setup_xfail "*-*-*"
 	    }
 	    gdb_test "print ${cbvfun}<$name> (${name}_var)" \
-- 
2.47.0

Re: [PATCH v2 2/3] gdb/testsuite: ask for DWARF5 in gdb.cp/pass-by-ref.exp

[Keith Seitz]

Hi, Gwen,

Thank you for your continuing efforts with clang!

On 10/15/24 7:38 AM, Guinevere Larsen wrote:
> From: Guinevere Larsen <blarsen@redhat.com>
> 
> The test gdb.cp/pass-by-ref.exp relies on some DWARF attributes that
> were only added in version 5. Some compilers (notably clang on fedoras
> older than 40) default to using DWARF4, which is why there are many
> XFAILs in the output.

I don't quite understand the assertion here, "... relies on DWARF
attributes that were only added in version 5."

pass-by-ref.exp was added to the repo a long time ago:

commit 41f1b6975dce5616800a8ff1acb544019c7bc132
Date:   Sun Sep 23 16:25:06 2007 +0000

[snip]
             * gdb.cp/pass-by-ref.cc, gdb.cp/pass-by-ref.exp: New files.

IIUC, in 2007 the only DWARF standard available was DWARF 3. [DWARF 4
is dated 2010.] Surely (paraphrase) "relies on DWARF 5" cannot
have always been true.

So while this may be true for clang, it cannot true for gcc. At least
not when this test was added.

If I test this patch with DTS 12 (gcc 12.1) on RHEL7 ('cause why not?):

$ gcc -v
Using built-in specs.
[snip]
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.1.1 20220628 (Red Hat 12.1.1-3) (GCC)
$ make check TESTS=gdb.cp/pass-by-ref.exp
[snip]
		=== gdb Summary ===

# of expected passes		833
# of unexpected successes	80

So perhaps the explanation/patch need to be limited to clang
specifically.

Keith

Re: [PATCH v2 2/3] gdb/testsuite: ask for DWARF5 in gdb.cp/pass-by-ref.exp

[Guinevere Larsen]

On 10/16/24 2:02 PM, Keith Seitz wrote:
> Hi, Gwen,
>
> Thank you for your continuing efforts with clang!
>
> On 10/15/24 7:38 AM, Guinevere Larsen wrote:
>> From: Guinevere Larsen <blarsen@redhat.com>
>>
>> The test gdb.cp/pass-by-ref.exp relies on some DWARF attributes that
>> were only added in version 5. Some compilers (notably clang on fedoras
>> older than 40) default to using DWARF4, which is why there are many
>> XFAILs in the output.
>
> I don't quite understand the assertion here, "... relies on DWARF
> attributes that were only added in version 5."
>
> pass-by-ref.exp was added to the repo a long time ago:
>
> commit 41f1b6975dce5616800a8ff1acb544019c7bc132
> Date:   Sun Sep 23 16:25:06 2007 +0000
>
> [snip]
>             * gdb.cp/pass-by-ref.cc, gdb.cp/pass-by-ref.exp: New files.
>
> IIUC, in 2007 the only DWARF standard available was DWARF 3. [DWARF 4
> is dated 2010.] Surely (paraphrase) "relies on DWARF 5" cannot
> have always been true.
>
> So while this may be true for clang, it cannot true for gcc. At least
> not when this test was added.

Checking my notes, the specific attributes I'm referencing here are 
DW_AT_deleted and DW_AT_defaulted.

According to this comment 
https://bugzilla.redhat.com/show_bug.cgi?id=2277419#c4 by Nikita Popov, 
they were only added to DWARF on version 5. In fact, the parts of the 
test I'm changing only date back to 2019, from this commit:

commit c855a9125ade61c046091373bafdae0c719118e0
Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Date:   Fri Dec 20 17:43:07 2019 +0100
         testsuite, cp: increase the coverage of testing pass-by-ref 
arguments

So some rewording is definitely in order, but those parts really do rely 
on DWARF 5 attributes. More thoughts on why clang and gcc are different 
below.

>
> If I test this patch with DTS 12 (gcc 12.1) on RHEL7 ('cause why not?):
>
> $ gcc -v
> Using built-in specs.
> [snip]
> Thread model: posix
> Supported LTO compression algorithms: zlib
> gcc version 12.1.1 20220628 (Red Hat 12.1.1-3) (GCC)
> $ make check TESTS=gdb.cp/pass-by-ref.exp
> [snip]
>         === gdb Summary ===
>
> # of expected passes        833
> # of unexpected successes    80
>
> So perhaps the explanation/patch need to be limited to clang
> specifically.

A small bit of git archaeology later, I also found this commit:

commit 5f440116e87a4613b888ab3f42c014468bd625d9
Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Date:   Wed Jan 29 09:05:20 2020 +0100

     testsuite, cp: add expected failures to pass-by-ref tests for 
certain compilers

     There exist expected failures in the pass-by-ref.exp and
     pass-by-ref-2.exp tests based on the GCC and Clang version.

     * GCC version <= 6 and Clang do not emit DW_AT_deleted and
DW_AT_defaulted.

<commit message snipped for brevity>

Looking at the release dates of gcc, version 7.1 was released on May 
2nd, 2017, 3 months after DWARF 5 was finalized.

So what I think is happening is that gcc emits those DWARF5 attributes 
regardless of which DWARF version is being used, while clang requires 
DWARF 5 to emit them. I will fix the if conditions to take this into 
account, thanks for the review!

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Keith
>

Re: [PATCH v2 2/3] gdb/testsuite: ask for DWARF5 in gdb.cp/pass-by-ref.exp

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> +get_debug_format
Guinevere> +set using_dwarf5 [test_debug_format "DWARF 5"]

It seems to me this will be a latent bug when DWARF 6 is released.

Tom

[PATCH v2 3/3] gdb/testsuite: introduce dwarf5 option to gdb_compile

[Guinevere Larsen]

From: Guinevere Larsen <blarsen@redhat.com>

A few tests on the testsuite require dwarf5 to work. Up until now, the
way to do this was to explicitly add the command line flag -gdwarf-5.
This isn't very portable, in case a compiler requires a different flag
to emit dwarf5.

This commit adds a new option to gdb_compile that would be able to add
the correct flag (if known) or error out in case we are unable to tell
which flag to use. It also changes the existing tests to use this
general option instead of hard coding -gdwarf-5.
---
 gdb/testsuite/gdb.arch/amd64-entry-value-param-dwarf5.exp | 2 +-
 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp             | 3 +--
 gdb/testsuite/gdb.dwarf2/gdb-index-types-dwarf5.exp       | 2 +-
 gdb/testsuite/gdb.fortran/assumedrank.exp                 | 2 +-
 gdb/testsuite/lib/gdb.exp                                 | 8 ++++++++
 5 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/amd64-entry-value-param-dwarf5.exp b/gdb/testsuite/gdb.arch/amd64-entry-value-param-dwarf5.exp
index 344c4abe456..5078d0c2b9a 100644
--- a/gdb/testsuite/gdb.arch/amd64-entry-value-param-dwarf5.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value-param-dwarf5.exp
@@ -19,7 +19,7 @@ set opts {}
 if [info exists COMPILE] {
     # make check RUNTESTFLAGS="gdb.arch/amd64-entry-value-param-dwarf5.exp COMPILE=1"
     standard_testfile .c .c
-    lappend opts optimize=-O2 additional_flags=-gdwarf-5
+    lappend opts optimize=-O2 dwarf5
 } else {
     require is_x86_64_m64_target
 }
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp b/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
index a6a99e86858..a7ca334869f 100644
--- a/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
@@ -24,8 +24,7 @@ standard_testfile .cc
 # we let that be the test of whether the target supports it.
 
 if { [prepare_for_testing "failed to prepare" "${testfile}" \
-          $srcfile {debug c++ additional_flags=-gdwarf-5 \
-                        additional_flags=-O0}] } {
+          $srcfile {debug c++ dwarf5 additional_flags=-O0}] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-types-dwarf5.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-types-dwarf5.exp
index 540e770a3bf..b97b55468e4 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-types-dwarf5.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-types-dwarf5.exp
@@ -17,7 +17,7 @@ standard_testfile
 
 set flags {}
 lappend flags {additional_flags=-fdebug-types-section}
-lappend flags {additional_flags=-gdwarf-5}
+lappend flags {dwarf5}
 
 if { [prepare_for_testing "failed to prepare" ${testfile} \
 	  $srcfile $flags] } {
diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp b/gdb/testsuite/gdb.fortran/assumedrank.exp
index 957dfefc7c7..32fd9d29ca7 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.exp
+++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
@@ -28,7 +28,7 @@ if { [test_compiler_info {gfortran-*} f90] &&
 }
 
 if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \
-	 {debug f90 additional_flags=-gdwarf-5}]} {
+	 {debug f90 dwarf5}]} {
     return -1
 }
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a73a84dfd97..99d3b1a787d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5676,6 +5676,7 @@ proc quote_for_host { args } {
 #   - no-build-id: Ensure the final binary does not include a build-id.
 #   - column-info/no-column-info: Enable/Disable generation of column table
 #     information.
+#   - dwarf5: Force compilation with dwarf-5 debug information.
 #
 # And here are some of the not too obscure options understood by DejaGnu that
 # influence the compilation:
@@ -5936,6 +5937,13 @@ proc gdb_compile {source dest type options} {
 		error "Option gno-column-info not supported by compiler."
 	    }
 
+	} elseif { $opt == "dwarf5" } {
+	    if {[test_compiler_info {gcc-*}] \
+		|| [test_compiler_info {clang-*}]} {
+		lappend new_options "additional_flags=-gdwarf-5"
+	    } else {
+		error "No idea how to force DWARF-5 in this compiler"
+	    }
         } else {
             lappend new_options $opt
         }
-- 
2.47.0

Re: [PATCH v2 3/3] gdb/testsuite: introduce dwarf5 option to gdb_compile

[Keith Seitz]

On 10/15/24 7:38 AM, Guinevere Larsen wrote:
> This commit adds a new option to gdb_compile that would be able to add
> the correct flag (if known) or error out in case we are unable to tell
> which flag to use. It also changes the existing tests to use this
> general option instead of hard coding -gdwarf-5.

This LGTM.

Reviewed-by: Keith Seitz <keiths@redhat.com>

If maintainers agree, I think this could go in independently of
the other patches in this series.

Keith

Re: [PATCH v2 3/3] gdb/testsuite: introduce dwarf5 option to gdb_compile

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> From: Guinevere Larsen <blarsen@redhat.com>
Guinevere> A few tests on the testsuite require dwarf5 to work. Up until now, the
Guinevere> way to do this was to explicitly add the command line flag -gdwarf-5.
Guinevere> This isn't very portable, in case a compiler requires a different flag
Guinevere> to emit dwarf5.

Guinevere> This commit adds a new option to gdb_compile that would be able to add
Guinevere> the correct flag (if known) or error out in case we are unable to tell
Guinevere> which flag to use. It also changes the existing tests to use this
Guinevere> general option instead of hard coding -gdwarf-5.

My first thought was maybe we'd want to future-proof this as well, but
in the end I think it doesn't matter -- it's fine to let these continue
to use DWARF 5 and if some compiler ever removes DWARF 5 support, (a)
I'll be retired and (b) it's easy to change to -gdwarf-23 or whatever.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH v2 3/3] gdb/testsuite: introduce dwarf5 option to gdb_compile

[Guinevere Larsen]

On 10/17/24 4:35 PM, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> Guinevere> From: Guinevere Larsen <blarsen@redhat.com>
> Guinevere> A few tests on the testsuite require dwarf5 to work. Up until now, the
> Guinevere> way to do this was to explicitly add the command line flag -gdwarf-5.
> Guinevere> This isn't very portable, in case a compiler requires a different flag
> Guinevere> to emit dwarf5.
>
> Guinevere> This commit adds a new option to gdb_compile that would be able to add
> Guinevere> the correct flag (if known) or error out in case we are unable to tell
> Guinevere> which flag to use. It also changes the existing tests to use this
> Guinevere> general option instead of hard coding -gdwarf-5.
>
> My first thought was maybe we'd want to future-proof this as well, but
> in the end I think it doesn't matter -- it's fine to let these continue
> to use DWARF 5 and if some compiler ever removes DWARF 5 support, (a)
> I'll be retired and (b) it's easy to change to -gdwarf-23 or whatever.
>
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
>
Thanks for the review! Do you agree with Keith that this can be pushed 
independent of the rest of the series?

Also, in case you're looking at patch 1, I have sent a v3 of this series 
(I just forgot to update the subject line of the cover letter). it's 
here: 
https://inbox.sourceware.org/gdb-patches/20241017175620.5876-2-guinevere@redhat.com/T/#t

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

Re: [PATCH v2 3/3] gdb/testsuite: introduce dwarf5 option to gdb_compile

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> Thanks for the review! Do you agree with Keith that this can be pushed
Guinevere> independent of the rest of the series?

Yeah.

Tom

Re: [PATCH v2 3/3] gdb/testsuite: introduce dwarf5 option to gdb_compile

[Guinevere Larsen]

On 10/17/24 5:13 PM, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> Guinevere> Thanks for the review! Do you agree with Keith that this can be pushed
> Guinevere> independent of the rest of the series?
>
> Yeah.
>
> Tom
>
oops, I missed this email. Thanks for the confirmation, I pushed the patch!

-- 
Cheers,
Guinevere Larsen
She/Her/Hers