[KDE/Mac] Review Request 121213: [OS X] correctly parse backtrace for demangling symbols
René J.V. Bertin
rjvbertin at gmail.com
Tue Nov 25 19:40:32 UTC 2014
> On Nov. 23, 2014, 12:20 a.m., Thomas Lübking wrote:
> > kdecore/io/kdebug.cpp, line 717
> > <https://git.reviewboard.kde.org/r/121213/diff/1/?file=329504#file329504line717>
> >
> > Why do you need to sharpen the match?
>
> René J.V. Bertin wrote:
> Because otherwise the symbol to be demangled is left with a trailing space, which causes the demangle to fail. Wanna know how I know?
>
> Thomas Lübking wrote:
> Don't know about the GCC behavior on trailing spaces, but one should probably have some "while (in.at(mangledNameEnd) == ' ') --mangledNameEnd;" loop?
>
> René J.V. Bertin wrote:
> One could, but why? The start is found with a string, I don't see why the end wouldn't be, given that that loop of yours will always loop just once ...
>
> Thomas Lübking wrote:
> ... or not at all or thrice - depending on how the mangler feels today.
>
> What I mean is that if untrimmed strings are a problem for demangling, one should seek to get rid of all the whitespace generically - i don't care about whether seeking for a string or char, this context is hardly performance critical.
>
> René J.V. Bertin wrote:
> this particular bit of whitespace has nothing to do with the mangler or how it feels today. It's formatting done by the backtrace function, which clearly always puts a space-padded `+` rather than one without padding as under Linux.
> Frankly, in general I'd agree with you, but here I don't see any reason to make the code more complex than it is.
>
> Thomas Lübking wrote:
> backtrace() is provided by glibc resp. the apple (darwin) libc (i hope).
> -> What provides /usr/include/execinfo.h on OSX?
> That's what is needed to be detected (I assume this would affect *all* BSD variants?)
>
> > I don't see any reason to make the code more complex than it is.
>
> If we're dealing w/ unspecified output and need to sanitize it, we may have to do this generically if we must consider random toolchains aside gcc+glibc.
> I know that you only care about OSX, but actually OSX is just a good testcase to see whether the code is "correct" or just happens to work.
>
> - Do you compile with gcc or clang?
> - Did you check whether the glibc backtrace() output is actually stripped?
> - What if we compile glibc w/ llvm and glibc backtrace() fires padded " + " which llvm __cxa_demangle() cannot handle? (ok, rethorical question ;-)
>
> René J.V. Bertin wrote:
> > backtrace() is provided by glibc resp. the apple (darwin) libc (i hope).
>
> Not that we have something called libc on OS X, but I think that just underlines my point. The C Library is more part of the system than it is of the C "package", so the only check on compiler that would make sense IMHO is on the system compiler, as far as that's possible (like the Apple-provided __APPLE_CC__).
>
> > What provides /usr/include/execinfo.h on OSX?
>
> If you're talking about packages: either the standard install, or Xcode and/or "Command Line Tools".
> One shouldn't take OS X as a guideline for any BSD variant, it might even have diverged too much to use BSD as a guideline for what ought to work on OS X.
>
> I agree about the sanitising/generalising, which is why I pulled the suppression of trailing whitespace out of the end token search (and you'll notice I don't just suppress spaces). I just don't think it'd make any sense to have a os/toolchain-specific constant and then the actual search one-liner. Just put that search statement in the conditional block and be done with it.
>
> This patch has currently been tested only on 10.9(.4), which means I can basically only test with clang. I can affirm though that the existing code doesn't demangle on 10.6 either, whether I use clang or gcc.
>
> > Did you check whether the glibc backtrace() output is actually stripped?
>
> I don't understand the question?
>
> This is what unparsed output from backtrace_symbols looks like (even adding the counter: isn't required):
>
> ```
> 0: "0 libkdecore.5.dylib 0x0000000106656f71 _Z14kRealBacktracei + 81"
> 1: "1 kwindowtest 0x0000000105e322e1 _ZN10TestWindow8slotOpenEv + 2753"
> 2: "2 kwindowtest 0x0000000105e33d68 _ZN10TestWindow18qt_static_metacallEP7QObjectN11QMetaObject4CallEiPPv + 184"
> 3: "3 QtCore 0x0000000106c8d3fd _ZN11QMetaObject8activateEP7QObjectPKS_iPPv + 1693"
> 4: "4 QtGui 0x0000000106faf419 _ZN7QAction8activateENS_11ActionEventE + 233"
> 5: "5 QtGui 0x00000001073141e4 _ZN22QAbstractButtonPrivate5clickEv + 84"
> 6: "6 QtGui 0x0000000107314fc8 _ZN15QAbstractButton17mouseReleaseEventEP11QMouseEvent + 88"
> 7: "7 QtGui 0x00000001073db67f _ZN11QToolButton17mouseReleaseEventEP11QMouseEvent + 15"
> 8: "8 QtGui 0x00000001070069bd _ZN7QWidget5eventEP6QEvent + 717"
> 9: "9 QtGui 0x0000000107314ed4 _ZN15QAbstractButton5eventEP6QEvent + 180"
> 10: "10 QtGui 0x00000001073dbb2a _ZN11QToolButton5eventEP6QEvent + 170"
> 11: "11 QtGui 0x0000000106fb840c _ZN19QApplicationPrivate13notify_helperEP7QObjectP6QEvent + 252"
> 12: "12 QtGui 0x0000000106fba357 _ZN12QApplication6notifyEP7QObjectP6QEvent + 2919"
> 13: "13 QtCore 0x0000000106c75516 _ZN16QCoreApplication14notifyInternalEP7QObjectP6QEvent + 118"
> 14: "14 QtGui 0x0000000106fb8df6 _ZN19QApplicationPrivate14sendMouseEventEP7QWidgetP11QMouseEventS1_S1_PS1_R8QPointerIS0_Eb + 470"
> 15: "15 QtGui 0x0000000106f67a2a _Z23qt_mac_handleMouseEventP7NSEventN6QEvent4TypeEN2Qt11MouseButtonEP7QWidgetb + 922"
> 16: "16 AppKit 0x00007fff9491f145 -[NSWindow sendEvent:] + 781"
> 17: "17 QtGui 0x0000000106f5f2f1 -[QCocoaWindow sendEvent:] + 113"
> 18: "18 AppKit 0x00007fff948c05d4 -[NSApplication sendEvent:] + 2021"
> 19: "19 QtGui 0x0000000106f640fe -[QNSApplication sendEvent:] + 78"
> 20: "20 AppKit 0x00007fff947109f9 -[NSApplication run] + 646"
> 21: "21 QtGui 0x0000000106f6cba0 _ZN19QEventDispatcherMac13processEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE + 528"
> 22: "22 QtCore 0x0000000106c728ad _ZN10QEventLoop4execE6QFlagsINS_17ProcessEventsFlagEE + 477"
> 23: "23 QtCore 0x0000000106c75ac7 _ZN16QCoreApplication4execEv + 199"
> 24: "24 kwindowtest 0x0000000105e33bdc main + 316"
> 25: "25 libdyld.dylib 0x00007fff93bb75fd start + 1"
> 26: "26 ??? 0x0000000000000002 0x0 + 2"
> ```
>
> which is converted to:
>
> ```
> 0: 0 libkdecore.5.dylib 0x0000000106656f71 kRealBacktrace(int) + 81
> 1: 1 kwindowtest 0x0000000105e322e1 TestWindow::slotOpen() + 2753
> 2: 2 kwindowtest 0x0000000105e33d68 TestWindow::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) + 184
> 3: 3 QtCore 0x0000000106c8d3fd QMetaObject::activate(QObject*, QMetaObject const*, int, void**) + 1693
> 4: 4 QtGui 0x0000000106faf419 QAction::activate(QAction::ActionEvent) + 233
> 5: 5 QtGui 0x00000001073141e4 QAbstractButtonPrivate::click() + 84
> 6: 6 QtGui 0x0000000107314fc8 QAbstractButton::mouseReleaseEvent(QMouseEvent*) + 88
> 7: 7 QtGui 0x00000001073db67f QToolButton::mouseReleaseEvent(QMouseEvent*) + 15
> 8: 8 QtGui 0x00000001070069bd QWidget::event(QEvent*) + 717
> 9: 9 QtGui 0x0000000107314ed4 QAbstractButton::event(QEvent*) + 180
> 10: 10 QtGui 0x00000001073dbb2a QToolButton::event(QEvent*) + 170
> 11: 11 QtGui 0x0000000106fb840c QApplicationPrivate::notify_helper(QObject*, QEvent*) + 252
> 12: 12 QtGui 0x0000000106fba357 QApplication::notify(QObject*, QEvent*) + 2919
> 13: 13 QtCore 0x0000000106c75516 QCoreApplication::notifyInternal(QObject*, QEvent*) + 118
> 14: 14 QtGui 0x0000000106fb8df6 QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) + 470
> 15: 15 QtGui 0x0000000106f67a2a qt_mac_handleMouseEvent(NSEvent*, QEvent::Type, Qt::MouseButton, QWidget*, bool) + 922
> 16: 16 AppKit 0x00007fff9491f145 -[NSWindow sendEvent:] + 781
> 17: 17 QtGui 0x0000000106f5f2f1 -[QCocoaWindow sendEvent:] + 113
> 18: 18 AppKit 0x00007fff948c05d4 -[NSApplication sendEvent:] + 2021
> 19: 19 QtGui 0x0000000106f640fe -[QNSApplication sendEvent:] + 78
> 20: 20 AppKit 0x00007fff947109f9 -[NSApplication run] + 646
> 21: 21 QtGui 0x0000000106f6cba0 QEventDispatcherMac::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 528
> 22: 22 QtCore 0x0000000106c728ad QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 477
> 23: 23 QtCore 0x0000000106c75ac7 QCoreApplication::exec() + 199
> 24: 24 kwindowtest 0x0000000105e33bdc main + 316
> 25: 25 libdyld.dylib 0x00007fff93bb75fd start + 1
> 26: 26 ??? 0x0000000000000002 0x0 + 2
> ```
>
> René J.V. Bertin wrote:
> Using a very simple standalone test application I have compared the `backtrace_symbols` output under 10.9.4, using clang and gcc-4.8. Apparently this particular case is simple enough to avoid C++ runtime conflicts (there's main(), a `show_backtrace` function that mimicks `kRealBacktrace` and optionally a `maybeDemangledName` when building as C++).
>
> - In pure C, both compilers give
> ```
> 0 backtrace 0x0000000104d57d9c show_backtrace + 60
> ```
>
> so the current parser would fail (but it's called from C++ so that's a moot point, or so I hope).
>
> - In C++ mode, both compilers give
> ```
> 0 backtrace 0x000000010656dbed _Z14show_backtracev + 61
> ```
>
> I'll attach the code so that it can be tested on other platforms too.
>
> Thomas Lübking wrote:
> Very good - I already felt stupid.
> The demangled is always nullptr here and the backtrace strings do not end w/ " + *" at all. No matter whether gcc, g++, clang or clang++
>
> *shrug*
>
> René J.V. Bertin wrote:
> Yeah, on Linux I get
>
> ```
> 0: ./backtrace() [0x400fa5]
> 1: ./backtrace() [0x4011d2]
> 2: /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7fbc5e1f7ec5]
> 3: ./backtrace() [0x400c49]
> ```
>
> is that comparable to what you're getting? If so, it would seem that backtrace or backtrace_symbols already do their own demangling...
>
> Thomas Lübking wrote:
> Yupp.
> This is of course nasty, because we cannot check "other" requirements - the entire demangling should be omitted on linux/glibc > v?.?
> (I will argue that this is what I meant by "how the mangler feels today")
>
> Depending on your raw output, I'd btw. suggest to search for " _Z" - a simple " _" could (too) easily appear in the lib name?
> For " + " ./. "+" + trimming, it becomes a coin flip between "future proof" and simplicitly - as demangling is apparently only relevant on eXoticOS™, *me* would now leave that choice to you.
I'll see if I still have that OpenIndiana VM, and what a BSD VM might teach me (but which flavour ...)
Anyway, it seems that the function is rather aptly named ... it may demangle, and it may not, and if so that might be because the input was already demangled :).
- René J.V.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121213/#review70796
-----------------------------------------------------------
On Nov. 25, 2014, 3:12 p.m., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121213/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 3:12 p.m.)
>
>
> Review request for KDE Software on Mac OS X and kdelibs.
>
>
> Repository: kdelibs
>
>
> Description
> -------
>
> This patch adapts the parser in `maybeDemangledName` to the backtrace obtained in `kRealBacktrace` on OS X.
>
>
> Diffs
> -----
>
> kdecore/io/kdebug.cpp 872a05a
>
> Diff: https://git.reviewboard.kde.org/r/121213/diff/
>
>
> Testing
> -------
>
> Works as expected on OS X 10.9.4 .
>
>
> File Attachments
> ----------------
>
> minimal test for backtrace functionality and parsing; can be built as C and C++
> https://git.reviewboard.kde.org/media/uploaded/files/2014/11/25/e9d48fea-2506-493c-b068-26a8e7d12b6c__backtrace.c
>
>
> Thanks,
>
> René J.V. Bertin
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20141125/46f4ad78/attachment-0001.html>
More information about the kde-mac
mailing list