[Qemu-devel] [PATCH] Fix TEXTREL in user targets when compiled with --enable-user-pie

[Qemu-devel] [PATCH] Fix TEXTREL in user targets when compiled with --enable-user-pie

[Kirill A. Shutemov]

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.

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)
 
-QEMU_CFLAGS+=-I..
+QEMU_CFLAGS+=-I$(SRC_PATH)/ $(CFLAGS_PIE)
 
 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

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

[Kirill A. Shutemov]

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.
>
> 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)
>
> -QEMU_CFLAGS+=-I..
> +QEMU_CFLAGS+=-I$(SRC_PATH)/ $(CFLAGS_PIE)
>
>  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
>
>

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

[Juan Quintela]

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

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

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

>From bdc26201bb83df3786722a63c3caf387914f8539 Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Tue, 15 Dec 2009 21:53:26 +0100
Subject: [PATCH] user_only: compile everything with -fpie


Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 Makefile.user |    1 +
 configure     |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Makefile.user b/Makefile.user
index 907e74b..e4b5def 100644
--- a/Makefile.user
+++ b/Makefile.user
@@ -2,6 +2,7 @@

 include ../config-host.mak
 include $(SRC_PATH)/rules.mak
+include config.mak

 .PHONY: all

diff --git a/configure b/configure
index cf1db86..d815c7a 100755
--- a/configure
+++ b/configure
@@ -2652,3 +2652,6 @@ d=libuser
 mkdir -p $d
 rm -f $d/Makefile
 ln -s $source_path/Makefile.user $d/Makefile
+if test "$static" = "no" -a "$user_pie" = "yes" ; then
+  echo "QEMU_CFLAGS+=-fpie" > $d/config.mak
+fi
-- 
1.6.5.2

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

[Kirill A. Shutemov]

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

diff --git a/Makefile.user b/Makefile.user
index 907e74b..75c92d4 100644
--- a/Makefile.user
+++ b/Makefile.user
@@ -2,10 +2,15 @@

 include ../config-host.mak
 include $(SRC_PATH)/rules.mak
+include config.mak

 .PHONY: all

-VPATH=$(SRC_PATH)
+# Do not take %.o from $(SRC_PATH), only %.c
+# All %.o for user targets should be built with -fpie, when
+# configured with --enable-user-pie, so we don't want to
+# take %.o from $(SRC_PATH), since they built without -fpie
+vpath %.c $(SRC_PATH)

 QEMU_CFLAGS+=-I..

diff --git a/configure b/configure
index 273b6b7..5f463b0 100755
--- a/configure
+++ b/configure
@@ -2652,3 +2652,6 @@ d=libuser
 mkdir -p $d
 rm -f $d/Makefile
 ln -s $source_path/Makefile.user $d/Makefile
+if test "$static" = "no" -a "$user_pie" = "yes" ; then
+  echo "QEMU_CFLAGS+=-fpie" > $d/config.mak
+fi
-- 
1.6.5.6

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

[Paolo Bonzini]

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

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

[Kirill A. Shutemov]

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

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

[Markus Armbruster]

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