413767 - Make doGetObjectPrincipal() faster yet.

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect, P1)

Description

[Johnny Stenback (:jst)]

Attachment: Use faster class/parent/private accessors.

Now that the JS engine stores the JS class, parent, proto, and private (and a couple of other properties too) in pre-allocated slots (obj->fslots) it's ok to access them w/o any need to check whether we need to lock or what not. IOW, it's ok to access the values in the slots directly. Given that caps spends a good chunk of its time in getting at the JS class, private, and parent, it should use the fastest possible way to get to that info. Another nice benefit of this is that to access these slots, we don't need a JS context, which means we can drop the context argument to doGetObjectPrincipal(). See attached patch.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

How can we be sure that we're not going to end up racing with some random object whose private data gets set on a different thread?

Comment 2

[Brendan Eich [:brendan]]

Since when is caps threadsafe? But even so, there's no hazard on architectures we support, because word loads and stores are not split up into smaller atomic ops (cc'ing wtc to confirm).

It's true that if we do support MT objects, two may race and there's no barrier to order their stores to obj->fslots[3] strongly -- but such objects need higher-order synchronization than JS object (scope) locking in any event, since even with such locking, races to update an MT object's private slot can happen.

MT objects should not change privates, or if they do, you'll need a way to make sure racing readers don't get a pointer to the old private, and hang onto it after it is recycled or invalidated somehow.

/be

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 298826 [details] [diff] [review]
Use faster class/parent/private accessors.

That's fair.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 298826 [details] [diff] [review]
Use faster class/parent/private accessors.

Comment how for the given native private types (JSFunction, nsIXPConnectWrappedNative, and nsISupports of a WN) the private pointer does not change over the lifetime of the JS object, so can't dangle, etc. Thanks,

/be

Comment 5

[Wan-Teh Chang]

(In reply to comment #2)
> there's no hazard on architectures we support, because word
> loads and stores are not split up into smaller atomic ops.

I believe this is true if the word is aligned.  But if you need
to load the *current* value of a word, The PowerPC Architecture,
Appendix E.1.1 says you need to use a pair of atomic instructions
(lwarx + stwcx.).  (The book calls this operation "fetch and no-op".)
I guess if you don't care your loads must read the current values,
you can use regular load instructions.  (I don't fully understand
this subject.)

Comment 6

[Mike Schroepfer]

So we going to take this?

Comment 7

[Johnny Stenback (:jst)]

Yes, I'll land it as soon as I see green on tinderbox.

Comment 8

[Johnny Stenback (:jst)]

Fix checked in.