Re: [PATCH][gdb/testsuite] Fix shlib compilation with target board unix/-pie/-fPIE

[Tom de Vries <tdevries@suse.de>]

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

On 12/16/20 6:22 PM, Tom de Vries wrote:
> On 12/14/20 6:21 PM, Simon Marchi wrote:
>> On 2020-12-13 11:41 a.m., Tom de Vries wrote:
>>> Hi,
>>>
>>> When running test-case gdb.base/info-shared.exp with target board
>>> unix/-pie/-fPIE, we run into:
>>> ...
>>> spawn -ignore SIGHUP gcc -fno-stack-protector \
>>>   outputs/gdb.base/info-shared/info-shared-solib1.c.o \
>>>   -fdiagnostics-color=never -fPIC -shared -Wl,-soname,info-shared-solib1.so \
>>>   -lm -fPIE -pie -o outputs/gdb.base/info-shared/info-shared-solib1.so^M
>>> ld: Scrt1.o: in function `_start':^M
>>> start.S:104: undefined reference to `main'^M
>>> collect2: error: ld returned 1 exit status^M
>>> compiler exited with status 1
>>> ...
>>>
>>> The intention of the -pie/-fPIE flags is to build and test PIE executables on
>>> platforms where that is not the default.  However, the flags clash with the
>>> flags required to build shared libraries.
>>>
>>> Fix this by filtering out PIE-related flags out of the multilib_flags settings
>>> in compile_shared_lib.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Any comments?
>>>
>>> Thanks,
>>> - Tom
>>>
>>> [gdb/testsuite] Fix shlib compilation with target board unix/-pie/-fPIE
>>>
>>> ---
>>>  gdb/testsuite/lib/gdb.exp | 39 ++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>> index e35d236018..071b5afa99 100644
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -4283,7 +4283,7 @@ proc gdb_compile_pthreads {source dest type options} {
>>>
>>>  # Build a shared library from SOURCES.
>>>
>>> -proc gdb_compile_shlib {sources dest options} {
>>> +proc gdb_compile_shlib_1 {sources dest options} {
>>>      set obj_options $options
>>>
>>>      set ada 0
>>> @@ -4416,6 +4416,43 @@ proc gdb_compile_shlib {sources dest options} {
>>>      return ""
>>>  }
>>>
>>> +# Build a shared library from SOURCES.  Ignore target boards PIE-related
>>> +# multilib_flags.
>>> +
>>> +proc gdb_compile_shlib {sources dest options} {
>>> +    global board
>>> +
>>> +    # Save multilib_flags.
>>> +    set board [target_info name]
>>> +    set save_multilib_flag [board_info $board multilib_flags]
>>> +
>>> +    # Ignore PIE-related setting in multilib_flags.
>>> +    set multilib_flag ""
>>> +    foreach op $save_multilib_flag {
>>> +	if { $op == "-pie" || $op == "-no-pie" \
>>> +		 || $op == "-fPIE" || $op == "-fno-PIE"} {
>>> +	} else {
>>> +	    append multilib_flag " $op"
>>> +	}
>>> +    }
>>> +    unset_board_info "multilib_flags"
>>> +    set_board_info multilib_flags "$multilib_flag"
>>> +    set code [catch {gdb_compile_shlib_1 $sources $dest $options} result]
>>> +
>>> +    # Restore multilib_flags.
>>> +    unset_board_info "multilib_flags"
>>> +    set_board_info multilib_flags $save_multilib_flag
>>> +
>>> +    if {$code == 1} {
>>> +	global errorInfo errorCode
>>> +	return -code error -errorinfo $errorInfo -errorcode $errorCode $result
>>> +    } elseif {$code > 1} {
>>> +	return -code $code $result
>>> +    }
>>> +
>>> +    return $result
>>
>> That sounds reasonable.  I wonder if the lines above could be simplified
>> to:
>>
>>   if {$code == 1} {
>>       ...
>>   }
>>
>>   return -code $code $result
>>
>> IOW, if it's fine to return an explicit -code even if $code is 0 / OK.
>>
> 
> Thanks for the review.
> 
> I copied this code from gdb_do_cache_wrap, so I left it as is for the
> commit.
> 
> This sounds like a good idea for a more broad refactoring.
> 

I thought this over a bit more, and came to the realization that what
this does is similar to save_vars, only for target board_info.

So, how about this refactoring?

Thanks,
- Tom



[-- Attachment #2: 0005-gdb-testsuite-Handle-target_board_info-switch-in-save_vars.patch --]
[-- Type: text/x-patch, Size: 4250 bytes --]

[gdb/testsuite] Handle --target_board_info switch in save_vars

Add a --target_board_info switch to save_vars such that we can do:
...
save_vars { --target_board_info multilib_flags } {
    unset_board_info multilib_flags
    set_board_info multilib_flags "$override_multilib_flags"
}
...
and use it in gdb_compile_shlib.

Tested on x86_64-linux.

gdb/testsuite/ChangeLog:

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

	* lib/gdb.exp (save_vars): Add --target_board_info support.
	(gdb_compile_shlib): Use save_vars.

---
 gdb/testsuite/lib/gdb.exp | 63 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e91023093e..4aae1fffe6 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2364,6 +2364,9 @@ proc proc_with_prefix {name arguments body} {
 # Run BODY in the context of the caller.  After BODY is run, the variables
 # listed in VARS will be reset to the values they had before BODY was run.
 #
+# A variable preceded by "--target_board_info" specifies a variable stored
+# in board_info for the target board, rather than a tcl variable.
+#
 # This is useful for providing a scope in which it is safe to temporarily
 # modify global variables, e.g.
 #
@@ -2386,9 +2389,28 @@ proc proc_with_prefix {name arguments body} {
 proc save_vars { vars body } {
     array set saved_scalars { }
     array set saved_arrays { }
+    array set saved_target_board_info { }
     set unset_vars { }
+    set unset_target_board_info { }
 
+    set target_board_info 0
     foreach var $vars {
+	if { $var == "--target_board_info" } {
+	    set target_board_info 1
+	    continue
+	}
+	if { $target_board_info } {
+	    set target_board_info 0
+	    global board board_info
+	    set board [target_info name]
+	    if { [info exists board_info($board,$var)] } {
+		set saved_target_board_info($var) [board_info $board $var]
+	    } else {
+		lappend unset_target_board_info $var
+	    }
+	    continue
+	}
+
 	# First evaluate VAR in the context of the caller in case the variable
 	# name may be a not-yet-interpolated string like env($foo)
 	set var [uplevel 1 list $var]
@@ -2415,10 +2437,23 @@ proc save_vars { vars body } {
 	uplevel 1 [list array set $var $value]
     }
 
+    foreach {var value} [array get saved_target_board_info] {
+	global board
+	set board [target_info name]
+	unset_board_info $var
+	set_board_info $var $value
+    }
+
     foreach var $unset_vars {
 	uplevel 1 [list unset -nocomplain $var]
     }
 
+    foreach var $unset_target_board_info {
+	global board
+	set board [target_info name]
+	unset_board_info $var
+    }
+
     if {$code == 1} {
 	global errorInfo errorCode
 	return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
@@ -4422,32 +4457,22 @@ proc gdb_compile_shlib_1 {sources dest options} {
 proc gdb_compile_shlib {sources dest options} {
     global board
 
-    # Save multilib_flags.
-    set board [target_info name]
-    set save_multilib_flag [board_info $board multilib_flags]
-
     # Ignore PIE-related setting in multilib_flags.
-    set multilib_flag ""
-    foreach op $save_multilib_flag {
+    set board [target_info name]
+    set multilib_flags_orig [board_info $board multilib_flags]
+    set multilib_flags ""
+    foreach op $multilib_flags_orig {
 	if { $op == "-pie" || $op == "-no-pie" \
 		 || $op == "-fPIE" || $op == "-fno-PIE"} {
 	} else {
-	    append multilib_flag " $op"
+	    append multilib_flags " $op"
 	}
     }
-    unset_board_info "multilib_flags"
-    set_board_info multilib_flags "$multilib_flag"
-    set code [catch {gdb_compile_shlib_1 $sources $dest $options} result]
-
-    # Restore multilib_flags.
-    unset_board_info "multilib_flags"
-    set_board_info multilib_flags $save_multilib_flag
 
-    if {$code == 1} {
-	global errorInfo errorCode
-	return -code error -errorinfo $errorInfo -errorcode $errorCode $result
-    } elseif {$code > 1} {
-	return -code $code $result
+    save_vars { --target_board_info multilib_flags } {
+	unset_board_info multilib_flags
+	set_board_info multilib_flags "$multilib_flags"
+	set result [gdb_compile_shlib_1 $sources $dest $options]
     }
 
     return $result