public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/testsuite] Limit default_target_compile override
@ 2020-06-19 12:10 Tom de Vries
  2020-06-19 14:18 ` Tom de Vries
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Tom de Vries @ 2020-06-19 12:10 UTC (permalink / raw)
  To: gdb-patches

Hi,

The file lib/future.exp contains an override of dejagnu's
default_target_compile.

The override is activated if dejagnu's default_target_compile is missing
support for one or more languages.

However, if the override is activated, it's active for all languages.

This unnecessarily extends the scope of potential problems in the override to
languages that don't need the override.

Fix this by limiting the scope of the override.

Also add a note stating for which languages the override is active, as a
reminder that support for those languages needs to be ported to dejagnu.  With
my system dejagnu 1.6.1, as well as with current dejagnu trunk, that gives us:
...
NOTE: Dejagnu's default_target_compile is missing support for Go, using \
  local override
NOTE: Dejagnu's default_target_compile is missing support for Rust, using \
  local override
...

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Limit default_target_compile override

gdb/testsuite/ChangeLog:

2020-06-19  Tom de Vries  <tdevries@suse.de>

	* lib/gdb.exp (gdb_note): New proc.
	* lib/future.exp (gdb_default_target_compile_1): Factor out of ...
	(gdb_default_target_compile): ... here.  Only call
	gdb_default_target_compile_1 if use_gdb_compile(<lang>) is set.
	(use_gdb_compile): Change to array.
	(toplevel): Update sets of use_gdb_compile to specify language.
	Warn about default_target_compile override.  Store dejagnu's version
	of default_target_compile in dejagnu_default_target_compile.

---
 gdb/testsuite/lib/future.exp | 68 +++++++++++++++++++++++++++++++++++---------
 gdb/testsuite/lib/gdb.exp    |  6 ++++
 2 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
index 62cc7e68a5..a46640c14c 100644
--- a/gdb/testsuite/lib/future.exp
+++ b/gdb/testsuite/lib/future.exp
@@ -172,7 +172,9 @@ proc gdb_find_eu-unstrip {} {
     return $eu_unstrip
 }
 
-proc gdb_default_target_compile {source destfile type options} {
+# Local version of default_target_compile, to be used for languages that
+# dejagnu's default_target_compile doesn't support.
+proc gdb_default_target_compile_1 {source destfile type options} {
     global target_triplet
     global tool_root_dir
     global CFLAGS_FOR_TARGET
@@ -627,40 +629,80 @@ proc gdb_default_target_compile {source destfile type options} {
     return ${comp_output}
 }
 
-# See if the version of dejaGNU being used to run the testsuite is
-# recent enough to contain support for building Ada programs or not.
-# If not, then use the functions above in place of the ones provided
-# by dejaGNU. This is only temporary (brobecker/2004-03-31).
+# If dejagnu's default_target_compile supports the language specified in
+# OPTIONS, use it.  Otherwise, use gdb_default_target_compile_1.
+proc gdb_default_target_compile {source destfile type options} {
+    global use_gdb_compile
+
+    set need_local 0
+    foreach i $options {
+
+	if { $i == "ada" || $i == "d" || $i == "go" || $i == "rust" } {
+	    set need_local [info exists use_gdb_compile($i)]
+	    break
+	}
+
+	if { $i == "c++" } {
+	    break
+	}
+
+	if { $i == "f77" || $i == "f90" } {
+	    set need_local [info exists use_gdb_compile(fortran)]
+	    break
+	}
+    }
+
+    if { $need_local } {
+	return [gdb_default_target_compile_1 $source $destfile $type $options]
+    }
+
+    return [dejagnu_default_target_compile $source $destfile $type $options]
+}
+
+# Array of languages for which dejagnu's default_target_compile is missing
+# support.
+array set use_gdb_compile [list]
+
+# Note missing support in dejagnu's default_target_compile.  This
+# needs to be fixed by porting the missing support to Dejagnu.
+set note_prefix "Dejagnu's default_target_compile is missing support for "
+set note_suffix	", using local override"
 
-set use_gdb_compile 0
 if {[info procs find_gnatmake] == ""} {
     rename gdb_find_gnatmake find_gnatmake
-    set use_gdb_compile 1
+    set use_gdb_compile(ada) 1
+    gdb_note [join [list $note_prefix "Ada" $note_suffix] ""]
 }
 
 if {[info procs find_gfortran] == ""} {
     rename gdb_find_gfortran find_gfortran
-    set use_gdb_compile 1
+    set use_gdb_compile(fortran) 1
+    gdb_note [join [list $note_prefix "Fortran" $note_suffix] ""]
 }
 
 if {[info procs find_go_linker] == ""} {
     rename gdb_find_go find_go
     rename gdb_find_go_linker find_go_linker
-    set use_gdb_compile 1
+    set use_gdb_compile(go) 1
+    gdb_note [join [list $note_prefix "Go" $note_suffix] ""]
 }
 
 if {[info procs find_gdc] == ""} {
     rename gdb_find_gdc find_gdc
-    set use_gdb_compile 1
+    set use_gdb_compile(d) 1
+    gdb_note [join [list $note_prefix "D" $note_suffix] ""]
 }
 
 if {[info procs find_rustc] == ""} {
     rename gdb_find_rustc find_rustc
-    set use_gdb_compile 1
+    set use_gdb_compile(rust) 1
+    gdb_note [join [list $note_prefix "Rust" $note_suffix] ""]
 }
 
-if {$use_gdb_compile} {
-    catch {rename default_target_compile {}}
+# If dejagnu's default_target_compile is missing support for any language,
+# override it.
+if { [array size use_gdb_compile] != 0 } {
+    catch {rename default_target_compile dejagnu_default_target_compile}
     rename gdb_default_target_compile default_target_compile
 }
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 480af7052f..7b243f5fff 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7393,5 +7393,11 @@ proc tuiterm_env { } {
     lappend gdb_finish_hooks tuiterm_env_finish
 }
 
+# Dejagnu has a version of note, but usage is not allowed outside of dejagnu.
+# Define a local version.
+proc gdb_note { message } {
+    verbose -- "NOTE: $message" 0
+}
+
 # Always load compatibility stuff.
 load_lib future.exp

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

* Re: [PATCH][gdb/testsuite] Limit default_target_compile override
  2020-06-19 12:10 [PATCH][gdb/testsuite] Limit default_target_compile override Tom de Vries
@ 2020-06-19 14:18 ` Tom de Vries
  2020-06-19 15:46 ` Tom Tromey
  2020-06-22 21:34 ` Luis Machado
  2 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries @ 2020-06-19 14:18 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey

