Skip to content

Commit 4d2ecf0

Browse files
committed
fix: work around LLVM -O2 miscompilation of lfs_dir_fetchmatch
LLVM's ThinLTO at -O2 miscompiles lfs_dir_fetchmatch(), causing directory metadata scans to return incorrect results. mkdir succeeds but subsequent stat/open fails with 'no directory entry'. The bug does not appear at -Oz (TinyGo's default), which is why the earlier Sync() approach seemed to help. Add __attribute__((optnone)) to lfs_dir_fetchmatch to prevent the miscompilation. Targeted approaches (volatile, noinline, memory barriers) were tested but none work - the issue is in how -O2 transforms the function's complex control flow as a whole. - littlefs/lfs.c: optnone on lfs_dir_fetchmatch, revert cache fix - littlefs/go_lfs.go: revert Sync() calls (not the actual fix) - littlefs/go_lfs_callbacks.go: fix uint32 size parameter - littlefs/go_lfs_test.go: directory persistence tests - examples/sd_mkdir_test/: hardware test firmware for SD card
1 parent 4c20469 commit 4d2ecf0

File tree

9 files changed

+1784
-86
lines changed

9 files changed

+1784
-86
lines changed

examples/sd_mkdir_test/main.go

Lines changed: 1152 additions & 0 deletions
Large diffs are not rendered by default.

littlefs/go_lfs.go

Lines changed: 9 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -186,48 +186,24 @@ func (l *LFS) Mount() error {
186186
}
187187

188188
func (l *LFS) Format() error {
189-
if err := errval(C.lfs_format(l.lfs, l.cfg)); err != nil {
190-
return err
191-
}
192-
if syncer, ok := l.dev.(tinyfs.Syncer); ok {
193-
return syncer.Sync()
194-
}
195-
return nil
189+
return errval(C.lfs_format(l.lfs, l.cfg))
196190
}
197191

198192
func (l *LFS) Unmount() error {
199-
if err := errval(C.lfs_unmount(l.lfs)); err != nil {
200-
return err
201-
}
202-
if syncer, ok := l.dev.(tinyfs.Syncer); ok {
203-
return syncer.Sync()
204-
}
205-
return nil
193+
return errval(C.lfs_unmount(l.lfs))
206194
}
207195

208196
func (l *LFS) Remove(path string) error {
209197
cs := cstring(path)
210198
defer C.free(unsafe.Pointer(cs))
211-
if err := errval(C.lfs_remove(l.lfs, cs)); err != nil {
212-
return err
213-
}
214-
if syncer, ok := l.dev.(tinyfs.Syncer); ok {
215-
return syncer.Sync()
216-
}
217-
return nil
199+
return errval(C.lfs_remove(l.lfs, cs))
218200
}
219201

220202
func (l *LFS) Rename(oldPath string, newPath string) error {
221203
cs1, cs2 := cstring(oldPath), cstring(newPath)
222204
defer C.free(unsafe.Pointer(cs1))
223205
defer C.free(unsafe.Pointer(cs2))
224-
if err := errval(C.lfs_rename(l.lfs, cs1, cs2)); err != nil {
225-
return err
226-
}
227-
if syncer, ok := l.dev.(tinyfs.Syncer); ok {
228-
return syncer.Sync()
229-
}
230-
return nil
206+
return errval(C.lfs_rename(l.lfs, cs1, cs2))
231207
}
232208

