* [PATCH] Testsuite: Ensure pie is disabled on some tests @ 2019-03-06 10:20 Alan Hayward 2019-03-19 10:45 ` [PING][PATCH] " Alan Hayward 2019-03-19 13:48 ` [PATCH] " Simon Marchi 0 siblings, 2 replies; 7+ messages in thread From: Alan Hayward @ 2019-03-06 10:20 UTC (permalink / raw) To: gdb-patches; +Cc: nd, Alan Hayward 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. --- gdb/testsuite/gdb.base/break-interp.exp | 4 ++++ gdb/testsuite/gdb.base/dump.exp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp index f85e8a650a..409c31e568 100644 --- a/gdb/testsuite/gdb.base/break-interp.exp +++ b/gdb/testsuite/gdb.base/break-interp.exp @@ -627,6 +627,10 @@ foreach ldprelink {NO YES} { if {$binpie != "NO"} { lappend opts {additional_flags=-fPIE} lappend opts {ldflags=-pie} + } else { + # Recent Debian/Ubuntu defaults PIE enabled. Ensure it is disabled. + lappend opts {additional_flags=-no-pie} + lappend opts {ldflags=-no-pie} } set dir ${exec}.d diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp index 44b0988b80..b2d7ef938a 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" } +# Ensure -pie is disabled as this breaks IHEX tests. +# (PIE defaults to enabled on recent Debian/Ubuntu). +lappend options "additional_flags=-no-pie" + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } { untested "failed to compile" return -1 -- 2.17.2 (Apple Git-113) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PING][PATCH] Testsuite: Ensure pie is disabled on some tests 2019-03-06 10:20 [PATCH] Testsuite: Ensure pie is disabled on some tests Alan Hayward @ 2019-03-19 10:45 ` Alan Hayward 2019-03-19 13:48 ` [PATCH] " Simon Marchi 1 sibling, 0 replies; 7+ messages in thread From: Alan Hayward @ 2019-03-19 10:45 UTC (permalink / raw) To: GDB Patches; +Cc: nd Ping for this. > On 6 Mar 2019, at 10:20, Alan Hayward <Alan.Hayward@arm.com> 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. > --- > gdb/testsuite/gdb.base/break-interp.exp | 4 ++++ > gdb/testsuite/gdb.base/dump.exp | 4 ++++ > 2 files changed, 8 insertions(+) > > diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp > index f85e8a650a..409c31e568 100644 > --- a/gdb/testsuite/gdb.base/break-interp.exp > +++ b/gdb/testsuite/gdb.base/break-interp.exp > @@ -627,6 +627,10 @@ foreach ldprelink {NO YES} { > if {$binpie != "NO"} { > lappend opts {additional_flags=-fPIE} > lappend opts {ldflags=-pie} > + } else { > + # Recent Debian/Ubuntu defaults PIE enabled. Ensure it is disabled. > + lappend opts {additional_flags=-no-pie} > + lappend opts {ldflags=-no-pie} > } > > set dir ${exec}.d > diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp > index 44b0988b80..b2d7ef938a 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" > } > > +# Ensure -pie is disabled as this breaks IHEX tests. > +# (PIE defaults to enabled on recent Debian/Ubuntu). > +lappend options "additional_flags=-no-pie" > + > if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } { > untested "failed to compile" > return -1 > -- > 2.17.2 (Apple Git-113) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Testsuite: Ensure pie is disabled on some tests 2019-03-06 10:20 [PATCH] Testsuite: Ensure pie is disabled on some tests Alan Hayward 2019-03-19 10:45 ` [PING][PATCH] " Alan Hayward @ 2019-03-19 13:48 ` Simon Marchi 2019-03-19 15:45 ` Alan Hayward 1 sibling, 1 reply; 7+ messages in thread From: Simon Marchi @ 2019-03-19 13:48 UTC (permalink / raw) To: Alan Hayward, gdb-patches; +Cc: nd 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"). > --- > gdb/testsuite/gdb.base/break-interp.exp | 4 ++++ > gdb/testsuite/gdb.base/dump.exp | 4 ++++ > 2 files changed, 8 insertions(+) > > diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp > index f85e8a650a..409c31e568 100644 > --- a/gdb/testsuite/gdb.base/break-interp.exp > +++ b/gdb/testsuite/gdb.base/break-interp.exp > @@ -627,6 +627,10 @@ foreach ldprelink {NO YES} { > if {$binpie != "NO"} { > lappend opts {additional_flags=-fPIE} > lappend opts {ldflags=-pie} > + } else { > + # Recent Debian/Ubuntu defaults PIE enabled. Ensure it is disabled. > + lappend opts {additional_flags=-no-pie} > + lappend opts {ldflags=-no-pie} > } I believe it won't be necessary here to change CFLAGS (which additional_flags does), only ldflags, to make the final executable a non-PIE. Anyway, you'll probably end up using the "nopie" gdb_compile flag. > > set dir ${exec}.d > diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp > index 44b0988b80..b2d7ef938a 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" > } > > +# Ensure -pie is disabled as this breaks IHEX tests. Can you just precise how it breaks the IHEX tests, as you did in the commit message (it causes the addresses to be out of range for that format)? This patch is ok with those fixed, thanks! Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Testsuite: Ensure pie is disabled on some tests 2019-03-19 13:48 ` [PATCH] " Simon Marchi @ 2019-03-19 15:45 ` Alan Hayward 2019-03-19 16:07 ` Simon Marchi 2019-03-19 16:36 ` Pedro Alves 0 siblings, 2 replies; 7+ messages in thread From: Alan Hayward @ 2019-03-19 15:45 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches, nd > 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. > >> --- >> gdb/testsuite/gdb.base/break-interp.exp | 4 ++++ >> gdb/testsuite/gdb.base/dump.exp | 4 ++++ >> 2 files changed, 8 insertions(+) >> diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp >> index f85e8a650a..409c31e568 100644 >> --- a/gdb/testsuite/gdb.base/break-interp.exp >> +++ b/gdb/testsuite/gdb.base/break-interp.exp >> @@ -627,6 +627,10 @@ foreach ldprelink {NO YES} { >> if {$binpie != "NO"} { >> lappend opts {additional_flags=-fPIE} >> lappend opts {ldflags=-pie} >> + } else { >> + # Recent Debian/Ubuntu defaults PIE enabled. Ensure it is disabled. >> + lappend opts {additional_flags=-no-pie} >> + lappend opts {ldflags=-no-pie} >> } > > I believe it won't be necessary here to change CFLAGS (which additional_flags does), only ldflags, to make the final executable a non-PIE. Anyway, you'll probably end up using the "nopie" gdb_compile flag. Agreed. > >> set dir ${exec}.d >> diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp >> index 44b0988b80..b2d7ef938a 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" >> } >> +# Ensure -pie is disabled as this breaks IHEX tests. > > Can you just precise how it breaks the IHEX tests, as you did in the commit message (it causes the addresses to be out of range for that format)? Done. > > This patch is ok with those fixed, thanks! > 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. 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. + 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" + } + set options [lreplace $options $pie $pie $flag] + + if [target_info exists gdb,ldpie_flag] { + set flag "ldflags=[target_info gdb,ldpie_flag]" + } else { + set flag "ldflags=-pie" + } + lappend options "$flag" + } + + # Replace the "nopie" option with the appropriate flags to disable PIE + # executables. set nopie [lsearch -exact $options nopie] if {$nopie != -1} { if [target_info exists gdb,nopie_flag] { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Testsuite: Ensure pie is disabled on some tests 2019-03-19 15:45 ` Alan Hayward @ 2019-03-19 16:07 ` Simon Marchi 2019-03-20 10:36 ` Alan Hayward 2019-03-19 16:36 ` Pedro Alves 1 sibling, 1 reply; 7+ messages in thread From: Simon Marchi @ 2019-03-19 16:07 UTC (permalink / raw) To: Alan Hayward; +Cc: gdb-patches, nd 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? > 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? Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Testsuite: Ensure pie is disabled on some tests 2019-03-19 16:07 ` Simon Marchi @ 2019-03-20 10:36 ` Alan Hayward 0 siblings, 0 replies; 7+ messages in thread From: Alan Hayward @ 2019-03-20 10:36 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches, nd > 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. diff --git a/gdb/testsuite/README b/gdb/testsuite/README index b5e75b9a79..db90ea4698 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,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 diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp index f85e8a650a..51e31f6503 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 { + # Debian9/Ubuntu16.10 onwards default 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..52ba5f8ebe 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" } +# 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 diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index f13f909c34..6800c74187 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,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] { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Testsuite: Ensure pie is disabled on some tests 2019-03-19 15:45 ` Alan Hayward 2019-03-19 16:07 ` Simon Marchi @ 2019-03-19 16:36 ` Pedro Alves 1 sibling, 0 replies; 7+ messages in thread From: Pedro Alves @ 2019-03-19 16:36 UTC (permalink / raw) To: Alan Hayward, Simon Marchi; +Cc: gdb-patches, nd 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-20 10:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-06 10:20 [PATCH] Testsuite: Ensure pie is disabled on some tests Alan Hayward 2019-03-19 10:45 ` [PING][PATCH] " Alan Hayward 2019-03-19 13:48 ` [PATCH] " Simon Marchi 2019-03-19 15:45 ` Alan Hayward 2019-03-19 16:07 ` Simon Marchi 2019-03-20 10:36 ` Alan Hayward 2019-03-19 16:36 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox