> 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 > >
