[PATCH] bpf: simulator: correct div, mod insn semantics
Andrew Burgess
andrew.burgess@embecosm.com
Sun Sep 6 13:23:55 GMT 2020
* David Faust via Gdb-patches <gdb-patches@sourceware.org> [2020-09-04 10:20:09 -0700]:
> The div and mod eBPF instructions are unsigned, but the semantic
> specification for the simulator incorrectly used signed operators.
> Correct them to unsigned versions, and correct the ALU tests in
> the simulator (which incorrectly assumed signed semantics).
>
> Tested in bpf-unknown-none.
>
> cpu/ChangeLog:
> 2020-09-04 David Faust <david.faust@oracle.com>
>
> * bpf.cpu (define-alu-instructions): Correct semantic operators
> for div, mod to unsigned versions.
>
> sim/ChangeLog:
> 2020-09-04 David Faust <david.faust@oracle.com>
>
> * bpf/sem-be.c: Regenerate.
> * bpf/sem-le.c: Likewise.
>
> sim/testsuite/ChangeLog:
> 2020-09-04 David Faust <david.faust@oracle.com>
>
> * sim/bpf/alu.s: Correct div and mod tests.
> * sim/bpf/alu32.s: Likewise.
> ---
> cpu/bpf.cpu | 4 ++--
> sim/bpf/sem-be.c | 16 ++++++++--------
> sim/bpf/sem-le.c | 16 ++++++++--------
> sim/testsuite/sim/bpf/alu.s | 26 +++++++++++++++++++-------
> sim/testsuite/sim/bpf/alu32.s | 31 +++++++++++++++++++++----------
> 5 files changed, 58 insertions(+), 35 deletions(-)
LGTM.
Thanks,
Andrew
>
> diff --git a/cpu/bpf.cpu b/cpu/bpf.cpu
> index 966500e36d1..eb7bf5caa5e 100644
> --- a/cpu/bpf.cpu
> +++ b/cpu/bpf.cpu
> @@ -487,12 +487,12 @@
> (daib add OP_CODE_ADD x-endian add)
> (daib sub OP_CODE_SUB x-endian sub)
> (daib mul OP_CODE_MUL x-endian mul)
> - (daib div OP_CODE_DIV x-endian div)
> + (daib div OP_CODE_DIV x-endian udiv)
> (daib or OP_CODE_OR x-endian or)
> (daib and OP_CODE_AND x-endian and)
> (daib lsh OP_CODE_LSH x-endian sll)
> (daib rsh OP_CODE_RSH x-endian srl)
> - (daib mod OP_CODE_MOD x-endian mod)
> + (daib mod OP_CODE_MOD x-endian umod)
> (daib xor OP_CODE_XOR x-endian xor)
> (daib arsh OP_CODE_ARSH x-endian sra)
> (daiu neg OP_CODE_NEG x-endian neg)
> diff --git a/sim/bpf/sem-be.c b/sim/bpf/sem-be.c
> index 12b65c77a13..0a3e927990d 100644
> --- a/sim/bpf/sem-be.c
> +++ b/sim/bpf/sem-be.c
> @@ -461,7 +461,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,divibe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = DIVDI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> + DI opval = UDIVDI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -482,7 +482,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,divrbe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = DIVDI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> + DI opval = UDIVDI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -503,7 +503,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,div32ibe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = DIVSI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> + USI opval = UDIVSI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> @@ -524,7 +524,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,div32rbe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = DIVSI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> + USI opval = UDIVSI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> @@ -881,7 +881,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,modibe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = MODDI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> + DI opval = UMODDI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -902,7 +902,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,modrbe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = MODDI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> + DI opval = UMODDI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -923,7 +923,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,mod32ibe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = MODSI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> + USI opval = UMODSI (CPU (h_gpr[FLD (f_dstbe)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> @@ -944,7 +944,7 @@ SEM_FN_NAME (bpfbf_ebpfbe,mod32rbe) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = MODSI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> + USI opval = UMODSI (CPU (h_gpr[FLD (f_dstbe)]), CPU (h_gpr[FLD (f_srcbe)]));
> CPU (h_gpr[FLD (f_dstbe)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> diff --git a/sim/bpf/sem-le.c b/sim/bpf/sem-le.c
> index 8bb1debfb7f..58a59b12422 100644
> --- a/sim/bpf/sem-le.c
> +++ b/sim/bpf/sem-le.c
> @@ -461,7 +461,7 @@ SEM_FN_NAME (bpfbf_ebpfle,divile) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = DIVDI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> + DI opval = UDIVDI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -482,7 +482,7 @@ SEM_FN_NAME (bpfbf_ebpfle,divrle) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = DIVDI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> + DI opval = UDIVDI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -503,7 +503,7 @@ SEM_FN_NAME (bpfbf_ebpfle,div32ile) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = DIVSI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> + USI opval = UDIVSI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> @@ -524,7 +524,7 @@ SEM_FN_NAME (bpfbf_ebpfle,div32rle) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = DIVSI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> + USI opval = UDIVSI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> @@ -881,7 +881,7 @@ SEM_FN_NAME (bpfbf_ebpfle,modile) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = MODDI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> + DI opval = UMODDI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -902,7 +902,7 @@ SEM_FN_NAME (bpfbf_ebpfle,modrle) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - DI opval = MODDI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> + DI opval = UMODDI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'D', opval);
> }
> @@ -923,7 +923,7 @@ SEM_FN_NAME (bpfbf_ebpfle,mod32ile) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = MODSI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> + USI opval = UMODSI (CPU (h_gpr[FLD (f_dstle)]), FLD (f_imm32));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> @@ -944,7 +944,7 @@ SEM_FN_NAME (bpfbf_ebpfle,mod32rle) (SIM_CPU *current_cpu, SEM_ARG sem_arg)
> SEM_PC vpc = SEM_NEXT_VPC (sem_arg, pc, 8);
>
> {
> - USI opval = MODSI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> + USI opval = UMODSI (CPU (h_gpr[FLD (f_dstle)]), CPU (h_gpr[FLD (f_srcle)]));
> CPU (h_gpr[FLD (f_dstle)]) = opval;
> CGEN_TRACE_RESULT (current_cpu, abuf, "gpr", 'x', opval);
> }
> diff --git a/sim/testsuite/sim/bpf/alu.s b/sim/testsuite/sim/bpf/alu.s
> index 6013ac7eb93..4dc37b1f01a 100644
> --- a/sim/testsuite/sim/bpf/alu.s
> +++ b/sim/testsuite/sim/bpf/alu.s
> @@ -40,11 +40,16 @@ main:
> ;; div
> div %r2, %r1
> fail_ne %r2, 0
> - div %r1, -10000
> - fail_ne %r1, -11007531
> + div %r1, 10000
> + fail_ne %r1, 11007531
> div %r1, %r1
> fail_ne %r1, 1
>
> + ;; div is unsigned
> + lddw %r1, -8
> + div %r1, 2
> + fail_ne %r1, 0x7ffffffffffffffc ; sign bits NOT maintained - large pos.
> +
> ;; and
> lddw %r1, 0xaaaaaaaa55555555
> and %r1, 0x55aaaaaa ; we still only have 32-bit imm.
> @@ -84,14 +89,21 @@ main:
>
> ;; mod
> mov %r1, 1025
> - mod %r1, -16
> + mod %r1, 16
> + fail_ne %r1, 1
> +
> + ;; mod is unsigned
> + mov %r1, 1025
> + mod %r1, -16 ; mod unsigned -> will treat as large positive
> + fail_ne %r1, 1025
> +
> + mov %r1, -25 ; -25 is 0xff..ffe7
> + mov %r2, 5 ; ... which when unsigned is a large positive
> + mod %r1, %r2 ; ... which is not evenly divisible by 5
> fail_ne %r1, 1
> - mov %r1, -25
> - mov %r2, 5
> - mod %r1, %r2
> - fail_ne %r1, 0
>
> ;; xor
> + mov %r1, 0
> xor %r1, %r2
> fail_ne %r1, 5
> xor %r1, 0x7eadbeef
> diff --git a/sim/testsuite/sim/bpf/alu32.s b/sim/testsuite/sim/bpf/alu32.s
> index fcd6699464f..e8d5062476c 100644
> --- a/sim/testsuite/sim/bpf/alu32.s
> +++ b/sim/testsuite/sim/bpf/alu32.s
> @@ -31,10 +31,15 @@ main:
> fail_ne32 %r1, 264
>
> ;; div
> - div32 %r1, %r2 ; r1 /= r2 (r1 = 264 / -6 = -44)
> - div32 %r1, -2 ; r1 /= -2 (r1 = 22)
> - div32 %r1, 2 ; r1 /= 2 (r1 = 11)
> - fail_ne32 %r1, 11
> + div32 %r1, 6
> + mov32 %r2, 11
> + div32 %r1, %r2
> + fail_ne32 %r1, 4
> +
> + ;; div is unsigned
> + mov32 %r1, -8 ; 0xfffffff8
> + div32 %r1, 2
> + fail_ne32 %r1, 0x7ffffffc ; sign bits are not preserved
>
> ;; and (bitwise)
> mov32 %r1, 0xb ; r1 = (0xb = 0b1011)
> @@ -70,13 +75,19 @@ main:
> ; i.e. upper-32 bits should be untouched
>
> ;; mod
> - mov32 %r1, -25
> - mov32 %r2, 4
> + mov32 %r1, 1025
> + mod32 %r1, 16
> + fail_ne32 %r1, 1
> +
> + ;; mod is unsigned
> + mov32 %r1, 1025
> + mod32 %r1, -16 ; when unsigned, much larger than 1025
> + fail_ne32 %r1, 1025
> +
> + mov32 %r1, -25 ; when unsigned, a large positive which is
> + mov32 %r2, 5 ; ... not evenly divisible by 5
> mod32 %r1, %r2
> - fail_ne32 %r1, -1
> - mov32 %r1, 25
> - mod32 %r1, 5
> - fail_ne32 %r1, 0
> + fail_ne32 %r1, 1
>
> ;; xor
> xor32 %r1, %r2
> --
> 2.26.2
>
More information about the Binutils
mailing list