diff --git a/parser/fuzz_test.go b/parser/fuzz_test.go index 42ccfa6..9dee9a1 100644 --- a/parser/fuzz_test.go +++ b/parser/fuzz_test.go @@ -94,6 +94,8 @@ func FuzzParseExpand(f *testing.F) { f.Add("#include \"no/such/header.h\"\n") f.Add("#line 7 \"f.s\"\n") f.Add("#define M(x) \\\n\tADD $x, R0 \\\n\tSUB $x, R1\nM(1)\n") // continuations + f.Add("#define M(...) x\nM(1)\n") // the C variadic spelling, a bad definition + f.Add("#define M(args...) x\nM(1)\n") // the GNU variadic spelling, one dotted name // A body that repeats its argument multiplies every nesting level; the // expansion must stop at the work budget, not produce the exponential // result it names. diff --git a/parser/preproc.go b/parser/preproc.go index cfc5d49..5bb1adc 100644 --- a/parser/preproc.go +++ b/parser/preproc.go @@ -238,6 +238,8 @@ func (pp *preproc) endif(line []token.Token) { // define parses "#define NAME[(formals)] body" into the macro table. The // body runs to the end of the logical line (the lexer has already spliced // backslash continuations) and stops at a comment, which never expands. +// A parameter list the toolchain would not accept is a bad definition: the +// whole #define is rejected, and the name stays unbound. func (pp *preproc) define(line []token.Token) { if len(line) < 3 || line[2].Kind != token.Ident { return @@ -246,22 +248,15 @@ func (pp *preproc) define(line []token.Token) { args := []string(nil) body := line[3:] // The definition is parameterised only when '(' follows the name - // directly; the toolchain separates "#define A(x)" from - // "#define A (x)" by adjacency, and so does the column check here. + // directly; the toolchain separates "#define A(x)" from "#define A (x)" + // by adjacency, and so does the column check here. if len(body) > 0 && body[0].Kind == token.LParen && body[0].Pos.Column == name.Pos.Column+utf8.RuneCountInString(name.Text) { - args = []string{} - i := 1 - for i < len(body) && body[i].Kind != token.RParen { - if body[i].Kind == token.Ident { - args = append(args, body[i].Text) - } - i++ - } - if i < len(body) { - body = body[i+1:] - } else { - body = nil + var ok bool + args, body, ok = formalList(body) + if !ok { + pp.errorf(name.Pos, "bad definition for macro %s", name.Text) + return } } if i := slices.IndexFunc(body, func(t token.Token) bool { return t.Kind == token.Comment }); i >= 0 { @@ -278,6 +273,48 @@ func (pp *preproc) define(line []token.Token) { pp.macros[name.Text] = ¯oDef{name: name.Text, args: args, body: stored} } +// formalList reads the parameter list at the head of a macro definition, the +// tokens after the '(' that follows the name. The list the toolchain +// accepts is identifiers separated by single commas and closed by ')': the C +// variadic spellings ("..." and the GNU "name...", a single token here +// because the lexer reads dots as identifier characters), an empty or +// trailing parameter, a missing comma, a stray token and an unterminated +// list are all rejected at the definition, so a use never silently binds to +// a parameter list the writer did not write. +func formalList(body []token.Token) (args []string, rest []token.Token, ok bool) { + i := 1 + expectName := true + for { + if i >= len(body) { + return nil, nil, false + } + switch body[i].Kind { + case token.RParen: + if expectName && len(args) > 0 { + return nil, nil, false // a trailing comma + } + if args == nil { + args = []string{} // zero parameters, still parameterised + } + return args, body[i+1:], true + case token.Ident: + if !expectName || strings.ContainsRune(body[i].Text, '.') { + return nil, nil, false + } + args = append(args, body[i].Text) + expectName = false + case token.Comma: + if expectName { + return nil, nil, false // an empty parameter + } + expectName = true + default: + return nil, nil, false + } + i++ + } +} + // bodyWithBreaks records the statement boundaries the continuations carry. // The lexer splices backslash-continued lines into one logical line, but the // toolchain keeps the newline as a token in the stored body, which is how a diff --git a/parser/preproc_test.go b/parser/preproc_test.go index 8183e28..09bf1d8 100644 --- a/parser/preproc_test.go +++ b/parser/preproc_test.go @@ -321,6 +321,55 @@ func TestWrongArgumentCountIsAnError(t *testing.T) { } } +// TestBadMacroDefinitionsAreRejected pins the preprocessor's reading of a +// parameter list against the toolchain's: the list is identifiers separated +// by single commas and closed by the parenthesis. The C variadic spellings +// ("...", and the GNU "name...", one token here because the lexer reads dots +// as identifier characters), an empty or trailing parameter, a missing +// comma, a stray token and an unterminated list are all a bad definition, +// rejected at the definition exactly as the toolchain rejects them, so a use +// never silently binds to a parameter list the writer did not write. +func TestBadMacroDefinitionsAreRejected(t *testing.T) { + bad := map[string]string{ + "ellipsis": "#define M(...) ADDQ $1, AX\n", + "ellipsis after a name": "#define M(a, ...) ADDQ a, AX\n", + "gnu dots": "#define M(args...) ADDQ args, AX\n", + "trailing comma": "#define M(a,) ADDQ a, AX\n", + "leading comma": "#define M(,a) ADDQ a, AX\n", + "missing comma": "#define M(a b) ADDQ a, AX\n", + "numeric parameter": "#define M(1) ADDQ $1, AX\n", + "unterminated parameter list": "#define M(a\n", + } + for name, def := range bad { + t.Run(name, func(t *testing.T) { + f, errs := ParseWithOptions("t_amd64.s", def+"TEXT ·f(SB), NOSPLIT, $0\n\tM(BX)\n\tRET\n", Options{Expand: true}) + if f == nil { + t.Fatal("ParseWithOptions returned a nil file") + } + if len(errs) == 0 || !strings.Contains(errs[0].Error(), "bad definition for macro M") { + t.Fatalf("got %v, want the definition rejected", errs) + } + }) + } + // The plain shapes stay accepted, with the invocation checked against + // the parameter count the writer wrote. + for _, def := range []string{ + "#define M() ADDQ $1, AX\n", + "#define M(a) ADDQ a, AX\n", + "#define M(a, b) ADDQ a, b\n", + } { + f, errs := ParseWithOptions("t_amd64.s", def+"TEXT ·f(SB), NOSPLIT, $0\n\tM(BX)\n\tRET\n", Options{Expand: true}) + if f == nil || len(f.Decls) == 0 { + t.Fatalf("%s: unusable file", def) + } + for _, err := range errs { + if strings.Contains(err.Error(), "bad definition") { + t.Fatalf("%s: definition wrongly rejected: %v", def, errs) + } + } + } +} + func TestConditionalsSelectOneBranch(t *testing.T) { _, got := expand(t, ` #define MODE2