[PATCH] D91533: [lib/Object] - Generalize the RelocationResolver API.

[George Rimar via Phabricator via llvm-commits]

grimar marked 3 inline comments as done.
grimar added inline comments.


================
Comment at: llvm/lib/Object/RelocationResolver.cpp:757
+  // contains the addend, provided by the caller.
+  return Resolver(/*Type=*/0, /*Offset=*/0, S, LocData,
+                  R.getRawDataRefImpl().p);
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > Is it simply not possible to provide Type and Offset information here? Otherwise, this code seems a little fragile to LLD deciding to try to use those fields for something.
> > Yes, it is not possible without an owning object :(
> > 
> > ```
> > inline uint64_t RelocationRef::getOffset() const {
> >   return OwningObject->getRelocationOffset(RelocationPimpl);
> > }
> > 
> > inline uint64_t RelocationRef::getType() const {
> >   return OwningObject->getRelocationType(RelocationPimpl);
> > }
> > ```
> I think we need some sort of note added to the LLD resolver then, indicating that the values will always be 0 then (possibly could be an assertion?), because the information isn't available. That would at least hint to future developers to not try to use them.
The `type` and `offset` fields are commented out currently in `lld/ELF/DWARF.cpp`:

```
  static uint64_t resolve(uint64_t /*type*/, uint64_t /*offset*/, uint64_t s,
                          uint64_t /*locData*/, int64_t addend) {
    return s + addend;
  }
```

Do you want me to add an explicit comment about them or uncomment and add an assertion?


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

https://reviews.llvm.org/D91533