From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42172 invoked by alias); 7 Apr 2017 10:27:36 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 42149 invoked by uid 89); 7 Apr 2017 10:27:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.99.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy=alter, trustworthy, H*M:162, ESP X-Spam-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: gnu.wildebeest.org Message-ID: <1491560851.8380.162.camel@klomp.org> Subject: Re: frame unwinding patches From: Mark Wielaard To: Ulf Hermann Cc: elfutils-devel@sourceware.org Date: Fri, 07 Apr 2017 10:27:00 -0000 In-Reply-To: <20170403211516.GB9584@stream> References: <1487201610-8381-1-git-send-email-mark@klomp.org> <3915502.JGE1jdPxOT@milian-kdab2> <75e83a7d-b372-3436-ba7a-3a49900e92dd@qt.io> <20170403211516.GB9584@stream> Content-Type: multipart/mixed; boundary="=-RvxwUIodIcoFX+2g+Fir" X-Mailer: Evolution 3.12.11 (3.12.11-22.el7) Mime-Version: 1.0 X-SW-Source: 2017-q2/txt/msg00030.txt.bz2 --=-RvxwUIodIcoFX+2g+Fir Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-length: 4707 On Mon, 2017-04-03 at 23:15 +0200, Mark Wielaard wrote: > On Mon, Apr 03, 2017 at 11:02:53AM +0200, Ulf Hermann wrote: > > > Ping? Any progress on merging this functionality upstream? > > > It can make quite a difference in unwinding. > >=20 > > The patches have also been in perfparser releases for over a year now. I > > would like to see them upstream. >=20 > Yes, sorry. The patches looked fine. Then I thought I should try > adding similar support to another arch to double check. And then > I got distracted by something else. I'll get back to is asap and > make sure they get in before the next release end of the month. OK. Sorry for the delay. In general I like these patches. I even made a i386 fp unwind backend function to play with the idea a bit more. See attached. Does that look sane? I do agree with Jan that frame pointer unwinding is notoriously untrustworthy. Even with some sanity checks it is hard to know whether you are doing a correct unwind. gdb gets away with it through pretty advanced frame sniffers, which take a lot of low-level compiler generation knowledge to get correct (and even then...). You only restore the pc, stack and frame pointer registers (maybe incorrectly). So afterwards, even if you got valid CFI data you might be stuck. Ideally we provide the user with a flag to check how trustworthy a frame is and/or an option to drop inexact unwinding completely. But that would be an extension for another time. I do think that in the context of dwfl_thread_getframes () it is appropriate. Currently all you can get from a Dwfl_Frame is the pc anyway. I do however have a problem with the testcases. I approved the --allow-unknown option for the backtrace fp core tests (commit f9971cb) but I now believe that was a mistake. It makes it really hard to see whether the test actually tests anything. In fact for my i386 case the testcase even succeeded when disabling the whole frame pointer unwinder fallback... I believe the issue with the missing symbol names is not caused by the frame pointer unwinding not matching real function addresses, because they should! But because of the way the testcase binaries are generated. What you do is the following: # gcc -static -O2 -fno-omit-frame-pointer -I ../ -I ../lib/ -D_GNU_SOURCE # -pthread -o backtrace.x86_64.fp.exec backtrace-child.c # # Then strip the .eh_frame and .eh_frame_hdr sections from the binary: # # strip -R .eh_frame backtrace.x86_64.fp.exec # strip -R .eh_frame_hdr backtrace.x86_64.fp.exec # # The core is generated by calling the binary with --gencore But the .eh_frame sections are allocated sections. Which means that by removing them strip will alter the address layout of the program. Which causes the symbol table addresses to not match up anymore. I think they should be generated with -fno-asynchronous-unwind-tables to avoid them being generated in the first place (in the main binary functions, but keep those from the static library functions pulled in): # gcc -static -O2 -fno-omit-frame-pointer -fno-asynchronous-unwind-tables \ # -D_GNU_SOURCE -pthread -o tests/backtrace.i386.fp.exec -I. -Ilib \ # tests/backtrace-child.c That way the difference between having a frame pointer unwinder or not is much clearer: $ eu-stack --exec ./backtrace.i386.fp.exec --core ./backtrace.i386.fp.core PID 12044 - core TID 12045: #0 0x008a7416 __kernel_vsyscall #1 0x08051ab9 raise #2 0x080485c1 sigusr2 eu-stack: dwfl_thread_getframes tid 12045 at 0x80485c0 in [exe]: No DWARF i= nformation found TID 12044: #0 0x008a7416 __kernel_vsyscall #1 0x0804ae30 pthread_join #2 0x0804847c main eu-stack: dwfl_thread_getframes tid 12044 at 0x804847b in [exe]: No DWARF i= nformation found $ LD_LIBRARY_PATH=3Dbackends:libelf:libdw src/stack --exec ./backtrace.i386= .fp.exec --core ./backtrace.i386.fp.core PID 12044 - core TID 12045: #0 0x008a7416 __kernel_vsyscall #1 0x08051ab9 raise #2 0x080485c1 sigusr2 #3 0x08048699 stdarg #4 0x08048702 backtracegen #5 0x0804871b start #6 0x0804a7cf start_thread #7 0x080746fe __clone TID 12044: #0 0x008a7416 __kernel_vsyscall #1 0x0804ae30 pthread_join #2 0x0804847c main #3 0x08053188 __libc_start_main #4 0x080481e1 _start src/stack: dwfl_thread_getframes tid 12044 at 0x80481e0 in [exe]: No DWARF = information found Then if we check for one of the function names in the middle of the frame pointer only unwound frames, like backtracegen, this would give us a much better test of whether things worked correctly. Could you try regenerating your test binaries with -fno-asynchronous-unwind-tables? Then I'll try to adapt the testsuite to check for the one of the frame pointer only unwound function names. Thanks, Mark --=-RvxwUIodIcoFX+2g+Fir Content-Disposition: inline; filename="i386_unwind.c" Content-Type: text/x-csrc; name="i386_unwind.c"; charset="UTF-8" Content-Transfer-Encoding: base64 Content-length: 3473 LyogR2V0IHByZXZpb3VzIGZyYW1lIHN0YXRlIGZvciBhbiBleGlzdGluZyBm cmFtZSBzdGF0ZSB1c2luZyBmcmFtZSBwb2ludGVycy4KICAgQ29weXJpZ2h0 IChDKSAyMDE3IFJlZCBIYXQsIEluYy4KICAgVGhpcyBmaWxlIGlzIHBhcnQg b2YgZWxmdXRpbHMuCgogICBUaGlzIGZpbGUgaXMgZnJlZSBzb2Z0d2FyZTsg eW91IGNhbiByZWRpc3RyaWJ1dGUgaXQgYW5kL29yIG1vZGlmeQogICBpdCB1 bmRlciB0aGUgdGVybXMgb2YgZWl0aGVyCgogICAgICogdGhlIEdOVSBMZXNz ZXIgR2VuZXJhbCBQdWJsaWMgTGljZW5zZSBhcyBwdWJsaXNoZWQgYnkgdGhl IEZyZWUKICAgICAgIFNvZnR3YXJlIEZvdW5kYXRpb247IGVpdGhlciB2ZXJz aW9uIDMgb2YgdGhlIExpY2Vuc2UsIG9yIChhdAogICAgICAgeW91ciBvcHRp b24pIGFueSBsYXRlciB2ZXJzaW9uCgogICBvcgoKICAgICAqIHRoZSBHTlUg R2VuZXJhbCBQdWJsaWMgTGljZW5zZSBhcyBwdWJsaXNoZWQgYnkgdGhlIEZy ZWUKICAgICAgIFNvZnR3YXJlIEZvdW5kYXRpb247IGVpdGhlciB2ZXJzaW9u IDIgb2YgdGhlIExpY2Vuc2UsIG9yIChhdAogICAgICAgeW91ciBvcHRpb24p IGFueSBsYXRlciB2ZXJzaW9uCgogICBvciBib3RoIGluIHBhcmFsbGVsLCBh cyBoZXJlLgoKICAgZWxmdXRpbHMgaXMgZGlzdHJpYnV0ZWQgaW4gdGhlIGhv cGUgdGhhdCBpdCB3aWxsIGJlIHVzZWZ1bCwgYnV0CiAgIFdJVEhPVVQgQU5Z IFdBUlJBTlRZOyB3aXRob3V0IGV2ZW4gdGhlIGltcGxpZWQgd2FycmFudHkg b2YKICAgTUVSQ0hBTlRBQklMSVRZIG9yIEZJVE5FU1MgRk9SIEEgUEFSVElD VUxBUiBQVVJQT1NFLiAgU2VlIHRoZSBHTlUKICAgR2VuZXJhbCBQdWJsaWMg TGljZW5zZSBmb3IgbW9yZSBkZXRhaWxzLgoKICAgWW91IHNob3VsZCBoYXZl IHJlY2VpdmVkIGNvcGllcyBvZiB0aGUgR05VIEdlbmVyYWwgUHVibGljIExp Y2Vuc2UgYW5kCiAgIHRoZSBHTlUgTGVzc2VyIEdlbmVyYWwgUHVibGljIExp Y2Vuc2UgYWxvbmcgd2l0aCB0aGlzIHByb2dyYW0uICBJZgogICBub3QsIHNl ZSA8aHR0cDovL3d3dy5nbnUub3JnL2xpY2Vuc2VzLz4uICAqLwoKI2lmZGVm IEhBVkVfQ09ORklHX0gKIyBpbmNsdWRlIDxjb25maWcuaD4KI2VuZGlmCgoj aW5jbHVkZSA8c3RkbGliLmg+CiNpbmNsdWRlIDxhc3NlcnQuaD4KCiNkZWZp bmUgQkFDS0VORCBpMzg2XwojaW5jbHVkZSAibGliZWJsX0NQVS5oIgoKLyog UmVnaXN0ZXIgbnVtYmVycyBmb3IgZnJhbWUgYW5kIHN0YWNrIHBvaW50ZXJz LiAgV2UgdGFrZSBhZHZhbnRhZ2Ugb2YKICAgdGhlbSBiZWluZyBuZXh0IHRv IGVhY2ggb3RoZXIgd2hlbiBjYWxsaW5nIGdldGZ1bmMgYW5kIHNldGZ1bmMu ICAqLwojZGVmaW5lIEVTUCA0CiNkZWZpbmUgRUJQIChFU1AgKyAxKQoKLyog TW9zdCBiYXNpYyBmcmFtZSBwb2ludGVyIGNoYXNpbmcgd2l0aCBFQlAgYXMg ZnJhbWUgcG9pbnRlci4KICAgUEMgPSAqKEZQICsgNCksIFNQID0gRlAgKyA4 LCBGUCA9ICpGUC4gICovCmJvb2wKaTM4Nl91bndpbmQgKEVibCAqZWJsIF9f YXR0cmlidXRlX18gKCh1bnVzZWQpKSwKCSAgICAgRHdhcmZfQWRkciBwYyBf X2F0dHJpYnV0ZV9fICgodW51c2VkKSksCgkgICAgIGVibF90aWRfcmVnaXN0 ZXJzX3QgKnNldGZ1bmMsIGVibF90aWRfcmVnaXN0ZXJzX2dldF90ICpnZXRm dW5jLAoJICAgICBlYmxfcGlkX21lbW9yeV9yZWFkX3QgKnJlYWRmdW5jLCB2 b2lkICphcmcsCgkgICAgIGJvb2wgKnNpZ25hbF9mcmFtZXAgX19hdHRyaWJ1 dGVfXyAoKHVudXNlZCkpKQp7CiAgLyogc3AgPSAwLCBmcCA9IDEgKi8KICBE d2FyZl9Xb3JkIHJlZ3NbMl07CgogIC8qIEdldCBjdXJyZW50IHN0YWNrIGFu ZCBmcmFtZSBwb2ludGVycy4gICovCiAgaWYgKCEgZ2V0ZnVuYyAoRVNQLCAy LCByZWdzLCBhcmcpKQogICAgcmV0dXJuIGZhbHNlOwoKICBEd2FyZl9Xb3Jk IHNwID0gcmVnc1swXTsKICBEd2FyZl9Xb3JkIGZwID0gcmVnc1sxXTsKCiAg LyogU2FuaXR5IGNoZWNrLiAgV2Ugb25seSBzdXBwb3J0IHRyYWRpdGlvbmFs IHN0YWNrIGZyYW1lcy4gICovCiAgaWYgKGZwID09IDAgfHwgc3AgPT0gMCB8 fCBmcCA8IHNwKQogICAgcmV0dXJuIGZhbHNlOwoKICAvKiBHZXQgdGhlIHJl dHVybiBhZGRyZXNzIGZyb20gdGhlIHN0YWNrLCBpdCBpcyBvdXIgbmV3IHBj LiAgKi8KICBEd2FyZl9Xb3JkIHJldF9hZGRyOwogIGlmICghIHJlYWRmdW5j IChmcCArIDQsICZyZXRfYWRkciwgYXJnKSB8fCByZXRfYWRkciA9PSAwKQog ICAgcmV0dXJuIGZhbHNlOwoKICAvKiBHZXQgbmV3IHNwIGFuZCBmcC4gICov CiAgc3AgPSBmcCArIDg7CiAgaWYgKCEgcmVhZGZ1bmMgKGZwLCAmZnAsIGFy ZykgfHwgZnAgPT0gMCB8fCBzcCA+PSBmcCkKICAgIHJldHVybiBmYWxzZTsK CiAgLyogU2V0IG5ldyBzcCwgZnAgYW5kIHBjLiAgKi8KICByZWdzWzBdID0g c3A7CiAgcmVnc1sxXSA9IGZwOwogIGlmICghIHNldGZ1bmMgKEVTUCwgMiwg cmVncywgYXJnKSB8fCAhIHNldGZ1bmMgKC0xLCAxLCAmcmV0X2FkZHIsIGFy ZykpCiAgICByZXR1cm4gZmFsc2U7CgogIHJldHVybiB0cnVlOwp9Cg== --=-RvxwUIodIcoFX+2g+Fir--