From 2dff89b658d6ab2592865c226f3f1078f418151d Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 4 Jul 2020 23:12:13 +0000 Subject: Issue #618 - Report source position information (line/column) Report source position information for module export resolution failures. Ref: BZ 1362098 --- js/src/builtin/Module.js | 37 +++++++++-------- js/src/builtin/ModuleObject.cpp | 79 ++++++++++++++++++++++++++++++------ js/src/builtin/ModuleObject.h | 26 +++++++++--- js/src/builtin/ReflectParse.cpp | 2 +- js/src/frontend/BytecodeCompiler.cpp | 2 +- js/src/js.msg | 8 ++-- js/src/vm/SelfHosting.cpp | 46 +++++++++++++++++++++ 7 files changed, 161 insertions(+), 39 deletions(-) (limited to 'js') diff --git a/js/src/builtin/Module.js b/js/src/builtin/Module.js index 064293670..b3365b505 100644 --- a/js/src/builtin/Module.js +++ b/js/src/builtin/Module.js @@ -436,8 +436,10 @@ function ModuleDeclarationEnvironmentSetup(module) let resolution = callFunction(module.resolveExport, module, e.exportName); assert(resolution.resolved || resolution.module, "Unexpected failure to resolve export in ModuleDeclarationEnvironmentSetup"); - if (!resolution.resolved) - return ResolutionError(resolution, "indirectExport", e.exportName) + if (!resolution.resolved) { + return ResolutionError(resolution, "indirectExport", e.exportName, + e.lineNumber, e.columnNumber) + } } // Steps 5-6 @@ -459,8 +461,10 @@ function ModuleDeclarationEnvironmentSetup(module) if (!resolution.resolved && !resolution.module) resolution.module = module; - if (!resolution.resolved) - return ResolutionError(resolution, "import", imp.importName); + if (!resolution.resolved) { + return ResolutionError(resolution, "import", imp.importName, + imp.lineNumber, imp.columnNumber); + } CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName); } @@ -470,7 +474,7 @@ function ModuleDeclarationEnvironmentSetup(module) } // 15.2.1.16.4.3 ResolutionError(module) -function ResolutionError(resolution, kind, name) +function ResolutionError(resolution, kind, name, line, column) { let module = resolution.module; assert(module !== null, @@ -483,21 +487,22 @@ function ResolutionError(resolution, kind, name) assert(kind === "import" || kind === "indirectExport", "Unexpected kind in ResolutionError"); - let message; + assert(line > 0, + "Line number should be present for all imports and indirect exports"); + + let errorNumber; if (kind === "import") { - message = resolution.ambiguous ? JSMSG_AMBIGUOUS_IMPORT - : JSMSG_MISSING_IMPORT; + errorNumber = resolution.ambiguous ? JSMSG_AMBIGUOUS_IMPORT + : JSMSG_MISSING_IMPORT; } else { - message = resolution.ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT - : JSMSG_MISSING_INDIRECT_EXPORT; + errorNumber = resolution.ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT + : JSMSG_MISSING_INDIRECT_EXPORT; } - try { - ThrowSyntaxError(message, name); - } catch (error) { - RecordModuleError(module, error); - throw error; - } + let message = GetErrorMessage(errorNumber) + ": " + name; + let error = CreateModuleSyntaxError(module, line, column, message); + RecordModuleError(module, error); + throw error; } // 15.2.1.16.5 ModuleEvaluate() diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp index 0a836c75b..09193c9ca 100644 --- a/js/src/builtin/ModuleObject.cpp +++ b/js/src/builtin/ModuleObject.cpp @@ -70,6 +70,15 @@ ModuleValueGetter(JSContext* cx, unsigned argc, Value* vp) return &value.toString()->asAtom(); \ } +#define DEFINE_UINT32_ACCESSOR_METHOD(cls, name) \ + uint32_t \ + cls::name() const \ + { \ + Value value = cls##_##name##Value(this); \ + MOZ_ASSERT(value.toInt32() >= 0); \ + return value.toInt32(); \ + } + /////////////////////////////////////////////////////////////////////////// // ImportEntryObject @@ -83,10 +92,14 @@ ImportEntryObject::class_ = { DEFINE_GETTER_FUNCTIONS(ImportEntryObject, moduleRequest, ModuleRequestSlot) DEFINE_GETTER_FUNCTIONS(ImportEntryObject, importName, ImportNameSlot) DEFINE_GETTER_FUNCTIONS(ImportEntryObject, localName, LocalNameSlot) +DEFINE_GETTER_FUNCTIONS(ImportEntryObject, lineNumber, LineNumberSlot) +DEFINE_GETTER_FUNCTIONS(ImportEntryObject, columnNumber, ColumnNumberSlot) DEFINE_ATOM_ACCESSOR_METHOD(ImportEntryObject, moduleRequest) DEFINE_ATOM_ACCESSOR_METHOD(ImportEntryObject, importName) DEFINE_ATOM_ACCESSOR_METHOD(ImportEntryObject, localName) +DEFINE_UINT32_ACCESSOR_METHOD(ImportEntryObject, lineNumber) +DEFINE_UINT32_ACCESSOR_METHOD(ImportEntryObject, columnNumber) /* static */ bool ImportEntryObject::isInstance(HandleValue value) @@ -101,6 +114,8 @@ GlobalObject::initImportEntryProto(JSContext* cx, Handle global) JS_PSG("moduleRequest", ImportEntryObject_moduleRequestGetter, 0), JS_PSG("importName", ImportEntryObject_importNameGetter, 0), JS_PSG("localName", ImportEntryObject_localNameGetter, 0), + JS_PSG("lineNumber", ImportEntryObject_lineNumberGetter, 0), + JS_PSG("columnNumber", ImportEntryObject_columnNumberGetter, 0), JS_PS_END }; @@ -119,8 +134,12 @@ GlobalObject::initImportEntryProto(JSContext* cx, Handle global) ImportEntryObject::create(ExclusiveContext* cx, HandleAtom moduleRequest, HandleAtom importName, - HandleAtom localName) + HandleAtom localName, + uint32_t lineNumber, + uint32_t columnNumber) { + MOZ_ASSERT(lineNumber > 0); + RootedObject proto(cx, cx->global()->getImportEntryPrototype()); RootedObject obj(cx, NewObjectWithGivenProto(cx, &class_, proto)); if (!obj) @@ -130,6 +149,8 @@ ImportEntryObject::create(ExclusiveContext* cx, self->initReservedSlot(ModuleRequestSlot, StringValue(moduleRequest)); self->initReservedSlot(ImportNameSlot, StringValue(importName)); self->initReservedSlot(LocalNameSlot, StringValue(localName)); + self->initReservedSlot(LineNumberSlot, Int32Value(lineNumber)); + self->initReservedSlot(ColumnNumberSlot, Int32Value(columnNumber)); return self; } @@ -147,11 +168,15 @@ DEFINE_GETTER_FUNCTIONS(ExportEntryObject, exportName, ExportNameSlot) DEFINE_GETTER_FUNCTIONS(ExportEntryObject, moduleRequest, ModuleRequestSlot) DEFINE_GETTER_FUNCTIONS(ExportEntryObject, importName, ImportNameSlot) DEFINE_GETTER_FUNCTIONS(ExportEntryObject, localName, LocalNameSlot) +DEFINE_GETTER_FUNCTIONS(ExportEntryObject, lineNumber, LineNumberSlot) +DEFINE_GETTER_FUNCTIONS(ExportEntryObject, columnNumber, ColumnNumberSlot) DEFINE_ATOM_OR_NULL_ACCESSOR_METHOD(ExportEntryObject, exportName) DEFINE_ATOM_OR_NULL_ACCESSOR_METHOD(ExportEntryObject, moduleRequest) DEFINE_ATOM_OR_NULL_ACCESSOR_METHOD(ExportEntryObject, importName) DEFINE_ATOM_OR_NULL_ACCESSOR_METHOD(ExportEntryObject, localName) +DEFINE_UINT32_ACCESSOR_METHOD(ExportEntryObject, lineNumber) +DEFINE_UINT32_ACCESSOR_METHOD(ExportEntryObject, columnNumber) /* static */ bool ExportEntryObject::isInstance(HandleValue value) @@ -167,6 +192,8 @@ GlobalObject::initExportEntryProto(JSContext* cx, Handle global) JS_PSG("moduleRequest", ExportEntryObject_moduleRequestGetter, 0), JS_PSG("importName", ExportEntryObject_importNameGetter, 0), JS_PSG("localName", ExportEntryObject_localNameGetter, 0), + JS_PSG("lineNumber", ExportEntryObject_lineNumberGetter, 0), + JS_PSG("columnNumber", ExportEntryObject_columnNumberGetter, 0), JS_PS_END }; @@ -192,8 +219,13 @@ ExportEntryObject::create(ExclusiveContext* cx, HandleAtom maybeExportName, HandleAtom maybeModuleRequest, HandleAtom maybeImportName, - HandleAtom maybeLocalName) + HandleAtom maybeLocalName, + uint32_t lineNumber, + uint32_t columnNumber) { + // Line and column numbers are optional for export entries since direct + // entries are checked at parse time. + RootedObject proto(cx, cx->global()->getExportEntryPrototype()); RootedObject obj(cx, NewObjectWithGivenProto(cx, &class_, proto)); if (!obj) @@ -204,6 +236,8 @@ ExportEntryObject::create(ExclusiveContext* cx, self->initReservedSlot(ModuleRequestSlot, StringOrNullValue(maybeModuleRequest)); self->initReservedSlot(ImportNameSlot, StringOrNullValue(maybeImportName)); self->initReservedSlot(LocalNameSlot, StringOrNullValue(maybeLocalName)); + self->initReservedSlot(LineNumberSlot, Int32Value(lineNumber)); + self->initReservedSlot(ColumnNumberSlot, Int32Value(columnNumber)); return self; } @@ -1025,9 +1059,11 @@ GlobalObject::initModuleProto(JSContext* cx, Handle global) /////////////////////////////////////////////////////////////////////////// // ModuleBuilder -ModuleBuilder::ModuleBuilder(ExclusiveContext* cx, HandleModuleObject module) +ModuleBuilder::ModuleBuilder(ExclusiveContext* cx, HandleModuleObject module, + const frontend::TokenStream& tokenStream) : cx_(cx), module_(cx, module), + tokenStream_(tokenStream), requestedModules_(cx, AtomVector(cx)), importedBoundNames_(cx, AtomVector(cx)), importEntries_(cx, ImportEntryVector(cx)), @@ -1052,6 +1088,7 @@ ModuleBuilder::buildTables() if (!localExportEntries_.append(exp)) return false; } else { + MOZ_ASSERT(exp->lineNumber()); RootedAtom exportName(cx_, exp->exportName()); RootedAtom moduleRequest(cx_, importEntry->moduleRequest()); RootedAtom importName(cx_, importEntry->importName()); @@ -1060,7 +1097,9 @@ ModuleBuilder::buildTables() exportName, moduleRequest, importName, - nullptr); + nullptr, + exp->lineNumber(), + exp->columnNumber()); if (!exportEntry || !indirectExportEntries_.append(exportEntry)) return false; } @@ -1069,6 +1108,7 @@ ModuleBuilder::buildTables() if (!starExportEntries_.append(exp)) return false; } else { + MOZ_ASSERT(exp->lineNumber()); if (!indirectExportEntries_.append(exp)) return false; } @@ -1133,8 +1173,12 @@ ModuleBuilder::processImport(frontend::ParseNode* pn) if (!importedBoundNames_.append(localName)) return false; + uint32_t line; + uint32_t column; + tokenStream_.srcCoords.lineNumAndColumnIndex(spec->pn_left->pn_pos.begin, &line, &column); + RootedImportEntryObject importEntry(cx_); - importEntry = ImportEntryObject::create(cx_, module, importName, localName); + importEntry = ImportEntryObject::create(cx_, module, importName, localName, line, column); if (!importEntry || !importEntries_.append(importEntry)) return false; } @@ -1165,7 +1209,7 @@ ModuleBuilder::processExport(frontend::ParseNode* pn) MOZ_ASSERT(spec->isKind(PNK_EXPORT_SPEC)); RootedAtom localName(cx_, spec->pn_left->pn_atom); RootedAtom exportName(cx_, spec->pn_right->pn_atom); - if (!appendExportEntry(exportName, localName)) + if (!appendExportEntry(exportName, localName, spec)) return false; } break; @@ -1230,12 +1274,12 @@ ModuleBuilder::processExportFrom(frontend::ParseNode* pn) if (spec->isKind(PNK_EXPORT_SPEC)) { RootedAtom bindingName(cx_, spec->pn_left->pn_atom); RootedAtom exportName(cx_, spec->pn_right->pn_atom); - if (!appendExportFromEntry(exportName, module, bindingName)) + if (!appendExportFromEntry(exportName, module, bindingName, spec->pn_left)) return false; } else { MOZ_ASSERT(spec->isKind(PNK_EXPORT_BATCH_SPEC)); RootedAtom importName(cx_, cx_->names().star); - if (!appendExportFromEntry(nullptr, module, importName)) + if (!appendExportFromEntry(nullptr, module, importName, spec)) return false; } } @@ -1264,19 +1308,30 @@ ModuleBuilder::hasExportedName(JSAtom* name) const } bool -ModuleBuilder::appendExportEntry(HandleAtom exportName, HandleAtom localName) +ModuleBuilder::appendExportEntry(HandleAtom exportName, HandleAtom localName, ParseNode* node) { + uint32_t line = 0; + uint32_t column = 0; + if (node) + tokenStream_.srcCoords.lineNumAndColumnIndex(node->pn_pos.begin, &line, &column); + Rooted exportEntry(cx_); - exportEntry = ExportEntryObject::create(cx_, exportName, nullptr, nullptr, localName); + exportEntry = ExportEntryObject::create(cx_, exportName, nullptr, nullptr, localName, + line, column); return exportEntry && exportEntries_.append(exportEntry); } bool ModuleBuilder::appendExportFromEntry(HandleAtom exportName, HandleAtom moduleRequest, - HandleAtom importName) + HandleAtom importName, ParseNode* node) { + uint32_t line; + uint32_t column; + tokenStream_.srcCoords.lineNumAndColumnIndex(node->pn_pos.begin, &line, &column); + Rooted exportEntry(cx_); - exportEntry = ExportEntryObject::create(cx_, exportName, moduleRequest, importName, nullptr); + exportEntry = ExportEntryObject::create(cx_, exportName, moduleRequest, importName, nullptr, + line, column); return exportEntry && exportEntries_.append(exportEntry); } diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h index 365fcd4bf..be0315215 100644 --- a/js/src/builtin/ModuleObject.h +++ b/js/src/builtin/ModuleObject.h @@ -24,6 +24,7 @@ class ModuleObject; namespace frontend { class ParseNode; +class TokenStream; } /* namespace frontend */ typedef Rooted RootedModuleObject; @@ -39,6 +40,8 @@ class ImportEntryObject : public NativeObject ModuleRequestSlot = 0, ImportNameSlot, LocalNameSlot, + LineNumberSlot, + ColumnNumberSlot, SlotCount }; @@ -48,10 +51,14 @@ class ImportEntryObject : public NativeObject static ImportEntryObject* create(ExclusiveContext* cx, HandleAtom moduleRequest, HandleAtom importName, - HandleAtom localName); + HandleAtom localName, + uint32_t lineNumber, + uint32_t columnNumber); JSAtom* moduleRequest() const; JSAtom* importName() const; JSAtom* localName() const; + uint32_t lineNumber() const; + uint32_t columnNumber() const; }; typedef Rooted RootedImportEntryObject; @@ -66,6 +73,8 @@ class ExportEntryObject : public NativeObject ModuleRequestSlot, ImportNameSlot, LocalNameSlot, + LineNumberSlot, + ColumnNumberSlot, SlotCount }; @@ -76,11 +85,15 @@ class ExportEntryObject : public NativeObject HandleAtom maybeExportName, HandleAtom maybeModuleRequest, HandleAtom maybeImportName, - HandleAtom maybeLocalName); + HandleAtom maybeLocalName, + uint32_t lineNumber, + uint32_t columnNumber); JSAtom* exportName() const; JSAtom* moduleRequest() const; JSAtom* importName() const; JSAtom* localName() const; + uint32_t lineNumber() const; + uint32_t columnNumber() const; }; typedef Rooted RootedExportEntryObject; @@ -304,7 +317,8 @@ class ModuleObject : public NativeObject class MOZ_STACK_CLASS ModuleBuilder { public: - explicit ModuleBuilder(ExclusiveContext* cx, HandleModuleObject module); + explicit ModuleBuilder(ExclusiveContext* cx, HandleModuleObject module, + const frontend::TokenStream& tokenStream); bool processImport(frontend::ParseNode* pn); bool processExport(frontend::ParseNode* pn); @@ -329,6 +343,7 @@ class MOZ_STACK_CLASS ModuleBuilder ExclusiveContext* cx_; RootedModuleObject module_; + const frontend::TokenStream& tokenStream_; RootedAtomVector requestedModules_; RootedAtomVector importedBoundNames_; RootedImportEntryVector importEntries_; @@ -339,9 +354,10 @@ class MOZ_STACK_CLASS ModuleBuilder ImportEntryObject* importEntryFor(JSAtom* localName) const; - bool appendExportEntry(HandleAtom exportName, HandleAtom localName); + bool appendExportEntry(HandleAtom exportName, HandleAtom localName, + frontend::ParseNode* node = nullptr); bool appendExportFromEntry(HandleAtom exportName, HandleAtom moduleRequest, - HandleAtom importName); + HandleAtom importName, frontend::ParseNode* node); bool maybeAppendRequestedModule(HandleAtom module); diff --git a/js/src/builtin/ReflectParse.cpp b/js/src/builtin/ReflectParse.cpp index a8065d6d1..5cb81355f 100644 --- a/js/src/builtin/ReflectParse.cpp +++ b/js/src/builtin/ReflectParse.cpp @@ -3741,7 +3741,7 @@ reflect_parse(JSContext* cx, uint32_t argc, Value* vp) if (!module) return false; - ModuleBuilder builder(cx, module); + ModuleBuilder builder(cx, module, parser.tokenStream); ModuleSharedContext modulesc(cx, module, &cx->global()->emptyGlobalScope(), builder); pn = parser.moduleBody(&modulesc); if (!pn) diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp index ccfe3cd2e..3c2bcc1ed 100644 --- a/js/src/frontend/BytecodeCompiler.cpp +++ b/js/src/frontend/BytecodeCompiler.cpp @@ -405,7 +405,7 @@ BytecodeCompiler::compileModule() module->init(script); - ModuleBuilder builder(cx, module); + ModuleBuilder builder(cx, module, parser->tokenStream); ModuleSharedContext modulesc(cx, module, enclosingScope, builder); ParseNode* pn = parser->moduleBody(&modulesc); if (!pn) diff --git a/js/src/js.msg b/js/src/js.msg index ee74c8dd5..9c508ebbd 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -573,10 +573,10 @@ MSG_DEF(JSMSG_REINIT_THIS, 0, JSEXN_REFERENCEERR, "super() called twice in // Modules MSG_DEF(JSMSG_BAD_DEFAULT_EXPORT, 0, JSEXN_SYNTAXERR, "default export cannot be provided by export *") -MSG_DEF(JSMSG_MISSING_INDIRECT_EXPORT, 1, JSEXN_SYNTAXERR, "indirect export '{0}' not found") -MSG_DEF(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, 1, JSEXN_SYNTAXERR, "ambiguous indirect export '{0}'") -MSG_DEF(JSMSG_MISSING_IMPORT, 1, JSEXN_SYNTAXERR, "import '{0}' not found") -MSG_DEF(JSMSG_AMBIGUOUS_IMPORT, 1, JSEXN_SYNTAXERR, "ambiguous import '{0}'") +MSG_DEF(JSMSG_MISSING_INDIRECT_EXPORT, 0, JSEXN_SYNTAXERR, "indirect export not found") +MSG_DEF(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, 0, JSEXN_SYNTAXERR, "ambiguous indirect export") +MSG_DEF(JSMSG_MISSING_IMPORT, 0, JSEXN_SYNTAXERR, "import not found") +MSG_DEF(JSMSG_AMBIGUOUS_IMPORT, 0, JSEXN_SYNTAXERR, "ambiguous import") MSG_DEF(JSMSG_MISSING_NAMESPACE_EXPORT, 0, JSEXN_SYNTAXERR, "export not found for namespace") MSG_DEF(JSMSG_MISSING_EXPORT, 1, JSEXN_SYNTAXERR, "local binding for export '{0}' not found") MSG_DEF(JSMSG_MODULE_INSTANTIATE_FAILED, 0, JSEXN_INTERNALERR, "attempt to re-instantiate module after failure") diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index 89750d61a..0dfeffc36 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -345,6 +345,50 @@ intrinsic_ThrowInternalError(JSContext* cx, unsigned argc, Value* vp) return false; } +static bool +intrinsic_GetErrorMessage(JSContext* cx, unsigned argc, Value* vp) +{ + CallArgs args = CallArgsFromVp(argc, vp); + MOZ_ASSERT(args.length() == 1); + MOZ_ASSERT(args[0].isInt32()); + + const JSErrorFormatString* errorString = GetErrorMessage(nullptr, args[0].toInt32()); + MOZ_ASSERT(errorString); + + MOZ_ASSERT(errorString->argCount == 0); + RootedString message(cx, JS_NewStringCopyZ(cx, errorString->format)); + if (!message) + return false; + + args.rval().setString(message); + return true; +} + +static bool +intrinsic_CreateModuleSyntaxError(JSContext* cx, unsigned argc, Value* vp) +{ + CallArgs args = CallArgsFromVp(argc, vp); + MOZ_ASSERT(args.length() == 4); + MOZ_ASSERT(args[0].isObject()); + MOZ_ASSERT(args[1].isInt32()); + MOZ_ASSERT(args[2].isInt32()); + MOZ_ASSERT(args[3].isString()); + + RootedModuleObject module(cx, &args[0].toObject().as()); + RootedString filename(cx, JS_NewStringCopyZ(cx, module->script()->filename())); + RootedString message(cx, args[3].toString()); + + RootedValue error(cx); + if (!JS::CreateError(cx, JSEXN_SYNTAXERR, nullptr, filename, args[1].toInt32(), + args[2].toInt32(), nullptr, message, &error)) + { + return false; + } + + args.rval().set(error); + return true; +} + /** * Handles an assertion failure in self-hosted code just like an assertion * failure in C++ code. Information about the failure can be provided in args[0]. @@ -2339,6 +2383,8 @@ static const JSFunctionSpec intrinsic_functions[] = { JS_FN("ThrowTypeError", intrinsic_ThrowTypeError, 4,0), JS_FN("ThrowSyntaxError", intrinsic_ThrowSyntaxError, 4,0), JS_FN("ThrowInternalError", intrinsic_ThrowInternalError, 4,0), + JS_FN("GetErrorMessage", intrinsic_GetErrorMessage, 1,0), + JS_FN("CreateModuleSyntaxError", intrinsic_CreateModuleSyntaxError, 4,0), JS_FN("AssertionFailed", intrinsic_AssertionFailed, 1,0), JS_FN("DumpMessage", intrinsic_DumpMessage, 1,0), JS_FN("OwnPropertyKeys", intrinsic_OwnPropertyKeys, 1,0), -- cgit v1.2.3