[MCExpr] avoid UB via negation of INT_MIN

[spatel]

Summary

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);

[spatel]

I've commented out the INT_MIN line in the test to unbreak sanitizer bots:
http://reviews.llvm.org/rL270128

For reference, the intended use of this test is for D20385 .

[majnemer]

lib/MC/MCExpr.cpp

669–673 ↗

(On Diff #57789)

I'd just go with:

Res = MCValue::get(Value.getSymB(), Value.getSymA(),
                   -(uint64_t)Value.getConstant());

[spatel]

Patch updated:

1. Use David's suggestion of casting to uint64_t.
2. Fix the other similar code where this could happen too.
3. Rebased after rL270128.

[majnemer]

LGTM

llvm/trunk/lib/MC/MCExpr.cpp

case MCUnaryExpr::LNot:
       if (!Value.isAbsolute())
         return false;
       Res = MCValue::get(!Value.getConstant());
       break;
     case MCUnaryExpr::Minus:
       /// -(a - b + const) ==> (b - a - const)
       if (Value.getSymA() && !Value.getSymB())
         return false;
+
+      // The cast avoids undefined behavior if the constant is INT64_MIN.
       Res = MCValue::get(Value.getSymB(), Value.getSymA(),
-                         -Value.getConstant());
+                         -(uint64_t)Value.getConstant());
       break;
     case MCUnaryExpr::Not:
       if (!Value.isAbsolute())
         return false;
       Res = MCValue::get(~Value.getConstant());
       break;
     case MCUnaryExpr::Plus:
       Res = Value;
case Binary: {
     // We only support a few operations on non-constant expressions, handle
     // those first.
     if (!LHSValue.isAbsolute() || !RHSValue.isAbsolute()) {
       switch (ABE->getOpcode()) {
       default:
         return false;
       case MCBinaryExpr::Sub:
         // Negate RHS and add.
+        // The cast avoids undefined behavior if the constant is INT64_MIN.
         return EvaluateSymbolicAdd(Asm, Layout, Addrs, InSet, LHSValue,
                                    RHSValue.getSymB(), RHSValue.getSymA(),
-                                   -RHSValue.getConstant(), Res);
+                                   -(uint64_t)RHSValue.getConstant(), Res);
 
       case MCBinaryExpr::Add:
         return EvaluateSymbolicAdd(Asm, Layout, Addrs, InSet, LHSValue,
                                    RHSValue.getSymA(), RHSValue.getSymB(),
                                    RHSValue.getConstant(), Res);
       }
     }
 

llvm/trunk/test/MC/X86/imm-comments.s

 # RUN: llvm-mc %s -triple=x86_64-unknown-unknown | FileCheck %s
 
 movb  $127, %al
 movb  $-128, %al
 
 movw  $32767, %ax
 movw  $-32768, %ax
 
 movl  $2147483647, %eax
 movl  $-2147483648, %eax
 
 movabsq $9223372036854775807, %rax
 
-# FIXME: This line causes UB failure.
-# 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
 
 # CHECK:  movw  $32767, %ax             # imm = 0x7FFF
 # CHECK:  movw  $-32768, %ax            # imm = 0xFFFFFFFFFFFF8000
 
 # CHECK:  movl  $2147483647, %eax       # imm = 0x7FFFFFFF
 # CHECK:  movl  $-2147483648, %eax      # imm = 0xFFFFFFFF80000000
 
 # CHECK:  movabsq $9223372036854775807, %rax # imm = 0x7FFFFFFFFFFFFFFF
-# FIXME-CHECK:  movabsq $-9223372036854775808, %rax # imm = 0x8000000000000000
+# CHECK:  movabsq $-9223372036854775808, %rax # imm = 0x8000000000000000