Patchwork [2/2] Build *-user targets as PIE

login
register
mail settings
Submitter Kirill A. Shutemov
Date Sept. 2, 2009, 3:21 p.m.
Message ID <1251904883-13706-2-git-send-email-kirill@shutemov.name>
Download mbox | patch
Permalink /patch/32829/
State Superseded
Headers show

Comments

Paolo Bonzini - Sept. 2, 2009, 2:24 p.m.
On 09/02/2009 05:21 PM, Kirill A. Shutemov wrote:
> Now we can drop link hack for i386 and fix text relocations on i386 host.

That's very nice---in fact on July 23rd I wrote:

 > BTW, maybe now the -Wl,-shared trick for self-virtualization can be
 > replaced with -fpie (position independent executable)?

so, thanks for doing that!

Paolo
Kirill A. Shutemov - Sept. 2, 2009, 2:35 p.m.
On Wed, Sep 2, 2009 at 5:24 PM, Paolo Bonzini<bonzini@gnu.org> wrote:
> On 09/02/2009 05:21 PM, Kirill A. Shutemov wrote:
>>
>> Now we can drop link hack for i386 and fix text relocations on i386 host.
>
> That's very nice---in fact on July 23rd I wrote:
>
>> BTW, maybe now the -Wl,-shared trick for self-virtualization can be
>> replaced with -fpie (position independent executable)?
>
> so, thanks for doing that!

I hope it will be applied...
Kirill A. Shutemov - Sept. 2, 2009, 3:21 p.m.
Now we can drop link hack for i386 and fix text relocations on i386 host.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 Makefile          |   10 +---------
 Makefile.target   |   19 +++++++++++++++----
 configure         |   17 ++++++-----------
 linux-user/main.c |   20 --------------------
 4 files changed, 22 insertions(+), 44 deletions(-)
Arnaud Patard (Rtp) - Sept. 2, 2009, 3:46 p.m.
"Kirill A. Shutemov" <kirill@shutemov.name> writes:

Hi,

[...]

> diff --git a/configure b/configure
> index 0d0162a..b501526 100755
> --- a/configure
> +++ b/configure
> @@ -2302,6 +2302,11 @@ if test "$target_softmmu" = "yes" ; then
>    esac
>  fi
>  
> +if test "$target_user_only" = "yes" -a "$static" = "no" ; then
> +  cflags="-fpie $cflags"
> +  ldflags="-pie $ldflags"
> +fi
> +

Please do that on per-arch basis. For instance, pie support tends to
be broken quite often on mips (afaik, it's currently broken on debian
unstable). I know qemu doesn't support mips host but it doesn't mean it
will never be supported - I need to find time to update to current git
and fix remaining bugs in my code before sending it for merge.
Unfortunately, this kind of patch will make sure it won't happen soon :(


Thanks,
Arnaud
Kirill A. Shutemov - Sept. 2, 2009, 3:52 p.m.
On Wed, Sep 2, 2009 at 6:46 PM, Arnaud Patard<arnaud.patard@rtp-net.org> wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
>
> Hi,
>
> [...]
>
>> diff --git a/configure b/configure
>> index 0d0162a..b501526 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2302,6 +2302,11 @@ if test "$target_softmmu" = "yes" ; then
>>    esac
>>  fi
>>
>> +if test "$target_user_only" = "yes" -a "$static" = "no" ; then
>> +  cflags="-fpie $cflags"
>> +  ldflags="-pie $ldflags"
>> +fi
>> +
>
> Please do that on per-arch basis. For instance, pie support tends to
> be broken quite often on mips (afaik, it's currently broken on debian
> unstable). I know qemu doesn't support mips host but it doesn't mean it
> will never be supported - I need to find time to update to current git
> and fix remaining bugs in my code before sending it for merge.
> Unfortunately, this kind of patch will make sure it won't happen soon :(
>
>
> Thanks,
> Arnaud
>

Will you happy if I provide option like --disable-user-pie to configure?

P.S. Sorry I forgot to put qemu-devil into CC.
Arnaud Patard (Rtp) - Sept. 2, 2009, 4:03 p.m.
"Kirill A. Shutemov" <kirill@shutemov.name> writes:

> On Wed, Sep 2, 2009 at 6:46 PM, Arnaud Patard<arnaud.patard@rtp-net.org> wrote:
>> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
>>
>> Hi,
>>
>> [...]
>>
>>> diff --git a/configure b/configure
>>> index 0d0162a..b501526 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2302,6 +2302,11 @@ if test "$target_softmmu" = "yes" ; then
>>>    esac
>>>  fi
>>>
>>> +if test "$target_user_only" = "yes" -a "$static" = "no" ; then
>>> +  cflags="-fpie $cflags"
>>> +  ldflags="-pie $ldflags"
>>> +fi
>>> +
>>
>> Please do that on per-arch basis. For instance, pie support tends to
>> be broken quite often on mips (afaik, it's currently broken on debian
>> unstable). I know qemu doesn't support mips host but it doesn't mean it
>> will never be supported - I need to find time to update to current git
>> and fix remaining bugs in my code before sending it for merge.
>> Unfortunately, this kind of patch will make sure it won't happen soon :(
>>
>>
>> Thanks,
>> Arnaud
>>
>
> Will you happy if I provide option like --disable-user-pie to configure?

