Skip to content
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

Use less stack intensive error mapping #2313

Merged
merged 5 commits into from Dec 14, 2022
Merged

Conversation

Henry-E
Copy link
Contributor

@Henry-E Henry-E commented Dec 14, 2022

closes: #2070

@vercel
Copy link

vercel bot commented Dec 14, 2022

Someone is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@andreihrs
Copy link
Contributor

lgtm! I reproduced this and seems like these changes fix the #2070 issue.

I was using https://github.com/andreihrs/anchor-error-reprod with all the accounts uncommented in OracleMappings struct. When I was running "cargo test-bpf", I was getting Program failed to complete: Access violation in stack frame 5 at address 0x200005ea0 of size 8 by instruction #2425 from anchor 0.25

With your latest fix, the tests pass properly. Thanks for this!

Maybe we can introduce a similarly sized account in tests to make sure other don't run into this issue again.

Much appreciated!

@Henry-E
Copy link
Contributor Author

Henry-E commented Dec 14, 2022

Great, I'll have to see if we can introduce downstream tests or include this one perhaps if it doesn't have too many other dependencies. Another more general solution is really just to be more conscious of heap usage in anchor, e.g. map_err is not good for heap usage because it automatically adds the error struct to the heap even if nothing is called. Or something roughly like that. It's similar to error_or_else where you want it to error lazily.

@Henry-E Henry-E merged commit f79f9da into coral-xyz:master Dec 14, 2022
@Henry-E Henry-E deleted the less-heap-err branch December 14, 2022 17:04
@Henry-E Henry-E changed the title Use less heap intensive error mapping Use less stack intensive error mapping Dec 20, 2022
@snawaz
Copy link
Contributor

snawaz commented Feb 10, 2023

I'm using anchor 0.26.0 which is supposed to include the fix in this PR (?). However, cargo build-sbf still gives me similar error messages:

Error: Function ZN14solana_program4vote5state9VoteState11deserialize17hc24c28c4eff79cffE Stack offset of 6344 exceeded max offset of 4096 by 2248 bytes, please minimize large stack variables
Error: Function ZN229$LT$solana_program..vote..state..vote_state_0_23_5..
..$LT$impl$u20$serde..de..Deserialize$u20$for$u20$solana_program..vote..state..vote_state_0_23_5..VoteState0_23_5$GT$..deserialize..__Visitor$u20$as$u20$serde..de..Visitor$GT$9visit_seq17h90250e16ba5fe33dE Stack offset of 5752 exceeded max offset of 4096 by 1656 bytes, please minimize large stack variables
Compiling voting-app v0.1.0 (/Users/snawaz/projects/voting-app/programs/voting-app)
Finished release [optimized] target(s) in 2.19s

Implies, this PR doesn't fix in all places?

@Henry-E
Copy link
Contributor Author

Henry-E commented Feb 10, 2023

This PR was fixing a stack error that was showing up for a limited range of programs for a very specific reason, it was introduced by a change made to anchor about showing errors.

No matter what anchor does as a dev you will always be able to exceed the stack. It's a memory buffer allowed to you by the solana runtime. If you're getting a stack error then you should follow the instructions and try to box accounts which will put them on the heap.

However, if there's a specific part of anchor you think might be causing this, like if there's some function or variables that are unnecessarily using up memory, then we can definitely look at fixing that. You would need to help us in tracking down / narrowing where the excessive memory usage was coming from in the anchor lang/ directory.

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.

Access violation when upgrading to anchor 0.25 on account init
3 participants