[libvirt] [PATCH 1/2] Simplify RELRO_LDFLAGS

[Guido Günther]

On Tue, Aug 20, 2013 at 01:23:18PM -0600, Eric Blake wrote:
> 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.

O.k. I think we lose more by not having eqaul AM_LDFLAGS everywhere
since there are already several places that did lack some flags since
it's error prone to maintain all locations correctly.

> 
> > +++ 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)

That would be fine with me but I'd do this as a separate cleanup since
AM_LDFLAGS currently only carries flags suitable for both lib and
executable builds.
Cheers,
 -- Guido

> 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
>