[PATCH] D16009: [lld]Non-MachO references shouldn't assert when checking for TLV.

Jim Grosbach via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 8 18:07:55 PST 2016


grosbach added inline comments.

================
Comment at: lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp:63
@@ -62,2 +62,3 @@
   bool isTLVAccess(const Reference &ref) const override {
-    assert(ref.kindNamespace() == Reference::KindNamespace::mach_o);
+    if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
+      return false;
----------------
lhames wrote:
> echristo wrote:
> > Can you add a comment explaining how you could get here? That said, is the check worth it now? i.e. do we really need the early exit?
> This is consistent with how GOT references (which are handled very similarly) are checked. We may try to improve on it in the future for performance reasons, but I'm happy with it for now.
A comment to that effect in the code seems a totally reasonable thing. The very fact that this patch is necessary is evidence that it's not completely obvious.


Repository:
  rL LLVM

http://reviews.llvm.org/D16009





More information about the llvm-commits mailing list