507395 - Incorrect check for atoms in Sampler::presweep

Status: VERIFIED FIXED

(Tamarin Graveyard :: Tools, defect)

Description

[Raul Hudea]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/532.0 (KHTML, like Gecko) Chrome/3.0.195.3 Safari/532.0
Build Identifier: 

In Sampler::presweep, there is the following check needed to keep the object's type alive:

  if (s.typeOrVTable > 7 && !GC::GetMark((void*)s.typeOrVTable))
  {
     GCWorkItem item((void*)s.typeOrVTable, (uint32)GC::Size((void*)s.typeOrVTable), true);
     core->gc->PushWorkItem(item);
  }

But "s.typeOrVTable > 7" is not enough to differentiate between basic types and atoms, because in recordAllocationInfo there is this code:

  if(typeOrVTable < 7 && core->codeContext() && core->codeContext()->domainEnv()) {
    typeOrVTable |= (uintptr)core->codeContext()->domainEnv()->toplevel();
  }

Example:

For an allocated String object, typeOrVTable = 2 (kStringType), but after executing the code above, typeOrVTable becomes greater than 
7.

When Sampler::presweep is executed and the String object is found, GCWorkItem will assert because typeOrVTable is not 8-byte aligned. 





Reproducible: Always

Comment 1

[Raul Hudea]

Attachment: Use atomPtr on typeOrVTable

Use atomPtr for typeOrVTable. Suggestion from treilly.

Comment 2

[Tommy Reilly]

Comment on attachment 391619 [details] [diff] [review]
Use atomPtr on typeOrVTable

naming nit, ptr would be a better name, since after the call to atomPtr its no longer an atom.

Shouldn't the first test be atom != NULL instead of !atom?

Comment 3

[Raul Hudea]

Attachment: Update patch addressing treilly's comments

Comment 4

[Tommy Reilly]

changeset:   2289:361724188074