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

unbounded heap growth crashes xst, xsnap (SIGSEGV) #567

Closed
dckc opened this issue Feb 19, 2021 · 6 comments
Closed

unbounded heap growth crashes xst, xsnap (SIGSEGV) #567

dckc opened this issue Feb 19, 2021 · 6 comments
Labels
confirmed issue reported has been reproduced fixed - please verify Issue has been fixed. Please verify and close.

Comments

@dckc
Copy link
Contributor

dckc commented Feb 19, 2021

Steps to Reproduce

  1. xst grow.js where grow.js is given below
    • whether release or debug build
  2. SIGSEGV
let stuff = '1234';
for (;;) {
  stuff += stuff;
}

It also crashes xsnap, debug and release.

Build environment: Linux
Target device: xst

cc @warner

somewhat related: Agoric/agoric-sdk#2276 , Agoric/agoric-sdk#2224

@phoddie
Copy link
Collaborator

phoddie commented Feb 20, 2021

Hmmm... yes, that happens. I know it used to work correctly. Interesting, the embedded targets seem fine. On an ESP32 for example, there's no crash: it lands in fxAbort with an error value of XS_NOT_ENOUGH_MEMORY_EXIT. So, strictly speaking, this looks like a bug in port to certain targets rather than a bug in XS itself.
image

@phoddie phoddie added the confirmed issue reported has been reproduced label Feb 20, 2021
@dckc
Copy link
Contributor Author

dckc commented Feb 23, 2021

yay! seems to be fixed in c22a50b.

@dckc dckc closed this as completed Feb 23, 2021
@phoddie
Copy link
Collaborator

phoddie commented Feb 23, 2021

It turns out to be a more complex issue than it first appeared. On systems with limited memory, it isn't a practical concern. On systems with lots of memory (gigabytes) it can be.

We put in a workaround for the problem you reported. It isn't a proper fix for all situations. We should come back to this at some point.

@dckc
Copy link
Contributor Author

dckc commented Feb 23, 2021

yeah... I saw txSize is 4 bytes... I can see how that could run into overflow.

I do prefer to track this until a proper fix is available. Memory safety is job 1!

@dckc dckc reopened this Feb 23, 2021
mkellner pushed a commit that referenced this issue Mar 15, 2021
mkellner pushed a commit that referenced this issue Mar 15, 2021
@phoddie phoddie added the fixed - please verify Issue has been fixed. Please verify and close. label Mar 23, 2021
@dckc
Copy link
Contributor Author

dckc commented Apr 9, 2021

I'm not sure what to verify; the symptoms I was seeing were fixed Feb 22 by c22a50b.

I don't have any way to provoke the more general problem handy.

@phoddie
Copy link
Collaborator

phoddie commented Apr 9, 2021

As the problems you could reproduce are resolved, this can be closed.

@phoddie phoddie closed this as completed Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed issue reported has been reproduced fixed - please verify Issue has been fixed. Please verify and close.
Projects
None yet
Development

No branches or pull requests

2 participants