[PATCH] D105210: [lld-macho] Ignore debug symbols for now.

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 1 18:44:24 PDT 2021


int3 added inline comments.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:154-164
+    InputSection *rFuncIsec = nullptr;
+    if (rFunc.referent.is<Symbol *>()) {
+      assert(rFunc.referent.get<Symbol *>()->kind() ==
+             Symbol::Kind::DefinedKind);
+      rFuncIsec = cast<Defined>(rFunc.referent.get<Symbol *>())->isec;
+    } else {
+      assert(r.offset ==
----------------
oontvoo wrote:
> int3 wrote:
> > this should be in a separate diff, and also needs a test :)
> Well, these are separate issue, yes, but without this change the link wouldn't succeed, ie., it'd trip on the assert on line 156 (on the LHS of the diff)
> IOWs, all  of these changes are necessary for the test to pass 
> 
> Since these are small enough, can you just let put them in one diff :) 
> 
could we not simplify bug_50812.o so that it doesn't trip the assert, then? I think it should be fairly straightforward if you convert it to YAML (or even just hand-craft a YAML test case from scratch) -- one of the advantages of yaml tests is that you can delete most of the extraneous stuff and just test the one code path you're interested in.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:156-157
+    if (rFunc.referent.is<Symbol *>()) {
+      assert(rFunc.referent.get<Symbol *>()->kind() ==
+             Symbol::Kind::DefinedKind);
+      rFuncIsec = cast<Defined>(rFunc.referent.get<Symbol *>())->isec;
----------------
this isn't necessary, `cast<>` does the assert in debug mode


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105210



More information about the llvm-commits mailing list