[Ltrace-devel] [PATCH] Fix libunwind support for MIPS

[Petr Machata]

Faraz Shahbazker <faraz.shahbazker at imgtec.com> writes:

> On 10/22/2015 02:43 AM, Vicente Olivert Riera wrote:
>> ping
>>
>> On 10/08/2015 04:59 PM, Vicente Olivert Riera wrote:
>>> /* Verify that we can safely cast arch_addr_t* to unw_word_t*. */
>>> (void)sizeof(char[1 - 2*(sizeof(unw_word_t) != sizeof(arch_addr_t))]);
>>>
>>> This check will always fail for MIPS 32-bit architectures (the only ones
>>> supported by ltrace) because unw_word_t is 64-bit sized (it's actually a
>>> uint64_t) and arch_add_t (which is void*) is 32-bit sized:
>
> As I understand, that check was deliberately designed to fail at compile time,
> where as your proposal defers it to run time. So one must ask under what
> conditions would the deferral and the related run-time overhead be justified?

I didn't check, but it looks as if on the arches where the compile-time
check would pass in the first place, gcc should be able to figure out
that the assert always passes and not emit any actual code for it.

> If MIPS 32-bit requires a cast-and-compare approach, then we would be
> better off restricting the change to MIPS 32-bit. You have to assume
> that the check is not relevant for mips32 and introduce an
> arch-specific UNW_2_ADDR_T macro which is just a cast in general, but
> calls the conversion+verification function for mips32. Alternatively
> (if the consensus here allows it), you could just assume that the
> check is not valid for mips32 but do it in a way that doesn't affect
> any other architecture.

This would work for me in general, though as I said--is it an actual
problem?  The proliferation of per-arch config tweaks is something
to avoid if possible.

Vincente, would you be able to look at the generated assembly on
e.g. x86_64?  Do one with your proposed patch, and then the same code
with the assert removed, and see if GCC is smart enough to realize the
assert is actually a NOP.

Thanks,
Petr