fix: clarify signed & unsigned for jump insts

[hsqStephenZhang]

content

fix the signed/unsigned comparisons in several jump instructions in the execute_program function

Key changes include:

• Added a new macro unsigned_u64! to convert immediate values to unsigned 64-bit integers (i can be simply written as inst.imm as u32 as u64, the macro is just for better readability, i can change it back to the simpler version if you wish)
• Updated the implementation of several jump instructions (JEQ_IMM, JGT_IMM, JGE_IMM, JLT_IMM, JLE_IMM, JNE_IMM) to use the new unsigned_u64! macro, ensuring correct handling of unsigned comparisons.

reference

• https://docs.kernel.org/bpf/standardization/instruction-set.html#jump-instructions

[qmonnet]

Thank you for this! Do you think you could add some corresponding non-regression tests, please?

Can you also please sign-off your commit?

[hsqStephenZhang]

sure

[hsqStephenZhang]

sorry, i might have made a mistake: the original cause of my problem is not a bug cause by signed extension of the i32 imm.

This pr was inspired by the note of "TODO: check this actually works as expected for signed / unsigned ops" in the code after i read the source code. Actually, the verifier can handle the situation: if the highest bit of i32 imm is 1, then the verifier will throw an error. May us still extend the imm as an unsigned 32 bit integer for better readability(or for removal of this todo comment)? If you think it's unnecessary, then this PR can be closed.

Sorry for the in convenience.

[qmonnet]

I don't mind fixing the code itself, we should do the proper cast even if the verifier may catch the mistake (in particular, because users could chose to use a different verifier that might not catch it).

[hsqStephenZhang]

hi, i've added some test. the test shall not pass before the first commit(unsigned extend), but it's working as expected now.

[qmonnet]

Thanks! Yes I saw you updated the PR, sorry I need a bit of time to look into this properly. I'll try to find a moment during the week.

[hsqStephenZhang]

https://elixir.bootlin.com/linux/v6.12.6/source/kernel/bpf/core.c#L2084

here is kernel's implementation of the interpreter, the 's' means signed, 'u' means unsigned. kernel's impl shows the solidity of rbpf's original design: the verifier will check if the immediate's range is valid, the interpreter will just convert i32 into u64 without considering the highest bit