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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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>
---
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;
 }
 
-- 
2.1.4


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