[ExecutionEngine] Modify static_casts to not be tautological in some COFF i386 relocations

[xiaobai]

Summary

I'm pretty sure an int32_t will always be between INT32_MAX and
INT32_MIN, inclusive.

[smeenai]

I'd wait for @compnerd to take a look before acting on my comments, since I don't have much context on this code.

lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFI386.h

146–147

A VA is unsigned, so "relocation underflow" doesn't make much sense to me. I'd just assert Result <= UINT32_MAX instead.

160–161

Same here.

198–199

Same here (though you'll need a cast to uint64_t).

[compnerd]

lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFI386.h

146–147

Yeah, given that it is a VA, we can remove the underflow check as it cannot happen.

[xiaobai]

Addressed comments and fixed incorrect test.

[compnerd]

SVN r316169

lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFI386.h

case COFF::IMAGE_REL_I386_ABSOLUTE:
       // This relocation is ignored.
       break;
     case COFF::IMAGE_REL_I386_DIR32: {
       // The target's 32-bit VA.
       uint64_t Result =
           RE.Sections.SectionA == static_cast<uint32_t>(-1)
               ? Value
               : Sections[RE.Sections.SectionA].getLoadAddressWithOffset(
                     RE.Addend);
-      assert(static_cast<int32_t>(Result) <= INT32_MAX &&
-             "relocation overflow");
-      assert(static_cast<int32_t>(Result) >= INT32_MIN &&
-             "relocation underflow");
+      assert(Result <= UINT32_MAX && "relocation overflow");
       DEBUG(dbgs() << "\t\tOffset: " << RE.Offset
                    << " RelType: IMAGE_REL_I386_DIR32"
                    << " TargetSection: " << RE.Sections.SectionA
                    << " Value: " << format("0x%08" PRIx32, Result) << '\n');
       writeBytesUnaligned(Result, Target, 4);
       break;
     }
     case COFF::IMAGE_REL_I386_DIR32NB: {
       // The target's 32-bit RVA.
       // NOTE: use Section[0].getLoadAddress() as an approximation of ImageBase
       uint64_t Result =
           Sections[RE.Sections.SectionA].getLoadAddressWithOffset(RE.Addend) -
           Sections[0].getLoadAddress();
-      assert(static_cast<int32_t>(Result) <= INT32_MAX &&
-             "relocation overflow");
-      assert(static_cast<int32_t>(Result) >= INT32_MIN &&
-             "relocation underflow");
+      assert(Result <= UINT32_MAX && "relocation overflow");
       DEBUG(dbgs() << "\t\tOffset: " << RE.Offset
                    << " RelType: IMAGE_REL_I386_DIR32NB"
                    << " TargetSection: " << RE.Sections.SectionA
                    << " Value: " << format("0x%08" PRIx32, Result) << '\n');
       writeBytesUnaligned(Result, Target, 4);
       break;
     }
     case COFF::IMAGE_REL_I386_REL32: {
       // 32-bit relative displacement to the target.
       uint64_t Result = RE.Sections.SectionA == static_cast<uint32_t>(-1)
                             ? Value
                             : Sections[RE.Sections.SectionA].getLoadAddress();
       Result = Result - Section.getLoadAddress() + RE.Addend - 4 - RE.Offset;
-      assert(static_cast<int32_t>(Result) <= INT32_MAX &&
+      assert(static_cast<int64_t>(Result) <= INT32_MAX &&
              "relocation overflow");
-      assert(static_cast<int32_t>(Result) >= INT32_MIN &&
+      assert(static_cast<int64_t>(Result) >= INT32_MIN &&
              "relocation underflow");
       DEBUG(dbgs() << "\t\tOffset: " << RE.Offset
                    << " RelType: IMAGE_REL_I386_REL32"
                    << " TargetSection: " << RE.Sections.SectionA
                    << " Value: " << format("0x%08" PRIx32, Result) << '\n');
       writeBytesUnaligned(Result, Target, 4);
       break;
     }
     case COFF::IMAGE_REL_I386_SECTION:
       // 16-bit section index of the section that contains the target.
-      assert(static_cast<int32_t>(RE.SectionID) <= INT16_MAX &&
+      assert(static_cast<uint32_t>(RE.SectionID) <= UINT16_MAX &&
              "relocation overflow");
-      assert(static_cast<int32_t>(RE.SectionID) >= INT16_MIN &&
-             "relocation underflow");
       DEBUG(dbgs() << "\t\tOffset: " << RE.Offset
                    << " RelType: IMAGE_REL_I386_SECTION Value: " << RE.SectionID
                    << '\n');
       writeBytesUnaligned(RE.SectionID, Target, 2);
       break;
     case COFF::IMAGE_REL_I386_SECREL:
       // 32-bit offset of the target from the beginning of its section.
-      assert(static_cast<int32_t>(RE.Addend) <= INT32_MAX &&
+      assert(static_cast<uint64_t>(RE.Addend) <= UINT32_MAX &&
              "relocation overflow");
-      assert(static_cast<int32_t>(RE.Addend) >= INT32_MIN &&
-             "relocation underflow");
       DEBUG(dbgs() << "\t\tOffset: " << RE.Offset
                    << " RelType: IMAGE_REL_I386_SECREL Value: " << RE.Addend
                    << '\n');
       writeBytesUnaligned(RE.Addend, Target, 2);
       break;
     default:
       llvm_unreachable("unsupported relocation type");
     }
   }
 
   void registerEHFrames() override {}
 };
 
 }
 
 #endif
 

test/ExecutionEngine/RuntimeDyld/X86/COFF_i386.s

 // RUN: llvm-mc -triple i686-windows -filetype obj -o %t.obj %s
-// RUN: llvm-rtdyld -triple i686-windows -dummy-extern _printf=0xfffffffd -dummy-extern _OutputDebugStringA@4=0xfffffffe -dummy-extern _ExitProcess@4=0xffffffff -verify -check=%s %t.obj
+// RUN: llvm-rtdyld -triple i686-windows -dummy-extern _printf=0x7ffffffd -dummy-extern _OutputDebugStringA@4=0x7ffffffe -dummy-extern _ExitProcess@4=0x7fffffff -verify -check=%s %t.obj
 
   .text
 
   .def _main
     .scl 2
     .type 32
   .endef
   .global _main
# rtdyld-check: decode_operand(rel4, 3) = __imp__ExitProcess
   retl
 
   .data
 
   .global __imp__OutputDebugStringA
   .align 4
 __imp__OutputDebugStringA:
   .long "_OutputDebugStringA@4"   // IMAGE_REL_I386_DIR32
-# rtdyld-check: *{4}__imp__OutputDebugStringA = 0xfffffffe
+# rtdyld-check: *{4}__imp__OutputDebugStringA = 0x7ffffffe
 
   .global __imp__ExitProcess
   .align 4
 __imp__ExitProcess:
   .long "_ExitProcess@4"      // IMAGE_REL_I386_DIR32
-# rtdyld-check: *{4}__imp__ExitProcess = 0xffffffff
+# rtdyld-check: *{4}__imp__ExitProcess = 0x7fffffff
 
   .global string
   .align 1
 string:
   .asciz "Hello World!\n"
 
   .global relocations
 relocations: