From: Andrew Cooper Date: Thu, 8 Sep 2016 17:52:46 +0000 (+0100) Subject: xen/x86: Fix build with clang following c/s 4fa0105 X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~423 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=4c47c47938ea24c73d9459f9f0b6923513772b5d;p=xen.git xen/x86: Fix build with clang following c/s 4fa0105 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 Reviewed-by: Jan Beulich Reviewed-by: George Dunlap --- diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index e3bfda5bec..cc25676c74 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 8d6661c5b6..21607bf34a 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; }