Re: [Libunwind-devel] [PATCH] arm: ptrace: Fix order of probing unwind tables

[Yichao Yu]

> As expected, your patchset regressed libunwind to the state when unw_step()
> fails to unwind a function that has [cantunwind] entry in ARM EXTBL and
> good DWARF info. Generally, this is because of your modification in
> unw_step() (src/arm/Gstep.c) that made arm_exidx_step() to be done first
> before dwarf_step().

Sounds like the fallback does not handle that very well. Seems like
the `ret == -UNW_ESTOPUNWIND` check should be removed. Does that make
the fallback works?

> No matter whether Ken Werner was right or not, he was the first who declared
> it, his patch was applied to libunwind and did not cause any problems to other
> people. When you contradict with any changes done before your work, it is very
> important for you to provide any proof that someone (e.g. Ken Werner) was not
> completely correct.
> That is what I mean.

I agree that there shouldn't be regression and if changing the logic
in the error checking patch fixes it, I still think it's the right way
to go for unwinding.
My proof would be that "more powerful" is not the right metric for
unwinding, reliability is.
My claim is that as long as the extbl info is present and is not
CANT_UNWIND, it has to be correct since c++ relies on it for exception
handling and not just backtrace/debugging. Or in another word, a bug
in the extbl unwind info is a much serious compiler bug than a bug in
the debug info.
For info's that are not present in it or for anything other than
unwinding, the "more powerful" argument would apply.

>> This is for unwinding of functions dumped to a shared object from a
>> LLVM JIT. We generate DWARF 4 debug info which does not make libunwind
>> too happy and I had to disable a DWARF version check so that could be
>> why the debug info unwinding fails sometimes. It could also be that
>> the debug info generated by LLVM is not accurate.
>
> It sounds like there is a bug in DWARF v4 parsing, but instead of
> debugging and fixing it, you just added "smart" version of
> UNW_ARM_UNWIND_METHOD environment variable.

Again, I totally agree that your use case (or any other cases that has
working unwinding with debug info) and I'm sorry that I didn't realize
that unwinding is aborted if a function has CANT_UNWIND so I think
that should be fixed and there shouldn't be regression with debug info
unwinding anymore.

> May be you can not use UNW_ARM_UNWIND_METHOD for some reason, ok.
> Neither me can. Some of the functions in our example do not have DWARF info,
> but present in ARM EXTBL.
>
>
>> For lookup up IP-to-function mapping, I'll definitely **NOT** use
>> EXTBL.
>
> You would not, but arm_exidx_step() actually does.

`arm_exidx_step` does not need to map ip to function, it only needs to
restore register values (unwinding).
IP to function is done in, e.g., unw_get_proc_info_by_ip etc and I
certainly wouldn't prefer getting extbl info for that. This is also
precisely why I added the flag instead of changing the default order
as in 92327a3. I believe the extbl first order only makes sense for
unwinding and not for any other users of the function.

>
>
> Regards,
> Vitaly.
>
> _______________________________________________
> Libunwind-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/libunwind-devel