Closed Bug 1278237 Opened 8 years ago Closed 8 years ago

TSan: data races security/nss/lib/freebl/mpi/mpi.c on mp_allocs, mp_frees, mp_copies

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jseward, Assigned: franziskus)

Details

Attachments

(3 files, 1 obsolete file)

Attached file TSan complaints
security/nss/lib/freebl/mpi/mpi.c has

  unsigned long mp_allocs;
  unsigned long mp_frees;
  unsigned long mp_copies;

These are incremented from different threads without any locking and 
cause TSan to complain.  I have no specific STR, because these races
are flagged quite quickly in "normal" surfing with a TSan-enabled build.
The obvious fix is to make them atomic.  I am not really sure
how to do that in portable C11, though.  Is the _Atomic qualifier
the right thing?
I think libmpi was never intended to be used this way. But since we're doing it we have to figure something out here.
Unfortunately we also have to work with C99 so _Atomic probably won't work (though it's available in gcc48).

Looking at what this code is doing; all three variables are only incremented but never really used as far as I can see. We can probably simply remove them. (They're used in a test that's not used to print memory allocations/frees/copies.)
Assignee: nobody → nobody
Component: Security → Libraries
Product: Core → NSS
Target Milestone: --- → 3.25
Version: unspecified → trunk
Attached patch remove_mp_counts.patch (obsolete) — Splinter Review
Assignee: nobody → franziskuskiefer
Attachment #8760263 - Flags: review?(ttaubert)
more clean up
Attachment #8760263 - Attachment is obsolete: true
Attachment #8760263 - Flags: review?(ttaubert)
Attachment #8760291 - Flags: review?(ttaubert)
Attachment #8760291 - Flags: review?(ttaubert) → review+
landed as https://hg.mozilla.org/projects/nss/rev/403152fc6621
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
jseward: the copy of NSS used in mozilla-central is periodically updated, so you'll need to wait for the next update to see this change in your TSan runs.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: