Don't filter out external Mach-O symbols

[nox]

Undefined symbols have their N_TYPE set to N_UNDF so they are already rejected by macho_defined_symbol.

See https://github.com/rust-lang/backtrace-rs/pull/299

[ianlancetaylor]

Can you show me a small test case that changes? I'd like to understand why the current testsuite didn't detect the problem. Thanks.

[nox]

AFAIK all your tests everywhere are about static C functions, which aren't N_EXT.

[nox]

Reading nm's man page on macOS:

If the symbol is local (non-external), the symbol's type is instead represented by the corresponding lowercase letter. A lower case u in a dynamic shared library indicates a undefined reference to a private external in another module in the same library.

So AFAIK it is indeed wrong to filter out external symbols here.

[nox]

Ok it is actually visible from current tests, except they fail whether or not this patch is in.

Without the patch, I get:

test1: [0]: missing file name or function name
test2: [0]: missing file name or function name
test3: [0]: missing file name or function name
test3: [2]: NULL syminfo name
test4: [0]: missing file name or function name
test5: NULL syminfo name
FAIL: backtrace_full noinline
FAIL: backtrace_full inline
FAIL: backtrace_simple noinline
FAIL: backtrace_simple inline
FAIL: backtrace_syminfo variable
FAIL dwarf5 (exit status: 1)

With the patch, I get:

test1: [0]: missing file name or function name
test2: [0]: missing file name or function name
test3: [0]: missing file name or function name
test3: [2]: unexpected syminfo name got main expected test3
test4: [0]: missing file name or function name
test5: unexpected syminfo size got 0 expected 4
FAIL: backtrace_full noinline
FAIL: backtrace_full inline
FAIL: backtrace_simple noinline
FAIL: backtrace_simple inline
FAIL: backtrace_syminfo variable
FAIL dwarf5 (exit status: 1)

Note how the NULL syminfo name failures go away.

[philipc]

The test3 syminfo failure with this patch is because test3 is compiled to a jump instead of a call, so it doesn't appear in the backtrace, and we get main instead.

The test5 failure is because Mach-O symbols don't have sizes.

The other failures are probably because ld doesn't support DWARF5. For btest, the failures with this patch are:

test1: [2]: got 489 expected 60
test1: [2]: got main expected test1
test3: [2]: got 491 expected 172
test3: [2]: got main expected test3
test3: [2]: unexpected syminfo name got main expected test3
test5: unexpected syminfo size got 0 expected 4
FAIL: backtrace_full noinline
PASS: backtrace_full inline
FAIL: backtrace_simple noinline
PASS: backtrace_simple inline
FAIL: backtrace_syminfo variable
FAIL btest (exit status: 1)

This lets us see that for test3 the DWARF agrees with the symbol table fix in this PR. And test1 is also compiled to a jump.

Note that if the tests were working as expected then none of the backtrace symbols would be external, so it's probably worth changing the tests to include a non-static function other than main.

[nox]

__attribute__((optnone)) is how you disable the jump optimisation in clang on macOS.

[nox]

@ianlancetaylor Are you satisfied by the current state of the patch? Is there anything I can do to help make it land?

[ianlancetaylor]

Sorry, I've just been busy. I'll get to this but at this point I don't know when.

[nox]

Fair! 😷

[cooljeanius]

I think this was addressed by 3fda5a8, i.e.: https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657095.html

[ianlancetaylor]

Yes, thanks. Sorry for forgetting about this pull request.