1750496 - Differential testing: MStringSplit incorrect conguent_to

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[lukas.bernhard]

Steps to reproduce:

On git commit afbff3f3ae087181d9988fe19e09cc15927fb7ff I encountered a miscomputation during differential fuzzing.
The issue seems to be an incorrect computation of congruent_to for the instruction MStringSplit.
The MIR instruction returns an array object; MIROps.yaml defines congruent_to = if_operands_equal.
Having two subsequent split instructions such as generated by:

const v28 = "aa".split('');
const v35 = "aa".split('');

causes GVN to replace usages of v35 with the very object in v28.
Hence the two objects behave as one; modifications to v28 also modify v35 (as it is the same object).
Marking s-s just as a precaution, may be a correctness-only problem.

obj-x86_64-pc-linux-gnu/dist/bin/js --no-threads --cpu-count=1 --ion-offthread-compile=off --fuzzing-safe --differential-testing --ion-warmup-threshold=100 --baseline-warmup-threshold=10 --small-function-length=4096 --inlining-entry-threshold=4 --trial-inlining-warmup-threshold=64 sample.js

function main() {
  for (let v1 = 0; v1 < 125; v1++) {
      const v28 = "aa".split('');
      v28[0] = 1;
      const v35 = "aa".split('');
      print(v35[0]);  // should be a, becomes 1 as v35 is replaced with v28
      print(Object.is(v35, v28)); // should be false, becomes true
  }
}
main();

Comment 1

[Nicolas B. Pierron [:nbp]]

Iain, Caroline, any idea from where this issue comes from?

Comment 2

[Caroline Cullen [:caroline]]

Looks as if when I was transcribing the ops to yaml, I accidentally added congruent_to for MStringSplit. I will fix that, thanks!

Comment 3

[Iain Ireland [:iain]]

Oh, good catch.

I don't think this is s-s. We end up in the same state as if the code were const v35 = v28. It's an invalid transformation, but it's internally consistent, so there's no opportunity for (eg) type confusion.

Comment 4

[Caroline Cullen [:caroline]]

Attachment: Bug 1750496 - Remove congruent_to for MStringSplit. r?iain!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ffac60c64852
Remove congruent_to for MStringSplit. r=iain

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/ffac60c64852