Skip to content

fix(runtime): go runtime when building in baremetal environment#1268

Merged
xushiwei merged 13 commits intogoplus:mainfrom
MeteorsLiu:emb-runtime
Sep 14, 2025
Merged

fix(runtime): go runtime when building in baremetal environment#1268
xushiwei merged 13 commits intogoplus:mainfrom
MeteorsLiu:emb-runtime

Conversation

@MeteorsLiu
Copy link
Contributor

@MeteorsLiu MeteorsLiu commented Sep 5, 2025

TODO: sigsetjmp(defer)

@MeteorsLiu MeteorsLiu changed the title fix(runtime): symbol not found when building in baremental environment fix(runtime): go runtime when building in baremental environment Sep 5, 2025
@codecov
Copy link

codecov bot commented Sep 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.22%. Comparing base (d5ad4d9) to head (dc59333).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1268   +/-   ##
=======================================
  Coverage   90.22%   90.22%           
=======================================
  Files          42       42           
  Lines       11931    11931           
=======================================
  Hits        10765    10765           
  Misses       1032     1032           
  Partials      134      134           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MeteorsLiu MeteorsLiu marked this pull request as ready for review September 8, 2025 08:03
@cpunion
Copy link
Collaborator

cpunion commented Sep 9, 2025

/review

@niupilot
Copy link

niupilot bot commented Sep 9, 2025

Code Review for PR #1268: Baremetal Runtime Support

Todo List:

  • Gather context and read changed files
  • Analyze baremetal runtime architecture
  • Review new baremetal-specific files
  • Review modifications to existing files
  • Check for potential issues and improvements
  • Provide comprehensive review feedback

🔍 Code Review Summary

This PR introduces baremetal runtime support for LLGO by creating platform-specific implementations that handle the constraints of bare-metal environments (no OS, limited standard library support).

🏗️ Architecture Analysis

The implementation uses Go's build tag system (//go:build baremetal) to provide specialized baremetal versions of runtime components:

  1. Simplified runtime behavior: Baremetal versions remove OS-dependent features like threading, debug stack traces, and complex exception handling
  2. Minimal resource usage: Single-threaded execution (maxprocs = 1) and simplified I/O operations
  3. Clean separation: Existing code is preserved with !baremetal tags while new baremetal-specific code is isolated

📋 Detailed Review

Strengths

  1. Excellent Build Tag Strategy: The use of build tags provides clean separation between baremetal and standard runtime implementations

    • runtime/internal/runtime/z_baremetal.go:1 vs runtime/internal/runtime/z_default.go:1
    • Consistent pattern across all modified files
  2. Proper Error Handling for Baremetal Constraints:

    • runtime/internal/clite/debug/debug_baremetal.go:18-40 - All debug functions properly panic with "not implemented" which is appropriate for baremetal environments
    • runtime/internal/runtime/z_baremetal.go:14-19 - Simplified panic handling without pthread dependencies
  3. Smart I/O Initialization:

    • runtime/internal/runtime/z_print.go:25-35 - Fallback initialization of stdin/stdout/stderr using /dev/* paths
    • runtime/internal/clite/stdio_baremetal.go:23-26 - Uninitialized file pointers that get set up later
  4. Consistent Format String Handling:

    • runtime/internal/runtime/z_baremetal.go:7-11 vs runtime/internal/runtime/z_default.go:11-15 - Different format strings for different architectures (32-bit vs 64-bit)

⚠️ Areas for Improvement

  1. Typo in Filename:

    • runtime/internal/lib/internal/syscall/unix/fcntl_baremental.go should be fcntl_baremetal.go for consistency with other files
  2. Missing Error Context:

    • runtime/internal/clite/debug/debug_baremetal.go:18-40 - While panicking is appropriate, consider providing more descriptive error messages that mention baremetal limitations
  3. Potential I/O Issues:

    • runtime/internal/runtime/z_print.go:25-35 - The hardcoded /dev/stdin, /dev/stdout paths may not exist in all baremetal environments. Consider adding error checking or making this configurable
  4. Redundant Code:

    • runtime/internal/lib/internal/syscall/unix/fcntl_baremental.go:9-15 - Both Fcntl and fcntl functions return the same thing. Consider if both are needed or if one should delegate to the other

🔒 Security Considerations

  1. Memory Safety: The code properly uses unsafe.Pointer in appropriate contexts and follows Go's unsafe usage patterns
  2. No Credential Exposure: No secrets, keys, or sensitive data handling detected
  3. Error Boundaries: Proper panic handling prevents undefined behavior in unsupported operations

🧪 Testing Considerations

Since this is baremetal support, traditional unit testing may not be applicable. Consider:

  1. Integration tests with actual baremetal targets
  2. Compilation tests to ensure build tags work correctly
  3. Mock baremetal environment testing

🎯 Specific Recommendations

  1. Fix filename typo: Rename fcntl_baremental.gofcntl_baremetal.go
  2. Enhance error messages: Add baremetal-specific context to panic messages
  3. Add documentation: Consider adding a brief comment explaining the baremetal constraints in key files
  4. I/O error handling: Add checks for failed file operations in the init function

🏁 Conclusion

This is a well-architected implementation that cleanly separates baremetal-specific behavior from the standard runtime. The code demonstrates good understanding of baremetal constraints and provides appropriate fallbacks. The primary issues are minor (typo, error messaging) and don't affect functionality.

Recommendation: Approve with minor fixes


👉 Want me to re-run this review? Just type /review in the comments! For more usage examples, visit CodeAgent GitHub Repository.

Comment on lines +8 to +11
printFormatPrefixInt = c.Str("%ld")
printFormatPrefixUInt = c.Str("%lu")
printFormatPrefixHex = c.Str("%lx")
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: some libc may not support %lld

@MeteorsLiu MeteorsLiu changed the title fix(runtime): go runtime when building in baremental environment fix(runtime): go runtime when building in baremetal environment Sep 10, 2025
@xushiwei xushiwei merged commit 30a3e73 into goplus:main Sep 14, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants