bugThe libunwind project - Bugs: bug #29672, debug_frame support for remote...

 
 

bug #29672: debug_frame support for remote unwinding

Submitted by:  None
Submitted on:  Mon 26 Apr 2010 07:13:55 AM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: None
Privacy: PublicAssigned to: None
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 28 May 2010 07:04:07 AM UTC, comment #13:

Great, thanks!

Verified the latest snapshot - working fine.

Andris Zeila <zeila>
Thu 27 May 2010 02:35:36 AM UTC, comment #12:

Patches applied.

Arun Sharma <aruns>
Project Administrator
Mon 17 May 2010 12:21:42 PM UTC, comment #11:

The applied patches visually look fine, though can't really test them without applying at least the first 6 debug frame related patches.

I updated the last three patches (libunwind-debug-frame-04.tar.gz) so they work with the latest sources.

The problem was because of int->long changes in src/dwarf/Gfind_proc_info-lsb.c:

Debug (15, "start_ip = %lx, end_ip = %lx\n",
(long) this_pi.start_ip, (long) this_pi.end_ip);

(plus some spaces replaced with tabs - i kept spaces in replacement code).

(file #20558)

Andris Zeila <zeila>
Sat 15 May 2010 07:21:35 PM UTC, comment #10:

I've applied 4 of your patches. Please verify that they look ok.

The rest don't seem to apply cleanly to the top of the tree.

$ stg import ../libunwind.test3/libunwind-debug-frame-03.tar.gz
Checking for changes in the working directory ... done
Importing patch "libunwind-debug-frame-03.tar" ... <stdin>:936: trailing whitespace.

<stdin>:940: trailing whitespace.

<stdin>:948: trailing whitespace.

error: patch failed: src/dwarf/Gfind_proc_info-lsb.c:583
error: src/dwarf/Gfind_proc_info-lsb.c: patch does not apply
stg import: Diff does not apply cleanly

More specifically:

  • Prefer comments in the patches themselves, rather than in the bug.
  • The errors above seem to be coming from *5.diff

git apply patches/libunwind-debug-frame-5.diff
patches/libunwind-debug-frame-5.diff:411: trailing whitespace.

patches/libunwind-debug-frame-5.diff:415: trailing whitespace.

patches/libunwind-debug-frame-5.diff:423: trailing whitespace.

error: patch failed: src/dwarf/Gfind_proc_info-lsb.c:583
error: src/dwarf/Gfind_proc_info-lsb.c: patch does not apply

Arun Sharma <aruns>
Project Administrator
Tue 11 May 2010 11:48:10 AM UTC, comment #9:

Here is a latest update. Fixed most of the commented issues. I'm planning to add ARM warning and namespace checking fixes later as a separate bug/patch.

Commit messages for the patches:

libunwind-debug-frame-1.diff:
-----------------------------
Adds missing register offsets for ARM, required by remote backtracing.

libunwind-debug-frame-2.diff:
-----------------------------
Current implementation fails to find separate debug symbols when .gnu_debuglink
is set to the same name of the target ELF basename (e.g. "libc.so.6" for
/lib/libc.so.6). This patch fixes this by ignoring separate debug files that
contain a .gnu_debuglink section.
It also fixes a small typo in a related Debug() line.

libunwind-debug-frame-3.diff:
-----------------------------
Adds path and pathlen arguments to tdep_get_elf_image(), used by
remote unwinding.

libunwind-debug-frame-4.diff:
-----------------------------
Adds ip argument to _UPTi_find_unwind_table, used by remote unwinding.

libunwind-debug-frame-5.diff:
-----------------------------
Factors out and exports dwarf_find_debug_frame() function.
Adds *dwarf_find_debug_frame symbol matching to check-namespace.sh.in script.
Updates configure.in to export enable_debug_frame setting used by check-namespace.sh.in
script.

libunwind-debug-frame-6.diff:
-----------------------------
Adds support for .debug_frame based unwinding in _UPTi_find_unwind_table() function.

libunwind-arm-frame-chain-1.diff:
---------------------------------
Adds an experimental frame chain unwinding for ARM targets, optionally configured by
UNW_ARM_UNWIND_METHOD (1 - debug_frame unwinding, 2 - frame chain unwinding,
3 - 1 & 2 (default)) environment variable.

(file #20508)

Andris Zeila <zeila>
Thu 06 May 2010 02:34:33 PM UTC, comment #8:

patches for other namespace and compiler warnings would be great. But I'd make them separate patches.

check-namespace.sh.in handles conditional namespace checks eg:

if [ x@enable_cxx_exceptions@ = xyes ]; then
check_cxx_abi
fi

perhaps you can do something similar for the debug frame checks.

On (5), yes you're right.

Arun Sharma <aruns>
Project Administrator
Thu 06 May 2010 02:17:46 PM UTC, comment #7:

About namespace check - it does fail on arm, but it does fail even without adding patches:
ERROR: Symbol "_ULarm_handle_signal_frame" missing.
ERROR: Extraneous symbols:
00001474 T _Uarm_get_elf_image
00036b78 A _bss_end_
00011604 A _bss_start_
000115ec D __data_start
00036b78 A _end_
00008e40 A __exidx_end
00008c20 A __exidx_start
00036b78 A bss_end_
ERROR: Symbol "_Uarm_handle_signal_frame" missing.
ERROR: Extraneous symbols:
00001450 T _Uarm_get_elf_image
0003be18 A _bss_end_
000168bc A _bss_start_
000168a4 D __data_start
0003be18 A _end_
0000e2a0 A __exidx_end
0000e070 A __exidx_start
0003be18 A bss_end_

I can hide unwi_unwind_method for ARM target, but dwarf_find_debug_frame is now used also by remote unwinding - it can't be hidden, can it?

Should i provide a patch for check-namespace.sh.in with ARM and debug_frame updates?

While i'm at it - I had few compilation warnings for ARM target. Should I make a patch to fix them?

Andris Zeila <zeila>
Thu 06 May 2010 11:54:16 AM UTC, comment #6:

2,3 Right. Initially unwind_method was meant as global setting. Only before posting the patch here i refactored it as ARM only setting (as it was not really used by the other targets). Will fix it.

3. Will check run-check-namespaces on ARM and fix it.

4. I'll prepare commit messages for patches. Unfortunately they aren't stored into any accessible repository.

5. Erm, i'm quite sure i moved (tried to?) it under CONFIG_DEBUG_FRAME define. Will recheck it.

6. Hmm, beats me. I'm not the author of the original debug_frame patches posted 1.5 years ago, I just updated them to work with the latest libunwind snapshot for integration purposes - so i could be missing things (though i have to take full blame for the last patch - ARM frame chain unwinding :)).
The #ifdef does seem to be unnecessary.

Andris Zeila <zeila>
Wed 05 May 2010 07:13:26 PM UTC, comment #5:

1. Coding style is GNU with spaces

Some patches with tabs went into the repo, but they should be treated as
bugs and hopefully will be fixed over time.

2. Why not put this in tdep/libunwind_i.h?

--- libunwind.orig/include/libunwind_i.h 2010-04-14 22:11:29.000000000 +0300
+++ libunwind/include/libunwind_i.h 2010-05-03 13:13:30.000000000 +0300
@@ -301,6 +301,18 @@
size_t size; /* (file-) size of the image */
};

+/* unwinding method selection support */
+#define UNW_ARM_METHOD_ALL 0xFF
+#define UNW_ARM_METHOD_DWARF 0x01
+#define UNW_ARM_METHOD_FRAME 0x02
+
+#if UNW_TARGET_ARM
+#define unwi_unwind_method UNWI_ARCH_OBJ(unwind_method)
+extern int unwi_unwind_method;
+#endif
+
+#define UNW_TRY_METHOD(x) (unwi_unwind_method & x)
+
#include "tdep/libunwind_i.h"

3. This code really belongs in src/arm/Ginit.c

--- libunwind.orig/src/mi/init.c 2010-04-14 22:11:29.000000000 +0300
+++ libunwind/src/mi/init.c 2010-05-03 13:13:30.000000000 +0300
@@ -40,6 +40,11 @@

#endif /* UNW_DEBUG */

+#if UNW_TARGET_ARM
+/* Unwinding methods to use. See UNW_METHOD_ enums */
+int unwi_unwind_method = UNW_ARM_METHOD_ALL;
+#endif
+
HIDDEN void
mi_init (void)
{
@@ -56,5 +61,15 @@
}
#endif

+#if UNW_TARGET_ARM
+ {
+ const char* str = getenv ("UNW_ARM_UNWIND_METHOD");
+ if (str)
+ {
+ unwi_unwind_method = atoi (str);
+ }
+ }
+#endif
+

3. Namespace issues

I think run-check-namespaces will still fail on ARM. Please use static scope
or HIDDEN to limit symbol visibility.

4. Please add a commit message to each of the patches.

Giving me a mbox of patches or pointing me at a git tree I can pull from
will speed up the review.

5. dwarf_find_debug_frame could you move it under CONFIG_DEBUG_FRAME?

6. ia64 doesn't use dwarf. Why is this ifdef needed?

--- libunwind.orig/src/dwarf/Gparser.c 2010-04-14 22:11:29.000000000 +0300
+++ libunwind/src/dwarf/Gparser.c 2010-05-03 13:10:08.000000000 +0300
@@ -73,6 +73,14 @@

as = c->as;
arg = c->as_arg;
+#ifndef UNW_TARGET_IA64
+ if (c->pi.flags & UNW_PI_FLAG_DEBUG_FRAME)
+ {
+ /* .debug_frame CFI is stored in local address space. */
+ as = unw_local_addr_space;
+ arg = NULL;
+ }
+#endif
a = unw_get_accessors (as);
curr_ip = c->pi.start_ip;

Arun Sharma <aruns>
Project Administrator
Mon 03 May 2010 10:57:36 AM UTC, comment #4:

Updated patches according to the comments:
1) Fixed unwi_unwind_method variable declaration and moved it and all related code under UNW_TARGET_ARM define. As it only affects ARM unwinding (at least currently), it doesn't make sense to be exported for all targets.

