1017154 - Fix RegExpShared read barriers (again)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

I don't think bug 1012642 was the right fix for RegExpShared read barriers when accessed from objects.  That bug added a barrier when copying RegExpShared pointers from one RegExpObject to another, but no barrier is tripped when getting the RegExpShared from an object to actually execute the regexp.

In this case there will be a RegExpGuard on the stack, which will trace the RegExpShared during root marking but which won't do anything during sweep.  So we could run through these steps to destroy the RegExpShared while it is still in use.

- start an IGC outside the RegExpGuard
- create the RegExpGuard, get the RegExpShared
- trace the RegExpObject and clear the pointer to its RegExpShared
- sweep, with the RegExpGuard still on the stack

Before bug 1010441 the RegExpGuard prevented the RegExpShared from being destroyed during sweep, but didn't prevent its JitCode from being destroyed.  So this is really a regression from the initial irregexp landing, bug 976446, before which the RegExpShared owned its jitcode outright.

I'm hoping that fixing this will fix bug 1013686 and bug 1013589.

Comment 1

[Brian Hackett [Laid off!]]

Oops, that first bug should be bug 1013586.

Comment 2

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9386b73250c

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/mozilla-central/rev/b9386b73250c

Comment 4

[Matt Wobensmith [:mwobensmith][:matt:]]

Brian, is there anything that can be done to verify this fix? I'm looking for a bug file or test. I've looked at the related bugs but I'm unsure what applies here.

If not, that's fine, just let me know. Thank you.

Comment 5

[Brian Hackett [Laid off!]]

(In reply to Matt Wobensmith from comment #4)
> Brian, is there anything that can be done to verify this fix? I'm looking
> for a bug file or test. I've looked at the related bugs but I'm unsure what
> applies here.
> 
> If not, that's fine, just let me know. Thank you.

Not easily, I found this by code inspection and never wrote a testcase (which can be hard to do with GC timing stuff).

Comment 6

[Matt Wobensmith [:mwobensmith][:matt:]]

OK, thanks Brian. I'm going to go ahead and cross it off of our list for verification, based on this info. Much appreciated.