Re: [PATCH] Fix TEXTREL in user targets when compiled with --enable-user-pie

On Tue, Dec 15, 2009 at 10:55 PM, Juan Quintela <quintela@trasno.org> wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>> Please, apply it.
>>
>> On Tue, Dec 15, 2009 at 3:10 AM, Kirill A. Shutemov
>> <kirill@shutemov.name> wrote:
>>> We really need compile _all_ sources for user target with -fpie when
>>> use --enable-user-pie.
>>>
>>> It's regression introduced by commit add16157d72454.
>>> Please, do not break it any more.
>
> Wishful thinking :p
> I bet that:
> a- Blue Swirl didn't do it on purpose
> b- our build system is very fragile, it will happens again.
>
> Learn to live with it.

:(

>>> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
>>> Cc: Blue Swirl <blauwirbel@gmail.com>
>>> ---
>>>  Makefile.user |    4 ++--
>>>  configure     |    1 +
>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Makefile.user b/Makefile.user
>>> index 907e74b..0be73df 100644
>>> --- a/Makefile.user
>>> +++ b/Makefile.user
>>> @@ -5,9 +5,9 @@ include $(SRC_PATH)/rules.mak
>>>
>>>  .PHONY: all
>>>
>>> -VPATH=$(SRC_PATH)
>>> +vpath %.c $(SRC_PATH)
>
> This change is needed?

Yes. We dont want to take %.o from $(SRC_PATH) since they built without -fpie.

>>>
>>> -QEMU_CFLAGS+=-I..
>>> +QEMU_CFLAGS+=-I$(SRC_PATH)/ $(CFLAGS_PIE)
>
> Why do you change it?
>
> QEMU_CFLAGS has this value in configure
> QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS"

Ok

>>>
>>>  obj-y =
>>>  obj-y += envlist.o path.o
>>> diff --git a/configure b/configure
>>> index 273b6b7..6200391 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2555,6 +2555,7 @@ if test "$target_user_only" = "yes" -a "$static" = "no" -a \
>>>        "$user_pie" = "yes" ; then
>>>   cflags="-fpie $cflags"
>>>   ldflags="-pie $ldflags"
>>> +  echo "CFLAGS_PIE=-fpie" >> $config_host_mak
>>>  fi
>>>
>>>  if test "$target_softmmu" = "yes" -a \( \
>>> --
>>> 1.6.5.6
>>>
>>>
>
> Can you test this other patch:

It doesn't work. It uses %.o from $(SRC_DIR). Updated version, based
on your patch:

From 5389617c7fa585126aab62c7a36e98116fa1f1aa Mon Sep 17 00:00:00 2001
From: Kirill A. Shutemov <kirill@shutemov.name>
Date: Wed, 16 Dec 2009 10:43:02 +0200
Subject: [PATCH] user_only: compile everything with -fpie

We really need compile _all_ sources for user target with -fpie when
use --enable-user-pie.

It's regression introduced by commit add16157d72454.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.user |    7 ++++++-
 configure     |    3 +++
 2 files changed, 9 insertions(+), 1 deletions(-)

On 12/16/2009 09:45 AM, Kirill A. Shutemov wrote:
>>>> >>>  -VPATH=$(SRC_PATH)
>>>> >>>  +vpath %.c $(SRC_PATH)
>> >
>> >  This change is needed?
> Yes. We dont want to take %.o from $(SRC_PATH) since they built without -fpie.

I think that's fine, it's kind of the purpose of vpath to only look for 
sources in the vpath; so it's safer to use "vpath" than "VPATH".  That 
said, I'd change it to

vpath %.c %.h $(SRC_PATH)

and I'd do so uniformly in all the directories that currently use VPATH.

Paolo

On Wed, Dec 16, 2009 at 11:15 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/16/2009 09:45 AM, Kirill A. Shutemov wrote:
>>>>>
>>>>> >>>  -VPATH=$(SRC_PATH)
>>>>> >>>  +vpath %.c $(SRC_PATH)
>>>
>>> >
>>> >  This change is needed?
>>
>> Yes. We dont want to take %.o from $(SRC_PATH) since they built without
>> -fpie.
>
> I think that's fine, it's kind of the purpose of vpath to only look for
> sources in the vpath; so it's safer to use "vpath" than "VPATH".  That said,
> I'd change it to
>
> vpath %.c %.h $(SRC_PATH)

We don't have make-targets which require %.h in Makefile.user, so it's noise.

>
> and I'd do so uniformly in all the directories that currently use VPATH.
>
> Paolo
>
>
>
>

"Kirill A. Shutemov" <kirill@shutemov.name> writes:

> On Wed, Dec 16, 2009 at 11:15 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 12/16/2009 09:45 AM, Kirill A. Shutemov wrote:
>>>>>>
>>>>>> >>>  -VPATH=$(SRC_PATH)
>>>>>> >>>  +vpath %.c $(SRC_PATH)
>>>>
>>>> >
>>>> >  This change is needed?
>>>
>>> Yes. We dont want to take %.o from $(SRC_PATH) since they built without
>>> -fpie.
>>
>> I think that's fine, it's kind of the purpose of vpath to only look for
>> sources in the vpath; so it's safer to use "vpath" than "VPATH".  That said,
>> I'd change it to
>>
>> vpath %.c %.h $(SRC_PATH)
>
> We don't have make-targets which require %.h in Makefile.user, so it's noise.

It may be noise now, but it the first guy who adds one there will be
grateful for it, and...

>> and I'd do so uniformly in all the directories that currently use VPATH.

... doing it uniformly makes more sense.