From 7de7a5ecf43a5c41b5170d0cb70cb744fdf9de9f Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 26 Nov 2024 11:36:41 +0100 Subject: pkg/compiler: allow manual consts to override auto-extracted consts Currently if const values in 2 .const files have different value, the compiler produces an error. This is problematic for auto-extacted consts since we extract them for only 1 arch now. So if a const has different values for different arches, auto-extacted consts may not reflect that, and we can get a mismatch with manual descriptions that has correct values for all arches. So if both manual and auto-extacted consts have different values, silently prefer the manual ones. I've tried to do some whitelisting of consts during auto-extaction, but the list is large and changing over time. This solution is not perfect since the manual descriptions may have a bug, and the mismatch is actually pointing to that bug. Maybe in future we could extract for all arches separately, or do something else. But let's do this for now. --- pkg/compiler/compiler_test.go | 21 ++++++++++++++++++++ pkg/compiler/const_file.go | 37 +++++++++++++++++++++++------------ pkg/compiler/consts.go | 2 +- pkg/compiler/testdata/auto.txt | 4 ++++ pkg/compiler/testdata/auto.txt.const | 6 ++++++ pkg/compiler/testdata/auto0.txt | 4 ++++ pkg/compiler/testdata/auto0.txt.const | 6 ++++++ 7 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 pkg/compiler/testdata/auto.txt create mode 100644 pkg/compiler/testdata/auto.txt.const create mode 100644 pkg/compiler/testdata/auto0.txt create mode 100644 pkg/compiler/testdata/auto0.txt.const diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index f43b2ccb3..711567163 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -137,6 +137,27 @@ func TestData(t *testing.T) { } } +func TestAutoConsts(t *testing.T) { + t.Parallel() + eh := func(pos ast.Pos, msg string) { + t.Errorf("%v: %v", pos, msg) + } + desc := ast.ParseGlob(filepath.Join("testdata", "auto*.txt"), eh) + if desc == nil { + t.Fatalf("parsing failed") + } + constFile := DeserializeConstFile(filepath.Join("testdata", "auto*.txt.const"), eh) + if constFile == nil { + t.Fatalf("const loading failed") + } + target := targets.List[targets.TestOS][targets.TestArch64] + consts := constFile.Arch(targets.TestArch64) + prog := Compile(desc, consts, target, eh) + if prog == nil { + t.Fatalf("compilation failed") + } +} + func TestFuzz(t *testing.T) { t.Parallel() for _, data := range []string{ diff --git a/pkg/compiler/const_file.go b/pkg/compiler/const_file.go index 1968092a4..0220da555 100644 --- a/pkg/compiler/const_file.go +++ b/pkg/compiler/const_file.go @@ -26,6 +26,10 @@ type ConstFile struct { type constVal struct { name string vals map[string]uint64 // arch -> value + // Set if the value for the arch is weak (come from auto.txt). + // Weak values are replaced on mismatch, instead of producing + // an error about mismatching values. + weak map[string]bool } const undefined = "???" @@ -40,30 +44,38 @@ func NewConstFile() *ConstFile { func (cf *ConstFile) AddArch(arch string, consts map[string]uint64, undeclared map[string]bool) error { cf.arches[arch] = true for name, val := range consts { - if err := cf.addConst(arch, name, val, true); err != nil { + if err := cf.addConst(arch, name, val, true, false); err != nil { return err } } for name := range undeclared { - if err := cf.addConst(arch, name, 0, false); err != nil { + if err := cf.addConst(arch, name, 0, false, false); err != nil { return err } } return nil } -func (cf *ConstFile) addConst(arch, name string, val uint64, declared bool) error { +func (cf *ConstFile) addConst(arch, name string, val uint64, declared, weak bool) error { cv := cf.m[name] if cv.vals == nil { cv.name = name cv.vals = make(map[string]uint64) - } - if val0, declared0 := cv.vals[arch]; declared && declared0 && val != val0 { - return fmt.Errorf("const=%v arch=%v has different values: %v[%v] vs %v[%v]", - name, arch, val, declared, val0, declared0) + cv.weak = make(map[string]bool) } if declared { + val0, declared0 := cv.vals[arch] + if declared0 { + if weak { + return nil + } + if !cv.weak[arch] && val != val0 { + return fmt.Errorf("const=%v arch=%v has different values: %v[%v] vs %v[%v]", + name, arch, val, declared, val0, declared0) + } + } cv.vals[arch] = val + cv.weak[arch] = weak } cf.m[name] = cv return nil @@ -196,6 +208,7 @@ func (cf *ConstFile) deserializeFile(data []byte, file, arch string, eh ast.Erro eh(pos, fmt.Sprintf(msg, args...)) return false } + weak := file == "auto.txt.const" s := bufio.NewScanner(bytes.NewReader(data)) var arches []string for ; s.Scan(); pos.Line++ { @@ -224,7 +237,7 @@ func (cf *ConstFile) deserializeFile(data []byte, file, arch string, eh ast.Erro } continue } - if !cf.parseConst(arches, name, val, errf) { + if !cf.parseConst(arches, name, val, weak, errf) { return false } } @@ -236,7 +249,7 @@ func (cf *ConstFile) deserializeFile(data []byte, file, arch string, eh ast.Erro type errft func(msg string, args ...interface{}) bool -func (cf *ConstFile) parseConst(arches []string, name, line string, errf errft) bool { +func (cf *ConstFile) parseConst(arches []string, name, line string, weak bool, errf errft) bool { var dflt map[string]uint64 for _, pair := range strings.Split(line, ",") { fields := strings.Split(pair, ":") @@ -274,13 +287,13 @@ func (cf *ConstFile) parseConst(arches []string, name, line string, errf errft) for _, arch := range fields[:len(fields)-1] { arch = strings.TrimSpace(arch) delete(dflt, arch) - if err := cf.addConst(arch, name, val, defined); err != nil { + if err := cf.addConst(arch, name, val, defined, weak); err != nil { return errf("%v", err) } } } for arch, val := range dflt { - if err := cf.addConst(arch, name, val, true); err != nil { + if err := cf.addConst(arch, name, val, true, weak); err != nil { return errf("%v", err) } } @@ -292,7 +305,7 @@ func (cf *ConstFile) parseOldConst(arch, name, line string, errf errft) bool { if err != nil { return errf("failed to parse int: %v", err) } - if err := cf.addConst(arch, name, val, true); err != nil { + if err := cf.addConst(arch, name, val, true, false); err != nil { return errf("%v", err) } return true diff --git a/pkg/compiler/consts.go b/pkg/compiler/consts.go index 62de4aa51..5e1ed9188 100644 --- a/pkg/compiler/consts.go +++ b/pkg/compiler/consts.go @@ -42,7 +42,7 @@ func FabricateSyscallConsts(target *targets.Target, constInfo map[string]*ConstI for _, info := range constInfo { for _, c := range info.Consts { if strings.HasPrefix(c.Name, target.SyscallPrefix) { - cf.addConst(target.Arch, c.Name, 0, true) + cf.addConst(target.Arch, c.Name, 0, true, false) } } } diff --git a/pkg/compiler/testdata/auto.txt b/pkg/compiler/testdata/auto.txt new file mode 100644 index 000000000..1e64e0a95 --- /dev/null +++ b/pkg/compiler/testdata/auto.txt @@ -0,0 +1,4 @@ +# Copyright 2024 syzkaller project authors. All rights reserved. +# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +foo(a const[FOO]) diff --git a/pkg/compiler/testdata/auto.txt.const b/pkg/compiler/testdata/auto.txt.const new file mode 100644 index 000000000..388aa936f --- /dev/null +++ b/pkg/compiler/testdata/auto.txt.const @@ -0,0 +1,6 @@ +# Copyright 2024 syzkaller project authors. All rights reserved. +# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +arches = 64 +SYS_foo = 11 +FOO = 22 diff --git a/pkg/compiler/testdata/auto0.txt b/pkg/compiler/testdata/auto0.txt new file mode 100644 index 000000000..9c3608caf --- /dev/null +++ b/pkg/compiler/testdata/auto0.txt @@ -0,0 +1,4 @@ +# Copyright 2024 syzkaller project authors. All rights reserved. +# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +foo$auto(a const[FOO]) diff --git a/pkg/compiler/testdata/auto0.txt.const b/pkg/compiler/testdata/auto0.txt.const new file mode 100644 index 000000000..4ba494b07 --- /dev/null +++ b/pkg/compiler/testdata/auto0.txt.const @@ -0,0 +1,6 @@ +# Copyright 2024 syzkaller project authors. All rights reserved. +# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +arches = 64 +SYS_foo = 1 +FOO = 2 -- cgit mrf-deployment