Closed Bug 848743 Opened 9 years ago Closed 9 years ago

BaselineCompiler: Differential Testing: Setter not called with baseline enabled

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(1 file)

The following testcase shows different behavior with options --ion-eager vs. --ion-eager --no-baseline on ionmonkey revision 8565e1fcdf91 (baseline branch):


var gTestcases = new Array();
var gTc = gTestcases.length;
var setterCalled = false;
function TestCase() {
  gTestcases[gTc++] = this;
}
for(var i = 0; i < 13; ++i) {
  var testcase = new TestCase();
}
Array.prototype.__defineSetter__(32, function() { setterCalled = true; });
for(var i = 0; i < 20; ++i) {
  var testcase = new TestCase();
}
setterCalled ? print("PASSED") : print("FAILED");


$ debug64/js --ion-eager test.js
FAILED

$ debug64/js --ion-eager --no-baseline test.js
PASSED
Great testcase. The problem is that the SetElem_DenseAdd stub has to check for indexed properties on the prototype before adding an element.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
The teleporting optimization does not work for indexes, so this patch guards on all shapes on the proto chain. Also fixes some very noisy Clang warnings (with this patch there are no new warnings compared to inbound).
Attachment #727191 - Flags: review?(kvijayan)
Comment on attachment 727191 [details] [diff] [review]
Patch

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

Looks good.

::: js/src/ion/BaselineIC.cpp
@@ +4340,5 @@
>      Register walker = regs.takeAny();
>      Register scratch = regs.takeAny();
>  
> +    // Use a local to silence Clang tautological-compare warning if NumHops is 0.
> +    size_t numHops = NumHops;

Would it be possible to make this 'const' and still eliminate the warnings?
Attachment #727191 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/def96e89be7e

(In reply to Kannan Vijayan [:djvj] from comment #3)
> 
> Would it be possible to make this 'const' and still eliminate the warnings?

I tried it but if I add 'const' Clang starts to complain again.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 877589
You need to log in before you can comment on or make changes to this bug.