Home Original page

[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