From patchwork Wed Mar 20 10:36:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 31910 Received: (qmail 125350 invoked by alias); 20 Mar 2019 10:36:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124812 invoked by uid 89); 20 Mar 2019 10:36:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=am, 2025 X-HELO: EUR03-AM5-obe.outbound.protection.outlook.com Received: from mail-eopbgr30070.outbound.protection.outlook.com (HELO EUR03-AM5-obe.outbound.protection.outlook.com) (40.107.3.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Mar 2019 10:36:27 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6cUP6mCE1h4PBBzwjYCAiBTaiP+eCWo82yXg3r57kgM=; b=Ldua+uyqmwD3aHbdAvlRz3gOdiyusgfRI4JVQE4zvN4lTsVLTt1ycdGMIDWsz7LShRawtTyQPT/btkpgPYpuQnq39t9z9kMMwqBNgLJQ8EtKgb1PVyn0utEdCk4dZRAs+GWNIs55UKcDnF+O61JhmJ4ABeD2N1/iQbJvRMG3z0c= Received: from AM4PR0802MB2129.eurprd08.prod.outlook.com (10.172.216.148) by AM4PR0802MB2180.eurprd08.prod.outlook.com (10.172.217.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1730.15; Wed, 20 Mar 2019 10:36:24 +0000 Received: from AM4PR0802MB2129.eurprd08.prod.outlook.com ([fe80::39ee:53d8:6053:d3fe]) by AM4PR0802MB2129.eurprd08.prod.outlook.com ([fe80::39ee:53d8:6053:d3fe%10]) with mapi id 15.20.1709.015; Wed, 20 Mar 2019 10:36:24 +0000 From: Alan Hayward To: Simon Marchi CC: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH] Testsuite: Ensure pie is disabled on some tests Date: Wed, 20 Mar 2019 10:36:24 +0000 Message-ID: <9F1CA01C-32AB-420F-9A4D-751D27264209@arm.com> References: <20190306102006.99150-1-alan.hayward@arm.com> <696945e9-4942-5285-0bb4-0bf899606f0a@simark.ca> <260313A3-D685-4306-8DE1-080D51BAE0BE@arm.com> In-Reply-To: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 Content-ID: MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-IsSubscribed: yes > On 19 Mar 2019, at 16:36, Pedro Alves 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 wrote: > > On 2019-03-19 11:45 a.m., Alan Hayward wrote: >>> On 19 Mar 2019, at 13:47, Simon Marchi 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 >>>> * 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 >> * 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] {