LegalizeJSInterface: Pick unique names for generated stubs

[akshat-lakhera]

Fixes #9243

LegalizeJSInterface could read freed memory when the module already contained a function with the name it wanted to give a generated stub.

Cause

For each illegal import, makeLegalStubForCalledImport builds two functions, a stub named legalfunc$ plus the import name and an import named legalimport$ plus the import name, and returns a raw pointer to the stub. The caller stores that pointer in illegalImportsToLegal, and Fixer::visitCall later uses it to rewrite calls.

Both functions were only added to the module when no function with that name already existed. When one did exist (in the issue, a user function called $legalfunc$imp), the stub was not added, so its unique_ptr freed it when the function returned. The pointer saved in illegalImportsToLegal then pointed at freed memory, and Fixer::visitCall read it. That's the heap-use-after-free in the report, and with a Release build the repro segfaults.

Fix

The stub and the legalimport function now get their names from Names::getValidFunctionName, which returns the requested name if it's free and otherwise adds a numeric suffix. Since the names are now always unique, both functions are always added and the "only if absent" checks are gone, so the stored pointer always refers to a function owned by the module. makeLegalStubForCalledImport is called once per illegal import, so this doesn't create duplicate stubs. Modules without a name collision get the same names as before.

Test

I added test/lit/passes/legalize-js-interface-name-collision.wast, based on the repro from the issue plus a colliding $legalimport$imp, with CHECK lines generated by scripts/update_lit_checks.py. With the fix the output uses $legalfunc$imp_4 and $legalimport$imp_4 and the pass completes, while on main the same input crashes wasm-opt. The existing legalize-js-interface lit tests still pass, and git clang-format is clean.

[gemini-code-assist]

Code Review

This pull request prevents name collisions in the LegalizeJSInterface pass by using Names::getValidFunctionName to generate unique names for legal imports and stubs, and adds a test case to verify this behavior. Feedback on the changes suggests a minor refactoring to directly use the return value of module->addFunction when adding the stub function, which is cleaner and more idiomatic than obtaining the raw pointer before moving the unique pointer.

[gemini-code-assist]

Instead of obtaining the raw pointer from the unique_ptr before moving it, we can directly use the return value of module->addFunction, which returns the raw pointer to the added function. This is cleaner and more idiomatic.

    auto* stubPtr = module->addFunction(std::move(stub));
    module->addFunction(std::move(legalIm));
    return stubPtr;

[akshat-lakhera]

cc @kripken @tlively for review when you get a chance.