[PATCH] D78888: [llvm-objcopy][MachO] Fix symbol table

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D78888#2007119 <https://reviews.llvm.org/D78888#2007119>, @alexshap wrote:

> @jhenderson - as i mentioned above in the summary of the diff (and in the comment as well)  (see also https://developer.apple.com/documentation/kernel/nlist_64/1583944-n_type?language=objc) for STABs the value of n_type is interpreted differently, in particular, those bitmasks (N_EXT/N_PEXT) are meaningless (to the best of my knowledge, unless I'm missing smth).


Yeah, I got that bit, sorry for not being clear. The reason I'm asking about it is because the `isExternalSymbol()` code checks that it is not a STAB and has N_EXT. That means we have a logical matrix of 4 code paths in this function: "not STAB, not N_EXT", "STAB, not N_EXT", "not STAB, N_EXT" and "STAB, N_EXT". It's possible due to the usage of the function (I haven't looked heavily at it), that the last of these (the one that isn't tested) can't actually be exercised. Of course, C++ lazy evaluation means that in its current state, only the STAB part is tested when it is a STAB, but it would be just as correct to rewrite the code as `return n_type & MachO::N_EXT && !isSTAB();`, so we should test that case (assuming we can get into this code at all in that situation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78888