Re: [Xen-devel] [PATCH] xen/x86: Fix build with clang following c/s 4fa0105

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 08/09/16 19:21, Andrew Cooper wrote:
> https://travis-ci.org/xen-project/xen/jobs/158494027#L2344
> 
> Clang complains:
> 
>   emulate.c:2016:14: error: comparison of unsigned enum expression < 0
>   is always false [-Werror,-Wtautological-compare]
>       if ( seg < 0 || seg >= ARRAY_SIZE(hvmemul_ctxt->seg_reg) )
>            ~~~ ^ ~
> 
> Clang is wrong to raise a warning like this.  The signed-ness of an enum is
> implementation defined in C, and robust code must not assume the choices made
> by the compiler.
> 
> In this case, dropping the < 0 check creates a latent bug which would result
> in an array underflow when compiled with a compiler which chooses a signed
> enum.
> 
> Work around the bug by explicitly pulling seg into an unsigned integer, and
> only perform the upper bounds check.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/emulate.c      | 19 +++++++++++--------
>  xen/arch/x86/mm/shadow/common.c |  9 +++++----
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index e3bfda5..cc25676 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1447,13 +1447,14 @@ static int hvmemul_write_segment(
>  {
>      struct hvm_emulate_ctxt *hvmemul_ctxt =
>          container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> +    unsigned int idx = seg;
>  
> -    if ( seg < 0 || seg >= ARRAY_SIZE(hvmemul_ctxt->seg_reg) )
> +    if ( idx >= ARRAY_SIZE(hvmemul_ctxt->seg_reg) )
>          return X86EMUL_UNHANDLEABLE;
>  
> -    hvmemul_ctxt->seg_reg[seg] = *reg;
> -    __set_bit(seg, &hvmemul_ctxt->seg_reg_accessed);
> -    __set_bit(seg, &hvmemul_ctxt->seg_reg_dirty);
> +    hvmemul_ctxt->seg_reg[idx] = *reg;
> +    __set_bit(idx, &hvmemul_ctxt->seg_reg_accessed);
> +    __set_bit(idx, &hvmemul_ctxt->seg_reg_dirty);
>  
>      return X86EMUL_OKAY;
>  }
> @@ -2012,12 +2013,14 @@ struct segment_register *hvmemul_get_seg_reg(
>      enum x86_segment seg,
>      struct hvm_emulate_ctxt *hvmemul_ctxt)
>  {
> -    if ( seg < 0 || seg >= ARRAY_SIZE(hvmemul_ctxt->seg_reg) )
> +    unsigned int idx = seg;
> +
> +    if ( idx >= ARRAY_SIZE(hvmemul_ctxt->seg_reg) )
>          return ERR_PTR(-X86EMUL_UNHANDLEABLE);
>  
> -    if ( !__test_and_set_bit(seg, &hvmemul_ctxt->seg_reg_accessed) )
> -        hvm_get_segment_register(current, seg, &hvmemul_ctxt->seg_reg[seg]);
> -    return &hvmemul_ctxt->seg_reg[seg];
> +    if ( !__test_and_set_bit(idx, &hvmemul_ctxt->seg_reg_accessed) )
> +        hvm_get_segment_register(current, idx, &hvmemul_ctxt->seg_reg[idx]);
> +    return &hvmemul_ctxt->seg_reg[idx];
>  }
>  
>  static const char *guest_x86_mode_to_str(int mode)
> diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
> index 8d6661c..21607bf 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -130,14 +130,15 @@ __initcall(shadow_audit_key_init);
>  static struct segment_register *hvm_get_seg_reg(
>      enum x86_segment seg, struct sh_emulate_ctxt *sh_ctxt)
>  {
> +    unsigned int idx = seg;
>      struct segment_register *seg_reg;
>  
> -    if ( seg < 0 || seg >= ARRAY_SIZE(sh_ctxt->seg_reg) )
> +    if ( idx >= ARRAY_SIZE(sh_ctxt->seg_reg) )
>          return ERR_PTR(-X86EMUL_UNHANDLEABLE);
>  
> -    seg_reg = &sh_ctxt->seg_reg[seg];
> -    if ( !__test_and_set_bit(seg, &sh_ctxt->valid_seg_regs) )
> -        hvm_get_segment_register(current, seg, seg_reg);
> +    seg_reg = &sh_ctxt->seg_reg[idx];
> +    if ( !__test_and_set_bit(idx, &sh_ctxt->valid_seg_regs) )
> +        hvm_get_segment_register(current, idx, seg_reg);
>      return seg_reg;
>  }
>  
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel