Repository: tetratelabs/wazero (go)

Issue:
`path_symlink` doesn't work
**Describe the bug**
The `path_symlink` WASI function doesn't seem to work. Always returning `noent` if `-mount dir` doesn't explicitly specify a guest path like `-mount dir:guest`.

**To Reproduce**

Compile this Rust snippet with `wasi` crate v0.11 (for the preview1 API).

```rust
fn main() {
    unsafe {
        let base_fd = 3;

        wasi::path_symlink("file", base_fd, "new-link").unwrap();
    }
}
```

For example:

```
cargo build --target wasm32-wasi
```

And run the produced Wasm binary with Wazero, preopening a directory with a file `file` in it. But don't map it to root.

```
wazero run -mount dir a.wasm
```



**Expected behavior**
Other runtimes like Wasmtime, WasmEdge, Wasmer, and WAMR can handle such a call and create a symlink.

**Environment (please complete the relevant information):**
- Go version: 1.12.1
- wazero Version: d4a49031b671b59fb832c95741df63b96f144104
- Host architecture: amd64
- OS: Darwin



Pull request:
Fix path_symlink only works when dir mounted to /
This commit fixes `path_symlink` not working when `-mount` is not supplied with a mapped directory.

fixes #2228

Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index a3559e5d..7575d77f 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -1955,16 +1955,16 @@ func pathSymlinkFn(_ context.Context, mod api.Module, params []uint64) experimen
 		return experimentalsys.EFAULT
 	}
 
-	newPathBuf, ok := mem.Read(newPath, newPathLen)
-	if !ok {
-		return experimentalsys.EFAULT
+	_, newPathName, errno := atPath(fsc, mod.Memory(), fd, newPath, newPathLen)
+	if errno != 0 {
+		return errno
 	}
 
 	return dir.FS.Symlink(
 		// Do not join old path since it's only resolved when dereference the link created here.
 		// And the dereference result depends on the opening directory's file descriptor at that point.
 		bufToStr(oldPathBuf),
-		path.Join(dir.Name, bufToStr(newPathBuf)),
+		newPathName,
 	)
 }
 
diff --git a/internal/engine/wazevo/backend/executable_context.go b/internal/engine/wazevo/backend/executable_context.go
index 81c6a6b6..8e9571b2 100644
--- a/internal/engine/wazevo/backend/executable_context.go
+++ b/internal/engine/wazevo/backend/executable_context.go
@@ -43,7 +43,7 @@ type ExecutableContextT[Instr any] struct {
 	labelPositionPool wazevoapi.Pool[LabelPosition[Instr]]
 	NextLabel         Label
 	// LabelPositions maps a label to the instructions of the region which the label represents.
-	LabelPositions     map[Label]*LabelPosition[Instr]
+	LabelPositions     []*LabelPosition[Instr]
 	OrderedBlockLabels []*LabelPosition[Instr]
 
 	// PerBlockHead and PerBlockEnd are the head and tail of the instruction list per currently-compiled ssa.BasicBlock.
@@ -67,7 +67,6 @@ func NewExecutableContextT[Instr any](
 		setNext:           setNext,
 		setPrev:           setPrev,
 		labelPositionPool: wazevoapi.NewPool[LabelPosition[Instr]](resetLabelPosition[Instr]),
-		LabelPositions:    make(map[Label]*LabelPosition[Instr]),
 		NextLabel:         LabelInvalid,
 	}
 }
@@ -97,11 +96,7 @@ func (e *ExecutableContextT[Instr]) StartBlock(blk ssa.BasicBlock) {
 	end := e.allocateNop0()
 	e.PerBlockHead, e.PerBlockEnd = end, end
 
-	labelPos, ok := e.LabelPositions[l]
-	if !ok {
-		labelPos = e.AllocateLabelPosition(l)
-		e.LabelPositions[l] = labelPos
-	}
+	labelPos := e.GetOrAllocateLabelPosition(l)
 	e.OrderedBlockLabels = append(e.OrderedBlockLabels, labelPos)
 	labelPos.Begin, labelPos.End = end, end
 	labelPos.SB = blk
@@ -146,8 +141,8 @@ func (e *ExecutableContextT[T]) FlushPendingInstructions() {
 func (e *ExecutableContextT[T]) Reset() {
 	e.labelPositionPool.Reset()
 	e.InstructionPool.Reset()
-	for l := Label(0); l <= e.NextLabel; l++ {
-		delete(e.LabelPositions, l)
+	for i := range e.LabelPositions {
+		e.LabelPositions[i] = nil
 	}
 	e.PendingInstructions = e.PendingInstructions[:0]
 	e.OrderedBlockLabels = e.OrderedBlockLabels[:0]
@@ -163,10 +158,17 @@ func (e *ExecutableContextT[T]) AllocateLabel() Label {
 	return e.NextLabel
 }
 
-func (e *ExecutableContextT[T]) AllocateLabelPosition(la Label) *LabelPosition[T] {
-	l := e.labelPositionPool.Allocate()
-	l.L = la
-	return l
+func (e *ExecutableContextT[T]) GetOrAllocateLabelPosition(l Label) *LabelPosition[T] {
+	if len(e.LabelPositions) <= int(l) {
+		e.LabelPositions = append(e.LabelPositions, make([]*LabelPosition[T], int(l)+1-len(e.LabelPositions))...)
+	}
+	ret := e.LabelPositions[l]
+	if ret == nil {
+		ret = e.labelPositionPool.Allocate()
+		ret.L = l
+		e.LabelPositions[l] = ret
+	}
+	return ret
 }
 
 func (e *ExecutableContextT[T]) GetOrAllocateSSABlockLabel(blk ssa.BasicBlock) Label {
diff --git a/internal/engine/wazevo/backend/isa/amd64/machine.go b/internal/engine/wazevo/backend/isa/amd64/machine.go
index 310ad220..61ae6f40 100644
--- a/internal/engine/wazevo/backend/isa/amd64/machine.go
+++ b/internal/engine/wazevo/backend/isa/amd64/machine.go
@@ -1906,8 +1906,10 @@ func (m *machine) InsertMove(dst, src regalloc.VReg, typ ssa.Type) {
 func (m *machine) Format() string {
 	ectx := m.ectx
 	begins := map[*instruction]backend.Label{}
-	for l, pos := range ectx.LabelPositions {
-		begins[pos.Begin] = l
+	for _, pos := range ectx.LabelPositions {
+		if pos != nil {
+			begins[pos.Begin] = pos.L
+		}
 	}
 
 	irBlocks := map[backend.Label]ssa.BasicBlockID{}
@@ -1950,7 +1952,10 @@ func (m *machine) encodeWithoutSSA(root *instruction) {
 		offset := int64(len(*bufPtr))
 		if cur.kind == nop0 {
 			l := cur.nop0Label()
-			if pos, ok := ectx.LabelPositions[l]; ok {
+			if int(l) >= len(ectx.LabelPositions) {
+				continue
+			}
+			if pos := ectx.LabelPositions[l]; pos != nil {
 				pos.BinaryOffset = offset
 			}
 		}
@@ -2005,7 +2010,7 @@ func (m *machine) Encode(ctx context.Context) (err error) {
 			switch cur.kind {
 			case nop0:
 				l := cur.nop0Label()
-				if pos, ok := ectx.LabelPositions[l]; ok {
+				if pos := ectx.LabelPositions[l]; pos != nil {
 					pos.BinaryOffset = offset
 				}
 			case sourceOffsetInfo:
@@ -2165,8 +2170,7 @@ func (m *machine) allocateBrTarget() (nop *instruction, l backend.Label) { //nol
 func (m *machine) allocateLabel() *labelPosition {
 	ectx := m.ectx
 	l := ectx.AllocateLabel()
-	pos := ectx.AllocateLabelPosition(l)
-	ectx.LabelPositions[l] = pos
+	pos := ectx.GetOrAllocateLabelPosition(l)
 	return pos
 }
 
diff --git a/internal/engine/wazevo/backend/isa/arm64/machine.go b/internal/engine/wazevo/backend/isa/arm64/machine.go
index 311b34f4..5f584f92 100644
--- a/internal/engine/wazevo/backend/isa/arm64/machine.go
+++ b/internal/engine/wazevo/backend/isa/arm64/machine.go
@@ -187,9 +187,8 @@ func (m *machine) allocateBrTarget() (nop *instruction, l label) {
 	l = ectx.AllocateLabel()
 	nop = m.allocateInstr()
 	nop.asNop0WithLabel(l)
-	pos := ectx.AllocateLabelPosition(l)
+	pos := ectx.GetOrAllocateLabelPosition(l)
 	pos.Begin, pos.End = nop, nop
-	ectx.LabelPositions[l] = pos
 	return
 }
 
@@ -285,7 +284,7 @@ func (m *machine) resolveRelativeAddresses(ctx context.Context) {
 				switch cur.kind {
 				case nop0:
 					l := cur.nop0Label()
-					if pos, ok := ectx.LabelPositions[l]; ok {
+					if pos := ectx.LabelPositions[l]; pos != nil {
 						pos.BinaryOffset = offset + size
 					}
 				case condBr:
@@ -432,8 +431,10 @@ func (m *machine) insertConditionalJumpTrampoline(cbr *instruction, currentBlk *
 func (m *machine) Format() string {
 	ectx := m.executableContext
 	begins := map[*instruction]label{}
-	for l, pos := range ectx.LabelPositions {
-		begins[pos.Begin] = l
+	for _, pos := range ectx.LabelPositions {
+		if pos != nil {
+			begins[pos.Begin] = pos.L
+		}
 	}
 
 	irBlocks := map[label]ssa.BasicBlockID{}
diff --git a/internal/engine/wazevo/ssa/basic_block.go b/internal/engine/wazevo/ssa/basic_block.go
index 0fc592a0..2e7069dc 100644
--- a/internal/engine/wazevo/ssa/basic_block.go
+++ b/internal/engine/wazevo/ssa/basic_block.go
@@ -49,21 +49,12 @@ type BasicBlock interface {
 	// ReturnBlock returns ture if this block represents the function return.
 	ReturnBlock() bool
 
-	// FormatHeader returns the debug string of this block, not including instruction.
-	FormatHeader(b Builder) string
-
 	// Valid is true if this block is still valid even after optimizations.
 	Valid() bool
 
 	// Sealed is true if this block has been sealed.
 	Sealed() bool
 
-	// BeginPredIterator returns the first predecessor of this block.
-	BeginPredIterator() BasicBlock
-
-	// NextPredIterator returns the next predecessor of this block.
-	NextPredIterator() BasicBlock
-
 	// Preds returns the number of predecessors of this block.
 	Preds() int
 
@@ -90,10 +81,9 @@ type (
 		rootInstr, currentInstr *Instruction
 		// params are Values that represent parameters to a basicBlock.
 		// Each parameter can be considered as an output of PHI instruction in traditional SSA.
-		params   []Value
-		predIter int
-		preds    []basicBlockPredecessorInfo
-		success  []*basicBlock
+		params  []Value
+		preds   []basicBlockPredecessorInfo
+		success []*basicBlock
 		// singlePred is the alias to preds[0] for fast lookup, and only set after Seal is called.
 		singlePred *basicBlock
 		// lastDefinitions maps Variable to its last definition in this block.
@@ -118,7 +108,7 @@ type (
 
 		// loopNestingForestChildren holds the children of this block in the loop nesting forest.
 		// Non-empty if and only if this block is a loop header (i.e. loopHeader=true)
-		loopNestingForestChildren []BasicBlock
+		loopNestingForestChildren wazevoapi.VarLength[BasicBlock]
 
 		// reversePostOrder is used to sort all the blocks in the function in reverse post order.
 		// This is used in builder.LayoutBlocks.
@@ -138,6 +128,9 @@ type (
 	}
 )
 
+// basicBlockVarLengthNil is the default nil value for basicBlock.loopNestingForestChildren.
+var basicBlockVarLengthNil = wazevoapi.NewNilVarLength[BasicBlock]()
+
 const basicBlockIDReturnBlock = 0xffffffff
 
 // Name implements BasicBlock.Name.
@@ -240,22 +233,6 @@ func (bb *basicBlock) NumPreds() int {
 	return len(bb.preds)
 }
 
-// BeginPredIterator implements BasicBlock.BeginPredIterator.
-func (bb *basicBlock) BeginPredIterator() BasicBlock {
-	bb.predIter = 0
-	return bb.NextPredIterator()
-}
-
-// NextPredIterator implements BasicBlock.NextPredIterator.
-func (bb *basicBlock) NextPredIterator() BasicBlock {
-	if bb.predIter >= len(bb.preds) {
-		return nil
-	}
-	pred := bb.preds[bb.predIter].blk
-	bb.predIter++
-	return pred
-}
-
 // Preds implements BasicBlock.Preds.
 func (bb *basicBlock) Preds() int {
 	return len(bb.preds)
@@ -297,7 +274,7 @@ func resetBasicBlock(bb *basicBlock) {
 	bb.unknownValues = bb.unknownValues[:0]
 	bb.lastDefinitions = wazevoapi.ResetMap(bb.lastDefinitions)
 	bb.reversePostOrder = -1
-	bb.loopNestingForestChildren = bb.loopNestingForestChildren[:0]
+	bb.loopNestingForestChildren = basicBlockVarLengthNil
 	bb.loopHeader = false
 	bb.sibling = nil
 	bb.child = nil
@@ -327,8 +304,8 @@ func (bb *basicBlock) addPred(blk BasicBlock, branch *Instruction) {
 	pred.success = append(pred.success, bb)
 }
 
-// FormatHeader implements BasicBlock.FormatHeader.
-func (bb *basicBlock) FormatHeader(b Builder) string {
+// formatHeader returns the string representation of the header of the basicBlock.
+func (bb *basicBlock) formatHeader(b Builder) string {
 	ps := make([]string, len(bb.params))
 	for i, p := range bb.params {
 		ps[i] = p.formatWithType(b)
@@ -390,7 +367,7 @@ func (bb *basicBlock) String() string {
 
 // LoopNestingForestChildren implements BasicBlock.LoopNestingForestChildren.
 func (bb *basicBlock) LoopNestingForestChildren() []BasicBlock {
-	return bb.loopNestingForestChildren
+	return bb.loopNestingForestChildren.View()
 }
 
 // LoopHeader implements BasicBlock.LoopHeader.
diff --git a/internal/engine/wazevo/ssa/builder.go b/internal/engine/wazevo/ssa/builder.go
index f5414079..0671b2a7 100644
--- a/internal/engine/wazevo/ssa/builder.go
+++ b/internal/engine/wazevo/ssa/builder.go
@@ -135,6 +135,7 @@ func NewBuilder() Builder {
 	return &builder{
 		instructionsPool:               wazevoapi.NewPool[Instruction](resetInstruction),
 		basicBlocksPool:                wazevoapi.NewPool[basicBlock](resetBasicBlock),
+		varLengthBasicBlockPool:        wazevoapi.NewVarLengthPool[BasicBlock](),
 		varLengthPool:                  wazevoapi.NewVarLengthPool[Value](),
 		valueAnnotations:               make(map[ValueID]string),
 		signatures:                     make(map[SignatureID]*Signature),
@@ -177,6 +178,8 @@ type builder struct {
 	dominators []*basicBlock
 	sparseTree dominatorSparseTree
 
+	varLengthBasicBlockPool wazevoapi.VarLengthPool[BasicBlock]
+
 	// loopNestingForestRoots are the roots of the loop nesting forest.
 	loopNestingForestRoots []BasicBlock
 
@@ -219,6 +222,7 @@ func (b *builder) Init(s *Signature) {
 	b.instructionsPool.Reset()
 	b.basicBlocksPool.Reset()
 	b.varLengthPool.Reset()
+	b.varLengthBasicBlockPool.Reset()
 	b.donePreBlockLayoutPasses = false
 	b.doneBlockLayout = false
 	b.donePostBlockLayoutPasses = false
@@ -566,7 +570,7 @@ func (b *builder) Format() string {
 	}
 	for bb := iterBegin(); bb != nil; bb = iterNext() {
 		str.WriteByte('\n')
-		str.WriteString(bb.FormatHeader(b))
+		str.WriteString(bb.formatHeader(b))
 		str.WriteByte('\n')
 
 		for cur := bb.Root(); cur != nil; cur = cur.Next() {
diff --git a/internal/engine/wazevo/ssa/pass_blk_layouts.go b/internal/engine/wazevo/ssa/pass_blk_layouts.go
index 9068180a..bf906391 100644
--- a/internal/engine/wazevo/ssa/pass_blk_layouts.go
+++ b/internal/engine/wazevo/ssa/pass_blk_layouts.go
@@ -143,7 +143,7 @@ func passLayoutBlocks(b *builder) {
 	if wazevoapi.SSAValidationEnabled {
 		for _, trampoline := range trampolines {
 			if _, ok := b.blkVisited[trampoline]; !ok {
-				panic("BUG: trampoline block not inserted: " + trampoline.FormatHeader(b))
+				panic("BUG: trampoline block not inserted: " + trampoline.formatHeader(b))
 			}
 			trampoline.validate(b)
 		}
diff --git a/internal/engine/wazevo/ssa/pass_cfg.go b/internal/engine/wazevo/ssa/pass_cfg.go
index 50cb9c47..1bec185e 100644
--- a/internal/engine/wazevo/ssa/pass_cfg.go
+++ b/internal/engine/wazevo/ssa/pass_cfg.go
@@ -180,7 +180,7 @@ func passBuildLoopNestingForest(b *builder) {
 			b.loopNestingForestRoots = append(b.loopNestingForestRoots, blk)
 		} else if n == ent {
 		} else if n.loopHeader {
-			n.loopNestingForestChildren = append(n.loopNestingForestChildren, blk)
+			n.loopNestingForestChildren = n.loopNestingForestChildren.Append(&b.varLengthBasicBlockPool, blk)
 		}
 	}
 
@@ -193,7 +193,7 @@ func passBuildLoopNestingForest(b *builder) {
 
 func printLoopNestingForest(root *basicBlock, depth int) {
 	fmt.Println(strings.Repeat("\t", depth), "loop nesting forest root:", root.ID())
-	for _, child := range root.loopNestingForestChildren {
+	for _, child := range root.loopNestingForestChildren.View() {
 		fmt.Println(strings.Repeat("\t", depth+1), "child:", child.ID())
 		if child.LoopHeader() {
 			printLoopNestingForest(child.(*basicBlock), depth+2)


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index 037d34d6..03c324f9 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -4508,7 +4508,7 @@ func Test_pathRemoveDirectory_Errors(t *testing.T) {
 	}
 }
 
-func Test_pathSymlink_errors(t *testing.T) {
+func Test_pathSymlink(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
 
 	dirName := "dir"
@@ -4536,14 +4536,46 @@ func Test_pathSymlink_errors(t *testing.T) {
 	ok = mem.Write(link, []byte(linkName))
 	require.True(t, ok)
 
-	t.Run("success", func(t *testing.T) {
-		requireErrnoResult(t, wasip1.ErrnoSuccess, mod, wasip1.PathSymlinkName,
-			uint64(file), uint64(len(fileName)), uint64(fd), uint64(link), uint64(len(linkName)))
-		require.Contains(t, log.String(), wasip1.ErrnoName(wasip1.ErrnoSuccess))
-		st, err := os.Lstat(joinPath(dirPath, linkName))
-		require.NoError(t, err)
-		require.Equal(t, st.Mode()&os.ModeSymlink, os.ModeSymlink)
-	})
+	successCases := []struct {
+		name    string
+		fd      int32
+		oldPath string
+		newPath string
+	}{
+		{
+			name:    "dir",
+			fd:      fd,
+			oldPath: fileName,
+			newPath: linkName,
+		},
+		{
+			name:    "preopened root",
+			fd:      sys.FdPreopen,
+			oldPath: fileName,
+			newPath: linkName,
+		},
+	}
+
+	for _, tt := range successCases {
+		tc := tt
+
+		t.Run(tc.name, func(t *testing.T) {
+			file := uint32(0xbb)
+			ok := mem.Write(file, []byte(fileName))
+			require.True(t, ok)
+
+			link := uint32(0xdd)
+			ok = mem.Write(link, []byte(linkName))
+			require.True(t, ok)
+
+			requireErrnoResult(t, wasip1.ErrnoSuccess, mod, wasip1.PathSymlinkName,
+				uint64(file), uint64(len(tc.oldPath)), uint64(tc.fd), uint64(link), uint64(len(tc.newPath)))
+			require.Contains(t, log.String(), wasip1.ErrnoName(wasip1.ErrnoSuccess))
+			st, err := os.Lstat(joinPath(dirPath, linkName))
+			require.NoError(t, err)
+			require.Equal(t, st.Mode()&os.ModeSymlink, os.ModeSymlink)
+		})
+	}
 
 	t.Run("errors", func(t *testing.T) {
 		for _, tc := range []struct {
@@ -4952,7 +4984,7 @@ func requireOpenFile(t *testing.T, tmpDir string, pathName string, data []byte,
 	if readOnly {
 		fsConfig = fsConfig.WithReadOnlyDirMount(tmpDir, "/")
 	} else {
-		fsConfig = fsConfig.WithDirMount(tmpDir, "/")
+		fsConfig = fsConfig.WithDirMount(tmpDir, "preopen")
 	}
 
 	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFSConfig(fsConfig))
diff --git a/internal/engine/wazevo/backend/isa/arm64/machine_test.go b/internal/engine/wazevo/backend/isa/arm64/machine_test.go
index 882b7384..943acff8 100644
--- a/internal/engine/wazevo/backend/isa/arm64/machine_test.go
+++ b/internal/engine/wazevo/backend/isa/arm64/machine_test.go
@@ -181,10 +181,10 @@ L200:
 
 			ectx := m.executableContext
 
-			originLabelPos := ectx.AllocateLabelPosition(originLabel)
+			originLabelPos := ectx.GetOrAllocateLabelPosition(originLabel)
 			originLabelPos.Begin = cbr
 			originLabelPos.End = linkInstr(cbr, end)
-			originNextLabelPos := ectx.AllocateLabelPosition(originLabelNext)
+			originNextLabelPos := ectx.GetOrAllocateLabelPosition(originLabelNext)
 			originNextLabelPos.Begin = originalEndNext
 			linkInstr(originLabelPos.End, originalEndNext)
 
diff --git a/internal/engine/wazevo/ssa/pass_cfg_test.go b/internal/engine/wazevo/ssa/pass_cfg_test.go
index f6c48e6e..ade6eeb9 100644
--- a/internal/engine/wazevo/ssa/pass_cfg_test.go
+++ b/internal/engine/wazevo/ssa/pass_cfg_test.go
@@ -669,7 +669,7 @@ func TestBuildLoopNestingForest(t *testing.T) {
 			for expBlkID, blk := range blocks {
 				expChildren := tc.expLNF.children[expBlkID]
 				var actualChildren []BasicBlockID
-				for _, child := range blk.loopNestingForestChildren {
+				for _, child := range blk.loopNestingForestChildren.View() {
 					actualChildren = append(actualChildren, child.(*basicBlock).id)
 				}
 				sort.Slice(actualChildren, func(i, j int) bool {