233209
func (l *LFS) Stat(path string) (os.FileInfo, error) {
@@ -247,13 +223,7 @@ func (l *LFS) Stat(path string) (os.FileInfo, error) {
247223
func (l *LFS) Mkdir(path string, _ os.FileMode) error {
248224
cs := (*C.char)(cstring(path))
249225
defer C.free(unsafe.Pointer(cs))
250-
if err := errval(C.lfs_mkdir(l.lfs, cs)); err != nil {
251-
return err
252-
}
253-
if syncer, ok := l.dev.(tinyfs.Syncer); ok {
254-
return syncer.Sync()
255-
}
256-
return nil
226+
return errval(C.lfs_mkdir(l.lfs, cs))
257227
}
258228

259229
func (l *LFS) Open(path string) (tinyfs.File, error) {
@@ -291,24 +261,6 @@ func (l *LFS) OpenFile(path string, flags int) (tinyfs.File, error) {
291261
return nil, err
292262
}
293263

294-
if flags&(os.O_CREATE|os.O_TRUNC) != 0 {
295-
if flags&os.O_TRUNC != 0 {
296-
if err := errval(C.lfs_file_sync(l.lfs, file.fileptr())); err != nil {
297-
file.Close()
298-
return nil, err
299-
}
300-
}
301-
if syncer, ok := l.dev.(tinyfs.Syncer); ok {
302-
if err := syncer.Sync(); err != nil {
303-
// Should we close and return error?
304-
// Maybe just log? But we can't log.
305-
// For now let's just return error and close.
306-
file.Close()
307-
return nil, err
308-
}
309-
}
310-
}
311-
312264
return file, nil
313265
}
314266

@@ -353,21 +305,14 @@ func (f *File) Close() error {
353305
C.free(f.hndl)
354306
f.hndl = nil
355307
}()
356-
var err error
357308
switch f.typ {
358309
case fileTypeReg:
359-
err = errval(C.lfs_file_close(f.lfs.lfs, f.fileptr()))
310+
return errval(C.lfs_file_close(f.lfs.lfs, f.fileptr()))
360311
case fileTypeDir:
361-
err = errval(C.lfs_dir_close(f.lfs.lfs, f.dirptr()))
312+
return errval(C.lfs_dir_close(f.lfs.lfs, f.dirptr()))
362313
default:
363314
panic("lfs: unknown typ for file handle")
364315
}
365-
if err != nil {
366-
return err
367-
}
368-
if syncer, ok := f.lfs.dev.(tinyfs.Syncer); ok {
369-
return syncer.Sync()
370-
}
371316
}
372317
return nil
373318
}
@@ -428,27 +373,12 @@ func (f *File) Stat() (os.FileInfo, error) {
428373

429374
// Sync synchronizes to storage so that any pending writes are written out.
430375
func (f *File) Sync() error {
431-
if err := errval(C.lfs_file_sync(f.lfs.lfs, f.fileptr())); err != nil {
432-
return err
433-
}
434-
if syncer, ok := f.lfs.dev.(tinyfs.Syncer); ok {
435-
return syncer.Sync()
436-
}
437-
return nil
376+
return errval(C.lfs_file_sync(f.lfs.lfs, f.fileptr()))
438377
}
439378

440379
// Truncate the size of the file to the specified size
441380
func (f *File) Truncate(size uint32) error {
442-
if err := errval(C.lfs_file_truncate(f.lfs.lfs, f.fileptr(), C.lfs_off_t(size))); err != nil {
443-
return err
444-
}
445-
if err := errval(C.lfs_file_sync(f.lfs.lfs, f.fileptr())); err != nil {
446-
return err
447-
}
448-
if syncer, ok := f.lfs.dev.(tinyfs.Syncer); ok {
449-
return syncer.Sync()
450-
}
451-
return nil
381+
return errval(C.lfs_file_truncate(f.lfs.lfs, f.fileptr(), C.lfs_off_t(size)))
452382
}
453383

454384
func (f *File) Write(buf []byte) (n int, err error) {

littlefs/go_lfs_callbacks.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const (
1515
)
1616

1717
//export go_lfs_block_device_read
18-
func go_lfs_block_device_read(ctx unsafe.Pointer, block uint32, offset uint32, buf unsafe.Pointer, size int) int {
18+
func go_lfs_block_device_read(ctx unsafe.Pointer, block uint32, offset uint32, buf unsafe.Pointer, size uint32) int {
1919
if debug {
2020
fmt.Printf("go_lfs_block_device_read: %v, %v, %v, %v, %v\n", ctx, block, offset, buf, size)
2121
}
@@ -27,7 +27,7 @@ func go_lfs_block_device_read(ctx unsafe.Pointer, block uint32, offset uint32, b
2727
}
2828

2929
//export go_lfs_block_device_prog
30-
func go_lfs_block_device_prog(ctx unsafe.Pointer, block uint32, offset uint32, buf unsafe.Pointer, size int) int {
30+
func go_lfs_block_device_prog(ctx unsafe.Pointer, block uint32, offset uint32, buf unsafe.Pointer, size uint32) int {
3131
if debug {
3232
fmt.Printf("go_lfs_block_device_prog: %v, %v, %v, %v, %v\n", ctx, block, offset, buf, size)
3333
}

littlefs/go_lfs_test.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,3 +397,187 @@ func TestDirectoryPersistence(t *testing.T) {
397397
t.Fatalf("'%s' exists but is not a directory", dirName)
398398
}
399399
}
400+
401+
// TestDirectoryWithFiles tests creating directories and adding files to them.
402+
// This covers the common use case of organizing files in subdirectories.
403+
func TestDirectoryWithFiles(t *testing.T) {
404+
fs, _, unmount := createTestFS(t, defaultConfig)
405+
defer unmount()
406+
407+
t.Run("CreateDirAndAddFile", func(t *testing.T) {
408+
// Create a directory
409+
check(t, fs.Mkdir("mydir", 0777))
410+
411+
// Create a file inside the directory
412+
f, err := fs.OpenFile("mydir/test.txt", os.O_CREATE|os.O_WRONLY)
413+
check(t, err)
414+
415+
content := []byte("Hello from inside a directory!")
416+
n, err := f.Write(content)
417+
check(t, err)
418+
if n != len(content) {
419+
t.Fatalf("expected to write %d bytes, wrote %d", len(content), n)
420+
}
421+
check(t, f.Close())
422+
423+
// Read back and verify
424+
f, err = fs.Open("mydir/test.txt")
425+
check(t, err)
426+
buf := make([]byte, 100)
427+
n, err = f.Read(buf)
428+
check(t, err)
429+
if string(buf[:n]) != string(content) {
430+
t.Fatalf("content mismatch: got %q, want %q", string(buf[:n]), string(content))
431+
}
432+
check(t, f.Close())
433+
})
434+
435+
t.Run("MultipleFilesInDir", func(t *testing.T) {
436+
check(t, fs.Mkdir("multidir", 0777))
437+
438+
files := []string{"file1.txt", "file2.txt", "file3.txt"}
439+
for _, fname := range files {
440+
path := "multidir/" + fname
441+
f, err := fs.OpenFile(path, os.O_CREATE|os.O_WRONLY)
442+
check(t, err)
443+
_, err = f.Write([]byte("content of " + fname))
444+
check(t, err)
445+
check(t, f.Close())
446+
}
447+
448+
// Verify all files exist
449+
for _, fname := range files {
450+
path := "multidir/" + fname
451+
info, err := fs.Stat(path)
452+
check(t, err)
453+
if info.IsDir() {
454+
t.Fatalf("expected file, got directory: %s", path)
455+
}
456+
}
457+
})
458+
459+
t.Run("NestedDirsWithFiles", func(t *testing.T) {
460+
// Create nested directory structure
461+
check(t, fs.Mkdir("level1", 0777))
462+
check(t, fs.Mkdir("level1/level2", 0777))
463+
check(t, fs.Mkdir("level1/level2/level3", 0777))
464+
465+
// Add files at each level
466+
levels := []string{"level1", "level1/level2", "level1/level2/level3"}
467+
for _, dir := range levels {
468+
path := dir + "/data.txt"
469+
f, err := fs.OpenFile(path, os.O_CREATE|os.O_WRONLY)
470+
check(t, err)
471+
_, err = f.Write([]byte("data at " + dir))
472+
check(t, err)
473+
check(t, f.Close())
474+
}
475+
476+
// Verify all files
477+
for _, dir := range levels {
478+
path := dir + "/data.txt"
479+
info, err := fs.Stat(path)
480+
check(t, err)
481+
if info.IsDir() {
482+
t.Fatalf("expected file, got directory: %s", path)
483+
}
484+
if info.Size() == 0 {
485+
t.Fatalf("file %s has zero size", path)
486+
}
487+
}
488+
})
489+
}
490+
491+
// TestDirectoryFilePersistence tests that files in directories persist after unmount/remount.
492+
func TestDirectoryFilePersistence(t *testing.T) {
493+
bd := tinyfs.NewMemoryDevice(testPageSize, testBlockSize, testBlockCount)
494+
fs := New(bd).Configure(defaultConfig)
495+
496+
check(t, fs.Format())
497+
check(t, fs.Mount())
498+
499+
// Create directory and file
500+
check(t, fs.Mkdir("persist_dir", 0777))
501+
502+
f, err := fs.OpenFile("persist_dir/important.txt", os.O_CREATE|os.O_WRONLY)
503+
check(t, err)
504+
content := []byte("This data must survive remount!")
505+
_, err = f.Write(content)
506+
check(t, err)
507+
check(t, f.Close())
508+
509+
// Unmount
510+
check(t, fs.Unmount())
511+
512+
// Remount with new instance
513+
fs2 := New(bd).Configure(defaultConfig)
514+
check(t, fs2.Mount())
515+
defer fs2.Unmount()
516+
517+
// Verify directory exists
518+
dirInfo, err := fs2.Stat("persist_dir")
519+
if err != nil {
520+
t.Fatalf("directory lost after remount: %v", err)
521+
}
522+
if !dirInfo.IsDir() {
523+
t.Fatalf("persist_dir is not a directory")
524+
}
525+
526+
// Verify file exists and has correct content
527+
f, err = fs2.Open("persist_dir/important.txt")
528+
if err != nil {
529+
t.Fatalf("file lost after remount: %v", err)
530+
}
531+
defer f.Close()
532+
533+
buf := make([]byte, 100)
534+
n, err := f.Read(buf)
535+
check(t, err)
536+
if string(buf[:n]) != string(content) {
537+
t.Fatalf("content corrupted after remount: got %q, want %q", string(buf[:n]), string(content))
538+
}
539+
}
540+
541+
// TestDeepNestedDirectories tests creating deeply nested directories and files.
542+
func TestDeepNestedDirectories(t *testing.T) {
543+
fs, _, unmount := createTestFS(t, defaultConfig)
544+
defer unmount()
545+
546+
// Create a deeply nested path
547+
path := "a/b/c/d/e/f/g"
548+
dirs := []string{"a", "a/b", "a/b/c", "a/b/c/d", "a/b/c/d/e", "a/b/c/d/e/f", "a/b/c/d/e/f/g"}
549+
550+
for _, dir := range dirs {
551+
check(t, fs.Mkdir(dir, 0777))
552+
}
553+
554+
// Create file at the deepest level
555+
filePath := path + "/deep_file.txt"
556+
f, err := fs.OpenFile(filePath, os.O_CREATE|os.O_WRONLY)
557+
check(t, err)
558+
content := []byte("This is a deeply nested file")
559+
_, err = f.Write(content)
560+
check(t, err)
561+
check(t, f.Close())
562+
563+
// Verify file
564+
info, err := fs.Stat(filePath)
565+
check(t, err)
566+
if info.IsDir() {
567+
t.Fatalf("expected file, got directory")
568+
}
569+
if info.Size() != int64(len(content)) {
570+
t.Fatalf("expected size %d, got %d", len(content), info.Size())
571+
}
572+
573+
// Read and verify content
574+
f, err = fs.Open(filePath)
575+
check(t, err)
576+
buf := make([]byte, 100)
577+
n, err := f.Read(buf)
578+
check(t, err)
579+
if string(buf[:n]) != string(content) {
580+
t.Fatalf("content mismatch")
581+
}
582+
check(t, f.Close())
583+
}

littlefs/lfs.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,6 @@ static int lfs_bd_flush(lfs_t *lfs,
185185
return err;
186186
}
187187

188-
if (pcache->block != LFS_BLOCK_NULL && pcache->block == rcache->block) {
189-
lfs_cache_drop(lfs, rcache);
190-
}
191-
192188
if (validate) {
193189
// check data on disk
194190
lfs_cache_drop(lfs, rcache);
@@ -1073,7 +1069,11 @@ static int lfs_dir_traverse(lfs_t *lfs,
10731069
}
10741070
#endif
10751071

1076-
static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs,
1072+
// Workaround for LLVM bug: this function gets miscompiled at -O2 and above,
1073+
// causing directory metadata scans to return incorrect results (mkdir succeeds
1074+
// but subsequent stat/open can't find the entry). Disabling optimization here
1075+
// prevents the miscompilation while keeping the rest of lfs.c fully optimized.
1076+
static __attribute__((optnone)) lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs,
10771077
lfs_mdir_t *dir, const lfs_block_t pair[2],
10781078
lfs_tag_t fmask, lfs_tag_t ftag, uint16_t *id,
10791079
int (*cb)(void *data, lfs_tag_t tag, const void *buffer), void *data) {

tests/hw/.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
builds/
2+
*.exe

tests/hw/go.mod

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
module tinygo.org/x/tinyfs/tests/hw
2+
3+
go 1.25.7
4+
5+
require go.bug.st/serial v1.6.4
6+
7+
require (
8+
github.com/creack/goselect v0.1.2 // indirect
9+
golang.org/x/sys v0.19.0 // indirect
10+
)

0 commit comments

Comments
 (0)