848602 - Add some random volatile annotations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

These fields are all racy. These annotations will protect us against a theoretical evil-genius compiler whose goal is to introduce bugs into every program it compiles.

Comment 1

[Terrence Cole [:terrence]]

Comment on attachment 721946 [details] [diff] [review]
patch

Review of attachment 721946 [details] [diff] [review]:
-----------------------------------------------------------------

Yup. Did you check if this makes us any slower?

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ed74a9e1a3e

If this makes us slower, I'll eat my hat!

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Something in this push caused bustage. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2da17db2a304

https://tbpl.mozilla.org/php/getParsedLog.php?id=20443040&tree=Mozilla-Inbound

In file included from ../../../js/src/jsalloc.h:11:0,
                 from ../../../js/src/jsalloc.cpp:8:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsalloc.o] Error 1
make[5]: *** Waiting for unfinished jobs....
In file included from ../../../js/src/jsalloc.h:11:0,
                 from ../../../js/src/jsapi.h:24,
                 from ../../../js/src/jscntxt.h:19,
                 from ../../../js/src/jscompartment.h:15,
                 from ../../../js/src/jsanalyze.h:15,
                 from ../../../js/src/jsanalyze.cpp:9:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsanalyze.o] Error 1
In file included from ../../../js/src/jsarray.cpp:18:0:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsarray.o] Error 1
In file included from ../../../js/src/jsapi.cpp:22:0:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsapi.o] Error 1

Comment 4

[Nicholas Nethercote [inactive]]

A non-working browser is a slower browser!  billm, time to start chewing.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Well, that was embarrassing. This time I checked that it compiles and doesn't regress performance (at least locally).

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

Comment 6

[Terrence Cole [:terrence]]

Nah, it executed in 0 time, that's clearly faster!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Julian Seward [:jseward]]

What effect do you expect the added volatiles to have?  My understanding
is that they are basically useless for inter-thread synchronisation.

http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming

Compilers are allowed to reorder ordinary memory references with respect to
volatile references, so it won't help make code any more correct in that
respect.