oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73.49k stars 2.71k forks source link

FFIType.u32 misbehavior #7007

Open Merulast opened 10 months ago

Merulast commented 10 months ago

What version of Bun is running?

1.0.10

What platform is your computer?

Linux 4.19.289-1-MANJARO x86_64 unknown

What steps can reproduce the bug?

Topic FFI. We have an given C Struct Color, that has a size of 32Bit = 4byte. In my case it's on raylib.

typedef struct Color {
    unsigned char r;        // Color red value
    unsigned char g;        // Color green value
    unsigned char b;        // Color blue value
    unsigned char a;        // Color alpha value
} Color;

Some methods, for example void DrawRectangle(int posX, int posY, int width, int height, Color color); demands to get this struct passed by value. (Other functions using color as byValue parameter also had been tested with the same result)

FFI dlopen parameters: DrawRectangle: {args:[FFIType.i32, FFIType.i32, FFIType.i32, FFIType.i32, FFIType.u32]},

The Plan: bypass the 'issue' of not beeing able to pass structs byValue (at least I dont know any way) with passing an u32 value.

Function for preparing data:

function rgba(r,g,b,a) {
  const arr = new Uint8Array([a,b,g,r]);
  return new DataView(arr.buffer).getUint32(0);
}

What is the expected behavior?

var color = undefined;

//1. Draws invisible magenta rect
color = rgba(255, 0, 255, 0);
DrawRectangle(100,100, 100, 100, color);
console.log(color.toString(16)); // = 00FF00FF

//2. Draws 50% transparent magenta rect
color = rgba(255, 0, 255, 127);
DrawRectangle(100,100, 100, 100, color);
console.log(color.toString(16)); // = 7FFF00FF

//3. Draws magenta rect 
color = rgba(255, 0, 255, 255);
DrawRectangle(100,100, 100, 100, color);
console.log(color.toString(16)); // = FFFF00FF

What do you see instead?

In case 3 we do se an about 25% visible, blue square. image

This is archived as soon as the most significant bit of alpha, which is also the most significant bit of the whole u32, gets set. So in a range of 128-255.

In that case also the function of all the other bytes becomes random from 'does nothing at all' up to 'gets black or blue'. As Example: An entire solid blue can be archived by using 255, 255, 0, 255 - thats make no sense at all.

Additional information

Even thought the console outputs show the expected values, I also computed them manually using bitmask. The outcome stay the same.

Since the kinda random, but deterministic behavior starts with the most significant bit, I could just guess that even thought u32 is used, at some point an signed value is beeing expected.

It would be way better to not have to use u32 at all, to just pass an buffer as argument in the first place. But so far only pointers seems to be supported.

Im new to bun. So I dont know where in the sourcecode this could be adressed. But I would like to look deeper into this, if someone could give me the right place to look at.

paperdave commented 10 months ago

Can observe incorrect values when uint32_t is used as the type, only when they cross the bounds of i32

image image

When I adjust to int32_t but keep the api as uint, the issue is gone.

image

When dumping the generated c binding, i spot something bad

image

I took some more time to look through and i have found a very interesting fix, will document it soon, it's quite complicated to explain.

Merulast commented 10 months ago

Just implemeted a test c-lib that litherally does nothing but looking deeper in this thing. Cant wait for more infos. Amazing that you looked into it :)

Edit: Can confirm, with int32_t its gone.

paperdave commented 10 months ago

seems the test cases i add fail in some cases, so this is not fully resolved yet.