Re: [PATCH] PR ld/20283: gold: Add a linker configure option --enable-relro

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Jun 22, 2016 at 3:43 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On 21 Jun 2016 19:45, H.J. Lu wrote:
>> On Tue, Jun 21, 2016 at 6:06 PM, Mike Frysinger wrote:
>> > On 21 Jun 2016 15:11, H.J. Lu wrote:
>> >> +# Decide if -z relro should be enabled in ELF linker by default.
>> >> +ac_default_ld_z_relro=unset
>> >> +# Provide a configure time option to override our default.
>> >> +AC_ARG_ENABLE(relro,
>> >> +           AS_HELP_STRING([--enable-relro],
>> >> +           [enable -z relro in ELF linker by default]),
>> >> +[case "${enableval}" in
>> >> +  no)  ac_default_ld_z_relro=0 ;;
>> >> +esac])dnl
>> >> +if test ${ac_default_ld_z_relro} = unset; then
>> >> +  ac_default_ld_z_relro=1
>> >> +fi
>> >
>> > any reason to not just write it like:
>> > AC_ARG_ENABLE(relro,
>> >         AS_HELP_STRING([--enable-relro],
>> >                 [enable -z relro in ELF linker by default]))
>> > if test "${enable_relro}" = "yes"; then
>> >   ac_default_ld_z_relro=1
>> > else
>> >   ac_default_ld_z_relro=0
>> > fi
>> >
>> > it's a bit simpler that way.
>>
>> I copied it from ld where ac_default_ld_z_relro is set to 1 unless
>> --disable-relro is used or not a Linux target. For gold, it becomes
>> unless --disable-relro is used.  It is easier for me to keep both ld
>> and gold similar.
>
> except that patch isn't merged yet, so you have time to change it too
> -mike

This is what I checked in.

-- 
H.J.

From eadfbc61639dfb967a3538e2ff5d465071569690 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 21 Jun 2016 14:17:06 -0700
Subject: [PATCH 2/2] gold: Add a linker configure option --enable-relro

Add a configure option --enable-relro to decide whether -z relro should
be enabled by default.  Default to yes.

	PR ld/20283
	* NEWS: Mention --enable-relro.
	* configure.ac: Add --enable-relro.
	(DEFAULT_LD_Z_RELRO): New.  Set by --enable-relro and default
	to 1.
	* config.in: Regenerated.
	* configure: Likewise.
	* options.h (General_options::relro): Default to
	DEFAULT_LD_Z_RELRO.
---
 gold/NEWS         |  3 +++
 gold/config.in    |  3 +++
 gold/configure    | 21 +++++++++++++++++++++
 gold/configure.ac | 17 +++++++++++++++++
 gold/options.h    |  2 +-
 5 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/gold/NEWS b/gold/NEWS
index 6f2cf3f..a400489 100644
--- a/gold/NEWS
+++ b/gold/NEWS
@@ -1,5 +1,8 @@
 Changes in 1.12:
 
+* Add a configure option --enable-relro to decide whether -z relro should
+  be enabled by default.  Default to yes.
+
 * Add support for s390, MIPS, AArch64, and TILE-Gx architectures.
 
 * Add support for STT_GNU_IFUNC symbols.
diff --git a/gold/configure.ac b/gold/configure.ac
index 695a2fd..90e2c22 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -114,6 +114,23 @@ if test "$plugins" = "yes"; then
 fi
 AM_CONDITIONAL(PLUGINS, test "$plugins" = "yes")
 
+# Decide if -z relro should be enabled in ELF linker by default.
+ac_default_ld_z_relro=unset
+# Provide a configure time option to override our default.
+AC_ARG_ENABLE(relro,
+	      AS_HELP_STRING([--enable-relro],
+	      [enable -z relro in ELF linker by default]),
+[case "${enableval}" in
+  yes)  ac_default_ld_z_relro=1 ;;
+  no)  ac_default_ld_z_relro=0 ;;
+esac])dnl
+if test "${ac_default_ld_z_relro}" = unset; then
+  ac_default_ld_z_relro=1
+fi
+AC_DEFINE_UNQUOTED(DEFAULT_LD_Z_RELRO,
+  $ac_default_ld_z_relro,
+  [Define to 1 if you want to enable -z relro in ELF linker by default.])
+
 AC_ARG_ENABLE([targets],
 [  --enable-targets        alternative target configurations],
 [case "${enableval}" in
diff --git a/gold/options.h b/gold/options.h
index 877e6d1..23c9658 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1336,7 +1336,7 @@ class General_options
   DEFINE_bool(origin, options::DASH_Z, '\0', false,
 	      N_("Mark DSO to indicate that needs immediate $ORIGIN "
 		 "processing at runtime"), NULL);
-  DEFINE_bool(relro, options::DASH_Z, '\0', false,
+  DEFINE_bool(relro, options::DASH_Z, '\0', DEFAULT_LD_Z_RELRO,
 	      N_("Where possible mark variables read-only after relocation"),
 	      N_("Don't mark variables read-only after relocation"));
   DEFINE_bool(text, options::DASH_Z, '\0', false,
-- 
2.5.5