// PTAL. Bisects to https://chromium.googlesource.com/v8/v8/+/38b1807a3f52b1cb1126eeb223c324e287a133ef

// Repro:

(function () {
  var dict = { toString() { result = v;} };
  for (var v of ['fontsize', 'sup']) {
    print("Var: " + v);
    String.prototype[v].call(dict);
    print("Result: " + result);
  }
})();

// Output for ignition vs turbo_opt:
# Compared x64,ignition with x64,ignition_turbo_opt
#
# Flags of x64,ignition:
--abort_on_stack_or_string_length_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 123 --turbo-filter=~ --noopt --suppress-asm-messages
# Flags of x64,ignition_turbo_opt:
--abort_on_stack_or_string_length_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --es-staging --random-seed 123 --always-opt --suppress-asm-messages
#
# Difference:
- Result: sup
+ Result: fontsize
#
### Start of configuration x64,ignition:
Var: fontsize
Result: fontsize
Var: sup
Result: sup

### End of configuration x64,ignition
#
### Start of configuration x64,ignition_turbo_opt:
Var: fontsize
Result: fontsize
Var: sup
Result: fontsize

### End of configuration x64,ignition_turbo_opt

Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

Interesting.

Another repro:

(function () {
  var dict = { toString() {
    print("toString called, v is " + v);
    result = v;
  }};
  for (var v of ['fontsize', 'sup']) {
    print("Var: " + v);
    String.prototype[v].call(dict);
    print("Result: " + result);
  }
})();

$ out/Debug/d8 ../foo319.js 
Var: fontsize
toString called, v is fontsize
Result: fontsize
Var: sup
toString called, v is sup
Result: sup


$ out/Debug/d8 ../foo319.js --always-opt
Var: fontsize
toString called, v is fontsize
Result: fontsize
Var: sup
toString called, v is fontsize <<<<< here it's different
Result: fontsize

Even simpler failure:

```
(function () {
  function setResult() {
    result = v;
  };
  for (var v of ['hello', 'goodbye']) {
    print("Var: " + v);
    setResult();
    print("Result: " + result);
  }
})();
```

It seems that |v| is being marked as "never assigned" after my change, for some reason. Digging further...

And the reason is that we are no longer inside a block scope when MarkLoopVariableAsAssigned() is called:

https://cs.chromium.org/chromium/src/v8/src/parsing/parser-base.h?rcl=22069a18b92fae0788161391a84a01360b3b9430&l=5850

It seems like the best fix would be to manually mark the loop variable as assigned for for-in/of loops. Will try to put such a fix together.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/0717ff34574b7294c2caa9a7821d5e45d983dff4

commit 0717ff34574b7294c2caa9a7821d5e45d983dff4
Author: Adam Klein <adamk@chromium.org>
Date: Thu Oct 05 19:16:29 2017

[parser] Ensure for-in/of loop variables are marked maybe_assigned

The code used to rely on all such loops having a block scope around
them, but that is no longer the case for loops whose loop variables
are VAR-declared.

This patch introduces a new DeclarationDescriptor::Kind for such
variables, and sets it during parsing, allowing the variable
declaration code to note them as assigned appropriately.

Bug:  chromium:768158 
Change-Id: I0cd60e8c8c735681be9dbb9344a93156af09c952
Reviewed-on: https://chromium-review.googlesource.com/701624
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48320}
[modify] https://crrev.com/0717ff34574b7294c2caa9a7821d5e45d983dff4/src/parsing/parser-base.h
[modify] https://crrev.com/0717ff34574b7294c2caa9a7821d5e45d983dff4/src/parsing/parser.cc
[modify] https://crrev.com/0717ff34574b7294c2caa9a7821d5e45d983dff4/src/parsing/pattern-rewriter.cc
[modify] https://crrev.com/0717ff34574b7294c2caa9a7821d5e45d983dff4/src/parsing/preparser.cc
[add] https://crrev.com/0717ff34574b7294c2caa9a7821d5e45d983dff4/test/mjsunit/regress/regress-crbug-768158.js

This isn't actually a stability bug, but a correctness bug. Still it would be good to fix.

This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

how critical is this bug for M62? Can it wait until M63, since we're only less than 10 days away from M62 stable?

It's a correctness regression, which could cause arbitrary behavior changes in existing code. What I can't do with any accuracy is say how much code could be affected, but I'm inclined to say I'd really rather not ship this regression in M62.

Also, I would say is that the fix is relatively low risk.

ClusterFuzz has detected this issue as fixed in range 48319:48320.

Detailed report: https://clusterfuzz.com/testcase?key=5349728895369216

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition:x64,ignition_turbo_opt
  sources: 064
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=47435:47436
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=48319:48320

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5349728895369216

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5349728895369216 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4af76561487f179c8a5cf0eaa4f0bf2c0263b45d

commit 4af76561487f179c8a5cf0eaa4f0bf2c0263b45d
Author: Adam Klein <adamk@chromium.org>
Date: Mon Oct 09 17:09:01 2017

Merged: [parser] Ensure for-in/of loop variables are marked maybe_assigned

Revision: 0717ff34574b7294c2caa9a7821d5e45d983dff4

BUG= chromium:768158 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=gsathya@chromium.org

Change-Id: I78ed9ea7ab6351b7c2be07466ed26272d6738803
Reviewed-on: https://chromium-review.googlesource.com/706146
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.2@{#60}
Cr-Branched-From: efa2ac4129d30c7c72e84c16af3d20b44829f990-refs/heads/6.2.414@{#1}
Cr-Branched-From: a861ebb762a60bf5cc2a274faee3620abfb06311-refs/heads/master@{#47693}
[modify] https://crrev.com/4af76561487f179c8a5cf0eaa4f0bf2c0263b45d/src/parsing/parser-base.h
[modify] https://crrev.com/4af76561487f179c8a5cf0eaa4f0bf2c0263b45d/src/parsing/parser.cc
[modify] https://crrev.com/4af76561487f179c8a5cf0eaa4f0bf2c0263b45d/src/parsing/pattern-rewriter.cc
[modify] https://crrev.com/4af76561487f179c8a5cf0eaa4f0bf2c0263b45d/src/parsing/preparser.cc
[add] https://crrev.com/4af76561487f179c8a5cf0eaa4f0bf2c0263b45d/test/mjsunit/regress/regress-crbug-768158.js