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

Playdate memory allocator #59

Closed
Nycto opened this issue Mar 2, 2024 · 6 comments · Fixed by #60
Closed

Playdate memory allocator #59

Nycto opened this issue Mar 2, 2024 · 6 comments · Fixed by #60

Comments

@Nycto
Copy link
Collaborator

Nycto commented Mar 2, 2024

Hiya!

I've been working on debugging a device-only crash that I think is related to memory corruption. I started a thread about it on the playdate forum here:

https://devforum.play.date/t/device-crash-for-c-nim-based-game/16205

While investigating, I noticed that it doesn't appear that Nim is using the malloc implementation provided by the playdate SDK. I tried adding logging into the memory allocating methods in setup.c from the Playdate SDK and they were never being called. It look like Nim directly calls malloc from <stdlib.h>, as seen here:

https://github.com/nim-lang/Nim/blob/version-2-0/lib/system/mm/malloc.nim#L5

Does that sound right? Have you looked into overriding the default Nim malloc implementation to use playdate's realloc? Or have I got this all wrong?

@Nycto
Copy link
Collaborator Author

Nycto commented Mar 3, 2024

I started playing around with using the playdate realloc method for everything in Nim: Nycto@memory

I've got things working on the simulator, but everything immediately crashes when trying to use it on device. Feel free to poke at this and see if it makes sense.

@samdze
Copy link
Owner

samdze commented Mar 3, 2024

I thought the _malloc_r, _realloc_r and _free_r definitions in setup.c (SDK folder) were enough to override the default malloc strategy, but I'm not sure if this is really the case.

What makes you think Nim wasn't using the SDK provided functions?

@Nycto
Copy link
Collaborator Author

Nycto commented Mar 3, 2024

This might just be a simulator thing, actually. When I completely comment out all the memory allocation functions in setup.c, I'm still able to compile and run the simulator. But I just tried compiling for the device and it looks like it's failing appropriately

@samdze
Copy link
Owner

samdze commented Mar 3, 2024

Ok, yes seems like compiling for the simulator just uses the std functions no matter what.
Maybe we should also try reverting back to switch("os", "any") compiling for the simulator too.
I remember removing this setting time ago.

@Nycto
Copy link
Collaborator Author

Nycto commented Mar 5, 2024

I have to admit to being a bit stumped so far. I’ve tried various compiler and linker flags, but nothing seems to impact the simulator. If you get a chance to look at this, I would love a second set of eyes.

@Nycto
Copy link
Collaborator Author

Nycto commented Mar 5, 2024

Since I was stuck, I switched back to working on this branch: https://github.com/Nycto/playdate-nim/tree/memory

I got it mostly working, though I still have one workflow failure to investigate. I think this might be the right path forward as it’s the official Nim way to override malloc.

I’m going to do some testing this morning to see if it resolves my memory corruption issues

Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 10, 2024
Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 16, 2024
Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 16, 2024
Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 17, 2024
Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 17, 2024
Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 17, 2024
Nycto added a commit to Nycto/playdate-nim that referenced this issue Mar 17, 2024
samdze pushed a commit that referenced this issue Mar 17, 2024
ninovanhooff pushed a commit to ninovanhooff/playdate-nim that referenced this issue Apr 4, 2024
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 a pull request may close this issue.

2 participants