[PATCH] D67340: [Object] Implement relocation resolver for COFF ARM/ARM64

Mon Sep 9 01:57:17 PDT 2019

MaskRay accepted this revision.
MaskRay added inline comments.

================
Comment at: lib/Object/RelocationResolver.cpp:443
+  case COFF::IMAGE_REL_ARM_ADDR32:
+    return (S + A) & 0xFFFFFFFF;
+  default:
----------------
ruiu wrote:
> mstorsjo wrote:
> > ruiu wrote:
> > > Is it OK to silently wrap-around an overflowed value? I wonder if we should report an error.
> > Not sure if the base values will be in range where overflow might even be expected.
> > 
> > In any case, this is the exact same existing code matching `resolveCOFFX86` and `resolveCOFFX86_64` (see the context of the diff), just with different arch-specific relocation names.
> Ah, OK. There precedents there in this file indeed.
On DWARF side, error reporting should be possible now after D63713 added `Error *Err` to DWARFDataExtractor::getRelocatedValue.

I'll take a stab at improving it. (In any case, this change should not be blocked by that change but I hope @mstorsjo can add test cases about relocation overflows when that is done)

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67340/new/

https://reviews.llvm.org/D67340