-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
os2.process_start uses the global allocator instead of context.allocator
which can lead to a panic/failed assert
#4265
Comments
context.allocator
which can lead to a paniccontext.allocator
which can lead to a panic/failed assert
I'm not sure how The crash is happening on this line of code: Odin/core/os/os2/process_linux.odin Line 446 in 8814170
For now figuring out what exactly is broken, and whether it's broken at all. I thought that maybe my
Yes, it shouldn't.
This is also expected, because |
Running hypothesis is that |
I believe E: |
Thank you for the prompt response. However, one disadvantage to this approach is that if someone wants to prevent any allocations using the default allocators, meaning they want to control how every and any allocation is done (for example on a tiny ARM 32 device), then the current code paired with the option “-default-to-nil-allocator” is misleading, this goal won’t be achieved. Behind the scenes, the default allocators are still being used silently sometimes. Now I don’t know if this goal is supported by Odin, or if the docs should clarify what this command line option really does? |
Context
odin report
output:Expected Behavior
When compiling with
-default-to-nil-allocator
and settingcontext.allocator
andcontext.temp_allocator
to custom allocators, I expectos2.process_start
to use these. But it does not.Current Behavior
os2.process_start
accessestemp_allocator()
meaning the default global temp allocator instead of usingcontext.allocator
orcontext.temp_allocator
. With the build option-default-to-nil-allocator
, that leads eventually to a panic or failed assert.Additionally, that means that it is not visible from the signature of the function that it allocates and we cannot provide it a specific allocator just for one call e.g.
os.process_start(desc, allocator=foobar)
.Steps to Reproduce
Here is a program that sets
context.allocator
andcontext.temp_allocator
to fixed arenas and tries to spawn a process (does not matter which one) withos2.process_start
. It will end up in a failed assert because it tries to use the default general temp allocator to allocate some memory (to convert the executable name to acstring
). This general temp allocator cannot allocate any memory because its backing allocator (the general purpose default allocator) is nil.Failure Logs
Run:
GDB:
Suggestion: A
allocator
should be present as one of the arguments ofos2.process_start
and be used inside it.The text was updated successfully, but these errors were encountered: