From c44c6603f51bd6588994ee2e2eb3a8ccf50abe6f Mon Sep 17 00:00:00 2001 From: tangtaizong666 Date: Sun, 26 Jul 2026 16:33:54 +0800 Subject: [PATCH] Fix -commonjs compilation crash and empty output (#1990) Compiling anything with -commonjs crashed with a null dereference: doCJSModule ran genBasicFunction without an active FunctionContext. The enqueued compilation lambda in genBasicFunction captures curFunction()->typedClassContext / legacyClassContext at enqueue time, and doCJSModule was the only genBasicFunction entry point that did not establish a context first. Create a top-level FunctionContext, mirroring doLazyFunction. With the crash fixed, CJS modules were still compiled to empty bytecode (CommonJS module count: 0) at the default optimization level, because the optimizer does not treat CJS module functions as roots: - deleteUnusedFunctionsAndVariables removed them: they have no IR users, as they are only referenced from the module's CJS module table. - analyzeFunctionCallsites marked them allCallsitesKnownInStrictMode/unreachable, gutting their bodies, since only the global function was exempted as "called by the runtime". FuncSigOpts already handles CJS module functions, so this brings the other two passes in line with it. Add a regression test that compiles and runs a CommonJS module at the default optimization level, -O0, and -O. Fixes #1990 --- lib/IRGen/ESTreeIRGen.cpp | 7 +++++++ lib/Optimizer/Scalar/FunctionAnalysis.cpp | 5 +++-- lib/Optimizer/Scalar/Utils.cpp | 5 +++-- test/hermes/cjs-module.js | 24 +++++++++++++++++++++++ 4 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 test/hermes/cjs-module.js diff --git a/lib/IRGen/ESTreeIRGen.cpp b/lib/IRGen/ESTreeIRGen.cpp index 7ed61dcda58..5ac9efd41e0 100644 --- a/lib/IRGen/ESTreeIRGen.cpp +++ b/lib/IRGen/ESTreeIRGen.cpp @@ -303,6 +303,13 @@ void ESTreeIRGen::doCJSModule( assert(Root && "no root in ESTreeIRGen"); auto *func = cast(Root); + // Create a top level FunctionContext that will never be executed, because + // genBasicFunction reads state from the current function context (e.g. the + // class contexts captured when enqueueing the compilation of the module + // function). + FunctionContext topLevelFunctionContext{ + this, Mod->getTopLevelFunction(), nullptr}; + // Take care of the additions to the global scope that this module could // have done. A module can only add ambient global properties. Look for new // ones (customData == nullptr) and declare them. diff --git a/lib/Optimizer/Scalar/FunctionAnalysis.cpp b/lib/Optimizer/Scalar/FunctionAnalysis.cpp index fb1fa639927..0a765727a7b 100644 --- a/lib/Optimizer/Scalar/FunctionAnalysis.cpp +++ b/lib/Optimizer/Scalar/FunctionAnalysis.cpp @@ -406,8 +406,9 @@ void analyzeFunctionCallsites(Function *F) { // Attempt to start from a position of knowing all callsites. F->getAttributesRef(M)._allCallsitesKnownInStrictMode = true; - if (F->isGlobalScope()) { - // global function is called by the runtime, so its callsites aren't known. + if (F->isGlobalScope() || M->findCJSModule(F)) { + // The global function and CommonJS module functions are called by the + // runtime, so their callsites aren't known. F->getAttributesRef(M)._allCallsitesKnownInStrictMode = false; } diff --git a/lib/Optimizer/Scalar/Utils.cpp b/lib/Optimizer/Scalar/Utils.cpp index 8a6c9c15f7f..6b8beed1c37 100644 --- a/lib/Optimizer/Scalar/Utils.cpp +++ b/lib/Optimizer/Scalar/Utils.cpp @@ -243,8 +243,9 @@ bool deleteUnusedFunctionsAndVariables(Module *M) { for (auto &F : *M) { // Delete any functions that do not have any uses other than in their own // bodies. The top level function does not have an explicit user, so check - // for it directly. - if (&F != M->getTopLevelFunction() && + // for it directly. CommonJS module functions are only referenced by the + // module's CJS module table, so they must also be kept. + if (&F != M->getTopLevelFunction() && !M->findCJSModule(&F) && llvh::all_of(F.getUsers(), [&F](Instruction *user) { // Use must be from another function to be meaningful. return user->getFunction() == &F; diff --git a/test/hermes/cjs-module.js b/test/hermes/cjs-module.js new file mode 100644 index 00000000000..93fec1e3365 --- /dev/null +++ b/test/hermes/cjs-module.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermes -commonjs %s | %FileCheck --match-full-lines %s +// RUN: %hermes -O0 -commonjs %s | %FileCheck --match-full-lines %s +// RUN: %hermes -O -commonjs %s | %FileCheck --match-full-lines %s + +// Regression test for #1990: compiling with -commonjs used to crash IRGen +// because doCJSModule ran without an active FunctionContext, and the +// optimizer used to delete CJS module functions as unused. + +print('cjs module start'); +// CHECK: cjs module start + +print(typeof exports, typeof require, typeof module); +// CHECK-NEXT: object function object + +exports.foo = 42; +print(module.exports.foo); +// CHECK-NEXT: 42