[JITLink][RISCV] Refactor range checking and alignment checking

[fourdim]

Summary

This patch refactors the range checking function to make it compatible with all relocation types and supports range checking for R_RISCV_BRANCH. Moreover, it refactors the alignment check functions.

[fourdim]

Include the recent commits and update the alignment check.

[fourdim]

Fix the variable's name.

[StephenFan]

llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h

375

Don't modify irrelevant parts

[fourdim]

llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h

375

I'm sorry but it is automatically modified by clang-format. Should I disable this?

[StephenFan]

llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h

375

I usually use git clang-format to clang-format the staged changes. This would not format irrelevant parts

[fourdim]

Revert partial clang format.

[StephenFan]

LGTM

[fourdim]

Remove unused parameter.

llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h

 /// Defines the scope in which this symbol should be visible:
 ///   Default -- Visible in the public interface of the linkage unit.
 ///   Hidden -- Visible within the linkage unit, but not exported from it.
 ///   Local -- Visible only within the LinkGraph.
 enum class Scope : uint8_t {
   Default,
   Hidden,
   Local
 };
 
 /// For debugging output.
 const char *getScopeName(Scope S);
 
 raw_ostream &operator<<(raw_ostream &OS, const Block &B);
 
 /// Symbol representation.
 ///

 /// Marks all symbols in a graph live. This can be used as a default,
 /// conservative mark-live implementation.
 Error markAllSymbolsLive(LinkGraph &G);
 
 /// Create an out of range error for the given edge in the given block.
 Error makeTargetOutOfRangeError(const LinkGraph &G, const Block &B,
                                 const Edge &E);
 
