[webkit-reviews] review denied: [Bug 229235] REGRESSION(r279256): Crash in JSC::FTL::saveAllRegisters : [Attachment 436769] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 30 14:14:51 PDT 2021


Yusuke Suzuki <ysuzuki at apple.com> has denied Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 229235: REGRESSION(r279256): Crash in JSC::FTL::saveAllRegisters
https://bugs.webkit.org/show_bug.cgi?id=229235

Attachment 436769: Patch

https://bugs.webkit.org/attachment.cgi?id=436769&action=review




--- Comment #26 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 436769
  --> https://bugs.webkit.org/attachment.cgi?id=436769
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436769&action=review

>> Source/JavaScriptCore/ftl/FTLSaveRestore.cpp:93
>>	}
> 
> Hi Mark, does this look OK?

This fix seems not OK to me since we are returning non-valid register if we are
reaching to MacroAssembler::lastRegister() (and if it is special register).
Can you investigate what is happening here? Why are we exhausting the
registers?
The pasted backtrace is,

#0  __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1  0x00007fbfe89ab862 in __GI_abort () at abort.c:79
#2  0x00007fbfe6e0caca in std::__replacement_assert(char const*, int, char
const*, char const*) (__file=<optimized out>, __line=<optimized out>,
__function=<optimized out>, __condition=<optimized out>) at
/usr/include/c++/11.1.0/x86_64-pc-linux-gnu/bits/c++config.h:504
#3  0x00007fbfe776fb2c in std::array<unsigned int, 1ul>::operator[](unsigned
long) const (__n=<optimized out>, this=<optimized out>) at
/usr/include/c++/11.1.0/array:196
#4  WTF::Bitmap<32ul, unsigned int>::get(unsigned long, WTF::Dependency) const
(dependency=..., n=<optimized out>, this=<optimized out>) at
/usr/src/debug/build/WTF/Headers/wtf/Bitmap.h:164
#5  JSC::RegisterSet::get(JSC::Reg) const (reg=..., this=<optimized out>) at
/usr/src/debug/webkitgtk-2.33.3/Source/JavaScriptCore/jit/RegisterSet.h:99
#6  JSC::FTL::(anonymous namespace)::Regs::Regs (this=<optimized out>) at
/usr/src/debug/webkitgtk-2.33.3/Source/JavaScriptCore/ftl/FTLSaveRestore.cpp:83
#7  JSC::FTL::saveAllRegisters(JSC::AssemblyHelpers&, char*) (jit=...,
scratchMemory=scratchMemory at entry=0x7fbfe026e688 "pC_,\277\177") at
/usr/src/debug/webkitgtk-2.33.3/Source/JavaScriptCore/ftl/FTLSaveRestore.cpp:11
1


So, in FTLSaveRestore.cpp:111, we define

Regs regs;

and this Regs constructor is doing,

   77	  Regs()
   78	  {
   79	      special = RegisterSet::stackRegisters();
   80	      special.merge(RegisterSet::reservedHardwareRegisters());
   81
   82	      first = MacroAssembler::firstRegister();
   83	      while (special.get(first))
   84		  first = MacroAssembler::nextRegister(first);
   85	  }

from the code, this is impossible to get exceeding first register since it is
`MacroAssembler::firstRegister()`.
And we are not including all registers into special registers. So
MacroAssember::nextRegister must succeed.


More information about the webkit-reviews mailing list