diff --git a/format/format.go b/format/format.go index f909623..e59de62 100644 --- a/format/format.go +++ b/format/format.go @@ -35,7 +35,12 @@ func Source(src string) string { inf := info{kind: kBlank, funcID: funcID} if len(line) > 0 { switch { - case line[0].Kind == token.Comment: + // A whole-line comment is layout of its own. A block comment + // ahead of code on the same line ("/* head */ MOVQ AX, BX") is + // legal assembly and must not swallow the statement after it, so + // only a lone comment is classified as one; anything else renders + // as an instruction line that carries the comment inline. + case line[0].Kind == token.Comment && len(line) == 1: inf.kind = kComment case line[0].Kind == token.Hash: inf.kind = kPreproc @@ -87,6 +92,10 @@ func Source(src string) string { for i, line := range lines { inf := infos[i] var out string + var last token.Token + if len(line) > 0 { + last = line[len(line)-1] + } switch inf.kind { case kBlank: out = "" @@ -99,7 +108,10 @@ func Source(src string) string { case kPreproc: out = renderPreproc(line) case kDirective: - out = line[0].Text + " " + renderOps(line[1:]) + out = line[0].Text + if ops := renderOps(line[1:]); ops != "" { + out += " " + ops + } inBody = line[0].Text == "TEXT" case kLabel: // Every label, and a trailing instruction, becomes its own @@ -121,9 +133,9 @@ func Source(src string) string { // A bare directive cannot start a line of its own (the // parser wants a symbol per line), so a directive sharing // the label's line stays there. - outs[len(outs)-1].text += " " + strings.TrimRight(renderOps(rest), " \t") + outs[len(outs)-1].text += " " + trimLineRight(renderOps(rest), rest[len(rest)-1]) } else if len(rest) > 0 && rest[0].Kind == token.Ident { - outs = append(outs, outLine{kind: kInstr, text: strings.TrimRight(renderInstr(rest, maxWidth[inf.funcID]), " \t")}) + outs = append(outs, outLine{kind: kInstr, text: trimLineRight(renderInstr(rest, maxWidth[inf.funcID]), rest[len(rest)-1])}) if strings.EqualFold(rest[0].Text, "RET") { inBody = false } @@ -140,11 +152,31 @@ func Source(src string) string { inBody = false } } - outs = append(outs, outLine{kind: inf.kind, text: strings.TrimRight(out, " \t")}) + if len(line) == 0 { + outs = append(outs, outLine{kind: inf.kind, text: ""}) + continue + } + outs = append(outs, outLine{kind: inf.kind, text: trimLineRight(out, last)}) } return normalizeSpacing(outs) } +// trimLineRight removes trailing spaces and tabs from a rendered line, which +// are layout the canonical form drops. The trim never eats into the text of +// the line's final token: a string or rune literal may legitimately end in +// whitespace, and that whitespace is the token's content, not spacing between +// tokens. A comment is the exception, because the lexer itself trims a +// comment token's trailing whitespace, so removing it cannot change what the +// next pass reads. s must end with the final token's text. +func trimLineRight(s string, last token.Token) string { + start := len(s) - len(last.Text) + t := strings.TrimRight(s, " \t") + if last.Kind == token.Comment || len(t) <= start { + return t + } + return s[:start] + last.Text +} + // Line classification, shared by the formatting passes. const ( kBlank = iota @@ -259,7 +291,14 @@ func renderPreproc(line []token.Token) string { // "#" directive [args] if len(line) >= 3 && line[1].Kind == token.Ident && line[1].Text == "include" && line[2].Kind == token.String { - return "#include " + line[2].Text + // Whatever follows the header name is stray, but it is the file's + // stray text: it renders after the name rather than vanishing, so + // re-lexing the output sees exactly the tokens the input carried. + out := "#include " + line[2].Text + if rest := renderOps(line[3:]); rest != "" { + out += " " + rest + } + return out } // The body of a directive, a macro definition included, is an ordinary // token run: rendering it through renderOps applies the same punctuation diff --git a/format/fuzz_test.go b/format/fuzz_test.go index 6bb8d8f..ee451df 100644 --- a/format/fuzz_test.go +++ b/format/fuzz_test.go @@ -4,16 +4,24 @@ package format import ( + "fmt" "os" "path/filepath" + "reflect" + "strings" "testing" + "sourcedock.dev/petrbalvin/gasm-sdk/ast" + "sourcedock.dev/petrbalvin/gasm-sdk/lexer" "sourcedock.dev/petrbalvin/gasm-sdk/parser" + "sourcedock.dev/petrbalvin/gasm-sdk/token" ) // FuzzFormatIdempotency hammers the formatter with arbitrary input. The -// contract: formatting twice equals formatting once, and input that parses -// cleanly still parses cleanly after formatting. The seed corpus carries the +// contract: formatting twice equals formatting once, the output re-lexes to +// the same tokens as the input (so formatting changes layout, never meaning), +// input that parses cleanly still parses cleanly after formatting, and its +// tree, positions aside, is unchanged. The seed corpus carries the // repository's kernels, so a plain `go test` run replays every seed as a // regression case and CI exercises them without any fuzzing budget. func FuzzFormatIdempotency(f *testing.F) { @@ -37,6 +45,16 @@ func FuzzFormatIdempotency(f *testing.F) { f.Add("//\r ") f.Add("// loop \r\t\nMOVQ AX, BX\n") f.Add("TEXT ·f(SB), NOSPLIT, $0 // tail\r\n\tMOVQ AX, BX\r\n\tRET\r\n") + // A block comment ahead of code on one line: the comment must not swallow + // the statement that follows it. + f.Add("/* head */ MOVQ AX, BX\n") + f.Add("/* head */ DATA d<>+0(SB)/8, $1\n") + // The exotic operand shapes the corpus taught the parser: register ranges, + // PC-relative jumps with a negative displacement and the U+2215 package + // path. + f.Add("TEXT ·f(SB), $0\n\tV4FMADDPS [Z0-Z3], Z2, Z1\n\tRET\n") + f.Add("TEXT ·f(SB), $0\n\tJMP -3(PC)\n\tRET\n") + f.Add("TEXT ·f(SB), $0\n\tCALL internal∕runtime∕atomic·Xchg(SB)\n\tRET\n") f.Fuzz(func(t *testing.T, src string) { once := Source(src) @@ -44,10 +62,188 @@ func FuzzFormatIdempotency(f *testing.F) { if once != twice { t.Fatalf("formatting is not idempotent:\nfirst: %q\nsecond: %q", once, twice) } - if _, errs := parser.Parse("in.s", src); len(errs) == 0 { - if _, errs := parser.Parse("out.s", once); len(errs) > 0 { - t.Fatalf("formatted output of clean input does not parse: %v\n%s", errs[0], once) - } + in, out := tokenView(src), tokenView(once) + if !sameView(in, out) { + t.Fatalf("formatting changed the token stream:\ninput: %v\noutput: %v\n%s", viewString(in), viewString(out), once) + } + file, errs := parser.Parse("in.s", src) + if len(errs) > 0 || hasStrayIllegal(src) { + // A file the parser already reports on, or one whose stray + // characters the formatter documents dropping, has no tree to + // preserve; the token view above already pinned its tokens. + return + } + reFile, errs := parser.Parse("out.s", once) + if len(errs) > 0 { + t.Fatalf("formatted output of clean input does not parse: %v\n%s", errs[0], once) + } + if got := scrub(reFile); !reflect.DeepEqual(got, scrub(file)) { + t.Fatalf("formatting changed the tree:\ninput: %s\noutput: %s\n%s", dump(scrub(file)), dump(got), once) } }) } + +// hasStrayIllegal reports whether src lexes to an Illegal token other than +// the operand brackets, which the formatter drops by contract. +func hasStrayIllegal(src string) bool { + for _, tok := range lexer.Tokenize(src) { + if tok.Kind == token.Illegal && tok.Text != "[" && tok.Text != "]" { + return true + } + } + return false +} + +// tokenView is the meaning of a source file as the assembler reads it: the +// token kinds and texts in order, ignoring line structure. Stray Illegal +// runes are dropped because the formatter documents dropping them; comment +// texts are compared trailing-whitespace-trimmed because line comments lose +// exactly that on the way through the lexer, and an unterminated block +// comment runs to the end of the file, so the formatter's mandatory final +// newline (and any line ending it lands after) is layout, not content the +// formatter destroyed. +type viewToken struct { + kind token.Kind + text string +} + +func tokenView(src string) []viewToken { + var out []viewToken + for _, tok := range lexer.Tokenize(src) { + switch tok.Kind { + case token.EOF, token.Newline: + continue + case token.Illegal: + if tok.Text != "[" && tok.Text != "]" { + continue + } + case token.Comment: + text := strings.TrimRight(tok.Text, " \t\r") + if strings.HasPrefix(text, "/*") && !strings.HasSuffix(text, "*/") { + text = strings.TrimRight(text, " \t\r\n") + } + out = append(out, viewToken{tok.Kind, text}) + continue + } + out = append(out, viewToken{tok.Kind, tok.Text}) + } + return out +} + +func sameView(a, b []viewToken) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func viewString(v []viewToken) string { + var b strings.Builder + for i, t := range v { + if i > 0 { + b.WriteByte(' ') + } + b.WriteString(t.kind.String() + "(" + t.text + ")") + } + return b.String() +} + +// scrub reduces a parsed file to what formatting must preserve: every field +// except source positions, the file path, and a preprocessor line's Raw text. +// Positions move with layout, the path is an input of the call, and Raw is a +// single-space join of the directive's tokens, whose input spelling the +// canonical operand spacing may legitimately re-glue ("#define A (x)" formats +// to "#define A(x)"). The token view already pins those tokens. +func scrub(f *ast.File) *ast.File { + v := reflect.ValueOf(f).Elem() + scrubValue(v) + v.FieldByName("Path").SetString("") + return f +} + +// scrubValue walks v and zeroes every token.Position it reaches, so two trees +// taken from the same text at different layouts compare equal. +func scrubValue(v reflect.Value) { + switch v.Kind() { + case reflect.Pointer, reflect.Interface: + if !v.IsNil() { + scrubValue(v.Elem()) + } + case reflect.Struct: + switch v.Type() { + case reflect.TypeFor[token.Position](): + v.Set(reflect.Zero(v.Type())) + return + case reflect.TypeFor[ast.Preproc](): + v.FieldByName("Raw").SetString("") + } + for _, field := range v.Fields() { + scrubValue(field) + } + case reflect.Slice, reflect.Array: + for i := 0; i < v.Len(); i++ { + scrubValue(v.Index(i)) + } + case reflect.Map: + for _, k := range v.MapKeys() { + scrubValue(v.MapIndex(k)) + } + } +} + +// dump renders a scrubbed tree as text, following pointers, because the fmt +// verbs stop at the first address inside a slice of interfaces. +func dump(v any) string { + var b strings.Builder + dumpValue(reflect.ValueOf(v), &b) + return b.String() +} + +func dumpValue(v reflect.Value, b *strings.Builder) { + switch v.Kind() { + case reflect.Pointer, reflect.Interface: + if v.IsNil() { + b.WriteString("nil") + return + } + dumpValue(v.Elem(), b) + case reflect.Struct: + b.WriteString(v.Type().Name() + "{") + for i := 0; i < v.NumField(); i++ { + if i > 0 { + b.WriteString(", ") + } + b.WriteString(v.Type().Field(i).Name + ":") + dumpValue(v.Field(i), b) + } + b.WriteString("}") + case reflect.Slice: + if v.IsNil() { + b.WriteString("nil") + return + } + b.WriteString("[") + for i := 0; i < v.Len(); i++ { + if i > 0 { + b.WriteString(", ") + } + dumpValue(v.Index(i), b) + } + b.WriteString("]") + case reflect.Map: + b.WriteString("map{") + for _, k := range v.MapKeys() { + dumpValue(k, b) + b.WriteString(":") + dumpValue(v.MapIndex(k), b) + } + b.WriteString("}") + default: + b.WriteString(fmt.Sprintf("%v", v)) + } +} diff --git a/format/testdata/fuzz/FuzzFormatIdempotency/182930100b51847b b/format/testdata/fuzz/FuzzFormatIdempotency/182930100b51847b new file mode 100644 index 0000000..d444606 --- /dev/null +++ b/format/testdata/fuzz/FuzzFormatIdempotency/182930100b51847b @@ -0,0 +1,2 @@ +go test fuzz v1 +string("#include\"00000000\"0000") diff --git a/format/testdata/fuzz/FuzzFormatIdempotency/51d239769677847e b/format/testdata/fuzz/FuzzFormatIdempotency/51d239769677847e new file mode 100644 index 0000000..bb2ccf9 --- /dev/null +++ b/format/testdata/fuzz/FuzzFormatIdempotency/51d239769677847e @@ -0,0 +1,2 @@ +go test fuzz v1 +string("/*\r")