Add ASSERT to catch incorrect declaration of GCMember

RESOLVED FIXED in Q3 11 - Serrano

Status

Tamarin
Garbage Collection (mmGC)
P2
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Brent Getlin, Assigned: Tommy Reilly)

Tracking

(Blocks: 1 bug)

unspecified
Q3 11 - Serrano

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Within the scope of a GC defined object (GCRoot, GCObject, GCFinalizedObject) method it's possible to declare a GCMember on the stack.  


void MyGCObject::MyFoo()
{

    //  This will likely cause crash as it attempts to add a write barrier to the stack 
    GCMember<GCObject> crash;

}


Solve it with a check on construction of the GCMember to see if the memory address is on the stack:

#ifdef DEBUG
        
        static bool IsAddressOnStack(void *address)
        {
                
                uintptr_t stackBase = VMPI_getThreadStackBase();
                char stackTop;
                return ((uintptr_t)address > (uintptr_t)&stackTop && (uintptr_t)address < stackBase);
                
        }
#endif
(Reporter)

Updated

7 years ago
Blocks: 565664

Updated

7 years ago
Assignee: nobody → treilly
Priority: -- → P3
Hardware: x86 → All
Target Milestone: --- → Q3 11 - Serrano

Updated

7 years ago
Priority: P3 → P2
(Assignee)

Comment 1

7 years ago
Created attachment 526290 [details] [diff] [review]
MErge GCMember-inlines.h and GCMemberBase-inlines and apply assert to all GCMember ctors
Attachment #526290 - Flags: review?(lhansen)

Comment 2

7 years ago
Comment on attachment 526290 [details] [diff] [review]
MErge GCMember-inlines.h and GCMemberBase-inlines and apply assert to all GCMember ctors

Not clear to me why you're checking "&(this->t)" and not simply "this".
Attachment #526290 - Flags: review?(lhansen) → review+
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> Not clear to me why you're checking "&(this->t)" and not simply "this".

Me neither.
(Assignee)

Updated

7 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

7 years ago
http://hg.mozilla.org/tamarin-redux/rev/57bcf95e5583
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.