[PATCH] D20434: [MCExpr] avoid UB via negation of INT_MIN

[Sanjay Patel via llvm-commits]

spatel created this revision.
spatel added reviewers: rafael, steven_wu, craig.topper.
spatel added a subscriber: llvm-commits.
Herald added a subscriber: mcrosier.

I accidentally exposed a bug in MCExpr::evaluateAsRelocatableImpl() with the test file added in:
http://reviews.llvm.org/rL269977

I don't know anything about MC. Is this a viable fix? 

Note that I see at least one other negation later in this file that seems like it could trigger the same UB:
  return EvaluateSymbolicAdd(Asm, Layout, Addrs, InSet, LHSValue,
    RHSValue.getSymB(), RHSValue.getSymA(),
    -RHSValue.getConstant(), Res);


http://reviews.llvm.org/D20434

Files:
  lib/MC/MCExpr.cpp
  test/MC/X86/imm-comments.s

Index: test/MC/X86/imm-comments.s
===================================================================
--- test/MC/X86/imm-comments.s
+++ test/MC/X86/imm-comments.s
@@ -10,7 +10,9 @@
 movl  $-2147483648, %eax
 
 movabsq	$9223372036854775807, %rax
-movabsq	$-9223372036854775808, %rax
+
+# This line should not induce undefined behavior via negation of INT64_MIN.
+movabsq	$-9223372036854775808, %rax 
 
 # CHECK:  movb  $127, %al
 # CHECK:  movb  $-128, %al
Index: lib/MC/MCExpr.cpp
===================================================================
--- lib/MC/MCExpr.cpp
+++ lib/MC/MCExpr.cpp
@@ -659,13 +659,20 @@
         return false;
       Res = MCValue::get(!Value.getConstant());
       break;
-    case MCUnaryExpr::Minus:
+    case MCUnaryExpr::Minus: {
       /// -(a - b + const) ==> (b - a - const)
       if (Value.getSymA() && !Value.getSymB())
         return false;
-      Res = MCValue::get(Value.getSymB(), Value.getSymA(),
-                         -Value.getConstant());
+
+      // Avoid undefined behavior: the negation of the minimum constant is the
+      // minimum constant.
+      int64_t ValConst = Value.getConstant();
+      if (ValConst != INT64_MIN)
+        ValConst = -ValConst;
+      
+      Res = MCValue::get(Value.getSymB(), Value.getSymA(), ValConst);
       break;
+    }
     case MCUnaryExpr::Not:
       if (!Value.isAbsolute())
         return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D20434.57789.patch
Type: text/x-patch
Size: 1397 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160519/cdfcfe30/attachment.bin>