Skip to content

Comments

asm: allow negative constants for builtin function calls#1806

Merged
lmb merged 1 commit intocilium:mainfrom
lmb:asm-call-neg-one
Jun 24, 2025
Merged

asm: allow negative constants for builtin function calls#1806
lmb merged 1 commit intocilium:mainfrom
lmb:asm-call-neg-one

Conversation

@lmb
Copy link
Contributor

@lmb lmb commented Jun 19, 2025

The work to encode a platform into various constant types made it so that decoding a call to a builtin helper with a negative value fails with

decoding instructions for section <sectionname>:
offset <offset>: invalid constant 0xffffffff

for a BPF instruction of "call -1". This is because we can't represent -1 as a tagged platform constant.

Allow negative constants by not transforming them into a platform constant at all. Adjust the platform tag size so that we never generate a platform constant with the high bit set. This avoids confusing it with a negative number when reinterpreting it as a signed number and ensures that trying to marshal such an instruction gives an error.

Fixes: #1797

@lmb
Copy link
Contributor Author

lmb commented Jun 19, 2025

@dylandreimerink @brycekahle PTAL

@lmb lmb marked this pull request as ready for review June 19, 2025 15:35
@lmb lmb requested a review from a team as a code owner June 19, 2025 15:35
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an easier fix than I thought. The builtin function call numbers can never get that high anyway.

Seems like we do have to adjust TestConstant so it iterates only over 0 to int32_max instead of 0 to uint32_max

The work to encode a platform into various constant types made it
so that decoding a call to a builtin helper with a negative value
fails with

    decoding instructions for section <sectionname>:
    offset <offset>: invalid constant 0xffffffff

for a BPF instruction of "call -1". This is because we can't
represent -1 as a tagged platform constant.

Allow negative constants by not transforming them into a platform
constant at all. Adjust the platform tag size so that we never
generate a platform constant with the high bit set. This avoids
confusing it with a negative number when reinterpreting it as a
signed number and ensures that trying to marshal such an
instruction gives an error.

Fixes: cilium#1797

Signed-off-by: Lorenz Bauer <[email protected]>
@lmb lmb force-pushed the asm-call-neg-one branch from 9859595 to bd1687f Compare June 24, 2025 08:17
@lmb lmb merged commit dab673c into cilium:main Jun 24, 2025
32 of 33 checks passed
@lmb lmb deleted the asm-call-neg-one branch June 24, 2025 11:03
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

Successfully merging this pull request may close these issues.

call -1 instructions no longer load without error

3 participants