Skip to content

fix(esp32): restore emulator float output and add regression coverage #1699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luoliwoshang wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix(esp32): restore emulator float output and add regression coverage #1699

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2f68148
fix(esp32): enable float formatted output regression coverage
luoliwoshang Mar 10, 2026
fe2cb9f
crosscompile/libc: compile full xtensa libm groups
luoliwoshang Mar 10, 2026
6d61f9f
crosscompile/libc: fix xtensa machine libm include paths
luoliwoshang Mar 10, 2026
fdb7c48
crosscompile/libc: add missing xtensa libm ef_acosh
luoliwoshang Mar 10, 2026
e5411f2
crosscompile: update newlib-esp32 group count assertion
luoliwoshang Mar 10, 2026
158fd2c
gofmt: fix internal/crosscompile/libc_test
luoliwoshang Mar 10, 2026
250f41e
embed: move esp serial regressions to testdata cases
luoliwoshang Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions _demo/embed/esp32/float-1685/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package main

type point struct {
x float64
y float64
}

type myPoint = point

func (p *point) scale(factor float64) {
p.x *= factor
p.y *= factor
}

func (p *myPoint) move(dx, dy float64) {
p.x += dx
p.y += dy
}

func pair(f float64) (int, float64) {
return 1, f
}

type bar struct {
pb *byte
f float32
}

type foo struct {
pb *byte
f float32
}

func xadd(a, b int) int {
return a + b
}

func double(v float64) float64 {
return v * 2
}

func main() {
pt := &myPoint{1, 2}
pt.scale(2)
pt.move(3, 4)
println(pt.x, pt.y)

i, f := pair(2.0)
println(i, f)

// Keep this case on the float-format path without triggering
// esp32 type-assert timeout cases tracked separately.
ret, ok := bar{}, false
println(ret.pb, ret.f, "notOk:", !ok)

ret2, ok2 := foo{}, true
println(ret2.pb, ret2.f, ok2)

println(xadd(1, 2), double(3.14))
}
59 changes: 59 additions & 0 deletions _demo/embed/test-esp-serial-startup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,58 @@ run_emulator_smoke() {
fi
}

extract_last_nonempty_lines() {
local n="$1"
awk -v n="$n" '
NF { out[++count] = $0 }
END {
if (n <= 0 || count == 0) {
exit
}
start = count - n + 1
if (start < 1) {
start = 1
}
for (i = start; i <= count; i++) {
print out[i]
}
}'
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The extract_last_nonempty_lines function is more complex than necessary and can be inefficient for large inputs as it buffers all non-empty lines in memory. You can achieve the same result more idiomatically and efficiently using a combination of standard shell utilities like grep and tail.

Suggested change
local n="$1"
awk -v n="$n" '
NF { out[++count] = $0 }
END {
if (n <= 0 || count == 0) {
exit
}
start = count - n + 1
if (start < 1) {
start = 1
}
for (i = start; i <= count; i++) {
print out[i]
}
}'
local n="$1"
# Use grep to filter out empty/whitespace-only lines and tail to get the last n lines.
# This is more memory-efficient than the awk implementation as it doesn't
# buffer the entire input.
grep -v '^[[:space:]]*$' | tail -n "$n"

}

run_case_and_compare() {
local target="$1"
local case_dir="$2"
local expected="$3"
local raw_output
local actual
local expected_lines

echo "Running: llgo run -a -target=${target} -emulator ${case_dir}"
if ! raw_output=$(llgo run -a -target="${target}" -emulator "${case_dir}" 2>&1); then
Copy link
Copy Markdown
Contributor

@xgopilot xgopilot bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_case_and_compare treats a non-zero exit code as a hard failure, but the existing run_emulator_smoke uses set +e and only validates output. ESP32 emulators often exit non-zero on normal completion (e.g. semihosting halt). This inconsistency could make the new regression test flaky in CI.

Consider matching the tolerance approach from run_emulator_smoke, or documenting why stricter handling is intentional here.

echo "✗ FAIL: command failed for ${case_dir}"
echo "$raw_output"
return 1
fi

expected_lines=$(printf "%s\n" "$expected" | awk 'NF { n++ } END { print n + 0 }')
actual=$(printf "%s\n" "$raw_output" | tr -d '\r' | extract_last_nonempty_lines "$expected_lines")
if [ "$actual" = "$expected" ]; then
echo "✓ PASS: $case_dir"
return 0
fi

echo "✗ FAIL: output mismatch for $case_dir"
echo "Expected:"
printf "%s\n" "$expected"
echo ""
echo "Got:"
printf "%s\n" "$actual"
echo ""
echo "Diff:"
diff -u <(printf "%s\n" "$expected") <(printf "%s\n" "$actual") || true
return 1
}

mkdir -p "$TEMP_DIR"

echo "==> Creating minimal test program..."
Expand All @@ -98,7 +150,14 @@ run_emulator_smoke "esp32c3-basic" "ESP32-C3" "Hello World"
build_target "esp32" "$ESP32_PREFIX" "ESP32"
run_emulator_smoke "esp32" "ESP32" "Hello World"

echo ""
echo "=== Regression: ESP32 float output (temporary) ==="
Copy link
Copy Markdown
Contributor

@xgopilot xgopilot bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (temporary) label here has no associated issue reference or removal criteria. Consider adding a link to the tracking issue (e.g. the struczero type-assert hang mentioned in the PR description) so this doesn't become stale.

pushd "$SCRIPT_DIR" > /dev/null
run_case_and_compare "esp32" "./esp32/float-1685" $'+5.000000e+00 +8.000000e+00\n1 +2.000000e+00\n0x0 +0.000000e+00 notOk: true\n0x0 +0.000000e+00 true\n3 +6.280000e+00'
popd > /dev/null

echo ""
echo "=== Smoke Tests Passed ==="
echo "✓ ESP32-C3 build + emulator run passed"
echo "✓ ESP32 build + emulator run passed"
echo "✓ ESP32 float output regression cases match expected output"
35 changes: 28 additions & 7 deletions internal/crosscompile/compile/libc/libc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,8 @@ func TestGetNewlibESP32ConfigXtensa(t *testing.T) {
}

// Test Groups configuration
if len(config.Groups) != 3 {
t.Errorf("Expected 2 groups, got %d", len(config.Groups))
if len(config.Groups) != 4 {
t.Errorf("Expected 4 groups, got %d", len(config.Groups))
} else {
// Group 0: libcrt0
group0 := config.Groups[0]
Expand Down Expand Up @@ -631,6 +631,28 @@ func TestGetNewlibESP32ConfigXtensa(t *testing.T) {
}
}

// Group 3: libm fpclassify symbols for _printf_float path.
group3 := config.Groups[3]
expectedOutput3 := "libm-fpclassify-" + target + ".a"
if group3.OutputFileName != expectedOutput3 {
t.Errorf("Group3 OutputFileName expected '%s', got '%s'", expectedOutput3, group3.OutputFileName)
}
for _, sample := range []string{
filepath.Join(baseDir, "newlib", "libm", "common", "s_fpclassify.c"),
filepath.Join(baseDir, "newlib", "libm", "common", "sf_fpclassify.c"),
} {
found := false
for _, file := range group3.Files {
if file == sample {
found = true
break
}
}
if !found {
t.Errorf("Expected file '%s' not found in group3 files", sample)
}
}

// Test LDFlags and CCFlags
if len(group0.LDFlags) == 0 {
t.Error("Expected non-empty LDFlags in group0")
Expand Down Expand Up @@ -698,8 +720,8 @@ func TestGroupConfiguration(t *testing.T) {

t.Run("Xtensa_GroupCount", func(t *testing.T) {
config := getNewlibESP32ConfigXtensa(baseDir, target)
if len(config.Groups) != 3 {
t.Errorf("Expected 2 groups for Xtensa, got %d", len(config.Groups))
if len(config.Groups) != 4 {
t.Errorf("Expected 4 groups for Xtensa, got %d", len(config.Groups))
}
})

Expand All @@ -726,12 +748,11 @@ func TestGroupConfiguration(t *testing.T) {
expectedNames := []string{
"libcrt0-" + target + ".a",
"libgloss-" + target + ".a",
"libc-" + target + ".a",
"libm-fpclassify-" + target + ".a",
}

for i, group := range config.Groups {
Copy link
Copy Markdown
Contributor

@xgopilot xgopilot bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old guard if i >= len(expectedNames) { return } was removed, but the loop still iterates over config.Groups. If a future change adds a group without updating expectedNames, this will panic with index-out-of-bounds instead of producing a clear test failure.

Consider bounding the loop or adding a length check:

Suggested change
for i, group := range config.Groups {
for i, group := range config.Groups {
if i >= len(expectedNames) {
t.Errorf("unexpected extra group %d: %s", i, group.OutputFileName)
break
}

if i >= len(expectedNames) {
return
}
if group.OutputFileName != expectedNames[i] {
t.Errorf("Group %d expected name '%s', got '%s'", i, expectedNames[i], group.OutputFileName)
}
Expand Down
23 changes: 23 additions & 0 deletions internal/crosscompile/compile/libc/newlibesp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,19 @@ func getNewlibESP32ConfigXtensa(baseDir, target string) compile.CompileConfig {
"-I" + libcDir,
}

// Keep this in sync with esp32 target ldflags:
// `--undefined=_printf_float` pulls nano-vfprintf float path, which
// requires libm fp classify symbols such as __fpclassifyd.
libmCommonCFlags := []string{
"-DHAVE_CONFIG_H",
"-D_LIBC",
"-D__ESP__",
"-isystem" + filepath.Join(libcDir, "include"),
"-I" + filepath.Join(baseDir, "newlib"),
"-idirafter" + filepath.Join(baseDir, "include"),
"-I" + filepath.Join(baseDir, "newlib", "libm", "common"),
}

return compile.CompileConfig{
ExportCFlags: libcIncludeDir,
Groups: []compile.CompileGroup{
Expand Down Expand Up @@ -2489,6 +2502,16 @@ func getNewlibESP32ConfigXtensa(baseDir, target string) compile.CompileConfig {
"-Wno-unused-command-line-argument",
}),
},
{
OutputFileName: fmt.Sprintf("libm-fpclassify-%s.a", target),
Files: []string{
filepath.Join(baseDir, "newlib", "libm", "common", "s_fpclassify.c"),
filepath.Join(baseDir, "newlib", "libm", "common", "sf_fpclassify.c"),
},
CFlags: append([]string{}, libmCommonCFlags...),
LDFlags: _libcLDFlags,
CCFlags: _libcCCFlags,
},
},
}
}
Expand Down
3 changes: 3 additions & 0 deletions targets/esp32.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
"default-stack-size": 2048,
"rtlib": "compiler-rt",
"libc": "newlib-esp32",
"ldflags": [
"--undefined=_printf_float"
],
"linkerscript": "targets/esp32.memory.elf.ld",
"binary-format": "esp32",
"flash-command": "esptool.py --chip=esp32 --port {port} write_flash 0x1000 {bin} -ff 80m -fm dout",
Expand Down
Loading