Remove interior pointer workarounds from String code

RESOLVED FIXED in Q1 12 - Brannan

Status

P4
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: edwsmith, Assigned: lhansen)

Tracking

unspecified
Q1 12 - Brannan
Bug Flags:
flashplayer-qrb +

Details

(Whiteboard: PACMAN)

(Reporter)

Description

9 years ago
Bug 541338 has made the GC able to pin objects when interior pointers to them are found on the stack.  This should enable us to remove workarounds from the String code that aimed to preserve the header pointer to string objects.  (grep for volatile in the code)
(Reporter)

Updated

9 years ago
Depends on: 541338
(Reporter)

Updated

9 years ago
Target Milestone: --- → Future
(Assignee)

Updated

9 years ago
Whiteboard: PACMAN

Updated

8 years ago
Summary: Remove interior poniter workarounds from String code → Remove interior pointer workarounds from String code
(Assignee)

Comment 1

8 years ago
Appears to apply only to 

    Stringp const volatile  m_str;

in StringIndexer in StringObject.h.  Since there are no comments there pointing to the bug that forced the 'volatile' to be inserted, it's tricky to exactly verify that we can remove it, but I'd be happy to offer a patch to do that...

Updated

8 years ago
Assignee: nobody → stejohns
Flags: flashplayer-qrb+

Comment 2

7 years ago
Lars, please submit the patch and close this out.
Assignee: stejohns → lhansen
Priority: -- → P4
Target Milestone: Future → Q1 12 - Brannan
(Assignee)

Comment 3

7 years ago
Some searching in Bugzilla has failed to turn up any bug that might be the source of the 'volatile' being added, so I'm just going to remove it and we're going to wait and see if our test suite trips over it on any platform, which is probably what happened last time.
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 4

7 years ago
changeset: 6561:d72ea908d9f0
user:      Lars T Hansen <lhansen@adobe.com>
summary:   Fix 546402 - Remove interior pointer workarounds from String code: strip out a now-redundant 'volatile' (authorization=dansmith)

http://hg.mozilla.org/tamarin-redux/rev/d72ea908d9f0

Comment 5

7 years ago
(In reply to Lars T Hansen from comment #3)
> Some searching in Bugzilla has failed to turn up any bug that might be the
> source of the 'volatile' being added, so I'm just going to remove it and
> we're going to wait and see if our test suite trips over it on any platform,
> which is probably what happened last time.

This is the changeset that introduced the volatile:
http://hg.mozilla.org/tamarin-redux/rev/8b3c7bde9273


changeset:   2418:8b3c7bde9273
user:        Steven Johnson <stejohns@adobe.com>
date:        Wed Sep 02 10:53:41 2009 -0700
files:       core/MultinameHashtable.h core/StringObject.h core/Traits.h
description:
StMNHTIterator needs to keep a reference to the MNHT it is using to keep it
from being collected.

StMNHTIterator, StTraitsBindingsIterator, StringIndexer all need their "keeper" fields to be marked volatile; turns out that agressive optimizing compilers can notice that these fields aren't ever used and omit the store. (r=treilly)
You need to log in before you can comment on or make changes to this bug.