On 6/19/20 2:10 PM, Tom de Vries wrote:
> Hi,
> 
> The file lib/future.exp contains an override of dejagnu's
> default_target_compile.
> 
> The override is activated if dejagnu's default_target_compile is missing
> support for one or more languages.
> 
> However, if the override is activated, it's active for all languages.
> 
> This unnecessarily extends the scope of potential problems in the override to
> languages that don't need the override.
> 
> Fix this by limiting the scope of the override.
> 

Interestingly, AFAICT this also seems to fix all the regressions with
dejagnu trunk.

Thanks,
- Tom

> Also add a note stating for which languages the override is active, as a
> reminder that support for those languages needs to be ported to dejagnu.  With
> my system dejagnu 1.6.1, as well as with current dejagnu trunk, that gives us:
> ...
> NOTE: Dejagnu's default_target_compile is missing support for Go, using \
>   local override
> NOTE: Dejagnu's default_target_compile is missing support for Rust, using \
>   local override
> ...
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Limit default_target_compile override
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-06-19  Tom de Vries  <tdevries@suse.de>
> 
> 	* lib/gdb.exp (gdb_note): New proc.
> 	* lib/future.exp (gdb_default_target_compile_1): Factor out of ...
> 	(gdb_default_target_compile): ... here.  Only call
> 	gdb_default_target_compile_1 if use_gdb_compile(<lang>) is set.
> 	(use_gdb_compile): Change to array.
> 	(toplevel): Update sets of use_gdb_compile to specify language.
> 	Warn about default_target_compile override.  Store dejagnu's version
> 	of default_target_compile in dejagnu_default_target_compile.
> 
> ---
>  gdb/testsuite/lib/future.exp | 68 +++++++++++++++++++++++++++++++++++---------
>  gdb/testsuite/lib/gdb.exp    |  6 ++++
>  2 files changed, 61 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
> index 62cc7e68a5..a46640c14c 100644
> --- a/gdb/testsuite/lib/future.exp
> +++ b/gdb/testsuite/lib/future.exp
> @@ -172,7 +172,9 @@ proc gdb_find_eu-unstrip {} {
>      return $eu_unstrip
>  }
>  
> -proc gdb_default_target_compile {source destfile type options} {
> +# Local version of default_target_compile, to be used for languages that
> +# dejagnu's default_target_compile doesn't support.
> +proc gdb_default_target_compile_1 {source destfile type options} {
>      global target_triplet
>      global tool_root_dir
>      global CFLAGS_FOR_TARGET
> @@ -627,40 +629,80 @@ proc gdb_default_target_compile {source destfile type options} {
>      return ${comp_output}
>  }
>  
> -# See if the version of dejaGNU being used to run the testsuite is
> -# recent enough to contain support for building Ada programs or not.
> -# If not, then use the functions above in place of the ones provided
> -# by dejaGNU. This is only temporary (brobecker/2004-03-31).
> +# If dejagnu's default_target_compile supports the language specified in
> +# OPTIONS, use it.  Otherwise, use gdb_default_target_compile_1.
> +proc gdb_default_target_compile {source destfile type options} {
> +    global use_gdb_compile
> +
> +    set need_local 0
> +    foreach i $options {
> +
> +	if { $i == "ada" || $i == "d" || $i == "go" || $i == "rust" } {
> +	    set need_local [info exists use_gdb_compile($i)]
> +	    break
> +	}
> +
> +	if { $i == "c++" } {
> +	    break
> +	}
> +
> +	if { $i == "f77" || $i == "f90" } {
> +	    set need_local [info exists use_gdb_compile(fortran)]
> +	    break
> +	}
> +    }
> +
> +    if { $need_local } {
> +	return [gdb_default_target_compile_1 $source $destfile $type $options]
> +    }
> +
> +    return [dejagnu_default_target_compile $source $destfile $type $options]
> +}
> +
> +# Array of languages for which dejagnu's default_target_compile is missing
> +# support.
> +array set use_gdb_compile [list]
> +
> +# Note missing support in dejagnu's default_target_compile.  This
> +# needs to be fixed by porting the missing support to Dejagnu.
> +set note_prefix "Dejagnu's default_target_compile is missing support for "
> +set note_suffix	", using local override"
>  
> -set use_gdb_compile 0
>  if {[info procs find_gnatmake] == ""} {
>      rename gdb_find_gnatmake find_gnatmake
> -    set use_gdb_compile 1
> +    set use_gdb_compile(ada) 1
> +    gdb_note [join [list $note_prefix "Ada" $note_suffix] ""]
>  }
>  
>  if {[info procs find_gfortran] == ""} {
>      rename gdb_find_gfortran find_gfortran
> -    set use_gdb_compile 1
> +    set use_gdb_compile(fortran) 1
> +    gdb_note [join [list $note_prefix "Fortran" $note_suffix] ""]
>  }
>  
>  if {[info procs find_go_linker] == ""} {
>      rename gdb_find_go find_go
>      rename gdb_find_go_linker find_go_linker
> -    set use_gdb_compile 1
> +    set use_gdb_compile(go) 1
> +    gdb_note [join [list $note_prefix "Go" $note_suffix] ""]
>  }
>  
>  if {[info procs find_gdc] == ""} {
>      rename gdb_find_gdc find_gdc
> -    set use_gdb_compile 1
> +    set use_gdb_compile(d) 1
> +    gdb_note [join [list $note_prefix "D" $note_suffix] ""]
>  }
>  
>  if {[info procs find_rustc] == ""} {
>      rename gdb_find_rustc find_rustc
> -    set use_gdb_compile 1
> +    set use_gdb_compile(rust) 1
> +    gdb_note [join [list $note_prefix "Rust" $note_suffix] ""]
>  }
>  
> -if {$use_gdb_compile} {
> -    catch {rename default_target_compile {}}
> +# If dejagnu's default_target_compile is missing support for any language,
> +# override it.
> +if { [array size use_gdb_compile] != 0 } {
> +    catch {rename default_target_compile dejagnu_default_target_compile}
>      rename gdb_default_target_compile default_target_compile
>  }
>  
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 480af7052f..7b243f5fff 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -7393,5 +7393,11 @@ proc tuiterm_env { } {
>      lappend gdb_finish_hooks tuiterm_env_finish
>  }
>  
> +# Dejagnu has a version of note, but usage is not allowed outside of dejagnu.
> +# Define a local version.
> +proc gdb_note { message } {
> +    verbose -- "NOTE: $message" 0
> +}
> +
>  # Always load compatibility stuff.
>  load_lib future.exp
> 

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

* Re: [PATCH][gdb/testsuite] Limit default_target_compile override
  2020-06-19 12:10 [PATCH][gdb/testsuite] Limit default_target_compile override Tom de Vries
  2020-06-19 14:18 ` Tom de Vries
@ 2020-06-19 15:46 ` Tom Tromey
  2020-06-22 21:34 ` Luis Machado
  2 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2020-06-19 15:46 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> 2020-06-19  Tom de Vries  <tdevries@suse.de>

Tom> 	* lib/gdb.exp (gdb_note): New proc.
Tom> 	* lib/future.exp (gdb_default_target_compile_1): Factor out of ...
Tom> 	(gdb_default_target_compile): ... here.  Only call
Tom> 	gdb_default_target_compile_1 if use_gdb_compile(<lang>) is set.
Tom> 	(use_gdb_compile): Change to array.
Tom> 	(toplevel): Update sets of use_gdb_compile to specify language.
Tom> 	Warn about default_target_compile override.  Store dejagnu's version
Tom> 	of default_target_compile in dejagnu_default_target_compile.

Thanks.  This looks reasonable to me.

Tom> +set note_suffix	", using local override"
 
There are some extra spaces in this assignment.

Tom

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

* Re: [PATCH][gdb/testsuite] Limit default_target_compile override
  2020-06-19 12:10 [PATCH][gdb/testsuite] Limit default_target_compile override Tom de Vries
  2020-06-19 14:18 ` Tom de Vries
  2020-06-19 15:46 ` Tom Tromey
@ 2020-06-22 21:34 ` Luis Machado
  2020-06-26 11:35   ` Luis Machado
  2020-06-29 14:31   ` [PATCH][gdb/testsuite] Handle early_flags in gdb_default_target_compile Tom de Vries
  2 siblings, 2 replies; 7+ messages in thread
From: Luis Machado @ 2020-06-22 21:34 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Hi,

This commit seems to have caused a few regressions for aarch64-linux:

-# of expected passes		75098
-# of unexpected failures	63
-# of expected failures		114
+# of expected passes		75036
+# of unexpected failures	149
+# of expected failures		111

They are distributed across a few different tests:

gdb.base/display.exp
gdb.base/jit-reader-simple.exp
gdb.base/shlib-call.exp
gdb.base/solib-weak.exp
gdb.base/step-test.exp
gdb.base/store.exp
gdb.base/type-opaque.exp
gdb.cp/ovldbreak.exp
gdb.multi/multi-target.exp
gdb.reverse/step-precsave.exp
gdb.reverse/step-reverse.exp

It seems mostly related to missing options in the compilation line. For 
example, display.exp is missing -fno-stack-protector, which leads to a 
different source stepping pattern than the test is actually expecting.


On 6/19/20 9:10 AM, Tom de Vries wrote:
> Hi,
> 
> The file lib/future.exp contains an override of dejagnu's
> default_target_compile.
> 
> The override is activated if dejagnu's default_target_compile is missing
> support for one or more languages.
> 
> However, if the override is activated, it's active for all languages.
> 
> This unnecessarily extends the scope of potential problems in the override to
> languages that don't need the override.
> 
> Fix this by limiting the scope of the override.
> 
> Also add a note stating for which languages the override is active, as a
> reminder that support for those languages needs to be ported to dejagnu.  With
> my system dejagnu 1.6.1, as well as with current dejagnu trunk, that gives us:
> ...
> NOTE: Dejagnu's default_target_compile is missing support for Go, using \
>    local override
> NOTE: Dejagnu's default_target_compile is missing support for Rust, using \
>    local override
> ...
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/testsuite] Limit default_target_compile override
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-06-19  Tom de Vries  <tdevries@suse.de>
> 
> 	* lib/gdb.exp (gdb_note): New proc.
> 	* lib/future.exp (gdb_default_target_compile_1): Factor out of ...
> 	(gdb_default_target_compile): ... here.  Only call
> 	gdb_default_target_compile_1 if use_gdb_compile(<lang>) is set.
> 	(use_gdb_compile): Change to array.
> 	(toplevel): Update sets of use_gdb_compile to specify language.
> 	Warn about default_target_compile override.  Store dejagnu's version
> 	of default_target_compile in dejagnu_default_target_compile.
> 
> ---
>   gdb/testsuite/lib/future.exp | 68 +++++++++++++++++++++++++++++++++++---------
>   gdb/testsuite/lib/gdb.exp    |  6 ++++
>   2 files changed, 61 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
> index 62cc7e68a5..a46640c14c 100644
> --- a/gdb/testsuite/lib/future.exp
> +++ b/gdb/testsuite/lib/future.exp
> @@ -172,7 +172,9 @@ proc gdb_find_eu-unstrip {} {
>       return $eu_unstrip
>   }
>   
> -proc gdb_default_target_compile {source destfile type options} {
> +# Local version of default_target_compile, to be used for languages that
> +# dejagnu's default_target_compile doesn't support.
> +proc gdb_default_target_compile_1 {source destfile type options} {
>       global target_triplet
>       global tool_root_dir
>       global CFLAGS_FOR_TARGET
> @@ -627,40 +629,80 @@ proc gdb_default_target_compile {source destfile type options} {
>       return ${comp_output}
>   }
>   
> -# See if the version of dejaGNU being used to run the testsuite is
> -# recent enough to contain support for building Ada programs or not.
> -# If not, then use the functions above in place of the ones provided
> -# by dejaGNU. This is only temporary (brobecker/2004-03-31).
> +# If dejagnu's default_target_compile supports the language specified in
> +# OPTIONS, use it.  Otherwise, use gdb_default_target_compile_1.
> +proc gdb_default_target_compile {source destfile type options} {
> +    global use_gdb_compile
> +
> +    set need_local 0
> +    foreach i $options {
> +
> +	if { $i == "ada" || $i == "d" || $i == "go" || $i == "rust" } {
> +	    set need_local [info exists use_gdb_compile($i)]
> +	    break
> +	}
> +
> +	if { $i == "c++" } {
> +	    break
> +	}
> +
> +	if { $i == "f77" || $i == "f90" } {
> +	    set need_local [info exists use_gdb_compile(fortran)]
> +	    break
> +	}
> +    }
> +
> +    if { $need_local } {
> +	return [gdb_default_target_compile_1 $source $destfile $type $options]
> +    }
> +
> +    return [dejagnu_default_target_compile $source $destfile $type $options]
> +}
> +
> +# Array of languages for which dejagnu's default_target_compile is missing
> +# support.
> +array set use_gdb_compile [list]
> +
> +# Note missing support in dejagnu's default_target_compile.  This
> +# needs to be fixed by porting the missing support to Dejagnu.
> +set note_prefix "Dejagnu's default_target_compile is missing support for "
> +set note_suffix	", using local override"
>   
> -set use_gdb_compile 0
>   if {[info procs find_gnatmake] == ""} {
>       rename gdb_find_gnatmake find_gnatmake
> -    set use_gdb_compile 1
> +    set use_gdb_compile(ada) 1
> +    gdb_note [join [list $note_prefix "Ada" $note_suffix] ""]
>   }
>   
>   if {[info procs find_gfortran] == ""} {
>       rename gdb_find_gfortran find_gfortran
> -    set use_gdb_compile 1
> +    set use_gdb_compile(fortran) 1
> +    gdb_note [join [list $note_prefix "Fortran" $note_suffix] ""]
>   }
>   
>   if {[info procs find_go_linker] == ""} {
>       rename gdb_find_go find_go
>       rename gdb_find_go_linker find_go_linker
> -    set use_gdb_compile 1
> +    set use_gdb_compile(go) 1
> +    gdb_note [join [list $note_prefix "Go" $note_suffix] ""]
>   }
>   
>   if {[info procs find_gdc] == ""} {
>       rename gdb_find_gdc find_gdc
> -    set use_gdb_compile 1
> +    set use_gdb_compile(d) 1
> +    gdb_note [join [list $note_prefix "D" $note_suffix] ""]
>   }
>   
>   if {[info procs find_rustc] == ""} {
>       rename gdb_find_rustc find_rustc
> -    set use_gdb_compile 1
> +    set use_gdb_compile(rust) 1
> +    gdb_note [join [list $note_prefix "Rust" $note_suffix] ""]
>   }
>   
> -if {$use_gdb_compile} {
> -    catch {rename default_target_compile {}}
> +# If dejagnu's default_target_compile is missing support for any language,
> +# override it.
> +if { [array size use_gdb_compile] != 0 } {
> +    catch {rename default_target_compile dejagnu_default_target_compile}
>       rename gdb_default_target_compile default_target_compile
>   }
>   
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 480af7052f..7b243f5fff 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -7393,5 +7393,11 @@ proc tuiterm_env { } {
>       lappend gdb_finish_hooks tuiterm_env_finish
>   }
>   
> +# Dejagnu has a version of note, but usage is not allowed outside of dejagnu.
> +# Define a local version.
> +proc gdb_note { message } {
> +    verbose -- "NOTE: $message" 0
> +}
> +
>   # Always load compatibility stuff.
>   load_lib future.exp
> 

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

* Re: [PATCH][gdb/testsuite] Limit default_target_compile override
  2020-06-22 21:34 ` Luis Machado
@ 2020-06-26 11:35   ` Luis Machado
  2020-06-29 14:31   ` [PATCH][gdb/testsuite] Handle early_flags in gdb_default_target_compile Tom de Vries
  1 sibling, 0 replies; 7+ messages in thread
From: Luis Machado @ 2020-06-26 11:35 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

Report here now, so it gets included in 10.1.

https://sourceware.org/bugzilla/show_bug.cgi?id=26175

On 6/22/20 6:34 PM, Luis Machado wrote:
> Hi,
> 
> This commit seems to have caused a few regressions for aarch64-linux:
> 
> -# of expected passes        75098
> -# of unexpected failures    63
> -# of expected failures        114
> +# of expected passes        75036
> +# of unexpected failures    149
> +# of expected failures        111
> 
> They are distributed across a few different tests:
> 
> gdb.base/display.exp
> gdb.base/jit-reader-simple.exp
> gdb.base/shlib-call.exp
> gdb.base/solib-weak.exp
> gdb.base/step-test.exp
> gdb.base/store.exp
> gdb.base/type-opaque.exp
> gdb.cp/ovldbreak.exp
> gdb.multi/multi-target.exp
> gdb.reverse/step-precsave.exp
> gdb.reverse/step-reverse.exp
> 
> It seems mostly related to missing options in the compilation line. For 
> example, display.exp is missing -fno-stack-protector, which leads to a 
> different source stepping pattern than the test is actually expecting.
> 
> 
> On 6/19/20 9:10 AM, Tom de Vries wrote:
>> Hi,
>>
>> The file lib/future.exp contains an override of dejagnu's
>> default_target_compile.
>>
>> The override is activated if dejagnu's default_target_compile is missing
>> support for one or more languages.
>>
>> However, if the override is activated, it's active for all languages.
>>
>> This unnecessarily extends the scope of potential problems in the 
>> override to
>> languages that don't need the override.
>>
>> Fix this by limiting the scope of the override.
>>
>> Also add a note stating for which languages the override is active, as a
>> reminder that support for those languages needs to be ported to 
>> dejagnu.  With
>> my system dejagnu 1.6.1, as well as with current dejagnu trunk, that 
>> gives us:
>> ...
>> NOTE: Dejagnu's default_target_compile is missing support for Go, using \
>>    local override
>> NOTE: Dejagnu's default_target_compile is missing support for Rust, 
>> using \
>>    local override
>> ...
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/testsuite] Limit default_target_compile override
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2020-06-19  Tom de Vries  <tdevries@suse.de>
>>
>>     * lib/gdb.exp (gdb_note): New proc.
>>     * lib/future.exp (gdb_default_target_compile_1): Factor out of ...
>>     (gdb_default_target_compile): ... here.  Only call
>>     gdb_default_target_compile_1 if use_gdb_compile(<lang>) is set.
>>     (use_gdb_compile): Change to array.
>>     (toplevel): Update sets of use_gdb_compile to specify language.
>>     Warn about default_target_compile override.  Store dejagnu's version
>>     of default_target_compile in dejagnu_default_target_compile.
>>
>> ---
>>   gdb/testsuite/lib/future.exp | 68 
>> +++++++++++++++++++++++++++++++++++---------
>>   gdb/testsuite/lib/gdb.exp    |  6 ++++
>>   2 files changed, 61 insertions(+), 13 deletions(-)
>>
>> diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
>> index 62cc7e68a5..a46640c14c 100644
>> --- a/gdb/testsuite/lib/future.exp
>> +++ b/gdb/testsuite/lib/future.exp
>> @@ -172,7 +172,9 @@ proc gdb_find_eu-unstrip {} {
>>       return $eu_unstrip
>>   }
>> -proc gdb_default_target_compile {source destfile type options} {
>> +# Local version of default_target_compile, to be used for languages that
>> +# dejagnu's default_target_compile doesn't support.
>> +proc gdb_default_target_compile_1 {source destfile type options} {
>>       global target_triplet
>>       global tool_root_dir
>>       global CFLAGS_FOR_TARGET
>> @@ -627,40 +629,80 @@ proc gdb_default_target_compile {source destfile 
>> type options} {
>>       return ${comp_output}
>>   }
>> -# See if the version of dejaGNU being used to run the testsuite is
>> -# recent enough to contain support for building Ada programs or not.
>> -# If not, then use the functions above in place of the ones provided
>> -# by dejaGNU. This is only temporary (brobecker/2004-03-31).
>> +# If dejagnu's default_target_compile supports the language specified in
>> +# OPTIONS, use it.  Otherwise, use gdb_default_target_compile_1.
>> +proc gdb_default_target_compile {source destfile type options} {
>> +    global use_gdb_compile
>> +
>> +    set need_local 0
>> +    foreach i $options {
>> +
>> +    if { $i == "ada" || $i == "d" || $i == "go" || $i == "rust" } {
>> +        set need_local [info exists use_gdb_compile($i)]
>> +        break
>> +    }
>> +
>> +    if { $i == "c++" } {
>> +        break
>> +    }
>> +
>> +    if { $i == "f77" || $i == "f90" } {
>> +        set need_local [info exists use_gdb_compile(fortran)]
>> +        break
>> +    }
>> +    }
>> +
>> +    if { $need_local } {
>> +    return [gdb_default_target_compile_1 $source $destfile $type 
>> $options]
>> +    }
>> +
>> +    return [dejagnu_default_target_compile $source $destfile $type 
>> $options]
>> +}
>> +
>> +# Array of languages for which dejagnu's default_target_compile is 
>> missing
>> +# support.
>> +array set use_gdb_compile [list]
>> +
>> +# Note missing support in dejagnu's default_target_compile.  This
>> +# needs to be fixed by porting the missing support to Dejagnu.
>> +set note_prefix "Dejagnu's default_target_compile is missing support 
>> for "
>> +set note_suffix    ", using local override"
>> -set use_gdb_compile 0
>>   if {[info procs find_gnatmake] == ""} {
>>       rename gdb_find_gnatmake find_gnatmake
>> -    set use_gdb_compile 1
>> +    set use_gdb_compile(ada) 1
>> +    gdb_note [join [list $note_prefix "Ada" $note_suffix] ""]
>>   }
>>   if {[info procs find_gfortran] == ""} {
>>       rename gdb_find_gfortran find_gfortran
>> -    set use_gdb_compile 1
>> +    set use_gdb_compile(fortran) 1
>> +    gdb_note [join [list $note_prefix "Fortran" $note_suffix] ""]
>>   }
>>   if {[info procs find_go_linker] == ""} {
>>       rename gdb_find_go find_go
>>       rename gdb_find_go_linker find_go_linker
>> -    set use_gdb_compile 1
>> +    set use_gdb_compile(go) 1
>> +    gdb_note [join [list $note_prefix "Go" $note_suffix] ""]
>>   }
>>   if {[info procs find_gdc] == ""} {
>>       rename gdb_find_gdc find_gdc
>> -    set use_gdb_compile 1
>> +    set use_gdb_compile(d) 1
>> +    gdb_note [join [list $note_prefix "D" $note_suffix] ""]
>>   }
>>   if {[info procs find_rustc] == ""} {
>>       rename gdb_find_rustc find_rustc
>> -    set use_gdb_compile 1
>> +    set use_gdb_compile(rust) 1
>> +    gdb_note [join [list $note_prefix "Rust" $note_suffix] ""]
>>   }
>> -if {$use_gdb_compile} {
>> -    catch {rename default_target_compile {}}
>> +# If dejagnu's default_target_compile is missing support for any 
>> language,
>> +# override it.
>> +if { [array size use_gdb_compile] != 0 } {
>> +    catch {rename default_target_compile dejagnu_default_target_compile}
>>       rename gdb_default_target_compile default_target_compile
>>   }
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 480af7052f..7b243f5fff 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -7393,5 +7393,11 @@ proc tuiterm_env { } {
>>       lappend gdb_finish_hooks tuiterm_env_finish
>>   }
>> +# Dejagnu has a version of note, but usage is not allowed outside of 
>> dejagnu.
>> +# Define a local version.
>> +proc gdb_note { message } {
>> +    verbose -- "NOTE: $message" 0
>> +}
>> +
>>   # Always load compatibility stuff.
>>   load_lib future.exp
>>

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

* [PATCH][gdb/testsuite] Handle early_flags in gdb_default_target_compile
  2020-06-22 21:34 ` Luis Machado
  2020-06-26 11:35   ` Luis Machado
@ 2020-06-29 14:31   ` Tom de Vries
  2020-06-29 15:24     ` Aktemur, Tankut Baris
  1 sibling, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2020-06-29 14:31 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

[ was: Re: [PATCH][gdb/testsuite] Limit default_target_compile override ]
On 6/22/20 11:34 PM, Luis Machado wrote:
> Hi,
> 
> This commit seems to have caused a few regressions for aarch64-linux:
> 
> -# of expected passes        75098
> -# of unexpected failures    63
> -# of expected failures        114
> +# of expected passes        75036
> +# of unexpected failures    149
> +# of expected failures        111
> 
> They are distributed across a few different tests:
> 
> gdb.base/display.exp
> gdb.base/jit-reader-simple.exp
> gdb.base/shlib-call.exp
> gdb.base/solib-weak.exp
> gdb.base/step-test.exp
> gdb.base/store.exp
> gdb.base/type-opaque.exp
> gdb.cp/ovldbreak.exp
> gdb.multi/multi-target.exp
> gdb.reverse/step-precsave.exp
> gdb.reverse/step-reverse.exp
> 
> It seems mostly related to missing options in the compilation line. For
> example, display.exp is missing -fno-stack-protector, which leads to a
> different source stepping pattern than the test is actually expecting.
> 

Hi Luis,

thanks for reporting this.

This should be fixed by attached patch.

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Handle-early_flags-in-gdb_default_target_compile.patch --]
[-- Type: text/x-patch, Size: 2048 bytes --]

[gdb/testsuite] Handle early_flags in gdb_default_target_compile

In gdb_default_target_compile, we use dejagnu's default_target_compile, unless
we need support for languages that are not yet support in the used dejagnu
version, in which case we use a local default_target_compile:
gdb_default_target_compile_1.

However, there's another reason to use the local default_target_compile: when
early_flags is used, because there's no dejagnu release available yet
supporting this.

Fix this by detecting and handling early_flags in gdb_default_target_compile.

Tested on x86_64-linux.

gdb/testsuite/ChangeLog:

2020-06-29  Tom de Vries  <tdevries@suse.de>

	PR testsuite/26175
	* lib/future.exp (gdb_default_target_compile): Detect and handle
	early_flags.

---
 gdb/testsuite/lib/future.exp | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
index ba00a31c19..1aabbe3233 100644
--- a/gdb/testsuite/lib/future.exp
+++ b/gdb/testsuite/lib/future.exp
@@ -634,25 +634,28 @@ proc gdb_default_target_compile_1 {source destfile type options} {
 proc gdb_default_target_compile {source destfile type options} {
     global use_gdb_compile
 
-    set need_local 0
+    set need_local_lang 0
+    set need_local_early_flags 0
     foreach i $options {
 
 	if { $i == "ada" || $i == "d" || $i == "go" || $i == "rust" } {
-	    set need_local [info exists use_gdb_compile($i)]
-	    break
+	    set need_local_lang [info exists use_gdb_compile($i)]
 	}
 
 	if { $i == "c++" } {
-	    break
+	    set need_local_lang 0
 	}
 
 	if { $i == "f77" || $i == "f90" } {
-	    set need_local [info exists use_gdb_compile(fortran)]
-	    break
+	    set need_local_lang [info exists use_gdb_compile(fortran)]
+	}
+
+        if { [regexp "^early_flags=" $i] } {
+	    set need_local_early_flags 1
 	}
     }
 
-    if { $need_local } {
+    if { $need_local_lang || $need_local_early_flags } {
 	return [gdb_default_target_compile_1 $source $destfile $type $options]
     }
 

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

* RE: [PATCH][gdb/testsuite] Handle early_flags in gdb_default_target_compile
  2020-06-29 14:31   ` [PATCH][gdb/testsuite] Handle early_flags in gdb_default_target_compile Tom de Vries
@ 2020-06-29 15:24     ` Aktemur, Tankut Baris
  0 siblings, 0 replies; 7+ messages in thread
From: Aktemur, Tankut Baris @ 2020-06-29 15:24 UTC (permalink / raw)
  To: Tom de Vries, Luis Machado, gdb-patches

On Monday, June 29, 2020 4:32 PM, Tom de Vries wrote:
> This should be fixed by attached patch.
> 
> Any comments?
> 
> Thanks,
> - Tom

Hi,

The patch fixed the regressions that I was seeing.

+        if { [regexp "^early_flags=" $i] } {

I think there is a tab vs. spaces issue for this line.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

end of thread, other threads:[~2020-06-29 15:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19 12:10 [PATCH][gdb/testsuite] Limit default_target_compile override Tom de Vries
2020-06-19 14:18 ` Tom de Vries
2020-06-19 15:46 ` Tom Tromey
2020-06-22 21:34 ` Luis Machado
2020-06-26 11:35   ` Luis Machado
2020-06-29 14:31   ` [PATCH][gdb/testsuite] Handle early_flags in gdb_default_target_compile Tom de Vries
2020-06-29 15:24     ` Aktemur, Tankut Baris

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