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

[Mike Frysinger <vapier at gentoo dot org>]

On 22 Jun 2016 06:33, H.J. Lu wrote:
> On Wed, Jun 22, 2016 at 6:16 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On 22 Jun 2016 05:41, H.J. Lu wrote:
> >> 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
> >>
> >> This is what I checked in.
> >
> > there was still no reason to not write it the way i suggested.
> > it's shorter and simpler.
> 
> It is better to make ld and gold consistent just in case
> we may need to change.

that is entirely irrelevant.  neither ld nor gold had this change
merged which meant you could fix both at the same time.  in fact,
Nick asked you to fix bugs in the ld version which my suggestion
above already had fixed.
-mike

Attachment: signature.asc
Description: Digital signature