s390: v8tests.cctest/test-platform/StackAlignment in v8 tests

[mhdawson]

• Version: master
• Platform: linux/s390
• Subsystem: dep/v8

Just added s390 to the v8 tests we run nightly. Shows failure in these tests:
https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel72-s390x,v8test=v8test/196/

We don't see these failures in the google sim runs for 390 nor do we see them in our internal builds on real hardware for 5.1

=== cctest/test-platform/StackAlignment ===
Command: /data/iojs/build/workspace/node-test-commit-v8-linux/nodes/rhel72-s390x/v8test/v8test/deps/v8/out/s390x.release/cctest --random-seed=2016406410 test-platform/StackAlignment --nohard-abort --nodead-code-elimination --nofold-constants
exit code: -11
--- CRASHED ---
=== cctest/test-platform/StackAlignment ===
Command: /data/iojs/build/workspace/node-test-commit-v8-linux/nodes/rhel72-s390x/v8test/v8test/deps/v8/out/s390x.release/cctest --random-seed=2016406410 --stress-opt --always-opt test-platform/StackAlignment --nohard-abort --nodead-code-elimination --nofold-constants
exit code: -11
--- CRASHED ---
=== cctest/test-platform/StackAlignment ===
Command: /data/iojs/build/workspace/node-test-commit-v8-linux/nodes/rhel72-s390x/v8test/v8test/deps/v8/out/s390x.release/cctest --random-seed=2016406410 --turbo test-platform/StackAlignment --nohard-abort --nodead-code-elimination --nofold-constants
exit code: -11
--- CRASHED ---

[john-yan]

Looks like the test is checking stack alignment requirement is met. I could not reproduce it on our systems.

[bnoordhuis]

Compiler issue, maybe? The test executes this on s390x:

__asm__ __volatile__("stg 15, %0" : "=g"(sp_addr));

Perhaps it solves the constraint in a way that emits wrong/illegal code?

@mhdawson If you can get a disassembly of the GetStackPointer() function, that might be helpful.

[joransiu]

Managed to reproduce this on a local system. @bnoordhuis was on the right track.

   0x80436278 <_Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE>:   stmg    %r13,%r15,104(%r15)
   0x8043627e <_Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE+6>: lay %r15,-168(%r15)
=> 0x80436284 <_Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE+12>:    stg %r15,-450104

We crashed on the STG, which ended up generating the wrong memory reference... -450104 doesn't look right at all. Need to dig into how the compiler expanded the sp_addr in the inlined asm.

GCC compiler level:

gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-4)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[joransiu]

Looking at the object file, we do see a reloc entry for the stg to reference sp_addr. Except, it's uncertain how you can manage to reference the static without a proper base register... it's as if the compiler has confused stg's memory operand as a relative offset instead.

Disassembly of section .bss._ZL7sp_addr:

0000000000000000 <_ZL7sp_addr>:
        ...

Disassembly of section .text._Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE:

0000000000000000 <_Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE>:
   0:   eb bf f0 58 00 24       stmg    %r11,%r15,88(%r15)
   6:   e3 f0 ff 20 ff 71       lay     %r15,-224(%r15)
   c:   b9 04 00 bf             lgr     %r11,%r15
  10:   e3 20 b0 a0 00 24       stg     %r2,160(%r11)
  16:   e3 f0 00 00 00 24       stg     %r15,0
                        18: R_390_20    .bss._ZL7sp_addr

Interestingly, compiling with GCC 4.8.3, we get a slightly different sequence with an extra larl instruction that is a relative instruction that can address sp_addr.

0000000000000000 <_Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE>:
   0:   eb df f0 68 00 24       stmg    %r13,%r15,104(%r15)
   6:   c0 10 00 00 00 00       larl    %r1,6 <_Z15GetStackPointerRKN2v820FunctionCallbackInfoINS_5ValueEEE+0x6>
                        8: R_390_PC32DBL        .bss._ZL7sp_addr+0x2
   c:   a7 fb ff 58             aghi    %r15,-168
  10:   e3 f0 10 00 00 24       stg     %r15,0(%r1)

[bnoordhuis]

Would changing the constraint from =g to =r suffice as a workaround for now? Untested:

diff --git a/deps/v8/test/cctest/test-platform.cc b/deps/v8/test/cctest/test-platform.cc
index 6012fd4..abcb475 100644
--- a/deps/v8/test/cctest/test-platform.cc
+++ b/deps/v8/test/cctest/test-platform.cc
@@ -25,7 +25,7 @@ void GetStackPointer(const v8::FunctionCallbackInfo<v8::Value>& args) {
 #elif V8_HOST_ARCH_MIPS64
   __asm__ __volatile__("sd $sp, %0" : "=g"(sp_addr));
 #elif defined(__s390x__) || defined(_ARCH_S390X)
-  __asm__ __volatile__("stg 15, %0" : "=g"(sp_addr));
+  __asm__ __volatile__("stg 15, %0" : "=r"(sp_addr));
 #elif defined(__s390__) || defined(_ARCH_S390)
   __asm__ __volatile__("st 15, %0" : "=g"(sp_addr));
 #elif defined(__PPC64__) || defined(_ARCH_PPC64)

[joransiu]

I believe what we want instead is =m, to force the sp_addr to be a memory operand.

[joransiu]

Sorry, was swamped today with other stuff. I've confirmed that =m constraint does resolve the issue. In this case, it's the more proper choice, given the specific assembly instruction (stg) we are trying to use. We don't have to rely on the compiler to try to work out which of the operand variants to treat the 'general' constraint.

I'd like to make the change to this into V8 as well, and will float it back over once it's committed there.

[joransiu]

V8 CL .. pending approval: https://codereview.chromium.org/2158523002/