From 83ec9e83a5dbe0538af89973dd976a7334749820 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 2 Sep 2023 13:11:36 +0200 Subject: [PATCH 1/2] compiler: add //go:noescape pragma This only works on declarations, not definitions. This is intentional: it follows the upstream Go implemetation. However, we might want to loosen this requirement at some point: TinyGo sometimes stores pointers in memory mapped I/O knowing they won't actually escape, but the compiler doesn't know about this. --- compiler/calls.go | 12 +++++++----- compiler/symbol.go | 23 ++++++++++++++++++++--- compiler/testdata/pragma.go | 9 +++++++++ compiler/testdata/pragma.ll | 8 ++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/compiler/calls.go b/compiler/calls.go index f4b76a5135..6400e634bd 100644 --- a/compiler/calls.go +++ b/compiler/calls.go @@ -19,8 +19,9 @@ const maxFieldsPerParam = 3 // useful while declaring or defining a function. type paramInfo struct { llvmType llvm.Type - name string // name, possibly with suffixes for e.g. struct fields - elemSize uint64 // size of pointer element type, or 0 if this isn't a pointer + name string // name, possibly with suffixes for e.g. struct fields + elemSize uint64 // size of pointer element type, or 0 if this isn't a pointer + flags paramFlags // extra flags for this parameter } // paramFlags identifies parameter attributes for flags. Most importantly, it @@ -28,9 +29,9 @@ type paramInfo struct { type paramFlags uint8 const ( - // Parameter may have the deferenceable_or_null attribute. This attribute - // cannot be applied to unsafe.Pointer and to the data pointer of slices. - paramIsDeferenceableOrNull = 1 << iota + // Whether this is a full or partial Go parameter (int, slice, etc). + // The extra context parameter is not a Go parameter. + paramIsGoParam = 1 << iota ) // createRuntimeCallCommon creates a runtime call. Use createRuntimeCall or @@ -195,6 +196,7 @@ func (c *compilerContext) getParamInfo(t llvm.Type, name string, goType types.Ty info := paramInfo{ llvmType: t, name: name, + flags: paramIsGoParam, } if goType != nil { switch underlying := goType.Underlying().(type) { diff --git a/compiler/symbol.go b/compiler/symbol.go index 9b9b1d10e8..48b4fafe10 100644 --- a/compiler/symbol.go +++ b/compiler/symbol.go @@ -33,6 +33,7 @@ type functionInfo struct { exported bool // go:export, CGo interrupt bool // go:interrupt nobounds bool // go:nobounds + noescape bool // go:noescape variadic bool // go:variadic (CGo only) inline inlineType // go:inline } @@ -127,11 +128,20 @@ func (c *compilerContext) getFunction(fn *ssa.Function) (llvm.Type, llvm.Value) c.addStandardDeclaredAttributes(llvmFn) dereferenceableOrNullKind := llvm.AttributeKindID("dereferenceable_or_null") - for i, info := range paramInfos { - if info.elemSize != 0 { - dereferenceableOrNull := c.ctx.CreateEnumAttribute(dereferenceableOrNullKind, info.elemSize) + for i, paramInfo := range paramInfos { + if paramInfo.elemSize != 0 { + dereferenceableOrNull := c.ctx.CreateEnumAttribute(dereferenceableOrNullKind, paramInfo.elemSize) llvmFn.AddAttributeAtIndex(i+1, dereferenceableOrNull) } + if info.noescape && paramInfo.flags¶mIsGoParam != 0 && paramInfo.llvmType.TypeKind() == llvm.PointerTypeKind { + // Parameters to functions with a //go:noescape parameter should get + // the nocapture attribute. However, the context parameter should + // not. + // (It may be safe to add the nocapture parameter to the context + // parameter, but I'd like to stay on the safe side here). + nocapture := c.ctx.CreateEnumAttribute(llvm.AttributeKindID("nocapture"), 0) + llvmFn.AddAttributeAtIndex(i+1, nocapture) + } } // Set a number of function or parameter attributes, depending on the @@ -394,6 +404,13 @@ func (c *compilerContext) parsePragmas(info *functionInfo, f *ssa.Function) { if hasUnsafeImport(f.Pkg.Pkg) { info.nobounds = true } + case "//go:noescape": + // Don't let pointer parameters escape. + // Following the upstream Go implementation, we only do this for + // declarations, not definitions. + if len(f.Blocks) == 0 { + info.noescape = true + } case "//go:variadic": // The //go:variadic pragma is emitted by the CGo preprocessing // pass for C variadic functions. This includes both explicit diff --git a/compiler/testdata/pragma.go b/compiler/testdata/pragma.go index 1f6badf7fb..1e6e967f53 100644 --- a/compiler/testdata/pragma.go +++ b/compiler/testdata/pragma.go @@ -106,3 +106,12 @@ var undefinedGlobalNotInSection uint32 //go:align 1024 //go:section .global_section var multipleGlobalPragmas uint32 + +//go:noescape +func doesNotEscapeParam(a *int, b []int, c chan int, d *[0]byte) + +// The //go:noescape pragma only works on declarations, not definitions. +// +//go:noescape +func stillEscapes(a *int, b []int, c chan int, d *[0]byte) { +} diff --git a/compiler/testdata/pragma.ll b/compiler/testdata/pragma.ll index 28e678359d..fa8015a90d 100644 --- a/compiler/testdata/pragma.ll +++ b/compiler/testdata/pragma.ll @@ -85,6 +85,14 @@ entry: declare void @main.undefinedFunctionNotInSection(ptr) #1 +declare void @main.doesNotEscapeParam(ptr nocapture dereferenceable_or_null(4), ptr nocapture, i32, i32, ptr nocapture dereferenceable_or_null(32), ptr nocapture, ptr) #1 + +; Function Attrs: nounwind +define hidden void @main.stillEscapes(ptr dereferenceable_or_null(4) %a, ptr %b.data, i32 %b.len, i32 %b.cap, ptr dereferenceable_or_null(32) %c, ptr %d, ptr %context) unnamed_addr #2 { +entry: + ret void +} + attributes #0 = { allockind("alloc,zeroed") allocsize(0) "alloc-family"="runtime.alloc" "target-features"="+bulk-memory,+mutable-globals,+nontrapping-fptoint,+sign-ext" } attributes #1 = { "target-features"="+bulk-memory,+mutable-globals,+nontrapping-fptoint,+sign-ext" } attributes #2 = { nounwind "target-features"="+bulk-memory,+mutable-globals,+nontrapping-fptoint,+sign-ext" } From a45b476331adb08cd08c85d23f2c4d630726fddb Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 2 Sep 2023 13:52:21 +0200 Subject: [PATCH 2/2] cgo: add support for `#cgo noescape` lines Here is the proposal: https://github.com/golang/go/issues/56378 They are documented here: https://pkg.go.dev/cmd/cgo@master#hdr-Optimizing_calls_of_C_code This would have been very useful to fix https://github.com/tinygo-org/bluetooth/issues/176 in a nice way. That bug is now fixed in a different way using a wrapper function, but once this new noescape pragma gets included in TinyGo we could remove the workaround and use `#cgo noescape` instead. --- cgo/cgo.go | 53 +++++++++++++++++++++++++++++++++++++ cgo/libclang.go | 10 ++++++- cgo/testdata/errors.go | 5 ++++ cgo/testdata/errors.out.go | 17 +++++++----- cgo/testdata/symbols.go | 5 ++++ cgo/testdata/symbols.out.go | 5 ++++ 6 files changed, 87 insertions(+), 8 deletions(-) diff --git a/cgo/cgo.go b/cgo/cgo.go index a90b307538..7ff6d2255c 100644 --- a/cgo/cgo.go +++ b/cgo/cgo.go @@ -18,6 +18,7 @@ import ( "go/scanner" "go/token" "path/filepath" + "sort" "strconv" "strings" @@ -42,6 +43,7 @@ type cgoPackage struct { fset *token.FileSet tokenFiles map[string]*token.File definedGlobally map[string]ast.Node + noescapingFuncs map[string]*noescapingFunc // #cgo noescape lines anonDecls map[interface{}]string cflags []string // CFlags from #cgo lines ldflags []string // LDFlags from #cgo lines @@ -80,6 +82,13 @@ type bitfieldInfo struct { endBit int64 // may be 0 meaning "until the end of the field" } +// Information about a #cgo noescape line in the source code. +type noescapingFunc struct { + name string + pos token.Pos + used bool // true if used somewhere in the source (for proper error reporting) +} + // cgoAliases list type aliases between Go and C, for types that are equivalent // in both languages. See addTypeAliases. var cgoAliases = map[string]string{ @@ -186,6 +195,7 @@ func Process(files []*ast.File, dir, importPath string, fset *token.FileSet, cfl fset: fset, tokenFiles: map[string]*token.File{}, definedGlobally: map[string]ast.Node{}, + noescapingFuncs: map[string]*noescapingFunc{}, anonDecls: map[interface{}]string{}, visitedFiles: map[string][]byte{}, } @@ -344,6 +354,22 @@ func Process(files []*ast.File, dir, importPath string, fset *token.FileSet, cfl }) } + // Show an error when a #cgo noescape line isn't used in practice. + // This matches upstream Go. I think the goal is to avoid issues with + // misspelled function names, which seems very useful. + var unusedNoescapeLines []*noescapingFunc + for _, value := range p.noescapingFuncs { + if !value.used { + unusedNoescapeLines = append(unusedNoescapeLines, value) + } + } + sort.SliceStable(unusedNoescapeLines, func(i, j int) bool { + return unusedNoescapeLines[i].pos < unusedNoescapeLines[j].pos + }) + for _, value := range unusedNoescapeLines { + p.addError(value.pos, fmt.Sprintf("function %#v in #cgo noescape line is not used", value.name)) + } + // Print the newly generated in-memory AST, for debugging. //ast.Print(fset, p.generated) @@ -409,6 +435,33 @@ func (p *cgoPackage) parseCGoPreprocessorLines(text string, pos token.Pos) strin } text = text[:lineStart] + string(spaces) + text[lineEnd:] + allFields := strings.Fields(line[4:]) + switch allFields[0] { + case "noescape": + // The code indicates that pointer parameters will not be captured + // by the called C function. + if len(allFields) < 2 { + p.addErrorAfter(pos, text[:lineStart], "missing function name in #cgo noescape line") + continue + } + if len(allFields) > 2 { + p.addErrorAfter(pos, text[:lineStart], "multiple function names in #cgo noescape line") + continue + } + name := allFields[1] + p.noescapingFuncs[name] = &noescapingFunc{ + name: name, + pos: pos, + used: false, + } + continue + case "nocallback": + // We don't do anything special when calling a C function, so there + // appears to be no optimization that we can do here. + // Accept, but ignore the parameter for compatibility. + continue + } + // Get the text before the colon in the #cgo directive. colon := strings.IndexByte(line, ':') if colon < 0 { diff --git a/cgo/libclang.go b/cgo/libclang.go index 794d4e81f1..54ff9f53fb 100644 --- a/cgo/libclang.go +++ b/cgo/libclang.go @@ -256,10 +256,18 @@ func (f *cgoFile) createASTNode(name string, c clangCursor) (ast.Node, any) { }, }, } + var doc []string if C.clang_isFunctionTypeVariadic(cursorType) != 0 { + doc = append(doc, "//go:variadic") + } + if _, ok := f.noescapingFuncs[name]; ok { + doc = append(doc, "//go:noescape") + f.noescapingFuncs[name].used = true + } + if len(doc) != 0 { decl.Doc.List = append(decl.Doc.List, &ast.Comment{ Slash: pos - 1, - Text: "//go:variadic", + Text: strings.Join(doc, "\n"), }) } for i := 0; i < numArgs; i++ { diff --git a/cgo/testdata/errors.go b/cgo/testdata/errors.go index 75828ce0f1..8813f83cf4 100644 --- a/cgo/testdata/errors.go +++ b/cgo/testdata/errors.go @@ -10,6 +10,11 @@ typedef struct { typedef someType noType; // undefined type +// Some invalid noescape lines +#cgo noescape +#cgo noescape foo bar +#cgo noescape unusedFunction + #define SOME_CONST_1 5) // invalid const syntax #define SOME_CONST_2 6) // const not used (so no error) #define SOME_CONST_3 1234 // const too large for byte diff --git a/cgo/testdata/errors.out.go b/cgo/testdata/errors.out.go index baadba68d2..15a47bfe70 100644 --- a/cgo/testdata/errors.out.go +++ b/cgo/testdata/errors.out.go @@ -1,14 +1,17 @@ // CGo errors: +// testdata/errors.go:14:1: missing function name in #cgo noescape line +// testdata/errors.go:15:1: multiple function names in #cgo noescape line // testdata/errors.go:4:2: warning: some warning // testdata/errors.go:11:9: error: unknown type name 'someType' -// testdata/errors.go:26:5: warning: another warning -// testdata/errors.go:13:23: unexpected token ), expected end of expression -// testdata/errors.go:21:26: unexpected token ), expected end of expression -// testdata/errors.go:16:33: unexpected token ), expected end of expression -// testdata/errors.go:17:34: unexpected token ), expected end of expression +// testdata/errors.go:31:5: warning: another warning +// testdata/errors.go:18:23: unexpected token ), expected end of expression +// testdata/errors.go:26:26: unexpected token ), expected end of expression +// testdata/errors.go:21:33: unexpected token ), expected end of expression +// testdata/errors.go:22:34: unexpected token ), expected end of expression // -: unexpected token INT, expected end of expression -// testdata/errors.go:30:35: unexpected number of parameters: expected 2, got 3 -// testdata/errors.go:31:31: unexpected number of parameters: expected 2, got 1 +// testdata/errors.go:35:35: unexpected number of parameters: expected 2, got 3 +// testdata/errors.go:36:31: unexpected number of parameters: expected 2, got 1 +// testdata/errors.go:3:1: function "unusedFunction" in #cgo noescape line is not used // Type checking errors after CGo processing: // testdata/errors.go:102: cannot use 2 << 10 (untyped int constant 2048) as C.char value in variable declaration (overflows) diff --git a/cgo/testdata/symbols.go b/cgo/testdata/symbols.go index fb585c2f87..c8029a1481 100644 --- a/cgo/testdata/symbols.go +++ b/cgo/testdata/symbols.go @@ -9,6 +9,10 @@ static void staticfunc(int x); // Global variable signatures. extern int someValue; + +void notEscapingFunction(int *a); + +#cgo noescape notEscapingFunction */ import "C" @@ -18,6 +22,7 @@ func accessFunctions() { C.variadic0() C.variadic2(3, 5) C.staticfunc(3) + C.notEscapingFunction(nil) } func accessGlobals() { diff --git a/cgo/testdata/symbols.out.go b/cgo/testdata/symbols.out.go index 569cb65f6a..1677959cd6 100644 --- a/cgo/testdata/symbols.out.go +++ b/cgo/testdata/symbols.out.go @@ -67,5 +67,10 @@ func C.staticfunc!symbols.go(x C.int) var C.staticfunc!symbols.go$funcaddr unsafe.Pointer +//export notEscapingFunction +//go:noescape +func C.notEscapingFunction(a *C.int) + +var C.notEscapingFunction$funcaddr unsafe.Pointer //go:extern someValue var C.someValue C.int