Skip to content

Integer overflow in function IDs #223

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

Open
joeltg opened this issue Mar 20, 2025 · 1 comment
Open

Integer overflow in function IDs #223

joeltg opened this issue Mar 20, 2025 · 1 comment

Comments

@joeltg
Copy link

joeltg commented Mar 20, 2025

After running a program in QuickJS for a long time, eventually it crashes with this error: QuickJSContext had no callback with id -32768.

Image

-32768 is the minimum int16 value. Looking at this line in context.ts, we see that this error is thrown when this.getFunction(fn_id) returns undefined. Function ids are set by incrementing protected fnNextId: number, starting at -32768.

It seems that after 2^16 function callbacks have been set, the next function ID is 2^16 = 32768, which overflows the int16 used to represent them in C, and gets returned back to JavaScript as -32768. The correct behavior is for context.ts to wrap around to -32768 when allocating the next function id in this line.

  newFunction(name: string, fn: VmFunctionImplementation<QuickJSHandle>): QuickJSHandle {
    if (this.fnNextId >= (1 << 15)) {
       this.fnNextId = -(1 << 15)
    }
    const fnId = this.fnNextId++ // (is 0 a valid fnId?)
    this.setFunction(fnId, fn)
    return this.memory.heapValueHandle(this.ffi.QTS_NewFunction(this.ctx.value, fnId, name))
  }

Note that I wasn't creating 2^16 functions simultaneously - just cumulative over history. In fact I wasn't actually creating new functions at all, just awaiting promises, which use newFunction internally.

@joeltg
Copy link
Author

joeltg commented Mar 20, 2025

Also ideally we would be able to delete functions from the map when they get disposed... I'm not sure if that's easy, difficult, or impossible.

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

No branches or pull requests

1 participant