This change allows compilation with...

This change allows compilation with
  "GYP_DEFINES: target_arch=x64 library=shared_library release_extra_cflags=-fPIC"

[Markus (顧孟勤), 2009-10-23 20:55:53]

This change makes the code compile in -fPIC mode, but I have no idea if the
change is actually correct. I don't quite understand what nacl_syscall.S does
and why you can use 32bit registers when accessing 64bit symbols. So, it is
quite possible my change needs some tweaking. Please review carefully.

[spotrh, 2009-12-17 19:54:56]

Doesn't resolve the issue for me, I still get:

/usr/bin/ld:
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/lib/libservice_runtime_x86_64.a(nacl_syscall.o):
relocation R_X86_64_32S against `nacl_user' can not be used when making a shared
object; recompile with -fPIC
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/lib/libservice_runtime_x86_64.a:
could not read symbols: Bad value

[Markus (顧孟勤), 2009-12-17 20:42:59]

Can you please dump the disassembly output and the relocation information and
see what code got generated? Ideally, you should do this both with "objdump" and
with "gcc -S".

[spotrh, 2009-12-18 06:18:06]

Okay. Lemme know if I did this wrong.

[spot@f10.x86-64 obj]$ objdump -Dr
./service_runtime_x86_64/service_runtime_x86_64/nacl_syscall.o

./service_runtime_x86_64/service_runtime_x86_64/nacl_syscall.o:     file format
elf64-x86-64


Disassembly of section .text:

0000000000000000 <NaClSyscallSeg>:
   0:   fc                      cld    
   1:   31 c0                   xor    %eax,%eax
   3:   66 8c e8                mov    %gs,%ax
   6:   c1 e8 03                shr    $0x3,%eax
   9:   67 8d 14 85 00 00 00    addr32 lea 0x0(,%eax,4),%edx
  10:   00 
                        d: R_X86_64_32S nacl_user
  11:   67 8b 12                addr32 mov (%edx),%edx
  14:   67 89 1a                addr32 mov %ebx,(%edx)
  17:   67 89 72 04             addr32 mov %esi,0x4(%edx)
  1b:   67 89 7a 08             addr32 mov %edi,0x8(%edx)
  1f:   67 89 6a 14             addr32 mov %ebp,0x14(%edx)
  23:   67 48 89 62 18          addr32 mov %rsp,0x18(%edx)
  28:   67 8d 14 85 00 00 00    addr32 lea 0x0(,%eax,4),%edx
  2f:   00 
                        2c: R_X86_64_32S        nacl_sys
  30:   67 8b 12                addr32 mov (%edx),%edx
  33:   67 8e 62 2c             addr32 mov 0x2c(%edx),%fs
  37:   67 8e 6a 2e             addr32 mov 0x2e(%edx),%gs
  3b:   67 8b 52 18             addr32 mov 0x18(%edx),%edx
  3f:   8e d1                   mov    %ecx,%ss
  41:   48 89 d4                mov    %rdx,%rsp
  44:   50                      push   %rax
  45:   e8 00 00 00 00          callq  4a <NaClSyscallSeg+0x4a>
                        46: R_X86_64_PC32       NaClSyscallCSegHook-0x4
  4a:   f4                      hlt    


[spot@f10.x86-64 obj]$ gcc -m64 -DNACL_BLOCK_SHIFT=5 -DNACL_BLOCK_SIZE=32
-DNACL_LINUX=1 -DNACL_OSX=0 -DNACL_WINDOWS=0 -D_BSD_SOURCE=1
-D_POSIX_C_SOURCE=199506 -D_XOPEN_SOURCE=600 -D_GNU_SOURCE=1
-D__STDC_LIMIT_MACROS=1 -DCHROMIUM_BUILD -DNACL_TARGET_SUBARCH=64
-DNACL_BUILD_SUBARCH=64 -DNDEBUG -DNVALGRIND
-I/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src -c -S
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S#
1
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S"
# 1 "<built-in>"
# 1 "<command-line>"
# 1
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S"
# 41
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S"
# 1
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/nacl_config.h"
1
# 38
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/nacl_config.h"
# 1
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/include/nacl_base.h"
1
# 39
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/nacl_config.h"
2
# 42
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S"
2

.globl NaClSyscallSeg
        .hidden NaClSyscallSeg
NaClSyscallSeg:





        cld
        xor %eax, %eax
        mov %gs, %ax
        shr $3, %eax




        lea nacl_user(,%eax,4), %edx



        movl 0(%edx), %edx
        movl %ebx, 0x0(%edx)
        movl %esi, 0x4(%edx)
        movl %edi, 0x8(%edx)
        movl %ebp, 0x14(%edx)
        mov %rsp, 0x18(%edx)
# 79
"/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S"
        lea nacl_sys(,%eax,4), %edx

        mov 0(%edx), %edx






        mov 0x2c(%edx), %fs
        mov 0x2e(%edx), %gs

        mov 0x18(%edx), %edx
        mov %ecx, %ss
        mov %rdx, %rsp
        push %rax
        call NaClSyscallCSegHook




        hlt

[Markus (顧孟勤), 2009-12-18 19:52:58]

You either didn't apply the patch to the correct source file, or the __PIC__
preprocessor symbol isn't defined for you. The latter should only happen if you
don't compile in -fPIC mode, I think. So, please check your compiler options.

[spotrh, 2009-12-18 21:47:09]

Okay, I forced -fPIC this time:

gcc -m64 -fPIC -DNACL_BLOCK_SHIFT=5 -DNACL_BLOCK_SIZE=32 -DNACL_LINUX=1
-DNACL_OSX=0 -DNACL_WINDOWS=0 -D_BSD_SOURCE=1 -D_POSIX_C_SOURCE=199506
-D_XOPEN_SOURCE=600 -D_GNU_SOURCE=1 -D__STDC_LIMIT_MACROS=1 -DCHROMIUM_BUILD
-DNACL_TARGET_SUBARCH=64 -DNACL_BUILD_SUBARCH=64 -DNDEBUG -DNVALGRIND
-I/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src -c -o
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/obj/service_runtime_x86_64/service_runtime_x86_64/nacl_switch.o
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_switch.S
gcc -m64 -fPIC -DNACL_BLOCK_SHIFT=5 -DNACL_BLOCK_SIZE=32 -DNACL_LINUX=1
-DNACL_OSX=0 -DNACL_WINDOWS=0 -D_BSD_SOURCE=1 -D_POSIX_C_SOURCE=199506
-D_XOPEN_SOURCE=600 -D_GNU_SOURCE=1 -D__STDC_LIMIT_MACROS=1 -DCHROMIUM_BUILD
-DNACL_TARGET_SUBARCH=64 -DNACL_BUILD_SUBARCH=64 -DNDEBUG -DNVALGRIND
-I/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src -c -o
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/obj/service_runtime_x86_64/service_runtime_x86_64/nacl_syscall.o
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S

This gets farther, but I am now hitting a new fPIC error:

flock
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/linker.lock
g++ -o
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/lib/libnacl.so
-L/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/lib
-pthread -Wl,--gc-sections -shared
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/obj/chrome/nacl/nacl/sel_main.os
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/obj/chrome/nacl/nacl/nacl_main.os
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/obj/chrome/nacl/nacl/nacl_thread.os
-Wl,--start-group -lX11 -lXrender -lXext -lrt -lpthread -lrt -lpthread -lrt
-lpthread -lrt -lpthread -lrt -lpthread -lrt -lpthread -lrt -lpthread -lrt
-lpthread -lrt -lpthread -lrt -lpthread -lrt -lpthread -lrt -lpthread -lrt
-lpthread -lrt -lpthread -lrt -lpthread -lrt -lpthread -lcommon -lglue
-lnpGoogleNaClPluginChrome -lsel -lncvalidate -lplatform_qual_lib -lnonnacl_srpc
-lnrd_xfer -lgoogle_nacl_imc_c -lplatform -lgio -lexpiration
-lnonnacl_util_chrome -lnonnacl_util_linux -lsel_ldr_launcher
-lgoogle_nacl_npruntime -lservice_runtime_x86 -lservice_runtime_x86_64
-Wl,--end-group
/usr/bin/ld:
/mnt/chromium/rpmbuild/BUILD/chromium-20091216svn34775/src/sconsbuild/Release/lib/libservice_runtime_x86_64.a(nacl_syscall.o):
relocation R_X86_64_PC32 against symbol `NaClSyscallCSegHook' can not be used
when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value
collect2: ld returned 1 exit status

