1

fix: Prevent stack overflow in recursive const types by 6d7a · Pull Request #169...

 1 month ago
source link: https://github.com/rust-lang/rust-analyzer/pull/16915
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

fix: Prevent stack overflow in recursive const types #16915

Merged

bors merged 2 commits into

rust-lang:master

from

6d7a:master

Mar 24, 2024

Conversation

Contributor

In the evaluation of const values of recursive types certain declarations could cause an endless call-loop within the interpreter (hir-ty’s create_memory_map), which would lead to a stack overflow.
This commit adds a check that prevents values that contain an address in their value (such as TyKind::Ref) from being allocated at the address they contain.
The commit also adds a test for this edge case.

rustbot

added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label

Mar 21, 2024

Member

@6d7a Thanks for the PR! I didn't understand how your patch addresses the problem. If I understand correctly, the problem is that TAG_TREE and VARIANT_TAG_TREE will get same address which will break things (here, and probably in other places too) but I'm not sure how this PR prevents that and how well it generalize to other cases.

In addition to that bug which we need to fix, infinite constants might exist, so we might need to add a depth limit or something like that to the create_memory_map function.

Contributor

Author

@HKalbasi thank you for your review. My reasoning in case of the fix was to prevent that reference values point to the location that they themselves reside in. However I can see that this is more of a hotfix for my particular case instead of a sustainable solution. Too much wishful thinking on my part.
I have removed the fix and added a stack depth check by propagating the Evaluator's stack depth limit along the call chain of create_memory_map. That prevents a terminal stack overflow, which makes rust-analyzer impossible to use in certain projects, and instead returns a soft internal error. I adapted the test accordingly.
I'd be interested in finding a way to better handle recursive const cases, though, but I guess that's for another PR.

Member

Thanks!
@bors r+

I'd be interested in finding a way to better handle recursive const cases, though, but I guess that's for another PR.

AFAIK there is no way to create infinite constants in stable Rust, so this is good for now.

Though we should find out why it is allocating the same address for different constants, as it can cause problems even without recursive types.

6d7a reacted with thumbs up emoji

Collaborator

📌 Commit 142ef76 has been approved by HKalbasi

It is now in the queue for this repository.

Collaborator

⌛ Testing commit 142ef76 with merge 3dfd4c1...

Collaborator

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 3dfd4c1 to master...

bors

merged commit 3dfd4c1 into

rust-lang:master

Mar 24, 2024

11 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

HKalbasi

Awaiting requested review from HKalbasi
Assignees

No one assigned

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

None yet

4 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK