Skip to content

Commit

Permalink
wasm: fix offset bug
Browse files Browse the repository at this point in the history
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
  • Loading branch information
srenatus authored and tsandall committed Mar 12, 2021
1 parent 83a7079 commit bcb7229
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 13 deletions.
16 changes: 3 additions & 13 deletions internal/compiler/wasm/optimizations.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,6 @@ func (c *Compiler) removeUnusedCode() error {
}
c.module.Names.Functions = funcNames

// functions we've compiled only get a new index
funcs := []funcCode{}
for _, f := range c.funcsCode {
oldIdx := c.funcs[f.name]
if _, ok := keepFuncs[oldIdx]; ok {
funcs = append(funcs, f)
}
}
c.funcsCode = funcs

// For anything that we don't want, replace the function code entries'
// expressions with `unreachable`.
// We do this because it lets the resulting wasm module pass `wasm-validate`,
Expand All @@ -217,9 +207,9 @@ func (c *Compiler) removeUnusedCode() error {
return fmt.Errorf("write code entry: %w", err)
}
for i := range c.module.Code.Segments {
if _, ok := keepFuncs[uint32(i)]; !ok {
idx := i - c.functionImportCount()
c.module.Code.Segments[idx].Code = buf.Bytes()
idx := i + c.functionImportCount()
if _, ok := keepFuncs[uint32(idx)]; !ok {
c.module.Code.Segments[i].Code = buf.Bytes()
}
}
return nil
Expand Down
56 changes: 56 additions & 0 deletions internal/compiler/wasm/optimizations_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package wasm

import (
"strings"
"testing"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/internal/planner"
)

func TestRemoveUnusedCode(t *testing.T) {
policy, err := planner.New().
WithQueries([]planner.QuerySet{
{
Name: "test",
Queries: []ast.Body{ast.MustParseBody(`input.foo = 1`)},
},
}).Plan()

if err != nil {
t.Fatal(err)
}

c := New().WithPolicy(policy)
mod, err := c.Compile()
if err != nil {
t.Fatal(err)
}

// NOTE(sr): our unused code elimination has the following invariant:
// if a function is not used, both its code and its name are removed
// from the code section, and name sections respectively.
idxToName := map[uint32]string{}
for _, nm := range mod.Names.Functions {
idxToName[nm.Index] = nm.Name
}
for i, seg := range mod.Code.Segments {
idx := i + c.functionImportCount()
noop := len(seg.Code) == 3
name, ok := idxToName[uint32(idx)]
if noop && ok {
t.Errorf("func[%d] has name (%s) and no code", idx, name)
}
if !noop && !ok {
t.Errorf("func[%d] has code but no name", idx)
}
}

// Having established that, we can check that this simple policy
// has no re2-related code by consulting the name map:
for _, nm := range mod.Names.Functions {
if strings.Contains(nm.Name, "re2") {
t.Errorf("expected no re2-related functions, found %s", nm.Name)
}
}
}

0 comments on commit bcb7229

Please sign in to comment.