Ideas? Do you want the gcc -S | objdump output?

[spotrh, 2009-12-18 21:48:10]

Also, for reference, this is the patch I am applying:

diff -up
chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S.shared
chromium-20091216svn34775/src/native_client/src/trusted/service_run$
---
chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S.shared
  2009-12-17 14:05:17.000000000 -0500
+++
chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/nacl_syscall.S
 2009-12-17 14:06:30.000000000 -0500
@@ -52,7 +52,12 @@ IDENTIFIER(NaClSyscallSeg):
         xor     %eax, %eax
         mov     %gs, %ax
         shr     $3, %eax
+#ifdef __PIC__
+        mov     IDENTIFIER(nacl_user)@GOTPCREL(%rip), %edx
+        lea     0(%edx,%eax,4), %edx
+#else
      	 lea     IDENTIFIER(nacl_user)(,%eax,4), %edx
+#endif

       	 /* check NaClThreadContext in sel_rt.h for the offsets */
         movl    0(%edx), %edx
@@ -67,7 +72,12 @@ IDENTIFIER(NaClSyscallSeg):
          * any registers having particular values.  we will clear/discard
          * caller-saved registers at system call return.
          */
+#ifdef __PIC__
+        mov     IDENTIFIER(nacl_sys)@GOTPCREL(%rip), %edx
+        lea     0(%edx,%eax,4), %edx
+#else
      	 lea     IDENTIFIER(nacl_sys)(,%eax,4), %edx
