[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] Simplify RELRO_LDFLAGS



On 08/20/2013 08:39 AM, Guido Günther wrote:
> by adding it to AM_LDFLAGS instead of every linking rule and
> by avoiding a forked grep.
> ---
> Daniel kind of nacked the AM_LDFLAGS part already but I think it's a
> reasonable cleanup. We should rather use AM_LDFLAGS everywhere which
> (we currently don't and which would be another cleanup). Or are there
> any reasons to not have a read only GOT?

Personally, I like the idea of using AM_LDFLAGS everywhere, but as
Daniel and I have opposing opinions, we'll need a third person to speak up.

> +++ b/m4/virt-linker-relro.m4
> @@ -22,10 +22,10 @@ AC_DEFUN([LIBVIRT_LINKER_RELRO],[
>      AC_MSG_CHECKING([for how to force completely read-only GOT table])
>  
>      RELRO_LDFLAGS=
> -    `$LD --help 2>&1 | grep -- "-z relro" >/dev/null` && \
> -        RELRO_LDFLAGS="-Wl,-z -Wl,relro"
> -    `$LD --help 2>&1 | grep -- "-z now" >/dev/null` && \
> -        RELRO_LDFLAGS="$RELRO_LDFLAGS -Wl,-z -Wl,now"
> +    case `$LD --help 2>&1` in
> +        *"-z relro"*) RELRO_LDFLAGS="-Wl,-z -Wl,relro" ;&

';&' is a bashism; it is not portable to configure scripts

> +        *"-z now"*)   RELRO_LDFLAGS="$RELRO_LDFLAGS -Wl,-z -Wl,now" ;;
> +    esac

Instead, this needs to be:

ld_help=`$LD --help 2>&1`
case $ld_help in
  *"-z relro"*) RELRO_LDFLAGS="-Wl,-z -Wl,relro" ;;
esac
case $ld_help in
  *"-z now"*) RELRO_LDFLAGS="$RELRO_LDFLAGS -Wl,-z -Wl,now" ;;
esac


> @@ -1812,7 +1814,6 @@ libvirt_la_LDFLAGS = \
>  		-version-info $(LIBVIRT_VERSION_INFO) \
>  		$(LIBVIRT_NODELETE) \
>  		$(AM_LDFLAGS) \
> -		$(RELRO_LDFLAGS) \
>  		$(CYGWIN_EXTRA_LDFLAGS) \
>  		$(MINGW_EXTRA_LDFLAGS) \
>  		$(NULL)

Hmm - maybe I see why Daniel expressed his opinion for not using
$(AM_LDFLAGS) - notice that libraries to NOT use $(PIE_LDFLAGS)...

> @@ -1964,7 +1963,6 @@ virtlockd_CFLAGS = \
>  virtlockd_LDFLAGS = \
>  		$(AM_LDFLAGS) \
>  		$(PIE_LDFLAGS) \
> -		$(RELRO_LDFLAGS) \
>  		$(CYGWIN_EXTRA_LDFLAGS) \
>  		$(MINGW_EXTRA_LDFLAGS) \
>  		$(NULL)

...but executables do.  But even then, maybe we want:

LIBRARY_LDFLAGS = $(AM_LDFLAGS)
EXEC_LDFLAGS = $(AM_LDFLAGS) $(PIE_LDFLAGS)

then use the appropriate $({LIBRARY,EXEC}_LDFLAGS) in each place, rather
than having lots of duplicate $(PIE_LDFLAGS)?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]