feat(EVM-1189): [AI] remove machine default start trap#541
feat(EVM-1189): [AI] remove machine default start trap#5410xVolosnikov wants to merge 1 commit intodevfrom
Conversation
0xVolosnikov
changed the title
Benchmark report
|
There was a problem hiding this comment.
Pull request overview
This PR cleans up low-level trap entry wiring in zksync_os by eliminating the legacy machine_default_start_trap symbol and making _machine_start_trap the explicit machine trap entry point.
Changes:
- Rename the assembly trap entry label from
machine_default_start_trapto_machine_start_trap. - Remove the linker-script
PROVIDEalias that previously mapped_machine_start_traptomachine_default_start_trap. - Remove an obsolete TODO related to the legacy trap entry point.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
zksync_os/src/main.rs |
Removes an obsolete TODO comment about the legacy trap entry symbol. |
zksync_os/src/lds/link.x |
Drops the legacy PROVIDE(_machine_start_trap = ...) alias. |
zksync_os/src/asm/asm_reduced.S |
Renames the exported trap entry symbol to _machine_start_trap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* # Start trap function override | ||
| By default uses the riscv crates default trap handler | ||
| but by providing the `_start_trap` symbol external crates can override. | ||
| */ | ||
| PROVIDE(_start_trap = default_start_trap); |
There was a problem hiding this comment.
Removing PROVIDE(_machine_start_trap = ...) changes _machine_start_trap from a weak/defaulted symbol into a required strong definition (now coming from asm_reduced.S). If downstream code previously overrode _machine_start_trap by defining it elsewhere, this will start failing with duplicate symbol errors. If override-ability is still desired, consider keeping a weak default (e.g., via PROVIDE or making the asm definition weak) or explicitly documenting that overrides are no longer supported.
| */ | ||
| .section .trap, "ax" | ||
| .global machine_default_start_trap | ||
| .global _machine_start_trap |
There was a problem hiding this comment.
Defining _machine_start_trap as a global (strong) symbol means it can no longer be overridden by another object file without causing a duplicate symbol link error. If the previous PROVIDE(_machine_start_trap = machine_default_start_trap) behavior was intentionally allowing overrides, consider marking this symbol weak (or restoring a weak alias pattern) to preserve that behavior while keeping the new name.
| .global _machine_start_trap | |
| .weak _machine_start_trap |
0xVolosnikov
changed the title
What ❔
_machine_start_trap.Why ❔
Is this a breaking change?
Checklist