From 9dfb5efa91fc0f051a1ddc88ace2867bb6b32275 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 5 May 2018 10:13:04 +0200 Subject: prog: simplify code Now that we don't have ReturnArg and only ResultArg's refer to other ResultArg's we can remove ArgUser/ArgUsed and devirtualize lots of code. --- prog/analysis.go | 7 +++-- prog/clone.go | 29 +++++++++---------- prog/encoding.go | 43 ++++++++++++++-------------- prog/encodingexec.go | 14 +++------- prog/prog.go | 79 +++++++++++++++++----------------------------------- prog/rand.go | 18 ++---------- prog/validation.go | 14 ++++------ sys/linux/init.go | 4 +-- sys/linux/sys.txt | 3 +- 9 files changed, 80 insertions(+), 131 deletions(-) diff --git a/prog/analysis.go b/prog/analysis.go index 0384394af..a7ef720f7 100644 --- a/prog/analysis.go +++ b/prog/analysis.go @@ -16,7 +16,7 @@ type state struct { target *Target ct *ChoiceTable files map[string]bool - resources map[string][]Arg + resources map[string][]*ResultArg strings map[string]bool ma *memAlloc va *vmaAlloc @@ -40,7 +40,7 @@ func newState(target *Target, ct *ChoiceTable) *state { target: target, ct: ct, files: make(map[string]bool), - resources: make(map[string][]Arg), + resources: make(map[string][]*ResultArg), strings: make(map[string]bool), ma: newMemAlloc(target.NumPages * target.PageSize), va: newVmaAlloc(target.NumPages), @@ -66,8 +66,9 @@ func (s *state) analyzeImpl(c *Call, resources bool) { } switch typ := arg.Type().(type) { case *ResourceType: + a := arg.(*ResultArg) if resources && typ.Dir() != DirIn { - s.resources[typ.Desc.Name] = append(s.resources[typ.Desc.Name], arg) + s.resources[typ.Desc.Name] = append(s.resources[typ.Desc.Name], a) // TODO: negative PIDs and add them as well (that's process groups). } case *BufferType: diff --git a/prog/clone.go b/prog/clone.go index e7d937318..51d801e28 100644 --- a/prog/clone.go +++ b/prog/clone.go @@ -8,11 +8,11 @@ func (p *Prog) Clone() *Prog { Target: p.Target, Calls: make([]*Call, len(p.Calls)), } - newargs := make(map[Arg]Arg) + newargs := make(map[*ResultArg]*ResultArg) for ci, c := range p.Calls { c1 := new(Call) c1.Meta = c.Meta - c1.Ret = clone(c.Ret, newargs) + c1.Ret = clone(c.Ret, newargs).(*ResultArg) c1.Args = make([]Arg, len(c.Args)) for ai, arg := range c.Args { c1.Args[ai] = clone(arg, newargs) @@ -27,7 +27,7 @@ func (p *Prog) Clone() *Prog { return p1 } -func clone(arg Arg, newargs map[Arg]Arg) Arg { +func clone(arg Arg, newargs map[*ResultArg]*ResultArg) Arg { var arg1 Arg switch a := arg.(type) { case *ConstArg: @@ -63,21 +63,18 @@ func clone(arg Arg, newargs map[Arg]Arg) Arg { a1 := new(ResultArg) *a1 = *a arg1 = a1 + if a1.Res != nil { + r := newargs[a1.Res] + a1.Res = r + if r.uses == nil { + r.uses = make(map[*ResultArg]bool) + } + r.uses[a1] = true + } + a1.uses = nil // filled when we clone the referent + newargs[a] = a1 default: panic("bad arg kind") } - if user, ok := arg1.(ArgUser); ok && *user.Uses() != nil { - r := newargs[*user.Uses()] - *user.Uses() = r - used := r.(ArgUsed) - if *used.Used() == nil { - *used.Used() = make(map[Arg]bool) - } - (*used.Used())[arg1] = true - } - if used, ok := arg1.(ArgUsed); ok { - *used.Used() = nil // filled when we clone the referent - newargs[arg] = arg1 - } return arg1 } diff --git a/prog/encoding.go b/prog/encoding.go index 7a38b680f..86577cd12 100644 --- a/prog/encoding.go +++ b/prog/encoding.go @@ -30,10 +30,10 @@ func (p *Prog) Serialize() []byte { } } buf := new(bytes.Buffer) - vars := make(map[Arg]int) + vars := make(map[*ResultArg]int) varSeq := 0 for _, c := range p.Calls { - if isUsed(c.Ret) { + if len(c.Ret.uses) != 0 { fmt.Fprintf(buf, "r%v = ", varSeq) vars[c.Ret] = varSeq varSeq++ @@ -53,16 +53,11 @@ func (p *Prog) Serialize() []byte { return buf.Bytes() } -func (target *Target) serialize(arg Arg, buf *bytes.Buffer, vars map[Arg]int, varSeq *int) { +func (target *Target) serialize(arg Arg, buf *bytes.Buffer, vars map[*ResultArg]int, varSeq *int) { if arg == nil { fmt.Fprintf(buf, "nil") return } - if isUsed(arg) { - fmt.Fprintf(buf, "", *varSeq) - vars[arg] = *varSeq - *varSeq++ - } switch a := arg.(type) { case *ConstArg: fmt.Fprintf(buf, "0x%x", a.Val) @@ -130,6 +125,11 @@ func (target *Target) serialize(arg Arg, buf *bytes.Buffer, vars map[Arg]int, va target.serialize(a.Option, buf, vars, varSeq) } case *ResultArg: + if len(a.uses) != 0 { + fmt.Fprintf(buf, "", *varSeq) + vars[a] = *varSeq + *varSeq++ + } if a.Res == nil { fmt.Fprintf(buf, "0x%x", a.Val) break @@ -155,7 +155,7 @@ func (target *Target) Deserialize(data []byte) (prog *Prog, err error) { Target: target, } p := newParser(data) - vars := make(map[string]Arg) + vars := make(map[string]*ResultArg) for p.Scan() { if p.EOF() || p.Char() == '#' { continue @@ -225,7 +225,7 @@ func (target *Target) Deserialize(data []byte) (prog *Prog, err error) { return } -func (target *Target) parseArg(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArg(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { r := "" if p.Char() == '<' { p.Parse('<') @@ -245,12 +245,14 @@ func (target *Target) parseArg(typ Type, p *parser, vars map[string]Arg) (Arg, e } } if r != "" { - vars[r] = arg + if res, ok := arg.(*ResultArg); ok { + vars[r] = res + } } return arg, nil } -func (target *Target) parseArgImpl(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArgImpl(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { switch p.Char() { case '0': return target.parseArgInt(typ, p) @@ -300,7 +302,7 @@ func (target *Target) parseArgInt(typ Type, p *parser) (Arg, error) { } } -func (target *Target) parseArgRes(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArgRes(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { id := p.Ident() var div, add uint64 if p.Char() == '/' { @@ -321,11 +323,8 @@ func (target *Target) parseArgRes(typ Type, p *parser, vars map[string]Arg) (Arg } add = v } - v, ok := vars[id] - if !ok || v == nil { - return target.defaultArg(typ), nil - } - if _, ok := v.(ArgUsed); !ok { + v := vars[id] + if v == nil { return target.defaultArg(typ), nil } arg := MakeResultArg(typ, v, 0) @@ -334,7 +333,7 @@ func (target *Target) parseArgRes(typ Type, p *parser, vars map[string]Arg) (Arg return arg, nil } -func (target *Target) parseArgAddr(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArgAddr(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { var typ1 Type switch t1 := typ.(type) { case *PtrType: @@ -407,7 +406,7 @@ func (target *Target) parseArgString(typ Type, p *parser) (Arg, error) { return MakeDataArg(typ, data), nil } -func (target *Target) parseArgStruct(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArgStruct(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { p.Parse('{') t1, ok := typ.(*StructType) if !ok { @@ -442,7 +441,7 @@ func (target *Target) parseArgStruct(typ Type, p *parser, vars map[string]Arg) ( return MakeGroupArg(typ, inner), nil } -func (target *Target) parseArgArray(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArgArray(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { p.Parse('[') t1, ok := typ.(*ArrayType) if !ok { @@ -471,7 +470,7 @@ func (target *Target) parseArgArray(typ Type, p *parser, vars map[string]Arg) (A return MakeGroupArg(typ, inner), nil } -func (target *Target) parseArgUnion(typ Type, p *parser, vars map[string]Arg) (Arg, error) { +func (target *Target) parseArgUnion(typ Type, p *parser, vars map[string]*ResultArg) (Arg, error) { t1, ok := typ.(*UnionType) if !ok { eatExcessive(p, true) diff --git a/prog/encodingexec.go b/prog/encodingexec.go index 4085262a9..9d918b155 100644 --- a/prog/encodingexec.go +++ b/prog/encodingexec.go @@ -91,7 +91,7 @@ func (p *Prog) SerializeForExec(buffer []byte) (int, error) { return } addr := p.Target.PhysicalAddr(ctx.Base) + ctx.Offset - if isUsed(arg) || csumUses[arg] { + if res, ok := arg.(*ResultArg); ok && len(res.uses) != 0 || csumUses[arg] { w.args[arg] = argInfo{Addr: addr} } if _, ok := arg.(*GroupArg); ok { @@ -152,7 +152,7 @@ func (p *Prog) SerializeForExec(buffer []byte) (int, error) { } // Generate the call itself. w.write(uint64(c.Meta.ID)) - if isUsed(c.Ret) { + if len(c.Ret.uses) != 0 { if _, ok := w.args[c.Ret]; ok { panic("argInfo is already created for return value") } @@ -168,15 +168,11 @@ func (p *Prog) SerializeForExec(buffer []byte) (int, error) { } // Generate copyout instructions that persist interesting return values. ForeachArg(c, func(arg Arg, _ *ArgCtx) { - if !isUsed(arg) { - return - } - switch arg.(type) { - case *ConstArg, *ResultArg: + if res, ok := arg.(*ResultArg); ok && len(res.uses) != 0 { // Create a separate copyout instruction that has own Idx. info := w.args[arg] if info.Ret { - break // Idx is already assigned above. + return // Idx is already assigned above. } info.Idx = copyoutSeq copyoutSeq++ @@ -185,8 +181,6 @@ func (p *Prog) SerializeForExec(buffer []byte) (int, error) { w.write(info.Idx) w.write(info.Addr) w.write(arg.Size()) - default: - panic("bad arg kind in copyout") } }) } diff --git a/prog/prog.go b/prog/prog.go index 1bec61643..ef09d46a9 100644 --- a/prog/prog.go +++ b/prog/prog.go @@ -15,7 +15,7 @@ type Prog struct { type Call struct { Meta *Syscall Args []Arg - Ret Arg + Ret *ResultArg } type Arg interface { @@ -23,24 +23,6 @@ type Arg interface { Size() uint64 } -// ArgUser is interface of an argument that uses value of another output argument. -type ArgUser interface { - Uses() *Arg -} - -// ArgUsed is interface of an argument that can be used by other arguments. -type ArgUsed interface { - Used() *map[Arg]bool -} - -func isUsed(arg Arg) bool { - used, ok := arg.(ArgUsed) - if !ok { - return false - } - return len(*used.Used()) != 0 -} - type ArgCommon struct { typ Type } @@ -259,23 +241,22 @@ func (arg *UnionArg) Size() uint64 { // Either holds constant value or reference another ResultArg. type ResultArg struct { ArgCommon - Res Arg // reference to arg which we use - OpDiv uint64 // divide result (executed before OpAdd) - OpAdd uint64 // add to result - Val uint64 // value used if Res is nil - uses map[Arg]bool // ArgResult args that use this arg + Res *ResultArg // reference to arg which we use + OpDiv uint64 // divide result (executed before OpAdd) + OpAdd uint64 // add to result + Val uint64 // value used if Res is nil + uses map[*ResultArg]bool // ArgResult args that use this arg } -func MakeResultArg(t Type, r Arg, v uint64) *ResultArg { +func MakeResultArg(t Type, r *ResultArg, v uint64) *ResultArg { arg := &ResultArg{ArgCommon: ArgCommon{typ: t}, Res: r, Val: v} if r == nil { return arg } - used := r.(ArgUsed) - if *used.Used() == nil { - *used.Used() = make(map[Arg]bool) + if r.uses == nil { + r.uses = make(map[*ResultArg]bool) } - (*used.Used())[arg] = true + r.uses[arg] = true return arg } @@ -291,14 +272,6 @@ func (arg *ResultArg) Size() uint64 { return arg.typ.Size() } -func (arg *ResultArg) Used() *map[Arg]bool { - return &arg.uses -} - -func (arg *ResultArg) Uses() *Arg { - return &arg.Res -} - // Returns inner arg for pointer args. func InnerArg(arg Arg) Arg { if t, ok := arg.Type().(*PtrType); ok { @@ -479,36 +452,34 @@ func replaceArg(arg, arg1 Arg) { func replaceResultArg(arg, arg1 *ResultArg) { // Remove link from `a.Res` to `arg`. if arg.Res != nil { - delete(*arg.Res.(ArgUsed).Used(), arg) + delete(arg.Res.uses, arg) } // Copy all fields from `arg1` to `arg` except for the list of args that use `arg`. - used := *arg.Used() + uses := arg.uses *arg = *arg1 - *arg.Used() = used + arg.uses = uses // Make the link in `arg.Res` (which is now `Res` of `arg1`) to point to `arg` instead of `arg1`. if arg.Res != nil { - delete(*arg.Res.(ArgUsed).Used(), arg1) - (*arg.Res.(ArgUsed).Used())[arg] = true + resUses := arg.Res.uses + delete(resUses, arg1) + resUses[arg] = true } } // removeArg removes all references to/from arg0 from a program. func removeArg(arg0 Arg) { ForeachSubArg(arg0, func(arg Arg, ctx *ArgCtx) { - if a, ok := arg.(*ResultArg); ok && a.Res != nil { - if !(*a.Res.(ArgUsed).Used())[arg] { - panic("broken tree") - } - delete(*a.Res.(ArgUsed).Used(), arg) - } - if used, ok := arg.(ArgUsed); ok { - for arg1 := range *used.Used() { - a1, ok := arg1.(*ResultArg) - if !ok { - panic("use references not ArgResult") + if a, ok := arg.(*ResultArg); ok { + if a.Res != nil { + uses := a.Res.uses + if !uses[a] { + panic("broken tree") } + delete(uses, a) + } + for arg1 := range a.uses { arg2 := MakeResultArg(arg1.Type(), nil, arg1.Type().Default()) - replaceResultArg(a1, arg2) + replaceResultArg(arg1, arg2) } } }) diff --git a/prog/rand.go b/prog/rand.go index 956e7c7e1..b86219368 100644 --- a/prog/rand.go +++ b/prog/rand.go @@ -285,7 +285,7 @@ func (r *randGen) createResource(s *state, res *ResourceType) (arg Arg, calls [] s1 := newState(r.target, s.ct) s1.analyze(calls[len(calls)-1]) // Now see if we have what we want. - var allres []Arg + var allres []*ResultArg for kind1, res1 := range s1.resources { if r.target.isCompatibleResource(kind, kind1) { allres = append(allres, res1...) @@ -303,7 +303,7 @@ func (r *randGen) createResource(s *state, res *ResourceType) (arg Arg, calls [] for _, c := range calls { ForeachArg(c, func(arg Arg, _ *ArgCtx) { if a, ok := arg.(*ResultArg); ok && a.Res != nil { - delete(*a.Res.(ArgUsed).Used(), arg) + delete(a.Res.uses, a) } }) } @@ -527,11 +527,8 @@ func (a *ResourceType) generate(r *randGen, s *state) (arg Arg, calls []*Call) { switch { case r.nOutOf(1000, 1011): // Get an existing resource. - var allres []Arg + var allres []*ResultArg for name1, res1 := range s.resources { - if name1 == "iocbptr" { - continue - } if r.target.isCompatibleResource(a.Desc.Name, name1) || r.oneOf(20) && r.target.isCompatibleResource(a.Desc.Kind[0], name1) { allres = append(allres, res1...) @@ -668,15 +665,6 @@ func (a *UnionType) generate(r *randGen, s *state) (arg Arg, calls []*Call) { func (a *PtrType) generate(r *randGen, s *state) (arg Arg, calls []*Call) { inner, calls := r.generateArg(s, a.Type) - // TODO(dvyukov): remove knowledge about iocb from prog. - if a.Type.Name() == "iocb" && len(s.resources["iocbptr"]) != 0 { - // It is weird, but these are actually identified by kernel by address. - // So try to reuse a previously used address. - addrs := s.resources["iocbptr"] - addr := addrs[r.Intn(len(addrs))].(*PointerArg) - arg = MakePointerArg(a, addr.Address, inner) - return arg, calls - } arg = r.allocAddr(s, a, inner.Size(), inner) return arg, calls } diff --git a/prog/validation.go b/prog/validation.go index 6dd44bc86..f6e0780fd 100644 --- a/prog/validation.go +++ b/prog/validation.go @@ -50,11 +50,9 @@ func (p *Prog) validateCall(ctx *validCtx, c *Call) error { if c.Ret == nil { return fmt.Errorf("syscall %v: return value is absent", c.Meta.Name) } - if ret, ok := c.Ret.(*ResultArg); !ok { - return fmt.Errorf("syscall %v: return value has wrong kind %v", c.Meta.Name, c.Ret) - } else if ret.Type() != nil && ret.Type().Dir() != DirOut { + if c.Ret.Type() != nil && c.Ret.Type().Dir() != DirOut { return fmt.Errorf("syscall %v: return value %v is not output", c.Meta.Name, c.Ret) - } else if ret.Res != nil || ret.Val != 0 || ret.OpDiv != 0 || ret.OpAdd != 0 { + } else if c.Ret.Res != nil || c.Ret.Val != 0 || c.Ret.OpDiv != 0 || c.Ret.OpAdd != 0 { return fmt.Errorf("syscall %v: return value %v is not empty", c.Meta.Name, c.Ret) } if c.Meta.Ret != nil { @@ -78,8 +76,8 @@ func validateArg(ctx *validCtx, c *Call, arg Arg) error { c.Meta.Name, arg) } ctx.args[arg] = true - if used, ok := arg.(ArgUsed); ok { - for u := range *used.Used() { + if used, ok := arg.(*ResultArg); ok { + for u := range used.uses { if u == nil { return fmt.Errorf("syscall %v: nil reference in uses for arg %+v", c.Meta.Name, arg) @@ -305,9 +303,9 @@ func validateArg(ctx *validCtx, c *Call, arg Arg) error { return fmt.Errorf("syscall %v: result arg %v references out-of-tree result: %#v -> %#v", c.Meta.Name, a.Type().Name(), arg, a.Res) } - if !(*a.Res.(ArgUsed).Used())[arg] { + if !a.Res.uses[a] { return fmt.Errorf("syscall %v: result arg '%v' has broken link (%+v)", - c.Meta.Name, a.Type().Name(), *a.Res.(ArgUsed).Used()) + c.Meta.Name, a.Type().Name(), a.Res.uses) } default: return fmt.Errorf("syscall %v: unknown arg '%v' kind", diff --git a/sys/linux/init.go b/sys/linux/init.go index 9fabafe72..e2370baab 100644 --- a/sys/linux/init.go +++ b/sys/linux/init.go @@ -275,8 +275,8 @@ func (arch *arch) generateTimespec(g *prog.Gen, typ0 prog.Type, old prog.Arg) (a Ret: prog.MakeReturnArg(meta.Ret), } calls = append(calls, gettime) - sec := prog.MakeResultArg(typ.Fields[0], tp.Inner[0], 0) - nsec := prog.MakeResultArg(typ.Fields[1], tp.Inner[1], 0) + sec := prog.MakeResultArg(typ.Fields[0], tp.Inner[0].(*prog.ResultArg), 0) + nsec := prog.MakeResultArg(typ.Fields[1], tp.Inner[1].(*prog.ResultArg), 0) msec := uint64(10) if g.NOutOf(1, 2) { msec = 30 diff --git a/sys/linux/sys.txt b/sys/linux/sys.txt index 8fd578629..10b35b2b4 100644 --- a/sys/linux/sys.txt +++ b/sys/linux/sys.txt @@ -212,7 +212,8 @@ resource io_ctx[intptr] io_setup(n int32, ctx ptr[out, io_ctx]) io_destroy(ctx io_ctx) io_getevents(ctx io_ctx, min_nr intptr, nr len[events], events ptr[out, array[io_event]], timeout ptr[in, timespec]) -# Prog knows that poiners passed in iocbpp needs to be forwarded to io_cancel. +# TODO: kernel identifies requets by address, so pointers passed to io_submit +# need to be forwarded to io_cancel somehow. io_submit(ctx io_ctx, nr len[iocbpp], iocbpp ptr[in, array[ptr[in, iocb]]]) io_cancel(ctx io_ctx, iocb ptr[in, iocb], res ptr[out, io_event]) -- cgit mrf-deployment