Last Comment Bug 623362 - nanojit: avoid harmless race on nHints[]
: nanojit: avoid harmless race on nHints[]
Status: RESOLVED FIXED
fixed-in-nanojit, fixed-in-tracemonkey
:
Product: Core Graveyard
Classification: Graveyard
Component: Nanojit (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Nicholas Nethercote [:njn]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-05 15:21 PST by Nicholas Nethercote [:njn]
Modified: 2014-03-17 08:00 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (2.45 KB, patch)
2011-03-06 19:09 PST, Nicholas Nethercote [:njn]
jseward: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2011-01-05 15:21:53 PST
nHints[] is a static variable.  It has a harmless race that Helgrind detected.  It's written by Assembler::nInit() and read by Assembler::hint(), and there's no locking.  It's harmless because the values written by Assembler::nInit() are always the same.  But it's worth fixing just to avoid the warning.

An easy fix would be to make it non-static.  It's one int32 per opcode, so the extra space is minimal, something like 512 bytes.  It would need to be manually zeroed, as well.
Comment 1 Nicholas Nethercote [:njn] 2011-03-06 19:09:07 PST
Created attachment 517335 [details] [diff] [review]
patch
Comment 2 Julian Seward [:jseward] 2011-03-14 06:46:35 PDT
Comment on attachment 517335 [details] [diff] [review]
patch

Yes, that appears to fix the race -- is no longer detectable.
I only checked on x64-linux, although I know this race also 
exists on arm and probably x86 too.  From the look of it this
patch fixes all targets, yes?
Comment 3 Nicholas Nethercote [:njn] 2011-03-15 01:40:03 PDT
(In reply to comment #2)
> From the look of it this patch fixes all targets, yes?

Yes.
Comment 4 Nicholas Nethercote [:njn] 2011-03-17 19:57:00 PDT
http://hg.mozilla.org/projects/nanojit-central/rev/8ab31790e8a1
Comment 5 Nicholas Nethercote [:njn] 2011-03-22 21:18:06 PDT
http://hg.mozilla.org/tracemonkey/rev/1c206a26f9ff
Comment 6 Tamarin Bot 2011-04-27 10:21:34 PDT
changeset: 6225:5b8d188e84a2
user:      Nicholas Nethercote <nnethercote@mozilla.com>
summary:   Bug 623362 - nanojit: avoid harmless race on nHints[].  r=jseward.

http://hg.mozilla.org/tamarin-redux/rev/5b8d188e84a2

Note You need to log in before you can comment on or make changes to this bug.