Testsuite: Ensure pie is disabled on some tests
Commit Message
> On 19 Mar 2019, at 16:36, Pedro Alves <palves@redhat.com> wrote:
>
> On 03/19/2019 03:45 PM, Alan Hayward wrote:
>> + # Recent Debian/Ubuntu defaults to PIE enabled. Ensure it is disabled.
>> + lappend opts {nopie}
>
> Please don't write "Recent", "New", etc. Imagine it's 2025 and you're
> reading this comment. What would "Recent" mean then? Spell out some
> version number where you observed this instead.
>
> Thanks,
> Pedro Alves
Fair enough. Updated with Ubuntu16.10 and Debian9.
> On 19 Mar 2019, at 16:07, Simon Marchi <simark@simark.ca> wrote:
>
> On 2019-03-19 11:45 a.m., Alan Hayward wrote:
>>> On 19 Mar 2019, at 13:47, Simon Marchi <simark@simark.ca> wrote:
>>>
>>> On 2019-03-06 5:20 a.m., Alan Hayward wrote:
>>>> Recent versions of Ubuntu and Debian default GCC to enable pie.
>>>> In dump.exp, pie will causes addresses to be out of range for IHEX.
>>>> In break-interp.exp, pie is explicitly set for some tests and assumed
>>>> to be disabled for the remainder.
>>>> Ensure pie is disabled for these tests when required.
>>>> gdb/testsuite/ChangeLog:
>>>> 2019-03-06 Alan Hayward <alan.hayward@arm.com>
>>>> * gdb.base/break-interp.exp: Ensure pie is disabled.
>>>> * gdb.base/dump.exp: Likewise.
>>>
>>> Hi Alan,
>>>
>>> The "nopie" flag to gdb_compile has been introduced recently for this, could you use that instead? See commit 6e8b1ab2fd4c ("Fix various tests to use -no-pie linker flag when needed”).
>> Aha! That’s exactly what I needed.
>> In my mind, we should have a “pie” flag too. I’ll add this in too.
>
> Good idea.
>
>> Updated. I would have pushed this new version - but - I’m a little unsure about adding the additional board flags.
>> Pie option adds both flag and ldflag versions to make sure it gets the cases where we’re just compiling to objects.
>
> Right, if we are looking to enable pie (versus disabling it), we need both.
>
>> This version ok?
>> 2019-03-19 Alan Hayward <alan.hayward@arm.com>
>> * README: Add pie options.
>> * gdb.base/break-interp.exp: Ensure pie is disabled.
>> * gdb.base/dump.exp: Likewise.
>> * lib/gdb.exp (gdb_compile): Add pie option.
>> diff --git a/gdb/testsuite/README b/gdb/testsuite/README
>> index b5e75b9a79..25381cdf04 100644
>> --- a/gdb/testsuite/README
>> +++ b/gdb/testsuite/README
>> @@ -482,6 +482,16 @@ gdb,no_thread_names
>> The target doesn't support thread names.
>> +gdb,pie_flag
>> +
>> + The flag required to force the compiler to produce position-independent
>> + executables.
>> +
>> +gdb,ldpie_flag
>> +
>> + The flag required to force the linker to produce position-independent
>> + executables.
>> +
>
> "pie_ldflag" sounds more natural to me than "ldpie_flag", unless you chose this name to be consistent with other existing options?
Switched.
>
>> gdb,nopie_flag
>> The flag required to force the compiler to produce non-position-independent
>> diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
>> index f85e8a650a..8bb853c041 100644
>> --- a/gdb/testsuite/gdb.base/break-interp.exp
>> +++ b/gdb/testsuite/gdb.base/break-interp.exp
>> @@ -625,8 +625,10 @@ foreach ldprelink {NO YES} {
>> lappend opts {debug}
>> }
>> if {$binpie != "NO"} {
>> - lappend opts {additional_flags=-fPIE}
>> - lappend opts {ldflags=-pie}
>> + lappend opts {pie}
>> + } else {
>> + # Recent Debian/Ubuntu defaults to PIE enabled. Ensure it is disabled.
>> + lappend opts {nopie}
>> }
>> set dir ${exec}.d
>> diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp
>> index 44b0988b80..56dcafd4cb 100644
>> --- a/gdb/testsuite/gdb.base/dump.exp
>> +++ b/gdb/testsuite/gdb.base/dump.exp
>> @@ -36,6 +36,10 @@ if {[istarget "spu*-*-*"]} then {
>> set is64bitonly "yes"
>> }
>> +# Recent Debian/Ubuntu defaults to PIE enabled. Ensure it is disabled as this
>> +# causes addresses to be out of range for IHEX.
>> +lappend options {nopie}
>> +
>> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } {
>> untested "failed to compile"
>> return -1
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index f13f909c34..259865f5fd 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -3461,6 +3461,7 @@ set gdb_saved_set_unbuffered_mode_obj ""
>> # dynamically load libraries at runtime. For example, on Linux, this adds
>> # -ldl so that the test can use dlopen.
>> # - nowarnings: Inhibit all compiler warnings.
>> +# - pie: Force creation of PIE executables.
>> # - nopie: Prevent creation of PIE executables.
>> #
>> # And here are some of the not too obscure options understood by DejaGnu that
>> @@ -3599,8 +3600,27 @@ proc gdb_compile {source dest type options} {
>> set options [lreplace $options $nowarnings $nowarnings $flag]
>> }
>> - # Replace the "nopie" option with the appropriate additional_flags
>> - # to disable PIE executables.
>> + # Replace the "pie" option with the appropriate flags to enable PIE
>> + # executables.
>> + set pie [lsearch -exact $options pie]
>> + if {$pie != -1} {
>> + if [target_info exists gdb,pie_flag] {
>> + set flag "additional_flags=[target_info gdb,pie_flag]"
>> + } else {
>> + set flag "additional_flags=-fpie"
>
> I know there's a difference between -fpie and -fPIE, but I don't really that what it is about. Did you choose -fpie on purpose? Since it matters on AArch64, among others, maybe you know the difference?
>
I hadn’t chosen -fpie on purpose. I’ve read into this a bit more now - it’s an awkward set of flags.
fPIE is the safer option and it’s what Ubuntu/Debian default to.
Updated the patch with PIE and added some explanations as to why.
Will push if there are no further comments.
@@ -482,6 +482,16 @@ gdb,no_thread_names
The target doesn't support thread names.
+gdb,pie_flag
+
+ The flag required to force the compiler to produce position-independent
+ executables.
+
+gdb,pie_ldflag
+
+ The flag required to force the linker to produce position-independent
+ executables.
+
gdb,nopie_flag
The flag required to force the compiler to produce non-position-independent
@@ -625,8 +625,10 @@ foreach ldprelink {NO YES} {
lappend opts {debug}
}
if {$binpie != "NO"} {
- lappend opts {additional_flags=-fPIE}
- lappend opts {ldflags=-pie}
+ lappend opts {pie}
+ } else {
+ # Debian9/Ubuntu16.10 onwards default to PIE enabled. Ensure it is disabled.
+ lappend opts {nopie}
}
set dir ${exec}.d
@@ -36,6 +36,10 @@ if {[istarget "spu*-*-*"]} then {
set is64bitonly "yes"
}
+# Debian9/Ubuntu16.10 onwards default to PIE enabled. Ensure it is disabled as
+# this causes addresses to be out of range for IHEX.
+lappend options {nopie}
+
if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } {
untested "failed to compile"
return -1
@@ -3461,6 +3461,7 @@ set gdb_saved_set_unbuffered_mode_obj ""
# dynamically load libraries at runtime. For example, on Linux, this adds
# -ldl so that the test can use dlopen.
# - nowarnings: Inhibit all compiler warnings.
+# - pie: Force creation of PIE executables.
# - nopie: Prevent creation of PIE executables.
#
# And here are some of the not too obscure options understood by DejaGnu that
@@ -3599,8 +3600,33 @@ proc gdb_compile {source dest type options} {
set options [lreplace $options $nowarnings $nowarnings $flag]
}
- # Replace the "nopie" option with the appropriate additional_flags
- # to disable PIE executables.
+ # Replace the "pie" option with the appropriate compiler and linker flags
+ # to enable PIE executables.
+ set pie [lsearch -exact $options pie]
+ if {$pie != -1} {
+ if [target_info exists gdb,pie_flag] {
+ set flag "additional_flags=[target_info gdb,pie_flag]"
+ } else {
+ # For safety, use fPIE rather than fpie. On AArch64, m68k, PowerPC
+ # and SPARC, fpie can cause compile errors due to the GOT exceeding
+ # a maximum size. On other architectures the two flags are
+ # identical (see the GCC manual). Note Debian9 and Ubuntu16.10
+ # onwards default GCC to using fPIE. If you do require fpie, then
+ # it can be set using the pie_flag.
+ set flag "additional_flags=-fPIE"
+ }
+ set options [lreplace $options $pie $pie $flag]
+
+ if [target_info exists gdb,pie_ldflag] {
+ set flag "ldflags=[target_info gdb,pie_ldflag]"
+ } else {
+ set flag "ldflags=-pie"
+ }
+ lappend options "$flag"
+ }
+
+ # Replace the "nopie" option with the appropriate linker flag to disable
+ # PIE executables. There are no compiler flags for this option.
set nopie [lsearch -exact $options nopie]
if {$nopie != -1} {
if [target_info exists gdb,nopie_flag] {