From f5810b1f7146890debad4e0733857080d659e5cf Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Tue, 7 May 2024 10:11:33 -0700 Subject: [PATCH 1/4] Add test for sign extension in unsigned jump Signed-off-by: Alan Jowett --- external/bpf_conformance | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/bpf_conformance b/external/bpf_conformance index 0eed408b1..3566334b7 160000 --- a/external/bpf_conformance +++ b/external/bpf_conformance @@ -1 +1 @@ -Subproject commit 0eed408b10ba22848886d31bfc2a77fa8adfc5ed +Subproject commit 3566334b7dd99c305eb45f161a40b94441451f4e From 93e06dfd21851db2a2dbf568816e740838749e47 Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Tue, 7 May 2024 11:19:12 -0700 Subject: [PATCH 2/4] Fix interpreter to match jit behavior Signed-off-by: Alan Jowett --- vm/ubpf_vm.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vm/ubpf_vm.c b/vm/ubpf_vm.c index 270ad73be..3b801392d 100644 --- a/vm/ubpf_vm.c +++ b/vm/ubpf_vm.c @@ -293,6 +293,7 @@ i32(uint64_t x) return x; } + #define IS_ALIGNED(x, a) (((uintptr_t)(x) & ((a)-1)) == 0) inline static uint64_t @@ -705,7 +706,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JGT_IMM: - if (reg[inst.dst] > u32(inst.imm)) { + if (reg[inst.dst] > inst.imm) { pc += inst.offset; } break; @@ -725,7 +726,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JGE_IMM: - if (reg[inst.dst] >= u32(inst.imm)) { + if (reg[inst.dst] >= inst.imm) { pc += inst.offset; } break; @@ -745,7 +746,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JLT_IMM: - if (reg[inst.dst] < u32(inst.imm)) { + if (reg[inst.dst] < inst.imm) { pc += inst.offset; } break; @@ -765,7 +766,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JLE_IMM: - if (reg[inst.dst] <= u32(inst.imm)) { + if (reg[inst.dst] <= inst.imm) { pc += inst.offset; } break; From 0dcf196c9edc3d95b5ebd80ba93d2c52d1cdcf20 Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Tue, 7 May 2024 11:32:58 -0700 Subject: [PATCH 3/4] Make sign extension explicit Signed-off-by: Alan Jowett --- vm/ubpf_vm.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/vm/ubpf_vm.c b/vm/ubpf_vm.c index 3b801392d..0aac1c9c3 100644 --- a/vm/ubpf_vm.c +++ b/vm/ubpf_vm.c @@ -293,6 +293,16 @@ i32(uint64_t x) return x; } +/** + * @brief Sign extend immediate value to a signed 64-bit value. + * + * @param[in] immediate The signed 32-bit immediate value to sign extend. + * @return The sign extended 64-bit value. + */ +static int64_t sign_extend_immediate(int32_t immediate) { + return (int64_t)immediate; + +} #define IS_ALIGNED(x, a) (((uintptr_t)(x) & ((a)-1)) == 0) @@ -686,7 +696,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret pc += inst.offset; break; case EBPF_OP_JEQ_IMM: - if (reg[inst.dst] == inst.imm) { + if (reg[inst.dst] == (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -706,7 +716,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JGT_IMM: - if (reg[inst.dst] > inst.imm) { + if (reg[inst.dst] > (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -726,7 +736,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JGE_IMM: - if (reg[inst.dst] >= inst.imm) { + if (reg[inst.dst] >= (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -746,7 +756,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JLT_IMM: - if (reg[inst.dst] < inst.imm) { + if (reg[inst.dst] < (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -766,7 +776,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JLE_IMM: - if (reg[inst.dst] <= inst.imm) { + if (reg[inst.dst] <= (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -786,7 +796,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSET_IMM: - if (reg[inst.dst] & inst.imm) { + if (reg[inst.dst] & (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -806,7 +816,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JNE_IMM: - if (reg[inst.dst] != inst.imm) { + if (reg[inst.dst] != (uint64_t)sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -826,7 +836,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSGT_IMM: - if ((int64_t)reg[inst.dst] > inst.imm) { + if ((int64_t)reg[inst.dst] > sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -846,7 +856,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSGE_IMM: - if ((int64_t)reg[inst.dst] >= inst.imm) { + if ((int64_t)reg[inst.dst] >= sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -866,7 +876,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSLT_IMM: - if ((int64_t)reg[inst.dst] < inst.imm) { + if ((int64_t)reg[inst.dst] < sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; @@ -886,7 +896,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSLE_IMM: - if ((int64_t)reg[inst.dst] <= inst.imm) { + if ((int64_t)reg[inst.dst] <= sign_extend_immediate(inst.imm)) { pc += inst.offset; } break; From 0854ecd70b6bf3be984674efde7b20013736a7f8 Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Tue, 7 May 2024 16:02:57 -0700 Subject: [PATCH 4/4] PR feedback Signed-off-by: Alan Jowett --- vm/ubpf_vm.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/vm/ubpf_vm.c b/vm/ubpf_vm.c index 0aac1c9c3..e2d71ae00 100644 --- a/vm/ubpf_vm.c +++ b/vm/ubpf_vm.c @@ -299,7 +299,7 @@ i32(uint64_t x) * @param[in] immediate The signed 32-bit immediate value to sign extend. * @return The sign extended 64-bit value. */ -static int64_t sign_extend_immediate(int32_t immediate) { +static int64_t i64(int32_t immediate) { return (int64_t)immediate; } @@ -696,7 +696,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret pc += inst.offset; break; case EBPF_OP_JEQ_IMM: - if (reg[inst.dst] == (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] == (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -716,7 +716,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JGT_IMM: - if (reg[inst.dst] > (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] > (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -736,7 +736,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JGE_IMM: - if (reg[inst.dst] >= (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] >= (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -756,7 +756,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JLT_IMM: - if (reg[inst.dst] < (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] < (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -776,7 +776,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JLE_IMM: - if (reg[inst.dst] <= (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] <= (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -796,7 +796,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSET_IMM: - if (reg[inst.dst] & (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] & (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -816,7 +816,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JNE_IMM: - if (reg[inst.dst] != (uint64_t)sign_extend_immediate(inst.imm)) { + if (reg[inst.dst] != (uint64_t)i64(inst.imm)) { pc += inst.offset; } break; @@ -836,7 +836,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSGT_IMM: - if ((int64_t)reg[inst.dst] > sign_extend_immediate(inst.imm)) { + if ((int64_t)reg[inst.dst] > i64(inst.imm)) { pc += inst.offset; } break; @@ -856,7 +856,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSGE_IMM: - if ((int64_t)reg[inst.dst] >= sign_extend_immediate(inst.imm)) { + if ((int64_t)reg[inst.dst] >= i64(inst.imm)) { pc += inst.offset; } break; @@ -876,7 +876,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSLT_IMM: - if ((int64_t)reg[inst.dst] < sign_extend_immediate(inst.imm)) { + if ((int64_t)reg[inst.dst] < i64(inst.imm)) { pc += inst.offset; } break; @@ -896,7 +896,7 @@ ubpf_exec(const struct ubpf_vm* vm, void* mem, size_t mem_len, uint64_t* bpf_ret } break; case EBPF_OP_JSLE_IMM: - if ((int64_t)reg[inst.dst] <= sign_extend_immediate(inst.imm)) { + if ((int64_t)reg[inst.dst] <= i64(inst.imm)) { pc += inst.offset; } break;