yes, it's fine as I hope that pie support will be fixed.

>
> P.S. Sorry I forgot to put qemu-devil into CC.

qemu-_devil_ ? :)


Arnaud
Riku Voipio - Sept. 2, 2009, 6:54 p.m.
On Wed, Sep 02, 2009 at 06:21:23PM +0300, Kirill A. Shutemov wrote:
> +if test "$target_user_only" = "yes" -a "$static" = "no" ; then
> +  cflags="-fpie $cflags"
> +  ldflags="-pie $ldflags"
> +fi

Do we really need to hadcode this in? we have --extra-cflags and
--extra-ldflags for setting non-typical flags. Other than that,
looks like a nice cleanup.
Kirill A. Shutemov - Sept. 2, 2009, 6:59 p.m.
On Wed, Sep 2, 2009 at 8:34 PM, Juan Quintela<quintela@trasno.org> wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>> Now we can drop link hack for i386 and fix text relocations on i386 host.
>>
>> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
>
> Some comments.
>
> This patch moves files from being compiled only once (in Makefile),
> to be compiled for aech target.
>
>> ---
>>  Makefile          |   10 +---------
>>  Makefile.target   |   19 +++++++++++++++----
>>  configure         |   17 ++++++-----------
>>  linux-user/main.c |   20 --------------------
>>  4 files changed, 22 insertions(+), 44 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index bdac9b3..634ea81 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -39,8 +39,6 @@ subdir-%:
>>       $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,)
>>
>>  $(filter %-softmmu,$(SUBDIR_RULES)): libqemu_common.a
>> -$(filter %-user,$(SUBDIR_RULES)): libqemu_user.a
>> -
>>
>>  ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
>>  romsubdir-%:
>> @@ -74,7 +72,7 @@ block-obj-y +=  $(addprefix block/, $(block-nested-y))
>>  # CPUs and machines.
>>
>>  obj-y = $(block-obj-y)
>> -obj-y += readline.o console.no host-utils.o
>> +obj-y += readline.o console.o
>
> What is the problem here? libqemu_common.o is not used for *-user targets?

No.

We need all object files for *-user built with -fpie. For softmmu they
build without
it.

>>
>> +ifdef CONFIG_USER_ONLY
>> +# hack to compile with -fpie for *-user targets
>> +obj-y += cutils-user.o cache-utils-user.o
>> +cutils-user.c cache-utils-user.c:
>> +     @echo "  LN     $(TARGET_DIR)$@"
>> +     @ln -s $(SRC_PATH)/$(@:%-user.c=%.c) $@
>> +endif
>
> Why is this needed?  Why cutils.o/cache-utils.o is not enough?
>
> I thought that:
>
> obj-$(CONFIG_USER_ONLY) += cutils.o cache-utils.o
>
> should be enough here.  Why is needed the link?

We need to build it with -fpie. Without symlink it will be linked with
cutils.o and
cache-utils.o which was built for softmmu without -fpie and we will get text
relocation in executable.

Patch

diff --git a/Makefile b/Makefile
index bdac9b3..634ea81 100644
--- a/Makefile
+++ b/Makefile
@@ -39,8 +39,6 @@  subdir-%:
 	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,)
 
 $(filter %-softmmu,$(SUBDIR_RULES)): libqemu_common.a
-$(filter %-user,$(SUBDIR_RULES)): libqemu_user.a
-
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 romsubdir-%:
@@ -74,7 +72,7 @@  block-obj-y +=  $(addprefix block/, $(block-nested-y))
 # CPUs and machines.
 
 obj-y = $(block-obj-y)
-obj-y += readline.o console.o host-utils.o
+obj-y += readline.o console.o
 
 obj-y += irq.o ptimer.o
 obj-y += i2c.o smbus.o smbus_eeprom.o max7310.o max111x.o wm8750.o
@@ -161,12 +159,6 @@  bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
 
 libqemu_common.a: $(obj-y)
 
-#######################################################################
-# user-obj-y is code used by qemu userspace emulation
-user-obj-y = cutils.o cache-utils.o path.o envlist.o host-utils.o
-
-libqemu_user.a: $(user-obj-y)
-
 ######################################################################
 
 qemu-img.o: qemu-img-cmds.h
diff --git a/Makefile.target b/Makefile.target
index f7d1919..f738617 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -31,7 +31,7 @@  all: $(PROGS)
 
 #########################################################
 # cpu emulator library
-libobj-y = exec.o translate-all.o cpu-exec.o translate.o
+libobj-y = exec.o translate-all.o cpu-exec.o translate.o host-utils.o
 libobj-y += tcg/tcg.o tcg/tcg-runtime.o
 libobj-$(CONFIG_SOFTFLOAT) += fpu/softfloat.o
 libobj-$(CONFIG_NOSOFTFLOAT) += fpu/softfloat-native.o
