Re: [Libunwind-devel] [PATCHv3 1/4] Add core dump unwinding support to libunwind

[Denys Vlasenko]

On 03/13/2012 04:26 AM, Arun Sharma wrote:
> I copied your patches + some build fixes for x64 to:
>
> https://github.com/adsharma/libunwind/commits/coredump2
>
> Please verify that everything still works on your end.

Checked out, built it, installed, built my program against it,
and tested that it still works. Thanks!


> Some loose ends
> to be tied up before I can push to libunwind HEAD:
>
> * tests/test-coredump-unwind doesn't test anything right now. An
> actual test that fails if future commits break things would be great.
> This might require a test core dump file for every platform you care
> about.

I think I can create an actual working test for you based on my
other program. Give me a few days...


> * _UCD_libs.h seems to have a different license. Please let me know if
> relicensing is possible.

Sure. It's a trivial file, I don't mind if you use
whatever license you like on it :)


> * I suspect ia64 is broken by 2.patch
>
> $ grep -i ia64 src/dwarf/Gfind_unwind_table.c
> #if UNW_TARGET_IA64
>
> ia64 is not a dwarf platform and shouldn't be  using stuff under
> src/dwarf. Please send me an incremental diff to move that code back
> to the original location.

Patch 2 moves ptrace-independent code from src/ptrace/_UPT_find_proc_info.c
to src/dwarf/{G,L}find_unwind_table.c.

Before my patch set:
the code under "#if UNW_TARGET_IA64" consists of IA64-specific
implementation of _UPTi_find_unwind_table() function.
This function is called from get_unwind_info() function.

I need to make it available for use by the analogous get_unwind_info()
function in src/coredump/_UCD_find_proc_info.c file.

Meaning that I would rather use a common code for that than
duplicate it in src/ptrace/* and src/coredump/*.

You say that it's wrong to call it *dwarf*_find_unwind_table
and put it into src/dwarf/*.
I'm ok with it.
Where do you think is the best place to have it?
I'll move it there.

--
vda