set/delete sequence assert fails (and breaks in operator)

RESOLVED FIXED in Q3 11 - Serrano

Status

Tamarin
Virtual Machine
P2
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: pnkfelix, Unassigned)

Tracking

unspecified
Q3 11 - Serrano
x86
Mac OS X
Dependency tree / graph
Bug Flags:
flashplayer-bug +

Details

Attachments

(1 attachment, 3 obsolete attachments)

Created attachment 439056 [details]
if this prints whaa, something is wrong

the following code:

    var arr = []
    
    arr[1] = 1
    delete arr[1]
    
    arr[0] = 1
    delete arr[0]
    
    if (1 in arr)
        print("whaaa")

Prints "whaaa" in a non-debug build (both in tr and tr-argo).

On a debug build, it signals the following assertion failure:

Assertion failed: "((ScriptObject::hasUintProperty (m_lowHTentry)))" ("/Users/fklockii/Dev/tamarin-redux/core/ArrayObject.cpp":80)

I found this while working on bug 559321
Created attachment 439059 [details]
if this prints whaa, something is wrong

(provide a bit more info about context)
Attachment #439056 - Attachment is obsolete: true
Created attachment 439060 [details]
if this prints whaa, something is wrong

(oops previous attach was for another ticket)
Attachment #439059 - Attachment is obsolete: true
Created attachment 439061 [details]
if this prints whaa, something is wrong

(... and i managed to screw up the last attachment too, sorry)
Attachment #439060 - Attachment is obsolete: true
This test case isn't perfect yet, because it should probably be separating modifications to a freshly allocatoin (clean) object/array/etc from modifications to one that has already undergone a series of modifications.  The current test bangs on one entity repeatedly; this is indeed important to test (see bug 559401) but so is the other case.
The assertion in ArrayObject::checkForSparseToDenseConversion seems bogus: the m_lowHTentry tracks the smallest element in the hash table, but AFAICT that element may be the DELETED sentinel.  So its not okay to assume that ScriptObject::hasUintProperty will be true for such an index.

(There is still the other bug to resolve, but this assertion should go away.)
(Reporter)

Updated

8 years ago
Depends on: 559565
(Reporter)

Updated

8 years ago
Blocks: 559321
As discussed in bug 559565, the assertion need not be bogus, *if* we change the code to maintain the invariant that checkForSparseToDenseConversion is assuming -- namely that m_lowHTentry never points to a deleted sentinel node.

The patches posted to bug 559565 ensure that del*Property enforces this rule, and in the process fix this bug.

Comment 7

8 years ago
Please confirm if this issue occurs in Coral or is an injection.
Target Milestone: --- → flash10.2
Issue occurs in Coral.

Updated

8 years ago
Priority: -- → P2
Since the fix to bug 559565 has been pushed, I think this can be resolved.
(Reporter)

Updated

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

Updated

8 years ago
Flags: flashplayer-bug+
You need to log in before you can comment on or make changes to this bug.