[PATCH] D67236: [MC] Fix undefined behavior in MCInstPrinter::formatHex

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: thegameg, shafik, vsk, dblaikie.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

Passing `UINT64_MIN` to `MCInstPrinter::formatHex` triggers undefined behavior because the negation of `-9223372036854775808` cannot be represented in type 'int64_t' (aka 'long long'). This patch puts a workaround in place to just print the hex value directly, without the leading `-`. A possible alternative involves using a small helper functions that uses (implementation) defined conversions to achieve the desirable value:

  static int64_t helper(int64_t V) {
    auto U = static_cast<uint64_t>(V);
    return V < 0 ? -U : U;
  }

The underlying problem is that MCInstPrinter::formatHex(int64_t) returns a `format_object<int64_t>` and should really return a `format_object<uint64_t>`. However, that's not possible because `formatImm` needs to be able to print both as decimal (where a signed is required) and hex (where we'd prefer to always have an unsigned).

  format_object<int64_t> formatImm(int64_t Value) const {
    return PrintImmHex ? formatHex(Value) : formatDec(Value);
  }


Repository:
  rL LLVM

https://reviews.llvm.org/D67236

Files:
  llvm/lib/MC/MCInstPrinter.cpp


Index: llvm/lib/MC/MCInstPrinter.cpp
===================================================================
--- llvm/lib/MC/MCInstPrinter.cpp
+++ llvm/lib/MC/MCInstPrinter.cpp
@@ -83,15 +83,19 @@
 }
 
 format_object<int64_t> MCInstPrinter::formatHex(int64_t Value) const {
-  switch(PrintHexStyle) {
+  switch (PrintHexStyle) {
   case HexStyle::C:
-    if (Value < 0)
+    if (Value == std::numeric_limits<int64_t>::min())
+      return format("0x%" PRIx64, Value);
+    else if (Value < 0)
       return format("-0x%" PRIx64, -Value);
     else
       return format("0x%" PRIx64, Value);
   case HexStyle::Asm:
-    if (Value < 0) {
-      if (needsLeadingZero((uint64_t)(-Value)))
+    if (Value == std::numeric_limits<int64_t>::min())
+      return format("%" PRIx64 "h", Value);
+    else if (Value < 0) {
+      if (needsLeadingZero(-(uint64_t)(Value)))
         return format("-0%" PRIx64 "h", -Value);
       else
         return format("-%" PRIx64 "h", -Value);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67236.218966.patch
Type: text/x-patch
Size: 968 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190905/ff78d1d8/attachment.bin>