Update libbacktrace to latest master

[ghost]

See rust-lang/libbacktrace#1

[alexcrichton]

Ok I did a bit of debugging locally to figure out what's going on there. The failing test case here is testing the accuracy of backtrace symbolization without debuginfo, where we still want at least some sort of human readable names. Something is coming out but not every symbol is being resolved when it should be.

The previous iteration of libbacktrace passed this test because it never actually called the callback in backtrace_syminfo, triggering a fallthrough to call dladdr instead. It looks like now backtrace_syminfo is indeed being called with the callback, only it's less useful than before.

Honestly I'm not actually entirely sure about the relationship between dladdr, backtrace_syminfo, and backtrace_pcinfo. My general understanding is that backtrace_pcinfo only works if there's dwraf, and dladdr and backtrace_syminfo are otherwise pretty equiavlent. I'm not sure if that's actually true in all the implementations today. We may be able to fix this by removing this fallback and forcing a fallback to dladdr instead. I'm not really sure if backtrace_syminfo will ever give more accurate information than dladdr. In this case at least dladdr does seem a bit more accurate.

[alexcrichton]

Ok I dug in some more. Applying this patch confirms that the symtab entry, which I believe is all that dladdr consults, is indeed correct and we can symbolize purely based on the symbol table. I believe this is what libbacktrace is trying to do, but presumably there's a bug in libbacktrace in parsing and/or managing the symbol table.

[philipc]

From a quick look, it is skipping over externally visible symbols, which seems wrong to me.

[alexcrichton]

Oh nice, thanks @philipc! Indeed deleting those two lines gets the test passing. @t6 would you be willing to try to get that patch into upstream libbacktrace?

[nox]

@alexcrichton Seems like the N_EXT condition should be replaced by N_UNDF, given the function is named macho_defined_symbol.

https://developer.apple.com/documentation/kernel/nlist_64/1583944-n_type?language=objc

[nox]

Ah never mind, it's just a N_TYPE and that's checked immediately afterwards in the switch statement so all that is needed is removing the N_EXT check indeed.

[nox]

@alexcrichton Do you think we could update libbacktrace with local patches instead of waiting for upstream? There is no ETA as to when they will reply.

ianlancetaylor/libbacktrace#34 (comment)

[alexcrichton]

Oops sorry didn't see that this had made progress! Thanks for the ping @nox.

Thanks for pushing this through @t6!

[nox]

You're welcome @alexcrichton. Can't wait for a new release! That will allow us to remove a [patch] in our top-level Cargo.toml file in Servo, which is nice.

[alexcrichton]

Ok I've published new releases with these updates as well.