Updated patch with fallback fixed here https://github.com/yuyichao/libunwind/commit/832b7644268cb11b1170729200309706ddf91f50
On Fri, Jan 20, 2017 at 3:40 PM, Yichao Yu <yyc1...@gmail.com> wrote: >> 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 >> Libunwind-devel@nongnu.org >> https://lists.nongnu.org/mailman/listinfo/libunwind-devel _______________________________________________ Libunwind-devel mailing list Libunwind-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/libunwind-devel
