Assemblyscript: WASI: Invalid clock event structure size

[torch2424]

cc @dcodeIO @MaxGraey

I brought this up a while ago, but just haven't had the time to open an issue with an isolated use case and all that. But I finally did! :smile:

So this is going back to the as-wasi sleep function I implemented a while back. In the meantime I just used the stub runtime to get around allocating / freeing memory properly, but this would cause a memory leak that we do not want to have.

I went ahead and created two isolated test cases:

1. An implementation using new ArrayBuffer. And has the error (at runtime): ~lib/rt/pure.ts:136:15: error: null
   

2. An implementation using __alloc / __free. And has the error (at runtime): ~lib/rt/tlsf.ts:279:13: error: null
   

Let me know if this is enough info! Thanks! :smile: :+1:

[dcodeIO]

From the examples my first guess would be that one of the structures given as pointers to poll_oneoff might be too small, in turn leading to the logic behind it writing too far into memory, leading to some sort of memory corruption, like overwriting a managed header and thus corrupting either memory manager or gc state.

Also, even though most likely not the cause here, the examples make use of both managed and unmanaged structures, like subscription_clock and event are unmanaged classes that are not automatically garbage collected (and need to be __freed), while ArrayBuffer is managed and dealt with automatically.

One way to debug this can be to increase the sizes (X, Y, Z) of the structures manually and see if it works with larger ones, which would indicate that either our bindings are missing something (subscription_clock, event) or our assumption on the size of neventsBuffer is wrong.

  static sleep(nanoseconds: i32): void {
    let clockSub = changetype<subscription_clock>(__alloc(offsetof<subscription_clock>() + X, 0));
    clockSub.userdata = 0;
    clockSub.clock_id = clockid.REALTIME;
    clockSub.timeout = nanoseconds;
    clockSub.precision = 10000;
    clockSub.type = eventtype.CLOCK;
    clockSub.flags = 0;

    let clockEvent = changetype<event>(__alloc(offsetof<event>() + Y, 0));

    let neventsBuffer: i32 = __alloc(4 + Z, 0);

    poll_oneoff(
      changetype<usize>(clockSub),
      changetype<usize>(clockEvent),
      1,
      changetype<usize>(neventsBuffer)
    );

    __free(neventsBuffer);
    __free(changetype<usize>(clockEvent));
    __free(changetype<usize>(clockSub));

Let me know :)

[MaxGraey]

Also you could try avoid allocations inside sleep function` by reusing static memory like here: https://github.com/jedisct1/as-wasi/pull/21/files#diff-5858db0a3a9b21c5aeba457de2d3b702R1011

[torch2424]

Dope I'll try these out @dcodeIO and @MaxGraey ! Thanks! :smile:

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[torch2424]

@dcodeIO I was able to fix this issue by implementing your suggested changes! :smile:

See this PR: https://github.com/jedisct1/as-wasi/pull/33

And doing your example code directly with X, Y, Z values: https://github.com/jedisct1/as-wasi/blob/2fe796db6ef02caee4997e533d626ce24ea822bb/assembly/as-wasi.ts#L1000

Seems like our clockevent must be incorrectly sized then on the Wasi side? :smile:

cc @MaxGraey

[dcodeIO]

Hmm, seems we are missing a field or two in one of the respective structures then. Or an entire structure for this particular kind of event. Or using the wrong structure. Must be somewhere to be found in the reference C implementation. I guess this can be sorted out by looking at some code doing the same in C and checking the structs used there versus our ones.

[torch2424]

@dcodeIO Yay! I helped haha :joy: Rad let me know if I can do anything more to help! Down to touch the compiler if I have to :smile:

[dcodeIO]

Looking into this again, from the WASI docs it seems that the event struct will always include the fd_readwrite data, which it states the clock event ignores (but iiuc still increases the size, probably setting that data to zeroes). This is currently modeled differently in our bindings as event_fd_readwrite extends event, and is likely the cause of the difference when just using event.

[dcodeIO]

Something similar might be the case for subscription, containing an u union as of the docs, indicating that the trailing data is always the same size, but depending on the type of the subscription is only partially used.

[dcodeIO]

WASI issue: https://github.com/WebAssembly/WASI/issues/266