-
Notifications
You must be signed in to change notification settings - Fork 87
Refactor moduledata parsing using field offset tables (Issue #55) #78
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.
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
Refactor moduledata parsing using field offset tables (Issue #55) #78
Conversation
) This commit refactors moduledata parsing for Go versions 1.16-1.24 to use a data-driven approach with field offset tables instead of repetitive version-specific switch statements. Changes: - Add objfile/layouts.go with generic field offset layout system - Add objfile/layouts_test.go with comprehensive test coverage - Refactor objfile/objfile.go to use generic parser for Go 1.16-1.24 - Reduce code from ~430 lines of switches to ~60 lines of generic code Benefits: - 85% code reduction for supported versions (430 → 62 lines) - Adding new Go versions requires only ~8 lines of layout data - Single parsing logic eliminates duplicate code - Comprehensive test suite ensures correctness Testing: - All existing tests pass - New tests verify layout offsets match struct definitions - Backward compatibility tests ensure identical output - Tested against real Go 1.17.2 binary (testproject) Fixes mandiant#55
bb7034e to
47c5458
Compare
|
Hello I made a few mistakes while committing I had pushed commits to wrong branch earlier and then push wrong commits to this branch as well now i fixed it. |
|
This is good work! It passes all the tests and looks reasonable to me. Before merging I'd like if it could be extended to cover the additional cases in the objfile For testing you can run the |
Extends the layout table approach to older Go versions (1.5-1.15), eliminating 228 lines of repetitive switch statements. Changes: - Add layout definitions for Go 1.5-1.6, 1.7, and 1.8-1.15 to layouts.go - Add validateAndConvertModuleData_Legacy() for Go 1.7-1.15 - Add validateAndConvertModuleData_Legacy_NoTypes() for Go 1.5-1.6 - Refactor objfile.go case "1.2" block (228 lines → 75 lines) - Add comprehensive tests for legacy versions (17 new test cases) Benefits: - 67% code reduction in legacy version handling - Consistent parsing approach across Go 1.5-1.24 (20 versions) - Improved maintainability and extensibility - Single source of truth for moduledata layouts Testing: - All existing unit tests pass - Added TestLayoutOffsets_Legacy_Versions (6 subtests) - Added TestVersionMapping_Legacy (11 subtests) - Total: 17 new test cases validating legacy version support Code Metrics: - objfile/objfile.go: 1,783 → 1,628 lines (-155 lines) - objfile/layouts.go: 436 → 564 lines (+128 lines infrastructure) - objfile/layouts_test.go: 380 → 590 lines (+210 lines tests) - Net: -155 lines of duplicate code eliminated Addresses maintainer feedback on PR mandiant#78 (moduledata portion). Type parsing refactoring (Phase 2) to follow in next commit.
Refactors type parsing (ParseType_impl) to use the layout table approach, eliminating 158 lines of repetitive version-specific switch statements. Changes: - Add RtypeLayout system to layouts.go for Go 1.5-1.24 - Add parseRtypeGeneric() for generic type parsing - Add getRtypeFieldOffset() helper function - Refactor ParseType_impl() to use generic parser (208 lines → 50 lines) - Add comprehensive tests for Rtype layouts (23 test cases) Benefits: - 76% code reduction in type parsing (208 lines → 50 lines) - Consistent parsing approach for all Go 1.5-1.24 type structures - Single source of truth for Rtype layouts - Improved maintainability Testing: - All existing tests pass - Added TestRtypeLayoutOffsets (3 subtests) - Added TestRtypeVersionMapping (20 subtests) - Total: 23 new test cases validating Rtype support Code Metrics: - objfile/objfile.go: 1,628 → 1,470 lines (-158 lines) - objfile/layouts.go: 564 → 786 lines (+222 lines infrastructure) - objfile/layouts_test.go: 590 → 748 lines (+158 lines tests) Combined with Phase 1: - Total code reduction: 313 lines from objfile.go - ModuleData + Type parsing: ~650 lines → ~125 lines (81% reduction) Completes maintainer feedback on PR mandiant#78 (type parsing portion).
|
@stevemk14ebr Hello and good day to you. I have ran build_test_files.sh for generating binaries 1.15-1.22 and ran go test and it passed all tests in my terminal |
objfile/layouts.go
Outdated
|
|
||
| // FieldInfo describes a single field's location and type in a binary structure | ||
| type FieldInfo struct { | ||
| Name string // Field name (e.g., "Text", "Types") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the name and type an enumeration instead of strings. This should be a bit faster and add more structure
|
The large switch on Interface runtime version in |
…adata with enums; generic parsers updated; tests adjusted; add InterfaceLayout scaffolding
|
This looks great to me, thanks for cleaning this up! It passes all my unit tests as well as my manual testing. |

Summary
This PR refactors moduledata parsing for Go versions 1.16-1.24 to eliminate repetitive version-specific switch statements by using a data-driven approach with field offset tables.
Problem
The current codebase has ~430 lines of duplicate code for parsing moduledata across different Go versions. Each version requires nearly identical parsing logic, making the code difficult to maintain and extend.
Before this PR:
Solution
Implemented a generic field offset layout system that:
After this PR:
Code reduction: 85% (430 lines → 62 lines)
Changes
New Files
objfile/layouts.go (394 lines)
FieldInfoandModuleDataLayoutstructs for defining binary layoutsparseModuleDataGeneric()- version-agnostic parsing functionvalidateAndConvertModuleData()- validation for Go 1.18+validateAndConvertModuleData_116()- validation for Go 1.16-1.17readPointer(),readSlice(),getFieldOffset()objfile/layouts_test.go (380 lines)
TestLayoutOffsets_Match_StructDefinitions- Verifies layout offsets match actual structsTestParseModuleDataGeneric_BackwardCompatibility- Ensures identical outputTestVersionMapping- Tests version alias handlingTestReadPointer- Tests pointer reading with different endiannessTestReadSlice- Tests Go slice structure parsingTestModuleDataIntermediate_FieldTypes- Verifies struct typesModified Files
objfile/objfile.go
Testing
✅ All tests passing
Unit Tests
Integration Testing
Test Coverage
Extensibility
Adding Go 1.25 support:
Future Work
This refactoring establishes a pattern that can be extended to:
Estimated additional savings: ~500-600 more lines could be eliminated by applying this pattern to type parsing.