On 1/24/22 15:18, Dumitru Ceara wrote:
> UB Sanitizer reports:
>   tests/test-hash.c:59:40: runtime error: shift exponent 64 is too large for 
> 64-bit type 'long unsigned int'
>       #0 0x44c3c9 in get_range128 tests/test-hash.c:59
>       #1 0x44cb2e in check_hash_bytes128 tests/test-hash.c:178
>       #2 0x44d14d in test_hash_main tests/test-hash.c:282
>       [...]
>   ofproto/ofproto-dpif-xlate.c:5607:45: runtime error: left shift of 65535 by 
> 16 places cannot be represented in type 'int'
>       #0 0x53fe9f in xlate_sample_action ofproto/ofproto-dpif-xlate.c:5607
>       #1 0x54d625 in do_xlate_actions ofproto/ofproto-dpif-xlate.c:7160
>       #2 0x553b76 in xlate_actions ofproto/ofproto-dpif-xlate.c:7806
>       #3 0x4fcb49 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237
>       #4 0x4fe02f in process_upcall ofproto/ofproto-dpif-upcall.c:1456
>       #5 0x4fda99 in upcall_cb ofproto/ofproto-dpif-upcall.c:1358
>       [...]
>   tests/test-util.c:89:23: runtime error: left shift of 1 by 31 places cannot 
> be represented in type 'int'
>       #0 0x476415 in test_ctz tests/test-util.c:89
>       [...]
>   lib/dpif-netlink.c:396:33: runtime error: left shift of 1 by 31 places 
> cannot be represented in type 'int'
>       #0 0x571b9f in dpif_netlink_open lib/dpif-netlink.c:396
> 
> Acked-by: Aaron Conole <[email protected]>
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> v3: Added Aaron's ack.
> ---
>  lib/dpif-netlink.c           |    2 +-
>  ofproto/ofproto-dpif-xlate.c |    3 ++-
>  tests/test-hash.c            |    2 +-
>  tests/test-util.c            |   13 ++++++-------
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 71e35ccddaae..06e1e8ca0283 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -85,7 +85,7 @@ enum { MAX_PORTS = USHRT_MAX };
>  #define EPOLLEXCLUSIVE (1u << 28)
>  #endif
>  
> -#define OVS_DP_F_UNSUPPORTED (1 << 31);
> +#define OVS_DP_F_UNSUPPORTED (1u << 31);
>  
>  /* This PID is not used by the kernel datapath when using dispatch per CPU,
>   * but it is required to be set (not zero). */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 6fb59e1702ec..d2b26f8ee27d 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -5618,7 +5618,8 @@ xlate_sample_action(struct xlate_ctx *ctx,
>  
>      /* Scale the probability from 16-bit to 32-bit while representing
>       * the same percentage. */
> -    uint32_t probability = (os->probability << 16) | os->probability;
> +    uint32_t probability =
> +        ((uint32_t) os->probability << 16) | os->probability;
>  
>      /* If ofp_port in flow sample action is equel to ofp_port,
>       * this sample action is a input port action. */
> diff --git a/tests/test-hash.c b/tests/test-hash.c
> index 5d3f8ea43f65..7c33922e3f02 100644
> --- a/tests/test-hash.c
> +++ b/tests/test-hash.c
> @@ -174,7 +174,7 @@ check_hash_bytes128(void (*hash)(const void *, size_t, 
> uint32_t, ovs_u128 *),
>  
>              set_bit128(&in2, j, n_bits);
>              hash(&in2, sizeof(ovs_u128), 0, &out2);
> -            for (ofs = 0; ofs < 128 - min_unique; ofs++) {
> +            for (ofs = 1; ofs < 128 - min_unique; ofs++) {

It looks like we're reducing the quality of the test here.
Shouldn't we change get_range128() function to return
"value->u64.lo & mask" if ofs == 0 instead?

>                  uint64_t bits1 = get_range128(&out1, ofs, unique_mask);
>                  uint64_t bits2 = get_range128(&out2, ofs, unique_mask);
>  
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to