nanojit: avoid harmless race on nHints[]

RESOLVED FIXED

Status

Core Graveyard
Nanojit
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
Created attachment 517335 [details] [diff] [review]
patch
Attachment #517335 - Flags: review?(jseward)
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?
Attachment #517335 - Flags: review?(jseward) → review+
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> From the look of it this patch fixes all targets, yes?

Yes.
(Assignee)

Comment 4

7 years ago
http://hg.mozilla.org/projects/nanojit-central/rev/8ab31790e8a1
Whiteboard: fixed-in-nanojit
(Assignee)

Comment 5

7 years ago
http://hg.mozilla.org/tracemonkey/rev/1c206a26f9ff
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey

Comment 6

6 years ago
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
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.