2) Rechecked patches for whitespace issues. In some places the original code had some peculiar mix of tabs and spaces. I wonder what is the indentation policy for libunwind project? It seems to be GNU style with spaces?

3) Fixed fread related warnings in load_debug_frame function.

(file #20402)

Andris Zeila <zeila>
Sun 02 May 2010 05:47:08 AM UTC, comment #3:

I have several instances of this warning:

warning: ignoring return value of fread, declared with attribute warn_unused_result

in the debug_frame related code. It'd be nice to fix these as well.

Arun Sharma <aruns>
Project Administrator
Sun 02 May 2010 05:45:22 AM UTC, comment #2:

Applying the patch introduces new test failures in run-check-namespaces

-e ERROR: Extraneous symbols:
0000000000007030 T _ULx86_64_dwarf_find_debug_frame
000000000020a140 D unwi_unwind_method

Could you please submit a patch to fix these failures? Don't think unwi_unwind_method needs to be a globally visible symbol.

In the future, please make sure that the patches apply with patch -p1 and free of whitespace issues (spaces before tabs etc).

Arun Sharma <aruns>
Project Administrator
Tue 27 Apr 2010 08:10:03 AM UTC, comment #1:

Few fixes:
1) Moved debug_frame related functionality (namely the exported dwarf_find_debug_frame function and it's call) under CONFIG_DEBUG_FRAME define
2) removed UNW_TARGET_ARM_ define (leftover from the initial patch)
3) removed whitespace changes form patch 6
4) renamed patch 7 (it's not really related to the debug_frame changes).

(file #20349)

Andris Zeila <zeila>
Mon 26 Apr 2010 07:13:55 AM UTC, original submission:

I'd like to resurrect the .debug_frame based remote unwinding support proposed with a series of patches quite a time ago http://lists.nongnu.org/archive/html/libunwind-devel/2008-07/msg00002.html

This is required to get remote unwinding working for ARM targets which is used in tools like functracer (http://wiki.maemo.org/Documentation/devtools/maemo5/functracer).

The attached archive contains the original set of the patches updated to work with the 19.04.2010 libunwind snapshot plus an experimental patch to introduce ARM unwinding based on frame chain following.

The original mailing list post contains notes about the first 6 patches, so i'll comment only the last one.

By default it will make libunwind to try frame chain based unwinding for ARM if the debug_frame unwinding fails. While debug_frame unwinding is much slower (and requires debug information), it's still more reliable (although the frame chain unwinding worked for us normally). Therefore a way to specify the ARM unwinding method was also added to this patch.

The UNW_ARM_UNWIND_METHOD environment variable controls the unwinding method used for ARM targets:
1 - debug_frame based unwinding (slow but reliable)
2 - frame chain based unwinding (faster, but not tested as much)
3 - first try (1) and switch to (2) only if the first failed.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #20558:  libunwind-debug-frame-04.tar.gz added by zeila (6KiB - application/x-gzip)
file #20508:  libunwind-debug-frame-03.tar.gz added by zeila (8KiB - application/x-gzip)
file #20402:  libunwind-debug-frame-02.tar.gz added by zeila (8KiB - application/x-gzip)
file #20349:  libunwind-debug-frame-01.tar.gz added by zeila (8KiB - application/x-gzip)
file #20333:  libunwind-debug-frame.tar.gz added by None (8KiB - application/x-gzip)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by aruns (Posted a comment)
  • -unavailable- added by zeila (Updated the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 27 May 2010 02:35:36 AM UTCarunsOpen/ClosedOpen=>Closed
    Mon 17 May 2010 12:21:42 PM UTCzeilaAttached File-=>Added libunwind-debug-frame-04.tar.gz, #20558
    Tue 11 May 2010 11:48:10 AM UTCzeilaAttached File-=>Added libunwind-debug-frame-03.tar.gz, #20508
    Mon 03 May 2010 10:57:36 AM UTCzeilaAttached File-=>Added libunwind-debug-frame-02.tar.gz, #20402
    Tue 27 Apr 2010 08:10:03 AM UTCzeilaAttached File-=>Added libunwind-debug-frame-01.tar.gz, #20349
    Mon 26 Apr 2010 07:13:56 AM UTCNoneAttached File-=>Added libunwind-debug-frame.tar.gz, #20333

    Back to the top


    Powered by Savane 3.1-cleanup1