> On July 27, 2014, 1:17 p.m., Thomas Lübking wrote: > > drkonqi/gdbhighlighter.cpp, line 74 > > <https://git.reviewboard.kde.org/r/119498/diff/1/?file=293510#file293510line74> > > > > an abort is not a crash ;-) > > > > If you hit this assert, the looked up (lineNr - 1) is somehow out of > > bounds, ie. there's no line with a key >= (lineNr -1) in the map. > > > > This should likely never happen on any system. > > Can you check what the lineNr actually is as compared to > > > > qDebug() << lineNr << lines.keys(); > > > > ? > > Ian Wadham wrote: > I did at one time have a few qDebug() statements to try and find what was > wrong with the algorithm. AFAICT it is trying to match one or more lines in > (const QString& text) with single (possibly long) lines of backtrace in > QMap<int, BacktraceLine>. I think it failed if one backtrace line was > formatted into three text lines or maybe if the last backtrace line was > formatted into two text lines. AFAICT (there is a real dearth of explanatory > comments) the code is merely introducing pretty colours, etc. into the > formatted text. As such, it should not abort Dr Konqi and lose the crash > report. > > Thomas Lübking wrote: > Of course it should not crash. > > The point is that the assert explicitly tells you that there's something > wrong with the code. > Skippping it won't fix the issu - that's like puting duct tape over a > warning sign on your cars dashboard to fix "no cooling water". > > > Since the lines map seems only used to map lines to textblocks, i assume > the issue is that the lines are eg. not "\n" terminated in gdb output on OSX > (no idea, though) and the only item in the map is that for the key "0" > > In this case that'd be the issue to fix. > > Ian Wadham wrote: > Please spare me the motoring analogies. To me, the ASSERT at this point, > is like bringing the car to a screeching halt or running it into the nearest > fence, simply because the glovebox light will not come on. I am a real-time > and O/S programmer from way back and have designed and written a few crash > procedures for different systems. They need to be rugged and simple (unlike > Dr Konqi IMHO) and to succeed no matter what. This used to be called failsafe > programming, graceful failure or graceful degradation. If an ASSERT and abort > technique has to be used in RT programming, to prevent potential database > corruption, it needs to be backed by an appropriate recovery and restart > procedure. > > Actually I think lines.end() is a legitimate return from > lines.lowerBound(lineNr - 1) in Dr K's algorithm (see > http://qt-project.org/doc/qt-5/qmap.html#lowerBound coding examples). The > lineNr can actually get ahead of the number of lines in the map by by too > much, if there are several multi-line entries in QString& text. Using an > ASSERT looks like lazy programming to me (Thinks: "I cannot see what to do in > this case, so I'll put in an ASSERT and see if it actually happens in > practice." ???). My solution is to re-use the last entry from the QMap. > Another might be to use "return;" instead of the ASSERT, leaving the last bit > of the highlighting incomplete, but not losing the valuable backtrace data. > > A better (more rugged) approach might be: > > get a line from "text" > if it is from the left-hand end of a backtrace entry (i.e. not a > continuation line) > get the next backtrace entry > if past last entry > return > re-format the line from "text" > > I would have used something like that in my patch, but I do not know > enough about the format of backtrace data to get the first "if" condition > right. How would I? > > Re "\n" characters, they occur as expected in the content of "QString& > text" on Apple OS X.
If it walks and talks like a crash ... we should not end up in the discussion deadlock I once had with my boss who claimed embedded code cannot crash (because there's no OS or whatelse to replace the application code). Anyway, I like Ian's suggestion to just return to the caller instead of exitting (and I concur with his lazy programming analysis). So if I understood correctly, DrKonqi does some reformatting of the backtrace before including it for upload with the bug report. What I haven't understood is how important this reformatting is. Could it be a thought to cancel the reformating procedure and return with the raw text, instead of with a partly formated text, when the assert condition triggers? About processing backtraces: recent OS X versions use lldb instead of gdb. How have you tackled that, Ian - added a dependency on port:gdb ? - RJVB ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119498/#review63254 ----------------------------------------------------------- On July 27, 2014, 11:16 a.m., Ian Wadham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119498/ > ----------------------------------------------------------- > > (Updated July 27, 2014, 11:16 a.m.) > > > Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and > Michael Pyne. > > > Repository: kde-runtime > > > Description > ------- > > When a KDE app crashes in Apple OS X, it just disappears from the screen. At > the most, the user is invited to report the crash to Apple. AFAIK this has > been a problem in KDE on Apple OS X for years, leading to frustration with > KDE among Apple users and MacPorts developers and an attitude among KDE > developers of "Why does nobody report the problem(s) on bugs.kde.org?" > > It is my strong belief that the failure to report crashes of KDE apps in > Apple OS X also exists in Frameworks. > > So far I have identified a number of portability bugs in KDE on Apple OS X: 1 > in KCrash, 1 in kdeinit4 and 5 in Dr Konqi. Three patches for Dr Konqi are > submitted in this review. Patches for KCrash and kdeinit4 are submitted in > part 1 of this review, against kdelibs. I am still investigating the other > two problems in Dr Konqi - and there could be more than two... > > In this review we have three portability problems: > > 1. On Apple OS X, Dr Konqi's dialog box hides itself underneath the main > window of the app that has just crashed, so is effectively useless. This > appears to be because Dr Konqi is started by a Linux/Unix method (fork() + > exec()?). If an app is started with the Apple OS X "open" command, it always > appears on top. The patch just raises the dialog box. > > 2. When formatting the backtrace output, Dr Konqi crashes (with an ASSERT) on > the last line. This appears to be an error in the algorithm used (i.e. also a > bug in Linux KDE), but the patch is treating it as an Apple OS X portability > problem for now. > > 3. Dr Konqi checks whether the user can save cookies in kcookiejar and, if > not, stops reporting the crash. On Apple OS X, cookies would be kept in > another browser (e.g. Safari or Firefox) and not in KDE's default browser > (Konqueror) and cookie jar. IMHO, Dr K should report the crash no matter > what, as long as it can connect to bugs.kde.org and log in. > > > Diffs > ----- > > drkonqi/reportassistantpages_bugzilla.cpp 86ca327 > drkonqi/gdbhighlighter.cpp 7cd0aa9 > drkonqi/main.cpp 75e060e > > Diff: https://git.reviewboard.kde.org/r/119498/diff/ > > > Testing > ------- > > Using Apple OS X 10.7.5 (Lion) on a MacBook Pro, I have installed KDE libs > via MacPorts (at version 4.12.5) and I have adapted kdesrc-build to run in an > Apple OS X environment and used it to test against the KDE 4.13 branch. I > have been testing with a KDE app that I can crash at will and using stderr > and Apple OS X Console log output to determine the outcome. > > Please note that I am the -only- KDE developer who has this kind of setup, > but I am NOT a KDE core developer. My experience before now has been in KDE > Games. However I used to be a UNIX and database guru before I retired in 1998. > > I NEED HELP from KDE -core- developers to proceed further. These problems > will also exist in Dr Konqi for KF 5, but I am as yet unable to build or test > Frameworks on Apple OS X and I cannot find Dr Konqi among the Frameworks > repositories. I am sure there are many more Apple OS X portability problems > in Dr Konqi and other KDE software. > > Without my patches, Dr Konqi, on Apple OS X, remains invisible to the user, > often fails to complete the backtrace report and then fails to connect to > bugs.kde.org. > > With my patches, Dr Konqi on Apple OS X can generate a full crash report, > including the backtrace and the results of the dialog with the user. > Sometimes, however, it fails to submit the completed report to bugs.kde.org. > This problem is still under investigation. > > I would not have got this far without help from Michael Pyne, Thomas Lübking > and several of the MacPorts developers, as well as the unfailing enthusiasm > and encouragement of my friend Marko Käning. > > > Thanks, > > Ian Wadham > >
