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

Tom de Vries
2020-12-19T17:44:17Z

[-- 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