[ARM] Fix crash with sampling profiler
authordinfuehr@igalia.com <dinfuehr@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2019 07:15:57 +0000 (07:15 +0000)
committerdinfuehr@igalia.com <dinfuehr@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2019 07:15:57 +0000 (07:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194772

Reviewed by Mark Lam.

JSTests:

Do not skip test since crash with sampling profiler is now fixed.

* stress/sampling-profiler-richards.js:

Source/JavaScriptCore:

sampling-profiler-richards.js was crashing with an enabled sampling profiler. add32
did not update the stack pointer in a single instruction. The src register was first
moved into the stack pointer, the immediate imm was added in a subsequent instruction.

This was problematic when a signal handler was invoked before applying the immediate,
when the stack pointer is still set to the temporary value. Avoid this by calculating src+imm in
a temporary register and then move it in one go into the stack pointer.

* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::add32):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241756 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/sampling-profiler-richards.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

index 40e6e8d..5b40495 100644 (file)
@@ -1,3 +1,14 @@
+2019-02-18  Dominik Infuehr  <dinfuehr@igalia.com>
+
+        [ARM] Fix crash with sampling profiler
+        https://bugs.webkit.org/show_bug.cgi?id=194772
+
+        Reviewed by Mark Lam.
+
+        Do not skip test since crash with sampling profiler is now fixed.
+
+        * stress/sampling-profiler-richards.js:
+
 2019-02-18  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Add LazyClassStructure::getInitializedOnMainThread
index 5ea41bd..7cf82b3 100644 (file)
@@ -1,6 +1,4 @@
-// [JSC] [Armv7] stress/sampling-profiler-richards.js crashes
-// https://bugs.webkit.org/show_bug.cgi?id=190426
-//@ skip if ["arm", "mips"].include?($architecture) and $hostOS == "linux"
+//@ skip if $architecture == "mips" and $hostOS == "linux"
 //@ skip if $architecture == "x86"
 //@ runDefault("--collectContinuously=1", "--useSamplingProfiler=1", "--collectSamplingProfilerDataForJSCShell=1")
 
index 2dd3a1c..ea3fd40 100644 (file)
@@ -1,3 +1,21 @@
+2019-02-18  Dominik Infuehr  <dinfuehr@igalia.com>
+
+        [ARM] Fix crash with sampling profiler
+        https://bugs.webkit.org/show_bug.cgi?id=194772
+
+        Reviewed by Mark Lam.
+
+        sampling-profiler-richards.js was crashing with an enabled sampling profiler. add32
+        did not update the stack pointer in a single instruction. The src register was first
+        moved into the stack pointer, the immediate imm was added in a subsequent instruction.
+
+        This was problematic when a signal handler was invoked before applying the immediate,
+        when the stack pointer is still set to the temporary value. Avoid this by calculating src+imm in
+        a temporary register and then move it in one go into the stack pointer.
+
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::add32):
+
 2019-02-18  Mark Lam  <mark.lam@apple.com>
 
         Fix DFG doesGC() for CompareEq/Less/LessEq/Greater/GreaterEq and CompareStrictEq nodes.
index c1fbb47..559fc26 100644 (file)
@@ -177,15 +177,15 @@ public:
 
     void add32(TrustedImm32 imm, RegisterID src, RegisterID dest)
     {
-        ARMThumbImmediate armImm = ARMThumbImmediate::makeUInt12OrEncodedImm(imm.m_value);
-
-        // For adds with stack pointer destination, moving the src first to sp is
-        // needed to avoid unpredictable instruction
+        // For adds with stack pointer destination avoid unpredictable instruction
         if (dest == ARMRegisters::sp && src != dest) {
-            move(src, ARMRegisters::sp);
-            src = ARMRegisters::sp;
+            add32(imm, src, dataTempRegister);
+            move(dataTempRegister, dest);
+            return;
         }
 
+        ARMThumbImmediate armImm = ARMThumbImmediate::makeUInt12OrEncodedImm(imm.m_value);
+
         if (armImm.isValid())
             m_assembler.add(dest, src, armImm);
         else {