Properly evaluate MCBinaryExpr with a constant on one side.

[colinl]

Summary

Previously these tests would fail with:
LLVM ERROR: expected relocatable expression

The logic is changed so EvaluateSymbolicAdd is only called if both sides have a relocatable expression.

If one side is a relocatable it combines the addends and creates a single relocatable expression.

[• rafael]

Please git clang-format the patch.

How is EvaluateSymbolicAdd failing? It would seem better to make it able to handle adding an absolute value.

test/MC/ARM/dot-symbol-relocation.s

17

Please add a newline to the end.

Please use unix newlines.

[• rafael]

Also, reduce the testcase all that is needed to fail currently:

.word 0x42 - .

It is not clear this patch handles '-' correctly. It should be inverting B and A, which is what the existing logic avoids to not produces a "null - B".

[• rafael]

.... is what the existing logic avoids to not produces a "null - B".

Turns out the correct solution is just to allow "null - B" since ELF
can represent that some times: r251818.

Thanks,
Rafael

[mcrosier]

Has Rafael addressed the issue? If so, can this be abandoned?

[colinl]

Yes, the crash I was seeing went away. Thanks for the reminder.

lib/MC/MCExpr.cpp

         !ABE->getRHS()->evaluateAsRelocatableImpl(RHSValue, Asm, Layout, Fixup,
                                                   Addrs, InSet))
       return false;
+    bool LHSAbs = LHSValue.isAbsolute();
+    bool RHSAbs = RHSValue.isAbsolute();
 
     // We only support a few operations on non-constant expressions, handle
     // those first.
-    if (!LHSValue.isAbsolute() || !RHSValue.isAbsolute()) {
+    if (!LHSAbs && !RHSAbs) {
       switch (ABE->getOpcode()) {
       default:
         return false;

     case MCBinaryExpr::Xor:  Result = LHS ^ RHS; break;
     }
 
+    // One side evaluates as absolute, adjust the relocatable addend
+    if (LHSAbs ^ RHSAbs) {
+      MCValue &RelValue = LHSAbs ? RHSValue : LHSValue;
+      switch (ABE->getOpcode()) {
+      default:
+        return false;
+      case MCBinaryExpr::Sub:
+      case MCBinaryExpr::Add:
+        break;
+      }
+      Res = MCValue::get(RelValue.getSymA(), RelValue.getSymB(),
+                         Result, RelValue.getRefKind());
+      return true;
+    }
+
     Res = MCValue::get(Result);
     return true;
   }

test/MC/ARM/dot-symbol-relocation.s

+# RUN: llvm-mc -filetype=obj -triple=arm -o - %s | \
+# RUN: llvm-objdump -triple=arm -r -s - | FileCheck %s
+
+# CHECK: 00000004 R_ARM_ABS32 .text
+# CHECK: 00000008 R_ARM_ABS32 .text
+# CHECK: 0000000c R_ARM_ABS32 .text
+# CHECK: 00000010 R_ARM_ABS32 .text
+# CHECK: 00000014 R_ARM_ABS32 .text
+# CHECK: 42000000 46000000 c6ffffff 4e000000
+# CHECK: 52000000 1c010000
+.word 0x42
+.word 0x42 - .
+.word . - 0x42
+.word 0x42 + .
+.word . + 0x42
+.word 0x42 + 0x42 + . + 0x42 + 0x42
 No newline at end of file