726226 - IonMonkey: reverse Pool code does not use the correct pool

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Marty Rosenberg [:mjrosenb]]

Attachment: /home/mrosenberg/patches/help-r0.patch

There are two separate pools that are carried around, one for forward references, one for backwards references.  When we go to update the backwards reference, we do a check based on the forward pool, not the backwards one.  I also cleaned up some edge cases that I'd been conservative about.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 596219 [details] [diff] [review]
/home/mrosenberg/patches/help-r0.patch

Review of attachment 596219 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/shared/IonAssemblerBufferWithConstantPools.h
@@ +57,2 @@
>      const int alignment;
> +  public:

nit: need newlines above public: and private:

@@ +125,4 @@
>      bool checkFullBackref(int poolOffset, int codeOffset) {
> +        if (!limitingUser.assigned()) {
> +            return false;
> +        }

nit: no braces needed here

@@ +143,5 @@
>          JS_ASSERT(!isBackref);
> +        // Not full if there aren't any uses.
> +        if (!limitingUser.assigned()) {
> +            return false;
> +        }

ditto

@@ +202,5 @@
>      bool isAligned(uint32 ptr) {
>          return ptr == align(ptr);
>      }
> +    bool getAlignment() {
> +        return alignment;

getAlignment returns a bool and alignment is an int.

@@ +414,5 @@
>          for (cur = &pools[numPoolKinds-1]; cur >= pools; cur--) {
>              // fetch the pool for the backwards half.
>              tmp = cur->other;
> +            if (p == cur) {
> +                tmp->updateLimiter(this->nextOffset());

nit: no braces needed here

@@ +446,4 @@
>          uint32 poolOffset = nextOffset;
>          Pool *tmp;
>          // If we need a guard instruction, reserve space for that.
> +        if (perforatedNode == NULL)

style nit: if (!perforatedNode)