@@ -80,9 +80,9 @@  ifdef CONFIG_LINUX_USER
 
 VPATH+=:$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
 QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user -I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
-
 obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
       elfload.o linuxload.o uaccess.o gdbstub.o gdbstub-xml.o
+obj-y += envlist.o path.o
 
 obj-$(TARGET_HAS_BFLT) += flatload.o
 obj-$(TARGET_HAS_ELFLOAD32) += elfload32.o
@@ -98,7 +98,7 @@  obj-arm-y += arm-semi.o
 
 obj-m68k-y += m68k-sim.o m68k-semi.o
 
-ARLIBS=../libqemu_user.a libqemu.a
+ARLIBS=libqemu.a
 endif #CONFIG_LINUX_USER
 
 #########################################################
@@ -116,6 +116,7 @@  LIBS+=-lmx
 
 obj-y = main.o commpage.o machload.o mmap.o signal.o syscall.o thunk.o \
         gdbstub.o gdbstub-xml.o
+obj-y += envlist.o path.o
 
 obj-i386-y += ioport-user.o
 
@@ -133,13 +134,23 @@  QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH)
 
 obj-y = main.o bsdload.o elfload.o mmap.o signal.o strace.o syscall.o \
         gdbstub.o gdbstub-xml.o uaccess.o
+obj-y += envlist.o path.o
 
 obj-i386-y += ioport-user.o
 
-ARLIBS=libqemu.a ../libqemu_user.a
+ARLIBS=libqemu.a
 
 endif #CONFIG_BSD_USER
 
+ifdef CONFIG_USER_ONLY
+# hack to compile with -fpie for *-user targets
+obj-y += cutils-user.o cache-utils-user.o
+cutils-user.c cache-utils-user.c:
+	@echo "  LN	$(TARGET_DIR)$@"
+	@ln -s $(SRC_PATH)/$(@:%-user.c=%.c) $@
+endif
+
+
 #########################################################
 # System emulator target
 ifdef CONFIG_SOFTMMU
diff --git a/configure b/configure
index 0d0162a..b501526 100755
--- a/configure
+++ b/configure
@@ -2302,6 +2302,11 @@  if test "$target_softmmu" = "yes" ; then
   esac
 fi
 
+if test "$target_user_only" = "yes" -a "$static" = "no" ; then
+  cflags="-fpie $cflags"
+  ldflags="-pie $ldflags"
+fi
+
 if test "$target_softmmu" = "yes" -a \( \
         "$TARGET_ARCH" = "microblaze" -o \
         "$TARGET_ARCH" = "cris" \) ; then
@@ -2323,16 +2328,6 @@  fi
 linker_script="-Wl,-T../config-host.ld -Wl,-T,\$(SRC_PATH)/\$(ARCH).ld"
 if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   case "$ARCH" in
-  i386)
-    if test "$gprof" = "yes" -o "$static" = "yes" ; then
-      ldflags="$linker_script $ldflags"
-    else
-      # WARNING: this LDFLAGS is _very_ tricky : qemu is an ELF shared object
-      # that the kernel ELF loader considers as an executable. I think this
-      # is the simplest way to make it self virtualizable!
-      ldflags="-Wl,-shared $ldflags"
-    fi
-    ;;
   sparc)
     # -static is used to avoid g1/g3 usage by the dynamic linker
     ldflags="$linker_script -static $ldflags"
@@ -2340,7 +2335,7 @@  if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   ia64)
     ldflags="-Wl,-G0 $linker_script -static $ldflags"
     ;;
-  x86_64|ppc|ppc64|s390|sparc64|alpha|arm|m68k|mips|mips64)
+  i386|x86_64|ppc|ppc64|s390|sparc64|alpha|arm|m68k|mips|mips64)
     ldflags="$linker_script $ldflags"
     ;;
   esac
diff --git a/linux-user/main.c b/linux-user/main.c
index a628c01..d3af2e2 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -54,26 +54,6 @@  const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 const char interp[] __attribute__((section(".interp"))) = "/lib/ld-linux.so.2";
 #endif
 
-/* for recent libc, we add these dummy symbols which are not declared
-   when generating a linked object (bug in ld ?) */
-#if (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3)) && !defined(CONFIG_STATIC)
-asm(".globl __preinit_array_start\n"
-    ".globl __preinit_array_end\n"
-    ".globl __init_array_start\n"
-    ".globl __init_array_end\n"
-    ".globl __fini_array_start\n"
-    ".globl __fini_array_end\n"
-    ".section \".rodata\"\n"
-    "__preinit_array_start:\n"
-    "__preinit_array_end:\n"
-    "__init_array_start:\n"
-    "__init_array_end:\n"
-    "__fini_array_start:\n"
-    "__fini_array_end:\n"
-    ".long 0\n"
-    ".previous\n");
-#endif
-
 /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
    we allocate a bigger stack. Need a better solution, for example
    by remapping the process stack directly at the right place */