+Error makeAlignmentError(llvm::orc::ExecutorAddr Loc, uint64_t Value, int N,
+                         const Edge &E);
+
 /// Base case for edge-visitors where the visitor-list is empty.
 inline void visitEdge(LinkGraph &G, Block *B, Edge &E) {}
 
 /// Applies the first visitor in the list to the given edge. If the visitor's
 /// visitEdge method returns true then we return immediately, otherwise we
 /// apply the next visitor.
 template <typename VisitorT, typename... VisitorTs>
 void visitEdge(LinkGraph &G, Block *B, Edge &E, VisitorT &&V,

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

static Expected<const Edge &> getRISCVPCRelHi20(const Edge &E) {
   return make_error<JITLinkError>(
       "No HI20 PCREL relocation type be found for LO12 PCREL relocation type");
 }
 
 static uint32_t extractBits(uint32_t Num, unsigned Low, unsigned Size) {
   return (Num & (((1ULL << Size) - 1) << Low)) >> Low;
 }
 
-inline Error checkAlignment(llvm::orc::ExecutorAddr loc, uint64_t v, int n,
-                            const Edge &E) {
-  if (v & (n - 1))
-    return make_error<JITLinkError>("0x" + llvm::utohexstr(loc.getValue()) +
-                                    " improper alignment for relocation " +
-                                    formatv("{0:d}", E.getKind()) + ": 0x" +
-                                    llvm::utohexstr(v) + " is not aligned to " +
-                                    Twine(n) + " bytes");
-  return Error::success();
+static inline bool isAlignmentCorrect(uint64_t Value, int N) {
+  return (Value & (N - 1)) ? false : true;
 }
 
-static inline bool isInRangeForImmS32(int64_t Value) {
-  return (Value >= std::numeric_limits<int32_t>::min() &&
-          Value <= std::numeric_limits<int32_t>::max());
+// Requires 0 < N <= 64.
+static inline bool isInRangeForImm(int64_t Value, int N) {
+  return Value == llvm::SignExtend64(Value, N);
 }
 
 class ELFJITLinker_riscv : public JITLinker<ELFJITLinker_riscv> {
   friend class JITLinker<ELFJITLinker_riscv>;
 
 public:
   ELFJITLinker_riscv(std::unique_ptr<JITLinkContext> Ctx,
                      std::unique_ptr<LinkGraph> G, PassConfiguration PassConfig)
Error applyFixup(LinkGraph &G, Block &B, const Edge &E) const {
     }
     case R_RISCV_64: {
       int64_t Value = (E.getTarget().getAddress() + E.getAddend()).getValue();
       *(little64_t *)FixupPtr = static_cast<uint64_t>(Value);
       break;
     }
     case R_RISCV_BRANCH: {
       int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
-      Error AlignmentIssue = checkAlignment(FixupAddress, Value, 2, E);
-      if (AlignmentIssue) {
-        return AlignmentIssue;
-      }
+      if (LLVM_UNLIKELY(!isInRangeForImm(Value >> 1, 12)))
+        return makeTargetOutOfRangeError(G, B, E);
+      if (LLVM_UNLIKELY(!isAlignmentCorrect(Value, 2)))
+        return makeAlignmentError(FixupAddress, Value, 2, E);
       uint32_t Imm31_25 =
           extractBits(Value, 5, 6) << 25 | extractBits(Value, 12, 1) << 31;
       uint32_t Imm11_7 =
           extractBits(Value, 1, 4) << 8 | extractBits(Value, 11, 1) << 7;
       uint32_t RawInstr = *(little32_t *)FixupPtr;
       *(little32_t *)FixupPtr = (RawInstr & 0x1FFF07F) | Imm31_25 | Imm11_7;
       break;
     }
     case R_RISCV_JAL: {
       int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
-      Error AlignmentIssue = checkAlignment(FixupAddress, Value, 2, E);
-      if (AlignmentIssue) {
-        return AlignmentIssue;
-      }
+      if (LLVM_UNLIKELY(!isInRangeForImm(Value >> 1, 20)))
+        return makeTargetOutOfRangeError(G, B, E);
+      if (LLVM_UNLIKELY(!isAlignmentCorrect(Value, 2)))
+        return makeAlignmentError(FixupAddress, Value, 2, E);
       uint32_t Imm20 = extractBits(Value, 20, 1) << 31;
       uint32_t Imm10_1 = extractBits(Value, 1, 10) << 21;
       uint32_t Imm11 = extractBits(Value, 11, 1) << 20;
       uint32_t Imm19_12 = extractBits(Value, 12, 8) << 12;
       uint32_t RawInstr = *(little32_t *)FixupPtr;
       *(little32_t *)FixupPtr = RawInstr | Imm20 | Imm10_1 | Imm11 | Imm19_12;
       break;
     }
     case R_RISCV_HI20: {
       int64_t Value = (E.getTarget().getAddress() + E.getAddend()).getValue();
       int64_t Hi = Value + 0x800;
-      if (LLVM_UNLIKELY(!isInRangeForImmS32(Hi)))
+      if (LLVM_UNLIKELY(!isInRangeForImm(Hi, 32)))
         return makeTargetOutOfRangeError(G, B, E);
       uint32_t RawInstr = *(little32_t *)FixupPtr;
       *(little32_t *)FixupPtr =
           (RawInstr & 0xFFF) | (static_cast<uint32_t>(Hi & 0xFFFFF000));
       break;
     }
     case R_RISCV_LO12_I: {
       // FIXME: We assume that R_RISCV_HI20 is present in object code and pairs
       // with current relocation R_RISCV_LO12_I. So here may need a check.
       int64_t Value = (E.getTarget().getAddress() + E.getAddend()).getValue();
       int32_t Lo = Value & 0xFFF;
       uint32_t RawInstr = *(little32_t *)FixupPtr;
       *(little32_t *)FixupPtr =
           (RawInstr & 0xFFFFF) | (static_cast<uint32_t>(Lo & 0xFFF) << 20);
       break;
     }
     case R_RISCV_CALL: {
       int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
       int64_t Hi = Value + 0x800;
-      if (LLVM_UNLIKELY(!isInRangeForImmS32(Hi)))
+      if (LLVM_UNLIKELY(!isInRangeForImm(Hi, 32)))
         return makeTargetOutOfRangeError(G, B, E);
       int32_t Lo = Value & 0xFFF;
       uint32_t RawInstrAuipc = *(little32_t *)FixupPtr;
       uint32_t RawInstrJalr = *(little32_t *)(FixupPtr + 4);
       *(little32_t *)FixupPtr =
           RawInstrAuipc | (static_cast<uint32_t>(Hi & 0xFFFFF000));
       *(little32_t *)(FixupPtr + 4) =
           RawInstrJalr | (static_cast<uint32_t>(Lo) << 20);
       break;
     }
     case R_RISCV_PCREL_HI20: {
       int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
       int64_t Hi = Value + 0x800;
-      if (LLVM_UNLIKELY(!isInRangeForImmS32(Hi)))
+      if (LLVM_UNLIKELY(!isInRangeForImm(Hi, 32)))
         return makeTargetOutOfRangeError(G, B, E);
       uint32_t RawInstr = *(little32_t *)FixupPtr;
       *(little32_t *)FixupPtr =
           (RawInstr & 0xFFF) | (static_cast<uint32_t>(Hi & 0xFFFFF000));
       break;
     }
     case R_RISCV_PCREL_LO12_I: {
       // FIXME: We assume that R_RISCV_PCREL_HI20 is present in object code and

llvm/lib/ExecutionEngine/JITLink/JITLink.cpp

else
       ErrStream << "<anonymous block> @ ";
 
     ErrStream << formatv("{0:x}", B.getAddress()) << " + "
               << formatv("{0:x}", E.getOffset()) << ")";
   }
   return make_error<JITLinkError>(std::move(ErrMsg));
 }
 
+Error makeAlignmentError(llvm::orc::ExecutorAddr Loc, uint64_t Value, int N,
+                         const Edge &E) {
+  return make_error<JITLinkError>("0x" + llvm::utohexstr(Loc.getValue()) +
+                                  " improper alignment for relocation " +
+                                  formatv("{0:d}", E.getKind()) + ": 0x" +
+                                  llvm::utohexstr(Value) +
+                                  " is not aligned to " + Twine(N) + " bytes");
+}
+
 Expected<std::unique_ptr<LinkGraph>>
 createLinkGraphFromObject(MemoryBufferRef ObjectBuffer) {
   auto Magic = identify_magic(ObjectBuffer.getBuffer());
   switch (Magic) {
   case file_magic::macho_object:
     return createLinkGraphFromMachOObject(ObjectBuffer);
   case file_magic::elf_relocatable:
     return createLinkGraphFromELFObject(ObjectBuffer);