Android's VMPI atomic-integer operations return bad post-inc/dec values

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: siwilkin, Unassigned)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Android's atomic-integer API returns pre-inc/dec values, whereas VMPI returns post-inc/dec values. This patch fixes VMPI's (trivial) conversion which is currently incorrect.
(Reporter)

Comment 1

8 years ago
Created attachment 515548 [details] [diff] [review]
Initial Patch. MQ rev 248
Attachment #515548 - Flags: superreview?(edwsmith)
Attachment #515548 - Flags: review?(kpalacz)
(Reporter)

Updated

8 years ago
Blocks: 575544
(Reporter)

Comment 2

8 years ago
Created attachment 515551 [details] [diff] [review]
Rebased. MQ rev 249
Attachment #515548 - Attachment is obsolete: true
Attachment #515548 - Flags: superreview?(edwsmith)
Attachment #515548 - Flags: review?(kpalacz)
(Reporter)

Updated

8 years ago
Attachment #515551 - Flags: review?(kpalacz)
Attachment #515551 - Flags: review?(fklockii)
Attachment #515551 - Flags: review?(fklockii) → review+

Comment 3

8 years ago
changeset: 6009:b8166dce7f69
user:      Simon Wilkinson <siwilkin>
summary:   Bug 637233: Fixes return values for Android atomic-inc/dec-and-get (r=fklockii, r pending kpalacz)

http://hg.mozilla.org/tamarin-redux/rev/b8166dce7f69
(Reporter)

Updated

8 years ago
Attachment #515551 - Flags: superreview?(edwsmith)
Attachment #515551 - Flags: review?(kpalacz)
Attachment #515551 - Flags: review+

Comment 4

8 years ago
I think my comment on bug 629435 actually applies here, not there.. copied:

> I see that VMPI_atomicIncAndGet32WithBarrier() is being called in a loop to
> make sure the right number of increments happen, in ST_vmpi_threads.st.  But
> the test doesn't check the exact correctness of the returned value (before or
> after value)
> each time.
> 
> worth adding more tests?

Updated

8 years ago
Attachment #515551 - Flags: superreview?(edwsmith) → superreview+
(Reporter)

Comment 5

8 years ago
(In reply to comment #4)
> I think my comment on bug 629435 actually applies here, not there.. copied:
> 
> > I see that VMPI_atomicIncAndGet32WithBarrier() is being called in a loop to
> > make sure the right number of increments happen, in ST_vmpi_threads.st.  But
> > the test doesn't check the exact correctness of the returned value (before or
> > after value)
> > each time.
> > 
> > worth adding more tests?

Seeing as though I messed it up so royally the first time around, it's probably worth adding a selftest.
(Reporter)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.