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

Fix infinite recursion for Debug::fmt #609

Closed
wants to merge 1 commit into from
Closed

Fix infinite recursion for Debug::fmt #609

wants to merge 1 commit into from

Conversation

jcdickinson
Copy link
Contributor

This Pull Request fixes/closes #608.

It changes the following:

  • Adds a test for stack overflow
  • Removes prototype from the Debug::fmt for Object

@codecov
Copy link

codecov bot commented Aug 7, 2020

Codecov Report

Merging #609 into master will increase coverage by 0.01%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #609      +/-   ##
==========================================
+ Coverage   72.51%   72.53%   +0.01%     
==========================================
  Files         179      179              
  Lines       12553    12565      +12     
==========================================
+ Hits         9103     9114      +11     
- Misses       3450     3451       +1     
Impacted Files Coverage Δ
boa/src/builtins/object/mod.rs 40.26% <85.71%> (+1.45%) ⬆️
boa/src/builtins/value/tests.rs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fd5533...1bec3c9. Read the comment docs.

Comment on lines +71 to +72
.field("properties", &self.properties)
.field("symbol_properties", &self.symbol_properties)
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't any of these overflow for example:

let x = {};
x.itSelf = x;

the itSelf property is contained in properties and this would cause a stack overflow, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prototype always overflows, I thought it valuable to get a fix in for that (because I suspect it is breaking the test262 PR) and then figure out how to approach this in a general way after some discussion.

@jcdickinson jcdickinson deleted the stackoverflow branch August 11, 2020 16:52
@Razican
Copy link
Member

Razican commented Aug 12, 2020

Why was this closed, @jcdickinson? I would be happy to have an patial fix for stack overflows.

@jcdickinson
Copy link
Contributor Author

@Razican I'll do a proper fix.

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.

Debug::fmt Causes Causes a Stack Overflow
3 participants