Skip to content

Commit

Permalink
Removes CustomSection except NameSection (#234)
Browse files Browse the repository at this point in the history
Per #232, we will be unexporting fields only used during instantiate. Any
additional custom sections are unusable otherwise, so this skips them.
In doing so, this eliminates a length check bug, some TODO and some
logic that enforced strict name uniqueness even though that only rule
only applied to the name section.

Note: this doesn't restrict future use of custom sections, ex for DWARF,
just we don't buffer sections we don't use anymore.

See https://www.w3.org/TR/wasm-core-1/#custom-section%E2%91%A0

Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
codefromthecrypt authored Feb 14, 2022
1 parent 25591e0 commit a6c7f96
Show file tree
Hide file tree
Showing 11 changed files with 97 additions and 171 deletions.
34 changes: 14 additions & 20 deletions wasm/binary/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ func DecodeModule(binary []byte) (*wasm.Module, error) {
for {
// TODO: except custom sections, all others are required to be in order, but we aren't checking yet.
// See https://www.w3.org/TR/wasm-core-1/#modules%E2%91%A0%E2%93%AA
sectionID := make([]byte, 1)
if _, err := io.ReadFull(r, sectionID); err == io.EOF {
sectionID, err := r.ReadByte()
if err == io.EOF {
break
} else if err != nil {
return nil, fmt.Errorf("read section id: %w", err)
}

sectionSize, _, err := leb128.DecodeUint32(r)
if err != nil {
return nil, fmt.Errorf("get size of section %s: %v", wasm.SectionIDName(sectionID[0]), err)
return nil, fmt.Errorf("get size of section %s: %v", wasm.SectionIDName(sectionID), err)
}

sectionContentStart := r.Len()
switch sectionID[0] {
switch sectionID {
case wasm.SectionIDCustom:
// First, validate the section and determine if the section for this name has already been set
name, nameSize, decodeErr := decodeUTF8(r, "custom section name")
Expand All @@ -55,25 +55,19 @@ func DecodeModule(binary []byte) (*wasm.Module, error) {
} else if name == "name" && m.NameSection != nil {
err = fmt.Errorf("redundant custom section %s", name)
break
} else if _, ok := m.CustomSections[name]; ok {
err = fmt.Errorf("redundant custom section %s", name)
break
}

// Now, either decode the NameSection or store an unsupported one
// TODO: Do we care to store something we don't use? We could also skip it!
data, dataErr := readCustomSectionData(r, sectionSize-nameSize)
if dataErr != nil {
err = dataErr
} else if name == "name" {
m.NameSection, err = decodeNameSection(data)
// Now, either decode the NameSection or skip an unsupported one
limit := sectionSize - nameSize
if name == "name" {
m.NameSection, err = decodeNameSection(r, uint64(limit))
} else {
if m.CustomSections == nil {
m.CustomSections = map[string][]byte{name: data}
} else {
m.CustomSections[name] = data
// Note: Not Seek because it doesn't err when given an offset past EOF. Rather, it leads to undefined state.
if _, err = io.CopyN(io.Discard, r, int64(limit)); err != nil {
return nil, fmt.Errorf("failed to skip name[%s]: %w", name, err)
}
}

case wasm.SectionIDType:
m.TypeSection, err = decodeTypeSection(r)
case wasm.SectionIDImport:
Expand Down Expand Up @@ -106,11 +100,11 @@ func DecodeModule(binary []byte) (*wasm.Module, error) {
}

if err != nil {
return nil, fmt.Errorf("section %s: %v", wasm.SectionIDName(sectionID[0]), err)
return nil, fmt.Errorf("section %s: %v", wasm.SectionIDName(sectionID), err)
}
}

functionCount, codeCount := m.SectionSize(wasm.SectionIDFunction), m.SectionSize(wasm.SectionIDCode)
functionCount, codeCount := m.SectionElementCount(wasm.SectionIDFunction), m.SectionElementCount(wasm.SectionIDCode)
if functionCount != codeCount {
return nil, fmt.Errorf("function and code section have inconsistent lengths: %d != %d", functionCount, codeCount)
}
Expand Down
53 changes: 25 additions & 28 deletions wasm/binary/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,6 @@ func TestDecodeModule(t *testing.T) {
name: "only name section",
input: &wasm.Module{NameSection: &wasm.NameSection{ModuleName: "simple"}},
},
{
name: "only custom section",
input: &wasm.Module{CustomSections: map[string][]byte{
"meme": {1, 2, 3, 4, 5, 6, 7, 8, 9, 0},
}},
},
{
name: "name section and a custom section",
input: &wasm.Module{
NameSection: &wasm.NameSection{ModuleName: "simple"},
CustomSections: map[string][]byte{
"meme": {1, 2, 3, 4, 5, 6, 7, 8, 9, 0},
},
},
},
{
name: "type section",
input: &wasm.Module{
Expand Down Expand Up @@ -107,6 +92,29 @@ func TestDecodeModule(t *testing.T) {
require.Equal(t, tc.input, m)
})
}
t.Run("skips custom section", func(t *testing.T) {
input := append(append(magic, version...),
wasm.SectionIDCustom, 0xf, // 15 bytes in this section
0x04, 'm', 'e', 'm', 'e',
1, 2, 3, 4, 5, 6, 7, 8, 9, 0)
m, e := DecodeModule(input)
require.NoError(t, e)
require.Equal(t, &wasm.Module{}, m)
})
t.Run("skips custom section, but not name", func(t *testing.T) {
input := append(append(magic, version...),
wasm.SectionIDCustom, 0xf, // 15 bytes in this section
0x04, 'm', 'e', 'm', 'e',
1, 2, 3, 4, 5, 6, 7, 8, 9, 0,
wasm.SectionIDCustom, 0x0e, // 14 bytes in this section
0x04, 'n', 'a', 'm', 'e',
subsectionIDModuleName, 0x07, // 7 bytes in this subsection
0x06, // the Module name simple is 6 bytes long
's', 'i', 'm', 'p', 'l', 'e')
m, e := DecodeModule(input)
require.NoError(t, e)
require.Equal(t, &wasm.Module{NameSection: &wasm.NameSection{ModuleName: "simple"}}, m)
})
}

func TestDecodeModule_Errors(t *testing.T) {
Expand All @@ -125,26 +133,15 @@ func TestDecodeModule_Errors(t *testing.T) {
input: []byte("\x00asm\x01\x00\x00\x01"),
expectedErr: "invalid version header",
},
{
name: "redundant custom section",
input: append(append(magic, version...),
wasm.SectionIDCustom, 0x09, // 9 bytes in this section
0x04, 'm', 'e', 'm', 'e',
subsectionIDModuleName, 0x03, 0x01, 'x',
wasm.SectionIDCustom, 0x09, // 9 bytes in this section
0x04, 'm', 'e', 'm', 'e',
subsectionIDModuleName, 0x03, 0x01, 'y'),
expectedErr: "section custom: redundant custom section meme",
},
{
name: "redundant name section",
input: append(append(magic, version...),
wasm.SectionIDCustom, 0x09, // 9 bytes in this section
0x04, 'n', 'a', 'm', 'e',
subsectionIDModuleName, 0x03, 0x01, 'x',
subsectionIDModuleName, 0x02, 0x01, 'x',
wasm.SectionIDCustom, 0x09, // 9 bytes in this section
0x04, 'n', 'a', 'm', 'e',
subsectionIDModuleName, 0x03, 0x01, 'x'),
subsectionIDModuleName, 0x02, 0x01, 'x'),
expectedErr: "section custom: redundant custom section name",
},
}
Expand Down
28 changes: 12 additions & 16 deletions wasm/binary/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,40 @@ var sizePrefixedName = []byte{4, 'n', 'a', 'm', 'e'}
// See https://www.w3.org/TR/wasm-core-1/#binary-format%E2%91%A0
func EncodeModule(m *wasm.Module) (bytes []byte) {
bytes = append(magic, version...)
if m.SectionSize(wasm.SectionIDType) > 0 {
if m.SectionElementCount(wasm.SectionIDType) > 0 {
bytes = append(bytes, encodeTypeSection(m.TypeSection)...)
}
if m.SectionSize(wasm.SectionIDImport) > 0 {
if m.SectionElementCount(wasm.SectionIDImport) > 0 {
bytes = append(bytes, encodeImportSection(m.ImportSection)...)
}
if m.SectionSize(wasm.SectionIDFunction) > 0 {
if m.SectionElementCount(wasm.SectionIDFunction) > 0 {
bytes = append(bytes, encodeFunctionSection(m.FunctionSection)...)
}
if m.SectionSize(wasm.SectionIDTable) > 0 {
if m.SectionElementCount(wasm.SectionIDTable) > 0 {
panic("TODO: TableSection")
}
if m.SectionSize(wasm.SectionIDMemory) > 0 {
if m.SectionElementCount(wasm.SectionIDMemory) > 0 {
bytes = append(bytes, encodeMemorySection(m.MemorySection)...)
}
if m.SectionSize(wasm.SectionIDGlobal) > 0 {
if m.SectionElementCount(wasm.SectionIDGlobal) > 0 {
panic("TODO: GlobalSection")
}
if m.SectionSize(wasm.SectionIDExport) > 0 {
if m.SectionElementCount(wasm.SectionIDExport) > 0 {
bytes = append(bytes, encodeExportSection(m.ExportSection)...)
}
if m.SectionSize(wasm.SectionIDStart) > 0 {
if m.SectionElementCount(wasm.SectionIDStart) > 0 {
bytes = append(bytes, encodeStartSection(*m.StartSection)...)
}
if m.SectionSize(wasm.SectionIDElement) > 0 {
if m.SectionElementCount(wasm.SectionIDElement) > 0 {
panic("TODO: ElementSection")
}
if m.SectionSize(wasm.SectionIDCode) > 0 {
if m.SectionElementCount(wasm.SectionIDCode) > 0 {
bytes = append(bytes, encodeCodeSection(m.CodeSection)...)
}
if m.SectionSize(wasm.SectionIDData) > 0 {
if m.SectionElementCount(wasm.SectionIDData) > 0 {
panic("TODO: DataSection")
}
if m.SectionSize(wasm.SectionIDCustom) > 0 {
for name, data := range m.CustomSections {
bytes = append(bytes, encodeCustomSection(name, data)...)
}

if m.SectionElementCount(wasm.SectionIDCustom) > 0 {
// >> The name section should appear only once in a module, and only after the data section.
// See https://www.w3.org/TR/wasm-core-1/#binary-namesec
if m.NameSection != nil {
Expand Down
28 changes: 0 additions & 28 deletions wasm/binary/encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,34 +32,6 @@ func TestModule_Encode(t *testing.T) {
0x06, // the Module name simple is 6 bytes long
's', 'i', 'm', 'p', 'l', 'e'),
},
{
name: "only custom section",
input: &wasm.Module{CustomSections: map[string][]byte{
"meme": {1, 2, 3, 4, 5, 6, 7, 8, 9, 0},
}},
expected: append(append(magic, version...),
wasm.SectionIDCustom, 0xf, // 15 bytes in this section
0x04, 'm', 'e', 'm', 'e',
1, 2, 3, 4, 5, 6, 7, 8, 9, 0),
},
{
name: "name section and a custom section", // name should encode last
input: &wasm.Module{
NameSection: &wasm.NameSection{ModuleName: "simple"},
CustomSections: map[string][]byte{
"meme": {1, 2, 3, 4, 5, 6, 7, 8, 9, 0},
},
},
expected: append(append(magic, version...),
wasm.SectionIDCustom, 0xf, // 15 bytes in this section
0x04, 'm', 'e', 'm', 'e',
1, 2, 3, 4, 5, 6, 7, 8, 9, 0,
wasm.SectionIDCustom, 0x0e, // 14 bytes in this section
0x04, 'n', 'a', 'm', 'e',
subsectionIDModuleName, 0x07, // 7 bytes in this subsection
0x06, // the Module name simple is 6 bytes long
's', 'i', 'm', 'p', 'l', 'e'),
},
{
name: "type section",
input: &wasm.Module{
Expand Down
15 changes: 9 additions & 6 deletions wasm/binary/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,30 @@ const (
// * LocalNames decode from subsection 2
//
// See https://www.w3.org/TR/wasm-core-1/#binary-namesec
func decodeNameSection(data []byte) (result *wasm.NameSection, err error) {
func decodeNameSection(r *bytes.Reader, limit uint64) (result *wasm.NameSection, err error) {
// TODO: add leb128 functions that work on []byte and offset. While using a reader allows us to reuse reader-based
// leb128 functions, it is less efficient, causes untestable code and in some cases more complex vs plain []byte.
r := bytes.NewReader(data)
result = &wasm.NameSection{}

// subsectionID is decoded if known, and skipped if not
var subsectionID uint8
// subsectionSize is the length to skip when the subsectionID is unknown
var subsectionSize uint32
for {
var bytesRead uint64
for limit > 0 {
if subsectionID, err = r.ReadByte(); err != nil {
if err == io.EOF {
return result, nil
}
// TODO: untestable as this can't fail for a reason beside EOF reading a byte from a buffer
return nil, fmt.Errorf("failed to read a subsection ID: %w", err)
}
limit--

// TODO: unused except when skipping. This means we can pass on a corrupt length of a known subsection
if subsectionSize, _, err = leb128.DecodeUint32(r); err != nil {
if subsectionSize, bytesRead, err = leb128.DecodeUint32(r); err != nil {
return nil, fmt.Errorf("failed to read the size of subsection[%d]: %w", subsectionID, err)
}
limit -= bytesRead

switch subsectionID {
case subsectionIDModuleName:
Expand All @@ -66,11 +67,13 @@ func decodeNameSection(data []byte) (result *wasm.NameSection, err error) {
}
default: // Skip other subsections.
// Note: Not Seek because it doesn't err when given an offset past EOF. Rather, it leads to undefined state.
if _, err := io.CopyN(io.Discard, r, int64(subsectionSize)); err != nil {
if _, err = io.CopyN(io.Discard, r, int64(subsectionSize)); err != nil {
return nil, fmt.Errorf("failed to skip subsection[%d]: %w", subsectionID, err)
}
}
limit -= uint64(subsectionSize)
}
return
}

func decodeFunctionNames(r *bytes.Reader) (wasm.NameMap, error) {
Expand Down
6 changes: 4 additions & 2 deletions wasm/binary/names_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package binary

import (
"bytes"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -201,7 +202,8 @@ func TestDecodeNameSection(t *testing.T) {
tc := tt

t.Run(tc.name, func(t *testing.T) {
ns, err := decodeNameSection(encodeNameSectionData(tc.input))
data := encodeNameSectionData(tc.input)
ns, err := decodeNameSection(bytes.NewReader(data), uint64(len(data)))
require.NoError(t, err)
require.Equal(t, tc.input, ns)
})
Expand Down Expand Up @@ -302,7 +304,7 @@ func TestDecodeNameSection_Errors(t *testing.T) {
tc := tt

t.Run(tc.name, func(t *testing.T) {
_, err := decodeNameSection(tc.input)
_, err := decodeNameSection(bytes.NewReader(tc.input), uint64(len(tc.input)))
require.EqualError(t, err, tc.expectedErr)
})
}
Expand Down
16 changes: 0 additions & 16 deletions wasm/binary/section.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@ import (
"github.com/tetratelabs/wazero/wasm/internal/leb128"
)

func readCustomSectionData(r *bytes.Reader, dataSize uint32) ([]byte, error) {
data := make([]byte, dataSize)
if _, err := io.ReadFull(r, data); err != nil {
return nil, fmt.Errorf("cannot read custom section data: %w", err)
}
return data, nil
}

func decodeTypeSection(r io.Reader) ([]*wasm.FunctionType, error) {
vs, _, err := leb128.DecodeUint32(r)
if err != nil {
Expand Down Expand Up @@ -218,14 +210,6 @@ func decodeDataSection(r *bytes.Reader) ([]*wasm.DataSegment, error) {
return result, nil
}

// encodeCustomSection encodes the opaque bytes for the given name as a SectionIDCustom
// See https://www.w3.org/TR/wasm-core-1/#binary-customsec
func encodeCustomSection(name string, data []byte) []byte {
// The contents of a custom section is the non-empty name followed by potentially empty opaque data
contents := append(encodeSizePrefixed([]byte(name)), data...)
return encodeSection(wasm.SectionIDCustom, contents)
}

// encodeSection encodes the sectionID, the size of its contents in bytes, followed by the contents.
// See https://www.w3.org/TR/wasm-core-1/#sections%E2%91%A0
func encodeSection(sectionID wasm.SectionID, contents []byte) []byte {
Expand Down
Loading

0 comments on commit a6c7f96

Please sign in to comment.