Skip to content

Commit

Permalink
Require max size for shared memory (tetratelabs#1473)
Browse files Browse the repository at this point in the history
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
  • Loading branch information
anuraaga authored May 16, 2023
1 parent 41c4ed5 commit 90eba1b
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 30 deletions.
62 changes: 39 additions & 23 deletions internal/integration_test/engine/threads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func TestThreadsInterpreter(t *testing.T) {
}

func incrementGuardedByMutex(t *testing.T, r wazero.Runtime) {
P := 8 // max count of goroutines
if testing.Short() { // Adjust down if `-test.short`
P = 4
}
tests := []struct {
fn string
}{
Expand Down Expand Up @@ -88,55 +92,59 @@ func incrementGuardedByMutex(t *testing.T, r wazero.Runtime) {
mod, err := r.Instantiate(testCtx, mutexWasm)
require.NoError(t, err)

hammer.NewHammer(t, 8, 30000).Run(func(name string) {
hammer.NewHammer(t, P, 30000).Run(func(name string) {
_, err := mod.ExportedFunction(tt.fn).Call(testCtx)
require.NoError(t, err)
}, func() {})

// Cheat that LE encoding can read both 32 and 64 bits
res, ok := mod.Memory().ReadUint32Le(8)
require.True(t, ok)
require.Equal(t, uint32(8*30000), res)
require.Equal(t, uint32(P*30000), res)
})
}
}

func atomicAdd(t *testing.T, r wazero.Runtime) {
P := 8 // max count of goroutines
if testing.Short() { // Adjust down if `-test.short`
P = 4
}
tests := []struct {
fn string
exp uint32
exp int
}{
{
fn: "run32",
exp: 8 * 30000,
exp: P * 30000,
},
{
fn: "run64",
exp: 8 * 30000,
exp: P * 30000,
},
{
fn: "run32_8",
// Overflows
exp: (8 * 30000) % (1 << 8),
exp: (P * 30000) % (1 << 8),
},
{
fn: "run32_16",
// Overflows
exp: (8 * 30000) % (1 << 16),
exp: (P * 30000) % (1 << 16),
},
{
fn: "run64_8",
// Overflows
exp: (8 * 30000) % (1 << 8),
exp: (P * 30000) % (1 << 8),
},
{
fn: "run64_16",
// Overflows
exp: (8 * 30000) % (1 << 16),
exp: (P * 30000) % (1 << 16),
},
{
fn: "run64_32",
exp: 8 * 30000,
exp: P * 30000,
},
}
for _, tc := range tests {
Expand All @@ -145,55 +153,59 @@ func atomicAdd(t *testing.T, r wazero.Runtime) {
mod, err := r.Instantiate(testCtx, addWasm)
require.NoError(t, err)

hammer.NewHammer(t, 8, 30000).Run(func(name string) {
hammer.NewHammer(t, P, 30000).Run(func(name string) {
_, err := mod.ExportedFunction(tt.fn).Call(testCtx)
require.NoError(t, err)
}, func() {})

// Cheat that LE encoding can read both 32 and 64 bits
res, ok := mod.Memory().ReadUint32Le(0)
require.True(t, ok)
require.Equal(t, tt.exp, res)
require.Equal(t, uint32(tt.exp), res)
})
}
}

func atomicSub(t *testing.T, r wazero.Runtime) {
P := 8 // max count of goroutines
if testing.Short() { // Adjust down if `-test.short`
P = 4
}
tests := []struct {
fn string
exp int32
exp int
}{
{
fn: "run32",
exp: -(8 * 30000),
exp: -(P * 30000),
},
{
fn: "run64",
exp: -(8 * 30000),
exp: -(P * 30000),
},
{
fn: "run32_8",
// Overflows
exp: (1 << 8) - ((8 * 30000) % (1 << 8)),
exp: (1 << 8) - ((P * 30000) % (1 << 8)),
},
{
fn: "run32_16",
// Overflows
exp: (1 << 16) - ((8 * 30000) % (1 << 16)),
exp: (1 << 16) - ((P * 30000) % (1 << 16)),
},
{
fn: "run64_8",
// Overflows
exp: (1 << 8) - ((8 * 30000) % (1 << 8)),
exp: (1 << 8) - ((P * 30000) % (1 << 8)),
},
{
fn: "run64_16",
// Overflows
exp: (1 << 16) - ((8 * 30000) % (1 << 16)),
exp: (1 << 16) - ((P * 30000) % (1 << 16)),
},
{
fn: "run64_32",
exp: -(8 * 30000),
exp: -(P * 30000),
},
}
for _, tc := range tests {
Expand All @@ -202,20 +214,24 @@ func atomicSub(t *testing.T, r wazero.Runtime) {
mod, err := r.Instantiate(testCtx, subWasm)
require.NoError(t, err)

hammer.NewHammer(t, 8, 30000).Run(func(name string) {
hammer.NewHammer(t, P, 30000).Run(func(name string) {
_, err := mod.ExportedFunction(tt.fn).Call(testCtx)
require.NoError(t, err)
}, func() {})

// Cheat that LE encoding can read both 32 and 64 bits
res, ok := mod.Memory().ReadUint32Le(0)
require.True(t, ok)
require.Equal(t, tt.exp, int32(res))
require.Equal(t, int32(tt.exp), int32(res))
})
}
}

func atomicXor(t *testing.T, r wazero.Runtime) {
P := 8 // max count of goroutines
if testing.Short() { // Adjust down if `-test.short`
P = 4
}
tests := []struct {
fn string
}{
Expand Down Expand Up @@ -249,7 +265,7 @@ func atomicXor(t *testing.T, r wazero.Runtime) {

mod.Memory().WriteUint32Le(0, 12345)

hammer.NewHammer(t, 8, 30000).Run(func(name string) {
hammer.NewHammer(t, P, 30000).Run(func(name string) {
_, err := mod.ExportedFunction(tt.fn).Call(testCtx)
require.NoError(t, err)
}, func() {})
Expand Down
11 changes: 9 additions & 2 deletions internal/wasm/binary/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@ func decodeMemory(
return nil, err
}

if shared && !enabledFeatures.IsEnabled(experimental.CoreFeaturesThreads) {
return nil, fmt.Errorf("shared memory requested but threads feature not enabled")
if shared {
if !enabledFeatures.IsEnabled(experimental.CoreFeaturesThreads) {
return nil, fmt.Errorf("shared memory requested but threads feature not enabled")
}
// This restriction may be lifted in the future.
// https://webassembly.github.io/threads/core/binary/types.html#memory-types
if maxP == nil {
return nil, fmt.Errorf("shared memory requires a maximum size to be specified")
}
}

min, capacity, max := memorySizer(min, maxP)
Expand Down
21 changes: 16 additions & 5 deletions internal/wasm/binary/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ func TestDecodeMemoryType_Errors(t *testing.T) {
max := wasm.MemoryLimitPages

tests := []struct {
name string
input []byte
expectedErr string
name string
input []byte
threadsEnabled bool
expectedErr string
}{
{
name: "max < min",
Expand All @@ -219,15 +220,25 @@ func TestDecodeMemoryType_Errors(t *testing.T) {
input: []byte{0x2, 0, 0x80, 0x80, 0x4},
expectedErr: "shared memory requested but threads feature not enabled",
},
{
name: "shared but no max",
input: []byte{0x2, 0, 0x80, 0x80, 0x4},
threadsEnabled: true,
expectedErr: "shared memory requires a maximum size to be specified",
},
}

for _, tt := range tests {
tc := tt

t.Run(tc.name, func(t *testing.T) {
// Allow test to work if threads is ever added to default features by explicitly removing threads features
features := api.CoreFeaturesV2
features = features.SetEnabled(experimental.CoreFeaturesThreads, false)
if tc.threadsEnabled {
features = features.SetEnabled(experimental.CoreFeaturesThreads, true)
} else {
// Allow test to work if threads is ever added to default features by explicitly removing threads features
features = features.SetEnabled(experimental.CoreFeaturesThreads, false)
}
_, err := decodeMemory(bytes.NewReader(tc.input), features, newMemorySizer(max, false), max)
require.EqualError(t, err, tc.expectedErr)
})
Expand Down

0 comments on commit 90eba1b

Please sign in to comment.