Bug 173573 – repatchIn needs to lock the CodeBlock's lock

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

Created attachment 313348 [details]
patch

[Yusuke Suzuki]

Comment on attachment 313348 [details]
patch

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

r=me

> Source/JavaScriptCore/jit/Repatch.cpp:560
>      if (tryRepatchIn(exec, base, ident, wasFound, slot, stubInfo) == GiveUpOnCache)
>          ftlThunkAwareRepatchCall(exec->codeBlock(), stubInfo.slowPathCallLocation(), operationIn);

BTW, how about passing the lock to these repatch functions? (const AbstractLocker&). Of course, it should be done in a separate patch b/c it involves other Get/Put changes.

[Saam Barati]

(In reply to Yusuke Suzuki from comment #2)
> Comment on attachment 313348 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313348&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jit/Repatch.cpp:560
> >      if (tryRepatchIn(exec, base, ident, wasFound, slot, stubInfo) == GiveUpOnCache)
> >          ftlThunkAwareRepatchCall(exec->codeBlock(), stubInfo.slowPathCallLocation(), operationIn);
> 
> BTW, how about passing the lock to these repatch functions? (const
> AbstractLocker&). Of course, it should be done in a separate patch b/c it
> involves other Get/Put changes.

Yup I’ll make a patch to do this. It took me a bit before I realized which functions were grabbing the lock. It’s not obvious that it’s held from some places.

[WebKit Commit Bot]

Comment on attachment 313348 [details]
patch

Clearing flags on attachment: 313348

Committed r218552: <http://trac.webkit.org/changeset/218552>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.