[PATCH] D107533: [lld-macho] Handle encoding personalities of same names but different kinds

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:195
+        //
+        // (See discussions/alternatives already  considered on D107533)
+        if (!defined->isExternal())
----------------
int3 wrote:
> 
 🧐 🧐 🧐


================
Comment at: lld/MachO/UnwindInfoSection.cpp:200
+      }
       if (auto *undefined = dyn_cast<Undefined>(s)) {
         treatUndefinedSymbol(*undefined);
----------------
int3 wrote:
> `symtab->find` will never return an Undefined, so we can use `else if` here
Ah, no actually - it *can* find an undefined (dylib) symbol. (The third testcase (`c.out`) covers this).


================
Comment at: lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s:17
+## check that we can link with 4 personalities without crashing:
+## ___gxx_personality_v0 (libc++.tbd) ___gxx_personality_v0(local), _personality_1 and _personality_2
+# RUN: %lld -pie  -lSystem -lc++  %t/user_2.o %t/combined.o -o %t/a.out
----------------
int3 wrote:
> let's try to be consistent with commas and spaces :) (same for the lines below)
(the 3 RUNs were consistent with each other tho)


================
Comment at: lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s:35
+# A-NEXT: address                    index name
+# A-DAG:  0x[[#%x,GXX_PERSONALITY_LOW:]] [[#]] ___gxx_personality_v0
+# A-DAG:  0x[[#%x,GXX_PERSONALITY_HI:]]  [[#]] ___gxx_personality_v0        
----------------
int3 wrote:
> suggestion: instead of `_LOW` and `_HI`, how about `GXX_PERSONALITY_DYLIB` and `GXX_PERSONALITY`?
But it's not the _DYLIB that's picked in b.out and c.out . so calling it that is a bit mis-leading, I think.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107533