Regression/new segfault with module loading

[rhuanjl]

@fatcerberus and I have found a new hard crash related to module loading in the latest 1.8, 1.9 and master branches.

I think this may have unfortunately been produced by #4551 I can't see how it would have caused it - but it's the only relevant change as far as I can see - @obastemur thoughts?

The issue as far as I can see is that if you have 1 module referenced via more than 1 path the parent is set with more than 1 name:
https://github.com/Microsoft/ChakraCore/blob/release/1.9/lib/Runtime/Language/SourceTextModuleRecord.cpp#L771

Putting an if and a printf into ModuleEvaluation around line 949
https://github.com/Microsoft/ChakraCore/blob/release/1.9/lib/Runtime/Language/SourceTextModuleRecord.cpp#L949

Shows that rootFunction appears to be nullptr for all paths other than the first one used.

The test case below reproduces this by having module2.js imported from module1.js in the same folder as it and module3.js in a different folder.

Whilst it's possible to paper over this error by just checking for rootFunction ==nullptr and if it is then calling GenerateRootFunction from line 949 I don't think that's the correct fix.

Repro

//test.js
WScript.LoadScriptFile("module1.js","module");

//module1.js
import mod2 from "./module2.js";
import {mod3} from "./folder/module3.js";

export function mod1()
{
	print("mod1");
}

mod2();
mod3();

//module2.js
export default function()
{
	print("mod2");
}

//folder/module3.js
import mod3 from "../module2.js";

export {mod3};

[obastemur]

@rhuanjl our internal tool also reported the regression yesterday. I'm looking into it. Thanks for the details.

[fatcerberus]

For completeness, here's another repro that only requires 2 modules and no subdirectories:

// test.js
WScript.LoadScriptFile('./main.js', 'module');

// main.js
import { pig } from './eaty.js';
import { cow } from '././eaty.js';
import { ape } from './././eaty.js';
pig();
cow();
ape();

// eaty.js
export function pig() {
	print("the pig ate it");
}
export function cow() {
	print("the cow ate the pig and a bunch of kitties");
}
export function ape() {
	print("the ape ate the cow and pig");
}

[rhuanjl]

@obastemur I've found a neatish fix and made a test - please let me know if you would like me to submit this as a PR or if you're already doing it - or feel free to use my commits for a PR if you want to.

The problem was:
https://github.com/Microsoft/ChakraCore/blob/release/1.9/lib/Runtime/Language/SourceTextModuleRecord.cpp#L833-L834
The same module can appear multiple times in one parent's list of children (under different names) this meant that shouldGenerateRoot function would be set to true and then to false again before the check to see if GenerateRootFunction should be called could occur.

Other note the currently disabled test for bug_issue_3257 would have caught this - not sure if we want a new test for it therefore - though I've made one for now. (As far as I can see that test appears to fail normally due to a mistake in the file path handling in ch that probably ought to be fixed.)

[obastemur]

@rhuanjl yes, that's the most part of the problem. I've been testing the changes, PR should be around shortly.