Add some random volatile annotations

RESOLVED FIXED in mozilla22

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

unspecified
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 721946 [details] [diff] [review]
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.
Attachment #721946 - Flags: review?(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?
Attachment #721946 - Flags: review?(terrence) → review+
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
A non-working browser is a slower browser!  billm, time to start chewing.
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
Nah, it executed in 0 time, that's clearly faster!
https://hg.mozilla.org/mozilla-central/rev/ba0702b7890d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
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.
You need to log in before you can comment on or make changes to this bug.