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

[grosbach]

Summary

Early exit identified as not being a TLV access is correct. In particular
the 'all' namespace is completely valid here for things like layout-after
references.

[echristo]

lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp

63

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?

[lhames]

LGTM. Thanks Jim!

lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp

63

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.

[echristo]

Whatever.

[grosbach]

lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp

63

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.

[lhames]

lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp

63

There are two things going on here:

1. When you check the kindValue you need to interpret it in the context of the kindNamespace and kindArch. We should have something to encapsulate that check so that it can't be accidentally dropped (causing another bug like this). There may already be something like this, I haven't gone looking for it yet.

2. I think ripRel32Tlv is generic, but currently its namespace and arch are getting set to mach_o x86_64 . They should probably be set to all/all instead (though I'll have to sanity check that), in which case we wouldn't need to query the ArchHandler about this at all.

Given how common this kind of check is, commenting this here probably isn't the right option. I'll file bugs to look into (1) and (2) instead.

[lhames]

lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp

63

For reference (though unfortunately it'll be primarily useful only to Apple people) I've filed rdar://problem/24119407 to track the issues above.

lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp

   }
 
   bool isTLVAccess(const Reference &ref) const override {
-    assert(ref.kindNamespace() == Reference::KindNamespace::mach_o);
+    if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
+      return false;
     assert(ref.kindArch() == Reference::KindArch::x86_64);
     return ref.kindValue() == ripRel32Tlv;
   }

test/mach-o/tlv-refkind.yaml

+# RUN: lld -flavor darwin -arch x86_64  %s %p/Inputs/libSystem.yaml \
+# RUN: -o %t
+# RUN: llvm-nm -m -n %t | FileCheck %s
+#
+#
+# Test that x86_64 checking for TLV on local symbols properly accounts for
+# references not in the mach_o:: namespace. Specifically in this case,
+# all::layout_after references.
+#
+--- !mach-o
+arch:            x86_64
+file-type:       MH_OBJECT
+flags:           [  ]
+compat-version:  0.0
+current-version: 0.0
+has-UUID:        false
+OS:              unknown
+sections:
+  - segment:         __TEXT
+    section:         __text
+    type:            S_REGULAR
+    attributes:      [ S_ATTR_PURE_INSTRUCTIONS, S_ATTR_SOME_INSTRUCTIONS ]
+    address:         0x0000000000000000
+    content:         [ 0xC3 ]
+  - segment:         __DATA
+    section:         __data
+    type:            S_REGULAR
+    attributes:      [  ]
+    address:         0x0000000000000001
+    content:         [ 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00 ]
+local-symbols:
+  - name:            l.blah
+    type:            N_SECT
+    sect:            2
+    value:           0x0000000000000001
+  - name:            l.blaz
+    type:            N_SECT
+    sect:            2
+    value:           0x0000000000000005
+global-symbols:
+  - name:            _foo
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            2
+    value:           0x0000000000000001
+  - name:            _main
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            1
+    value:           0x0000000000000000
+page-size:       0x00000000
+...
+
+
+# CHECK: (undefined) external dyld_stub_binder (from libSystem)
+# CHECK-NEXT: external _main