+#endif
       	 mov     0(%edx), %edx
         /*
          * %cs and %ds already taken care of by NaCl_trampoline_seg_code
diff -up
chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/service_runtime_x86_64.gyp.BAD
chromium-20091216svn34775/src/native_client/src/trusted/se$
---
chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/service_runtime_x86_64.gyp.BAD
 2009-12-18 15:17:32.000000000 -0500
+++
chromium-20091216svn34775/src/native_client/src/trusted/service_runtime/arch/x86_64/service_runtime_x86_64.gyp	2009-12-18
15:21:08.000000000 -0500
@@ -40,6 +40,13 @@
         'nacl_syscall.S',
         'sel_rt_64.c',
       ],
+      'conditions': [
+        [ 'library=="shared_library"', {
+            'asflags': [
+              '-fPIC',
+            ],
+        }],
+      ],
     },
   ],
 }

[khimg, 2009-12-21 10:37:38]

Translation looks sane to me, but one question arises: why are we using addr32
prefix everywhere? It don't buys us anyway except some slowdown!

After "lea ..., %edx" %rdx and %edx are the same and we can use (%rdx)
everywhere without worry - this generates smaller code and faster (not by much,
but faster - I've checked this when we've played with specCPU2000).

[tuduce, 2009-12-21 10:57:19]

On 2009/12/21 10:37:38, khim wrote:
> Translation looks sane to me, but one question arises: why are we using addr32
> prefix everywhere? It don't buys us anyway except some slowdown!

Victor, thank you for looking into this.

This code does not work as it is. I have a CL which changes the addr32
as well.

I think is OK to check this CL in as it is. My CL is not yet ready to be checked
in, and this code is anyway not executed right now.

Gregory and Ian know more than me about the impact of gyp changes though.

[tuduce, 2009-12-22 07:38:59]

LGTM

[sehr (please use chromium), 2010-01-07 17:23:37]

On 2009/12/22 07:38:59, tuduce wrote:
> LGTM

Markus,

We need to get Brad Nelson to look at the gyp changes.  Please add bradnelson to
the reviewers.

Second, you'll need to submit this change to the NaCl source tree, and then
someone will need to change the chrome DEPS file to have a revision number for
NaCl that includes your change.  I'm going to need to update the DEPS file in
the next couple of days, so if you let me know when your CL is in, I'll let you
know when the DEPS file change is reflected.

David

[Markus (顧孟勤), 2010-01-07 17:30:15]

I don't know how to check out and work in the NaCl tree. If you point me at the
right documentation, I can probably figure it out. But maybe it would be easier
if you or Brad just applied these two patches directly to your tree. There only
a very small number of lines that need to be changed, so this should be quite
easy to do.

[Evan Martin, 2010-02-15 13:19:57]

On 2010/01/07 17:30:15, Markus (顧孟勤) wrote:
> I don't know how to check out and work in the NaCl tree. If you point me at
the
> right documentation, I can probably figure it out. But maybe it would be
easier
> if you or Brad just applied these two patches directly to your tree. There
only
> a very small number of lines that need to be changed, so this should be quite
> easy to do.

Ping.  What became of this patch?

[Mark Seaborn, 2010-02-15 14:45:49]

I don't think this patch is necessary any more.

The code has since been changed to be position-independent by using PC-relative
addressing.  Making this conditional on __PIC__ is not necessary on x86-64.

See
http://code.google.com/p/nativeclient/source/browse/trunk/src/native_client/s...

http://codereview.chromium.org/332015/diff/7001/7003
File src/trusted/service_runtime/arch/x86_64/nacl_syscall.S (right):

http://codereview.chromium.org/332015/diff/7001/7003#newcode56
src/trusted/service_runtime/arch/x86_64/nacl_syscall.S:56: mov    
IDENTIFIER(nacl_user)@GOTPCREL(%rip), %edx
I'm not sure if this is correct.
It might need an extra dereference to fetch the address from the GOT.
In any case, going via the GOT is not necessary on x86-64, which allows
%rip-relative addressing.

http://codereview.chromium.org/332015/diff/7001/7003#newcode96
src/trusted/service_runtime/arch/x86_64/nacl_syscall.S:96: call   
IDENTIFIER(NaClSyscallCSegHook)@PLT
Calling via the PLT is not necessary to make this PIC.
CALL instructions are position independent anyway.
Calling via PLT is for cross-library calls.

[tuduce, 2010-02-16 08:23:38]

This patch was applied by
http://code.google.com/p/nativeclient/source/detail?r=1306
and latter changed as Mark described by
http://code.google.com/p/nativeclient/source/detail?r=1455
So, this issue can be closed.

[Evan Martin, 2010-02-16 09:03:14]

On 2010/02/16 08:23:38, tuduce wrote:
> This patch was applied by
> http://code.google.com/p/nativeclient/source/detail?r=1306
> and latter changed as Mark described by
> http://code.google.com/p/nativeclient/source/detail?r=1455
> So, this issue can be closed.

I will close this issue.

BTW, we still get a native client error in the shared build:
 /usr/local/google/home/eisinger/chrome/bin/ld: error:
out/Debug/obj.target/native_client/src/trusted/service_runtime/arch/x86_64/libservice_runtime_x86_64.a(nacl_syscall_64.o):
unsupported reloc 23 against local symbol
collect2: ld returned 1 exit status
make: * [out/Debug/lib.target/chrome/libnacl.so] Error 1
make: * Waiting for unfinished jobs....

Do you know where I should bring this up?