From 3d71c762b2345706a1b041b523616a54185b6fc8 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Tue, 12 Sep 2023 19:00:54 +0200 Subject: [PATCH] FFI: disallow interface specification on locals (#886) ## Summary Change the language specification and implementation to disallow using the `.nodecl`, `.header`, `.importc`, `.importjs`, `.exportc`, `.extern`, and `.dynlib` pragmas on local 'var'/'let' definitions -- locals exist only inside routines and thus cannot be foreign. ## Details Except for parameters, which can be detected through the `skParam` kind, locals cannot be distinguished from globals through just the symbol kind. Therefore, they need to be detected through dedicated logic that inspects the full symbol. This check is implemented by the new `isLocal` procedure added to the `pragmas` module. The `sfGlobal` flag cannot be used for detecting globals at this point, as the flag is only included on the symbols for declarations outside of routines *after* pragma processing is finished. Instead, `isLocal` checks for whether the owner of the symbol is a module -- nonetheless testing for the `sfGlobal` flag ensures that declarations with `.global` can also use the FFI. During application of the FFI pragmas, it is now first checked whether the applied-to symbol is that of a local, and if it is, an error is returned. The error uses the new `adSemExternalLocalNotAllowed` diagnostic plus associated report. Finally, the documentation for the pragmas is updated. ### Code-generator cleanup - the name mangling in the C code generator is updated to not inspect the `extname` for parameters and locals -- the string is now always empty for those - testing for the `sfImportc` and `extNoDecl` flag for locals is removed from the JS code generator -- the flags are never present on the symbol now ### Other adjustments The only place in the compiler where using the FFI with locals was relied on is with the `asyncjs` module. There, `.importjs` is used for getting access to JavaScript's `undefined` value. However, using a local is not necessary for achieving this -- a `proc` using `.importjs` works as well. --- compiler/ast/ast_types.nim | 4 +++- compiler/ast/report_enums.nim | 2 ++ compiler/backend/ccgtypes.nim | 6 ++---- compiler/backend/jsgen.nim | 2 +- compiler/front/cli_reporter.nim | 7 ++++++- compiler/front/msgs.nim | 1 + compiler/sem/pragmas.nim | 37 ++++++++++++++++++++++++++++++--- doc/manual.rst | 20 ++++++++++-------- lib/js/asyncjs.nim | 7 ++++--- 9 files changed, 64 insertions(+), 22 deletions(-) diff --git a/compiler/ast/ast_types.nim b/compiler/ast/ast_types.nim index 5a26573b49b..e36781800df 100644 --- a/compiler/ast/ast_types.nim +++ b/compiler/ast/ast_types.nim @@ -1064,6 +1064,7 @@ type # pragmas adSemInvalidPragma adSemIllegalCustomPragma + adSemExternalLocalNotAllowed adSemStringLiteralExpected adSemIntLiteralExpected adSemOnOrOffExpected @@ -1341,7 +1342,8 @@ type adSemExpectedRangeType, adSemExpectedLabel, adSemContinueCannotHaveLabel, - adSemUnavailableLocation: + adSemUnavailableLocation, + adSemExternalLocalNotAllowed: discard of adSemExpectedIdentifierInExpr: notIdent*: PNode diff --git a/compiler/ast/report_enums.nim b/compiler/ast/report_enums.nim index 535ba38733b..9dad3e44b13 100644 --- a/compiler/ast/report_enums.nim +++ b/compiler/ast/report_enums.nim @@ -655,6 +655,8 @@ type rsemPropositionExpected rsemIllegalCustomPragma ## supplied pragma is not a legal custom pragma, and cannot be attached + rsemExternalLocalNotAllowed + ## a local is specified to be part of an external interface rsemNoReturnHasReturn ## a routine marked as no return, has a return type rsemImplicitPragmaError diff --git a/compiler/backend/ccgtypes.nim b/compiler/backend/ccgtypes.nim index fe31f776f46..89605fb76d3 100644 --- a/compiler/backend/ccgtypes.nim +++ b/compiler/backend/ccgtypes.nim @@ -46,8 +46,7 @@ proc mangleParamName(c: ConfigRef; s: PSym): Rope = ## we cannot use 'sigConflicts' here since we don't have access to a BProc. ## Fortunately C's scoping rules are sane enough so that that doesn't ## cause any trouble. - result = s.extname - if result == "": + if true: var res = s.name.s.mangle if isKeyword(s.name) or c.cppDefines.contains(res): res.add "_0" @@ -57,8 +56,7 @@ proc mangleParamName(c: ConfigRef; s: PSym): Rope = proc mangleLocalName(p: BProc; s: PSym): Rope = assert s.kind in skLocalVars+{skTemp} #assert sfGlobal notin s.flags - result = s.extname - if result == "": + if true: var key: string shallowCopy(key, s.name.s.mangle) let counter = p.sigConflicts.getOrDefault(key) diff --git a/compiler/backend/jsgen.nim b/compiler/backend/jsgen.nim index 8210d31e6e2..83264cdfb09 100644 --- a/compiler/backend/jsgen.nim +++ b/compiler/backend/jsgen.nim @@ -1703,7 +1703,7 @@ proc genDef(p: PProc, it: CgNode) = assert it[0].kind == cnkSym let v = it[0].sym let name = mangleName(p.module, v) - if exfNoDecl notin v.extFlags and sfImportc notin v.flags: + if true: genLineDir(p, it) genVarInit(p, v, name, it[1]) diff --git a/compiler/front/cli_reporter.nim b/compiler/front/cli_reporter.nim index f3d7b4155c2..ae031669eda 100644 --- a/compiler/front/cli_reporter.nim +++ b/compiler/front/cli_reporter.nim @@ -1822,6 +1822,10 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string = of rsemIllegalCustomPragma: result = "cannot attach a custom pragma to '$1'" % r.symstr + of rsemExternalLocalNotAllowed: + result = "parameters and local 'let'/'var' cannot be part of an" & + " external interface" + of rsemCallingConventionMismatch: assert false, "REMOVE" @@ -3290,7 +3294,8 @@ func astDiagToLegacyReport(conf: ConfigRef, diag: PAstDiag): Report {.inline.} = adSemExpectedRangeType, adSemExpectedLabel, adSemContinueCannotHaveLabel, - adSemUnavailableLocation: + adSemUnavailableLocation, + adSemExternalLocalNotAllowed: semRep = SemReport( location: some diag.location, reportInst: diag.instLoc.toReportLineInfo, diff --git a/compiler/front/msgs.nim b/compiler/front/msgs.nim index a0c27828077..47d2fa0f81a 100644 --- a/compiler/front/msgs.nim +++ b/compiler/front/msgs.nim @@ -442,6 +442,7 @@ func astDiagToLegacyReportKind*( of adSemCannotImportItself: rsemCannotImportItself of adSemInvalidPragma: rsemInvalidPragma of adSemIllegalCustomPragma: rsemIllegalCustomPragma + of adSemExternalLocalNotAllowed: rsemExternalLocalNotAllowed of adSemStringLiteralExpected: rsemStringLiteralExpected of adSemIntLiteralExpected: rsemIntLiteralExpected of adSemOnOrOffExpected: rsemOnOrOffExpected diff --git a/compiler/sem/pragmas.nim b/compiler/sem/pragmas.nim index ef851771b07..995816373a3 100644 --- a/compiler/sem/pragmas.nim +++ b/compiler/sem/pragmas.nim @@ -159,6 +159,16 @@ proc illegalCustomPragma*(c: PContext; n: PNode, s: PSym): PNode = n, PAstDiag(kind: adSemIllegalCustomPragma, customPragma: s)) +func isLocal(s: PSym): bool = + ## Returns whether `s` is a parameter or local `var`/`let`. + # XXX: ideally, we could rely on ``sfGlobal``, but for declarations + # outside of routines, the flag is currently only included after pragma + # processing + s.kind in skLocalVars and sfGlobal notin s.flags and s.owner.kind != skModule + +proc disallowedExternalLocal(c: PContext, n: PNode): PNode = + c.config.newError(n, PAstDiag(kind: adSemExternalLocalNotAllowed)) + proc pragmaAsm*(c: PContext, n: PNode): tuple[marker: char, err: PNode] = ## Gets the marker out of an asm stmts pragma list ## Returns ` as the default marker if no other markers are found @@ -1063,6 +1073,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = let k = whichPragma(it) case k of wExportc: + if isLocal(sym): + return disallowedExternalLocal(c, it) + let extLit = semExternName(c.config, getOptionalStrLit(c, it, "$1")) result = @@ -1074,6 +1087,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = incl(sym.flags, sfUsed) # avoid wrong hints of wImportc: + if isLocal(sym): + return disallowedExternalLocal(c, it) + let nameLit = semExternName(c.config, getOptionalStrLit(c, it, "$1")) case nameLit.kind of nkError: @@ -1099,6 +1115,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = processImportCompilerProc(c, sym, name) result = it of wExtern: + if isLocal(sym): + return disallowedExternalLocal(c, it) + let name = semExternName(c.config, getStrLitNode(c, it)) result = if name.isError: @@ -1111,6 +1130,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = assert sym.kind == skTemplate incl(sym.flags, sfDirty) of wImportJs: + if isLocal(sym): + return disallowedExternalLocal(c, it) + # note: don't use ``semExternName`` here. The operand is *not* an # external name, but rather a *pattern* string let nameLit = getOptionalStrLit(c, it, "$1") @@ -1156,8 +1178,11 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = else: c.config.newError(it, PAstDiag(kind: adSemAlignRequiresPowerOfTwo)) of wNodecl: - result = noVal(c, it) - incl(sym.extFlags, exfNoDecl) + if isLocal(sym): + result = disallowedExternalLocal(c, it) + else: + result = noVal(c, it) + incl(sym.extFlags, exfNoDecl) of wPure, wAsmNoStackFrame: result = noVal(c, it) incl(sym.flags, sfPure) @@ -1189,6 +1214,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = result = noVal(c, it) sym.flags.incl {sfGlobal, sfPure} of wHeader: + if isLocal(sym): + return disallowedExternalLocal(c, it) + let path = getStrLitNode(c, it) # the path or an error if path.isError: return path @@ -1219,7 +1247,10 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode = result = noVal(c, it) incl(sym.flags, sfWasForwarded) of wDynlib: - result = processDynLib(c, it, sym) + if isLocal(sym): + result = disallowedExternalLocal(c, it) + else: + result = processDynLib(c, it, sym) of wCompilerProc, wCore: result = noVal(c, it) # compilerproc may not get a string! cppDefine(c.graph.config, sym.name.s) diff --git a/doc/manual.rst b/doc/manual.rst index 6ca6295d4c0..87d89cf3592 100644 --- a/doc/manual.rst +++ b/doc/manual.rst @@ -6945,8 +6945,8 @@ in C). nodecl pragma ------------- -The `nodecl` pragma can be applied to almost any symbol (variable, proc, -type, etc.) and is sometimes useful for interoperability with C: +The `nodecl` pragma can be applied to the symbol of procedures, types, +globals, and constants and is sometimes useful for interoperability with C: It tells Nim that it should not generate a declaration for the symbol in the C code. For example: @@ -6963,8 +6963,9 @@ However, the `header` pragma is often the better alternative. Header pragma ------------- The `header` pragma is very similar to the `nodecl` pragma: It can be -applied to almost any symbol and specifies that it should not be declared -and instead, the generated code should contain an `#include`:c:\: +applied to the symbol of procedures, types, globals, and constants and +specifies that it should not be declared and instead, the generated code +should contain an `#include`:c:\: .. code-block:: Nim type @@ -7340,9 +7341,9 @@ are documented here. Importc pragma -------------- -The `importc` pragma provides a means to import a proc or a variable -from C. The optional argument is a string containing the C identifier. If -the argument is missing, the C name is the Nim identifier *exactly as +The `importc` pragma provides a means to import a procecdure, type, or global +variable from C. The optional argument is a string containing the C identifier. +If the argument is missing, the C name is the Nim identifier *exactly as spelled*: .. code-block:: @@ -7371,7 +7372,7 @@ is available and a literal dollar sign must be written as ``$$``. Exportc pragma -------------- -The `exportc` pragma provides a means to export a type, a variable, or a +The `exportc` pragma provides a means to export a type, global, or a procedure to C. Enums and constants can't be exported. The optional argument is a string containing the C identifier. If the argument is missing, the C name is the Nim identifier *exactly as spelled*: @@ -7408,6 +7409,7 @@ mangling. The string literal passed to `extern` can be a format string: In the example, the external name of `p` is set to `prefixp`. Only ``$1`` is available and a literal dollar sign must be written as ``$$``. +Only the name mangling of procedures, globals, and constants can be changed. Bycopy pragma ------------- @@ -7467,7 +7469,7 @@ a static error. Usage with inheritance should be defined and documented. Dynlib pragma for import ------------------------ -With the `dynlib` pragma, a procedure or a variable can be imported from +With the `dynlib` pragma, a procedure or global variable can be imported from a dynamic library (``.dll`` files for Windows, ``lib*.so`` files for UNIX). The non-optional argument has to be the name of the dynamic library: diff --git a/lib/js/asyncjs.nim b/lib/js/asyncjs.nim index 861fe3fe21f..7193beff9db 100644 --- a/lib/js/asyncjs.nim +++ b/lib/js/asyncjs.nim @@ -80,7 +80,8 @@ proc replaceReturn(node: var NimNode) = var son = node[z] let jsResolve = ident("jsResolve") if son.kind == nnkReturnStmt: - let value = if son[0].kind != nnkEmpty: nnkCall.newTree(jsResolve, son[0]) else: jsResolve + let value = if son[0].kind != nnkEmpty: nnkCall.newTree(jsResolve, son[0]) + else: nnkCall.newTree(jsResolve) node[z] = nnkReturnStmt.newTree(value) elif son.kind == nnkAsgn and son[0].kind == nnkIdent and $son[0] == "result": node[z] = nnkAsgn.newTree(son[0], nnkCall.newTree(jsResolve, son[1])) @@ -119,7 +120,7 @@ proc generateJsasync(arg: NimNode): NimNode = var resolve: NimNode if isVoid: resolve = quote: - var `jsResolve` {.importjs: "undefined".}: Future[void] + proc `jsResolve`: Future[void] {.importjs: "(undefined)", used.} else: resolve = quote: proc jsResolve[T](a: T): Future[T] {.importjs: "#", used.} @@ -132,7 +133,7 @@ proc generateJsasync(arg: NimNode): NimNode = if len(code) > 0 and isVoid: var voidFix = quote: - return `jsResolve` + return `jsResolve`() result.body.add(voidFix) let asyncPragma = quote: