Memory Access Crashes in Playdate Game Developed with Zig

Help
,
1

I’ve been working on Playdate SDK for Zig based off of GitHub - DanB91/Zig-Playdate-Template: Starter code for a Playdate program written in Zig

I’ll preface this by saying that DanB91 has done all the preliminary work to get Zig running on the Playdate. Cross compiling for novel targets isn’t my area of expertise.

I’ve been running into some memory allocation issues and weird panics when running in the Playdate Simulator. For example, a call to math.Vector2.toVector2i() causes a Zig panic in @intFromFloat.

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	       0x18b58311c __pthread_kill + 8
1   libsystem_pthread.dylib       	       0x18b5bacc0 pthread_kill + 288
2   libsystem_c.dylib             	       0x18b4caa50 abort + 180
3   pdex.dylib                    	       0x118850c90 posix.abort + 12 (posix.zig:689)
4   pdex.dylib                    	       0x118848964 debug.panicImpl + 148 (debug.zig:501)
5   pdex.dylib                    	       0x118844914 builtin.default_panic + 104 (builtin.zig:845)
6   pdex.dylib                    	       0x118853ea8 math.Vector2.toVector2i + 252 (math.zig:44)
7   pdex.dylib                    	       0x11884e698 main.drawLines + 112 (main.zig:190)
8   pdex.dylib                    	       0x11884e8b4 main.drawAsteroid + 412 (main.zig:302)
9   pdex.dylib                    	       0x11884efa0 main.render + 1068 (main.zig:712)
10  pdex.dylib                    	       0x1188468e0 main.update_and_render + 284 (main.zig:870)
11  Playdate Simulator            	       0x104523f4c pd_update + 504
12  Playdate Simulator            	       0x104577450 -[PCPlaydateSimulator update] + 508
13  Foundation                    	       0x18c7bbfe8 __NSThreadPerformPerform + 264
14  CoreFoundation                	       0x18b697d28 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 28
15  CoreFoundation                	       0x18b697cbc __CFRunLoopDoSource0 + 176
16  CoreFoundation                	       0x18b697a2c __CFRunLoopDoSources0 + 244
17  CoreFoundation                	       0x18b69661c __CFRunLoopRun + 828
18  CoreFoundation                	       0x18b695c2c CFRunLoopRunSpecific + 608
19  HIToolbox                     	       0x195bee448 RunCurrentEventLoopInMode + 292
20  HIToolbox                     	       0x195bee284 ReceiveNextEventCommon + 648
21  HIToolbox                     	       0x195bedfdc _BlockUntilNextEventMatchingListInModeWithFilter + 76
22  AppKit                        	       0x18ee70f90 _DPSNextEvent + 660
23  AppKit                        	       0x18f644b94 -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 716
24  AppKit                        	       0x18ee6443c -[NSApplication run] + 476
25  AppKit                        	       0x18ee3b708 NSApplicationMain + 880
26  dyld                          	       0x18b241058 start + 2224

Here is the function in question:

    pub fn toVector2i(self: @This()) Vector2i {
        const x = @round(self.x);
        const y = @round(self.y);
        const xi: i32 = @intFromFloat(x);
        const yi: i32 = @intFromFloat(y);
        return Vector2i.init(xi, yi);
    }

Secondly, I’ve been trying to implement a std.mem.Allocator using the playdate.system.realloc function. Allocating seems to work, but resizing and freeing just crashes the simulator.

My naive attempt at an allocator can be found here: Zig-Playdate-SDK/src/sdk/mem.zig at main · deevus/Zig-Playdate-SDK · GitHub

SDK: GitHub - deevus/Zig-Playdate-SDK
Game: GitHub - deevus/zigsteroids-playdate: asteroids in zig

1 Like
2

I’m just looking at your allocator implementation - I haven’t seen your other code, but it looks like you have a bug in your resize function:

    fn resize(
        ctx: *anyopaque,
        buf: []u8,
        _: u8,
        new_size: usize,
        _: usize,
    ) bool {
        const self: *PlaydateAllocator = @alignCast(@ptrCast(ctx));

        _ = self.api.system.realloc(buf.ptr, new_size);
        // It's assumed that the consumer of this API will handle the returned pointer correctly.
        return true; // Return true on successful resize.
    }

You’re always returning true. What if it didn’t resize and you asked for a larger one… consider the following from the standard library:

       pub fn addManyAt(self: *Self, index: usize, count: usize) Allocator.Error![]T {
            const new_len = try addOrOom(self.items.len, count);

            if (self.capacity >= new_len)
                return addManyAtAssumeCapacity(self, index, count);

            // Here we avoid copying allocated but unused bytes by
            // attempting a resize in place, and falling back to allocating
            // a new buffer and doing our own copy. With a realloc() call,
            // the allocator implementation would pointlessly copy our
            // extra capacity.
            const new_capacity = growCapacity(self.capacity, new_len);
            const old_memory = self.allocatedSlice();
            if (self.allocator.resize(old_memory, new_capacity)) {
                self.capacity = new_capacity;
                return addManyAtAssumeCapacity(self, index, count);
            }

            // Make a new allocation, avoiding `ensureTotalCapacity` in order
            // to avoid extra memory copies.
            const new_memory = try self.allocator.alignedAlloc(T, alignment, new_capacity);
            const to_move = self.items[index..];

It has to check if it has the capacity. So it tries a resize and if that fails, it needs to allocate a larger size. In your case, if you always return true, it’s going to look like a resize happened even if it didn’t and then you may try to read/write to something out of bounds.

I’m assuming that the realloc function attempts to handle this under the hood? If it fails at resizing, does it try to assign back to the pointer that you’ve provided? I’m not sure how that would be the case though, because it looks like you’re passing that pointer by value so the new address would only get assigned to a temporary.

1 Like
3

This probably comes down to my lack of knowledge around how memory allocators generally work internally.

I’m not sure how to check if it resized or not. It doesn’t say much in the docs

I suppose if it resized, the pointers would not be the same?

My current implementation is to just use Allocator.noResize

4

I’m going to point you to this awesome post by @squeek502: Is it Ok to use std.heap.page_allocator directly? - #7 by squeek502

To answer your question, resize does not require a new pointer. It asks for a bigger chunk of memory in place. Let’s go back to that code example I posted:

            if (self.allocator.resize(old_memory, new_capacity)) {
                self.capacity = new_capacity;
                return addManyAtAssumeCapacity(self, index, count);
            }

So if resize works, it does not reassign the pointer - it just increases the self.capacity meaning that the array list can take in more items with its current allocation position. In the second half, you’ll see this:

            const new_memory = try self.allocator.alignedAlloc(T, alignment, new_capacity);
            // removed for sake of example...
            self.allocator.free(old_memory);
            self.items = new_memory[0..new_len];
            self.capacity = new_memory.len;

So it gets new memory, and then overwrites it’s current items member (which is the slice of used items) and also assigns its extra capacity.

If you fail to resize then you need a new pointer. In that case, you need to return false because it failed, signalling you to do something else to get the memory you need.

5

I’m assuming here that realloc in their case is actually a version of the C realloc: Use of realloc() - GeeksforGeeks

In this case my friend, you’re kind of in a bad spot. It looks like it is… their docs say: Allocates heap space if *ptr* is NULL, else reallocates the given pointer. If *size* is zero, frees the given pointer.

Here’s a comment from a libc implementation of realloc (I’d post the whole thing, but you only need to read the first two chunks of text).

/*
  realloc(void* p, size_t n)
  Returns a pointer to a chunk of size n that contains the same data
  as does chunk p up to the minimum of (n, p's size) bytes, or null
  if no space is available.

  The returned pointer may or may not be the same as p. The algorithm
  prefers extending p when possible, otherwise it employs the
  equivalent of a malloc-copy-free sequence.

  ...

The second part is the most important: The returned pointer may or may not be the same as p.

In other words, if it needs to allocate new space, it returns a pointer to that new space. In your case, you’re tossing that pointer away and moving forward with the old pointer to now invalid memory. This api mostly likely will not work with the Allocator interface. Resize only returns a bool and you have no way to get that new pointer back out.

In both cases (returning true or false) you’re stuck.If you return true you’ll never get back your allocation if it changes. In the case you decide to bypass it and return false, the realloc function can still try to free the memory. Then, you’ll get a double free in the case where it thought you still needed to free the old memory or it’s actually the same pointer and you’ll free that one instead.

I’d have to really think hard about how to work this into your current situation and make it work with standard utilities but you’re going to have bugs galore going down this road. I strongly suggest thinking about handling this another way.

If you’re linking to c… why not just use the std.heap.c_allocator instead? It’s all setup to do what you need if you’re working with Zig. I don’t know much about this framework you’re using, so maybe that isn’t an option.

2 Likes
6

I’m not sure if it’s possible to link libc. I’ll have to do some research.

7

Confirmed that it is not possible to link to libc with the playdate. It is used in the simulator only.

8

@intFromFloat invokes safety-checked UB and panics when the value is NaN or is outside the bounds of values representable by the destination integer type (± 2 billion for i32). I tried briefly copy/pasting your Vector2/Vector2i definitions and running them on the device and simulator (Windows) and they work fine as long as the values are within range of i32, so I suspect the problem is that the magnitude of the values is too large.

You want to take a look at how std.heap.raw_c_allocator is designed. I have my own separate (very WIP) Playdate bindings/SDK package and I implemented my allocator like this:

There are some differences because my allocator is global and does not have a ctx, but the general logic should be the same.

(My package also has a custom panic handler that works on device, if you want to take inspiration from that as well.)

3 Likes
9

Thank you. I’ll have to try to debug why some calls to @intFromFloat are going out of bounds.

On Windows at least, I’m having problems getting any stack traces from the simulator if it crashes. It just immediately implodes and the window closes.

Also I tried your allocator and I can also get it to crash the simulator in zigsteroids. Again, I’m not getting any error logs on Windows. Any ideas?

10

I think you simply have UB in your program.

At first sight it seems that in this section you have a callback that creates an area allocator as a local variable and then you create arraylists that will hold pointers to it for longer than the lifetime of the function itself.

Either this or some other location where something similar happens is probably the main cause of your crashes.

2 Likes
11

Yep you were right about that. That fixed the crashes related to the allocator.

This probably comes down to my inexperience with systems programming.

12

we all go through it in the beginning.

enjoy your journey :^)

3 Likes