X Tutup
Skip to content

Commit 7682017

Browse files
rscrsc
committed
cmd/compile: detect and diagnose invalid //go: directive placement
Thie CL changes cmd/compile/internal/syntax to give the gc half of the compiler more control over pragma handling, so that it can prepare better errors, diagnose misuse, and so on. Before, the API between the two was hard-coded as a uint16. Now it is an interface{}. This should set us up better for future directives. In addition to the split, this CL emits a "misplaced compiler directive" error for any directive that is in a place where it has no effect. I've certainly been confused in the past by adding comments that were doing nothing and not realizing it. This should help avoid that kind of confusion. The rule, now applied consistently, is that a //go: directive must appear on a line by itself immediately before the declaration specifier it means to apply to. See cmd/compile/doc.go for precise text and test/directive.go for examples. This may cause some code to stop compiling, but that code was broken. For example, this code formerly applied the //go:noinline to f (not c) but now will fail to compile: //go:noinline const c = 1 func f() {} Change-Id: Ieba9b8d90a27cfab25de79d2790a895cefe5296f Reviewed-on: https://go-review.googlesource.com/c/go/+/228578 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
1 parent 4f27e1d commit 7682017

File tree

10 files changed

+345
-88
lines changed

10 files changed

+345
-88
lines changed

‎src/cmd/compile/doc.go‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -195,30 +195,58 @@ directive can skip over a directive like any other comment.
195195
// Line directives typically appear in machine-generated code, so that compilers and debuggers
196196
// will report positions in the original input to the generator.
197197
/*
198-
The line directive is an historical special case; all other directives are of the form
199-
//go:name and must start at the beginning of a line, indicating that the directive is defined
200-
by the Go toolchain.
198+
The line directive is a historical special case; all other directives are of the form
199+
//go:name, indicating that they are defined by the Go toolchain.
200+
Each directive must be placed its own line, with only leading spaces and tabs
201+
allowed before the comment.
202+
Each directive applies to the Go code that immediately follows it,
203+
which typically must be a declaration.
201204
202205
//go:noescape
203206
204-
The //go:noescape directive specifies that the next declaration in the file, which
205-
must be a func without a body (meaning that it has an implementation not written
206-
in Go) does not allow any of the pointers passed as arguments to escape into the
207-
heap or into the values returned from the function. This information can be used
208-
during the compiler's escape analysis of Go code calling the function.
207+
The //go:noescape directive must be followed by a function declaration without
208+
a body (meaning that the function has an implementation not written in Go).
209+
It specifies that the function does not allow any of the pointers passed as
210+
arguments to escape into the heap or into the values returned from the function.
211+
This information can be used during the compiler's escape analysis of Go code
212+
calling the function.
213+
214+
//go:uintptrescapes
215+
216+
The //go:uintptrescapes directive must be followed by a function declaration.
217+
It specifies that the function's uintptr arguments may be pointer values
218+
that have been converted to uintptr and must be treated as such by the
219+
garbage collector. The conversion from pointer to uintptr must appear in
220+
the argument list of any call to this function. This directive is necessary
221+
for some low-level system call implementations and should be avoided otherwise.
222+
223+
//go:noinline
224+
225+
The //go:noinline directive must be followed by a function declaration.
226+
It specifies that calls to the function should not be inlined, overriding
227+
the compiler's usual optimization rules. This is typically only needed
228+
for special runtime functions or when debugging the compiler.
229+
230+
//go:norace
231+
232+
The //go:norace directive must be followed by a function declaration.
233+
It specifies that the function's memory accesses must be ignored by the
234+
race detector. This is most commonly used in low-level code invoked
235+
at times when it is unsafe to call into the race detector runtime.
209236
210237
//go:nosplit
211238
212-
The //go:nosplit directive specifies that the next function declared in the file must
213-
not include a stack overflow check. This is most commonly used by low-level
214-
runtime sources invoked at times when it is unsafe for the calling goroutine to be
215-
preempted.
239+
The //go:nosplit directive must be followed by a function declaration.
240+
It specifies that the function must omit its usual stack overflow check.
241+
This is most commonly used by low-level runtime code invoked
242+
at times when it is unsafe for the calling goroutine to be preempted.
216243
217244
//go:linkname localname [importpath.name]
218245
219-
The //go:linkname directive instructs the compiler to use ``importpath.name'' as the
220-
object file symbol name for the variable or function declared as ``localname'' in the
221-
source code.
246+
This special directive does not apply to the Go code that follows it.
247+
Instead, the //go:linkname directive instructs the compiler to use ``importpath.name''
248+
as the object file symbol name for the variable or function declared as ``localname''
249+
in the source code.
222250
If the ``importpath.name'' argument is omitted, the directive uses the
223251
symbol's default object file symbol name and only has the effect of making
224252
the symbol accessible to other packages.

‎src/cmd/compile/internal/gc/lex.go‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,18 @@ func isQuoted(s string) bool {
2828
return len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"'
2929
}
3030

31+
type PragmaFlag int16
32+
3133
const (
3234
// Func pragmas.
33-
Nointerface syntax.Pragma = 1 << iota
34-
Noescape // func parameters don't escape
35-
Norace // func must not have race detector annotations
36-
Nosplit // func should not execute on separate stack
37-
Noinline // func should not be inlined
38-
NoCheckPtr // func should not be instrumented by checkptr
39-
CgoUnsafeArgs // treat a pointer to one arg as a pointer to them all
40-
UintptrEscapes // pointers converted to uintptr escape
35+
Nointerface PragmaFlag = 1 << iota
36+
Noescape // func parameters don't escape
37+
Norace // func must not have race detector annotations
38+
Nosplit // func should not execute on separate stack
39+
Noinline // func should not be inlined
40+
NoCheckPtr // func should not be instrumented by checkptr
41+
CgoUnsafeArgs // treat a pointer to one arg as a pointer to them all
42+
UintptrEscapes // pointers converted to uintptr escape
4143

4244
// Runtime-only func pragmas.
4345
// See ../../../../runtime/README.md for detailed descriptions.
@@ -50,7 +52,24 @@ const (
5052
NotInHeap // values of this type must not be heap allocated
5153
)
5254

53-
func pragmaValue(verb string) syntax.Pragma {
55+
const (
56+
FuncPragmas = Nointerface |
57+
Noescape |
58+
Norace |
59+
Nosplit |
60+
Noinline |
61+
NoCheckPtr |
62+
CgoUnsafeArgs |
63+
UintptrEscapes |
64+
Systemstack |
65+
Nowritebarrier |
66+
Nowritebarrierrec |
67+
Yeswritebarrierrec
68+
69+
TypePragmas = NotInHeap
70+
)
71+
72+
func pragmaFlag(verb string) PragmaFlag {
5473
switch verb {
5574
case "go:nointerface":
5675
if objabi.Fieldtrack_enabled != 0 {

‎src/cmd/compile/internal/gc/noder.go‎

Lines changed: 85 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ func (p *noder) node() {
241241
p.setlineno(p.file.PkgName)
242242
mkpackage(p.file.PkgName.Value)
243243

244+
if pragma, ok := p.file.Pragma.(*Pragma); ok {
245+
p.checkUnused(pragma)
246+
}
247+
244248
xtop = append(xtop, p.decls(p.file.DeclList)...)
245249

246250
for _, n := range p.linknames {
@@ -313,6 +317,10 @@ func (p *noder) importDecl(imp *syntax.ImportDecl) {
313317
return // avoid follow-on errors if there was a syntax error
314318
}
315319

320+
if pragma, ok := imp.Pragma.(*Pragma); ok {
321+
p.checkUnused(pragma)
322+
}
323+
316324
val := p.basicLit(imp.Path)
317325
ipkg := importfile(&val)
318326

@@ -363,6 +371,10 @@ func (p *noder) varDecl(decl *syntax.VarDecl) []*Node {
363371
exprs = p.exprList(decl.Values)
364372
}
365373

374+
if pragma, ok := decl.Pragma.(*Pragma); ok {
375+
p.checkUnused(pragma)
376+
}
377+
366378
p.setlineno(decl)
367379
return variter(names, typ, exprs)
368380
}
@@ -384,6 +396,10 @@ func (p *noder) constDecl(decl *syntax.ConstDecl, cs *constState) []*Node {
384396
}
385397
}
386398

399+
if pragma, ok := decl.Pragma.(*Pragma); ok {
400+
p.checkUnused(pragma)
401+
}
402+
387403
names := p.declNames(decl.NameList)
388404
typ := p.typeExprOrNil(decl.Type)
389405

@@ -438,11 +454,13 @@ func (p *noder) typeDecl(decl *syntax.TypeDecl) *Node {
438454

439455
param := n.Name.Param
440456
param.Ntype = typ
441-
param.Pragma = decl.Pragma
442457
param.Alias = decl.Alias
443-
if param.Alias && param.Pragma != 0 {
444-
yyerror("cannot specify directive with type alias")
445-
param.Pragma = 0
458+
if pragma, ok := decl.Pragma.(*Pragma); ok {
459+
if !decl.Alias {
460+
param.Pragma = pragma.Flag & TypePragmas
461+
pragma.Flag &^= TypePragmas
462+
}
463+
p.checkUnused(pragma)
446464
}
447465

448466
nod := p.nod(decl, ODCLTYPE, n, nil)
@@ -493,10 +511,13 @@ func (p *noder) funcDecl(fun *syntax.FuncDecl) *Node {
493511
f.Func.Nname.Name.Defn = f
494512
f.Func.Nname.Name.Param.Ntype = t
495513

496-
pragma := fun.Pragma
497-
f.Func.Pragma = fun.Pragma
498-
if pragma&Systemstack != 0 && pragma&Nosplit != 0 {
499-
yyerrorl(f.Pos, "go:nosplit and go:systemstack cannot be combined")
514+
if pragma, ok := fun.Pragma.(*Pragma); ok {
515+
f.Func.Pragma = pragma.Flag & FuncPragmas
516+
if pragma.Flag&Systemstack != 0 && pragma.Flag&Nosplit != 0 {
517+
yyerrorl(f.Pos, "go:nosplit and go:systemstack cannot be combined")
518+
}
519+
pragma.Flag &^= FuncPragmas
520+
p.checkUnused(pragma)
500521
}
501522

502523
if fun.Recv == nil {
@@ -1479,13 +1500,58 @@ var allowedStdPragmas = map[string]bool{
14791500
"go:generate": true,
14801501
}
14811502

1503+
// *Pragma is the value stored in a syntax.Pragma during parsing.
1504+
type Pragma struct {
1505+
Flag PragmaFlag // collected bits
1506+
Pos []PragmaPos // position of each individual flag
1507+
}
1508+
1509+
type PragmaPos struct {
1510+
Flag PragmaFlag
1511+
Pos syntax.Pos
1512+
}
1513+
1514+
func (p *noder) checkUnused(pragma *Pragma) {
1515+
for _, pos := range pragma.Pos {
1516+
if pos.Flag&pragma.Flag != 0 {
1517+
p.yyerrorpos(pos.Pos, "misplaced compiler directive")
1518+
}
1519+
}
1520+
}
1521+
1522+
func (p *noder) checkUnusedDuringParse(pragma *Pragma) {
1523+
for _, pos := range pragma.Pos {
1524+
if pos.Flag&pragma.Flag != 0 {
1525+
p.error(syntax.Error{Pos: pos.Pos, Msg: "misplaced compiler directive"})
1526+
}
1527+
}
1528+
}
1529+
14821530
// pragma is called concurrently if files are parsed concurrently.
1483-
func (p *noder) pragma(pos syntax.Pos, text string) syntax.Pragma {
1484-
switch {
1485-
case strings.HasPrefix(text, "line "):
1531+
func (p *noder) pragma(pos syntax.Pos, blankLine bool, text string, old syntax.Pragma) syntax.Pragma {
1532+
pragma, _ := old.(*Pragma)
1533+
if pragma == nil {
1534+
pragma = new(Pragma)
1535+
}
1536+
1537+
if text == "" {
1538+
// unused pragma; only called with old != nil.
1539+
p.checkUnusedDuringParse(pragma)
1540+
return nil
1541+
}
1542+
1543+
if strings.HasPrefix(text, "line ") {
14861544
// line directives are handled by syntax package
14871545
panic("unreachable")
1546+
}
14881547

1548+
if !blankLine {
1549+
// directive must be on line by itself
1550+
p.error(syntax.Error{Pos: pos, Msg: "misplaced compiler directive"})
1551+
return pragma
1552+
}
1553+
1554+
switch {
14891555
case strings.HasPrefix(text, "go:linkname "):
14901556
f := strings.Fields(text)
14911557
if !(2 <= len(f) && len(f) <= 3) {
@@ -1513,7 +1579,8 @@ func (p *noder) pragma(pos syntax.Pos, text string) syntax.Pragma {
15131579
p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("invalid library name %q in cgo_import_dynamic directive", lib)})
15141580
}
15151581
p.pragcgo(pos, text)
1516-
return pragmaValue("go:cgo_import_dynamic")
1582+
pragma.Flag |= pragmaFlag("go:cgo_import_dynamic")
1583+
break
15171584
}
15181585
fallthrough
15191586
case strings.HasPrefix(text, "go:cgo_"):
@@ -1530,18 +1597,19 @@ func (p *noder) pragma(pos syntax.Pos, text string) syntax.Pragma {
15301597
if i := strings.Index(text, " "); i >= 0 {
15311598
verb = verb[:i]
15321599
}
1533-
prag := pragmaValue(verb)
1600+
flag := pragmaFlag(verb)
15341601
const runtimePragmas = Systemstack | Nowritebarrier | Nowritebarrierrec | Yeswritebarrierrec
1535-
if !compiling_runtime && prag&runtimePragmas != 0 {
1602+
if !compiling_runtime && flag&runtimePragmas != 0 {
15361603
p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("//%s only allowed in runtime", verb)})
15371604
}
1538-
if prag == 0 && !allowedStdPragmas[verb] && compiling_std {
1605+
if flag == 0 && !allowedStdPragmas[verb] && compiling_std {
15391606
p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("//%s is not allowed in the standard library", verb)})
15401607
}
1541-
return prag
1608+
pragma.Flag |= flag
1609+
pragma.Pos = append(pragma.Pos, PragmaPos{flag, pos})
15421610
}
15431611

1544-
return 0
1612+
return pragma
15451613
}
15461614

15471615
// isCgoGeneratedFile reports whether pos is in a file

‎src/cmd/compile/internal/gc/syntax.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package gc
88

99
import (
1010
"cmd/compile/internal/ssa"
11-
"cmd/compile/internal/syntax"
1211
"cmd/compile/internal/types"
1312
"cmd/internal/obj"
1413
"cmd/internal/objabi"
@@ -483,7 +482,7 @@ type Param struct {
483482
// OTYPE
484483
//
485484
// TODO: Should Func pragmas also be stored on the Name?
486-
Pragma syntax.Pragma
485+
Pragma PragmaFlag
487486
Alias bool // node is alias for Ntype (only used when type-checking ODCLTYPE)
488487
}
489488

@@ -565,7 +564,7 @@ type Func struct {
565564
Endlineno src.XPos
566565
WBPos src.XPos // position of first write barrier; see SetWBPos
567566

568-
Pragma syntax.Pragma // go:xxx function annotations
567+
Pragma PragmaFlag // go:xxx function annotations
569568

570569
flags bitset16
571570
numDefers int // number of defer calls in the function

0 commit comments

Comments
 (0)
X Tutup