[PATCH] Call get_compiler_info directly from test_compiler_info if needed.

[PATCH] Call get_compiler_info directly from test_compiler_info if needed.

[Andrew Burgess]

Ran into a case where we call test_compiler_info without
first calling get_compiler_info.

I was going to just add an extra call to get_compiler_info,
but then I got all carried away.... with this patch
test_compiler_info now calls get_compiler_info if needed.

I also remove a couple of uses of the compiler_info
variable and replace with calls to test_compiler_info instead.

OK to apply?


    gdb/testsuite/ChangeLog
    
    	* gdb.base/watchpoint.exp (test_complex_watchpoint): Remove use of
    	compiler_info global, instead call test_compiler_info.
    	* gdb.cp/temargs.exp: Same.
    	* lib/gdb.exp (compiler_info): Change initial value to empty string.
    	(test_compiler_info): Call get_compiler_info if compiler_info is
    	empty string.
    	(skip_vsx_tests): No longer need to call get_compiler_info.
    	(skip_altivec_tests): Same.

diff --git a/gdb/testsuite/gdb.base/watchpoint.exp b/gdb/testsuite/gdb.base/watchpoint.exp
index 9576a9e..73dc5cc 100644
--- a/gdb/testsuite/gdb.base/watchpoint.exp
+++ b/gdb/testsuite/gdb.base/watchpoint.exp
@@ -464,12 +464,12 @@ proc test_complex_watchpoint {} {
 		pass $test
 	    }
 	    -re "can't compute CFA for this frame.*\r\n$gdb_prompt $" {
-		global compiler_info no_hw
+		global no_hw
 
 		# GCC < 4.5.0 does not get LOCATIONS_VALID set by dwarf2read.c.
 		# Therefore epilogue unwinder gets applied which is
 		# incompatible with dwarf2_frame_cfa.
-		verbose -log "compiler_info: $compiler_info"
+		verbose -log "compiler_info: [test_compiler_info]"
 		if {$no_hw && ([test_compiler_info {gcc-[0-3]-*}]
 			       || [test_compiler_info {gcc-4-[0-4]-*}])} {
 		    xfail "$test (old GCC has broken watchpoints in epilogues)"
diff --git a/gdb/testsuite/gdb.cp/temargs.exp b/gdb/testsuite/gdb.cp/temargs.exp
index 4cd9da5..31cde41 100644
--- a/gdb/testsuite/gdb.cp/temargs.exp
+++ b/gdb/testsuite/gdb.cp/temargs.exp
@@ -34,7 +34,7 @@ if {![runto_main]} {
 # NOTE: prepare_for_testing calls get_compiler_info, which we need
 # for the test_compiler_info calls.
 # gcc 4.4 and earlier don't emit enough info for some of our template tests.
-verbose -log "compiler_info: $compiler_info"
+verbose -log "compiler_info: [test_compiler_info]"
 set have_older_template_gcc 0
 set have_pr_41736_fixed 1
 set have_pr_45024_fixed 1
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 2c1cf29..6340e80 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2027,10 +2027,6 @@ gdb_caching_proc skip_altivec_tests {
 
     # Make sure we have a compiler that understands altivec.
     set compile_flags {debug nowarnings}
-    if [get_compiler_info] {
-       warning "Could not get compiler info"
-       return 1
-    }
     if [test_compiler_info gcc*] {
         set compile_flags "$compile_flags additional_flags=-maltivec"
     } elseif [test_compiler_info xlc*] {
@@ -2110,10 +2106,6 @@ gdb_caching_proc skip_vsx_tests {
 
     # Make sure we have a compiler that understands altivec.
     set compile_flags {debug nowarnings quiet}
-    if [get_compiler_info] {
-       warning "Could not get compiler info"
-       return 1
-    }
     if [test_compiler_info gcc*] {
         set compile_flags "$compile_flags additional_flags=-mvsx"
     } elseif [test_compiler_info xlc*] {
@@ -2401,7 +2393,7 @@ proc skip_libstdcxx_probe_tests {} {
     return $ok
 }
 
-set compiler_info		"unknown"
+set compiler_info		""
 set gcc_compiled		0
 set hp_cc_compiler		0
 set hp_aCC_compiler		0
@@ -2552,6 +2544,14 @@ proc get_compiler_info {{arg ""}} {
 proc test_compiler_info { {compiler ""} } {
     global compiler_info
 
+    # Check that compiler_info has been initialised
+    if [string match "" $compiler_info] {
+	if [get_compiler_info] {
+	    warning "Could not get compiler info"
+	    return -1
+	}
+    }
+
      # if no arg, return the compiler_info string
 
      if [string match "" $compiler] {

Re: [PATCH] Call get_compiler_info directly from test_compiler_info if needed.

[Pedro Alves]

On 12/10/2013 02:43 PM, Andrew Burgess wrote:
>  proc test_compiler_info { {compiler ""} } {
>      global compiler_info
>  
> +    # Check that compiler_info has been initialised
> +    if [string match "" $compiler_info] {
> +	if [get_compiler_info] {
> +	    warning "Could not get compiler info"
> +	    return -1
> +	}
> +    }

What about get_compiler_info's $arg?

-- 
Pedro Alves

Re: [PATCH] Call get_compiler_info directly from test_compiler_info if needed.

[Andrew Burgess]

On 10/12/2013 3:00 PM, Pedro Alves wrote:
> On 12/10/2013 02:43 PM, Andrew Burgess wrote:
>>  proc test_compiler_info { {compiler ""} } {
>>      global compiler_info
>>  
>> +    # Check that compiler_info has been initialised
>> +    if [string match "" $compiler_info] {
>> +	if [get_compiler_info] {
>> +	    warning "Could not get compiler info"
>> +	    return -1
>> +	}
>> +    }
> 
> What about get_compiler_info's $arg?

In the cases where I removed calls to get_compiler_info no arg was being
passed anyway, my assumption then is that in most cases the "default"
result of get_compiler_info is fine.

If you really want to pass some args to get_compiler_info you can still
do that in a separate call, and I left in place (in gdb.exp) a call that
does just this.

The code in test_compiler_info will not overwrite an existing
compiler_info value, so if you've taken care to call get_compiler_info
yourself then all should still work as expected.

Thanks,
Andrew

Re: [PATCH] Call get_compiler_info directly from test_compiler_info if needed.

[Pedro Alves]

On 12/10/2013 03:16 PM, Andrew Burgess wrote:
> On 10/12/2013 3:00 PM, Pedro Alves wrote:
>> On 12/10/2013 02:43 PM, Andrew Burgess wrote:
>>>  proc test_compiler_info { {compiler ""} } {
>>>      global compiler_info
>>>  
>>> +    # Check that compiler_info has been initialised
>>> +    if [string match "" $compiler_info] {
>>> +	if [get_compiler_info] {
>>> +	    warning "Could not get compiler info"
>>> +	    return -1
>>> +	}
>>> +    }
>>
>> What about get_compiler_info's $arg?
> 
> In the cases where I removed calls to get_compiler_info no arg was being
> passed anyway, my assumption then is that in most cases the "default"
> result of get_compiler_info is fine.
> 
> If you really want to pass some args to get_compiler_info you can still
> do that in a separate call, and I left in place (in gdb.exp) a call that
> does just this.
> 
> The code in test_compiler_info will not overwrite an existing
> compiler_info value, so if you've taken care to call get_compiler_info
> yourself then all should still work as expected.

Yes, but, if the interface will be "you don't need to call
get_compiler_info yourself, except ...", is there any gain in
doing this here?  The current rule is "you need to call
get_compiler_info yourself, period.", which seems easier to
explain.  E.g., here:

> @@ -2027,10 +2027,6 @@ gdb_caching_proc skip_altivec_tests {
>
>      # Make sure we have a compiler that understands altivec.
>      set compile_flags {debug nowarnings}
> -    if [get_compiler_info] {
> -       warning "Could not get compiler info"
> -       return 1
> -    }
>      if [test_compiler_info gcc*] {
>          set compile_flags "$compile_flags additional_flags=-maltivec"
>      } elseif [test_compiler_info xlc*] {

Before, the removed get_compiler_info would always overwrite
whatever was in the compiler_info before.  Now, if the previous
call to get_compiler_info happened to get an $arg,
test_compiler_info will now reuse the wrong compiler_info.

It seems to me that it should be get_compiler_info that
caches its results, not test_compiler_info, taking into
account $arg.  And then, if we want to get rid of the
need to call get_compiler_info before test_compiler_info,
then test_compiler_info should have likewise an $arg parameter
that gets passed down to the get_compiler_info call.

-- 
Pedro Alves

Call get_compiler_info before test_compiler_info. (was: Re: [PATCH] Call get_compiler_info directly from test_compiler_info if needed.)

[Andrew Burgess]

On 10/12/2013 3:49 PM, Pedro Alves wrote:
> On 12/10/2013 03:16 PM, Andrew Burgess wrote:
>> On 10/12/2013 3:00 PM, Pedro Alves wrote:
> 
>> @@ -2027,10 +2027,6 @@ gdb_caching_proc skip_altivec_tests {
>>
>>      # Make sure we have a compiler that understands altivec.
>>      set compile_flags {debug nowarnings}
>> -    if [get_compiler_info] {
>> -       warning "Could not get compiler info"
>> -       return 1
>> -    }
>>      if [test_compiler_info gcc*] {
>>          set compile_flags "$compile_flags additional_flags=-maltivec"
>>      } elseif [test_compiler_info xlc*] {
> 
> Before, the removed get_compiler_info would always overwrite
> whatever was in the compiler_info before.  Now, if the previous
> call to get_compiler_info happened to get an $arg,
> test_compiler_info will now reuse the wrong compiler_info.

OK, I see.  Here's a simpler solution that just adds the missing
call to get_compiler_info.

The example I found where this is an issue was: gdb.trace/pending.exp,
which pre-patch fails to build for me (when I run just that test), but
after the patch does run.  There are probably other examples around.

OK to apply?

Thanks,
Andrew

    gdb/testsuite/ChangeLog
    
    	* lib/gdb.exp (gdb_compile_shlib): Call get_compiler_info before
    	calling test_compiler_info.

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 2c1cf29..eddfb9d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2812,6 +2812,14 @@ proc gdb_compile_pthreads {source dest type options} {
 proc gdb_compile_shlib {sources dest options} {
     set obj_options $options
 
+    set info_options ""
+    if { [lsearch -exact $options "c++"] >= 0 } {
+	set info_options "c++"
+    }
+    if [get_compiler_info ${info_options}] {
+       return -1
+    }
+
     switch -glob [test_compiler_info] {
         "xlc-*" {
             lappend obj_options "additional_flags=-qpic"

Re: Call get_compiler_info before test_compiler_info.

[Pedro Alves]

On 12/10/2013 04:10 PM, Andrew Burgess wrote:
> On 10/12/2013 3:49 PM, Pedro Alves wrote:
>> On 12/10/2013 03:16 PM, Andrew Burgess wrote:
>>> On 10/12/2013 3:00 PM, Pedro Alves wrote:
>>
>>> @@ -2027,10 +2027,6 @@ gdb_caching_proc skip_altivec_tests {
>>>
>>>      # Make sure we have a compiler that understands altivec.
>>>      set compile_flags {debug nowarnings}
>>> -    if [get_compiler_info] {
>>> -       warning "Could not get compiler info"
>>> -       return 1
>>> -    }
>>>      if [test_compiler_info gcc*] {
>>>          set compile_flags "$compile_flags additional_flags=-maltivec"
>>>      } elseif [test_compiler_info xlc*] {
>>
>> Before, the removed get_compiler_info would always overwrite
>> whatever was in the compiler_info before.  Now, if the previous
>> call to get_compiler_info happened to get an $arg,
>> test_compiler_info will now reuse the wrong compiler_info.
> 
> OK, I see.  Here's a simpler solution that just adds the missing
> call to get_compiler_info.
> 
> The example I found where this is an issue was: gdb.trace/pending.exp,
> which pre-patch fails to build for me (when I run just that test), but
> after the patch does run.  There are probably other examples around.
> 
> OK to apply?

OK, but please update the proc's intro comment:

# Build a shared library from SOURCES.  You must use get_compiler_info
# first.

proc gdb_compile_shlib {sources dest options} {

-- 
Pedro Alves

Re: Call get_compiler_info before test_compiler_info.

[Andrew Burgess]

On 10/12/2013 4:26 PM, Pedro Alves wrote:
> On 12/10/2013 04:10 PM, Andrew Burgess wrote:
>> OK to apply?
> 
> OK, but please update the proc's intro comment:
> 
> # Build a shared library from SOURCES.  You must use get_compiler_info
> # first.
> 
> proc gdb_compile_shlib {sources dest options} {

Have pushed with truncated comment.

thanks,
Andrew