Improve readability of crash reports

[bonefish]

A few suggestions for improving the readability of the crash reports:

• Use a table format for the images, areas, and semaphores sections. Moving the name column rightmost, everything could be neatly aligned and would take a lot less horizontal space.
• Move the images, areas, and semaphores sections to the end of the report. I believe the threads section is usually the most interesting and should be topmost.
• In the areas section use hex format for the area size (or an end address), so it is easier to see whether an address falls into an area.
• Sort the images by address.
• Consider making the images table format similar to the areas table, maybe even two lines per image.

[anevilyak]

Reordered the sections in hrev45539. Will see what I can come up with as far as the other suggestions go.

[anevilyak]

Attached an example for feedback based on the above suggestions. Ideally viewed with a fixed width font.

[phoudoin]

Looks way nicer... but now the Active threads list deserves being a table like others ;-)

Active Threads:
ID      State       Name
--------------------------------------------------------------------------------	    3320    Running     Debugger (main)
3321    Running	    team 3320 debug task
3351    Running     worker
3352    Running     DebugReportGenerator
3353    Running	    team 3349 debug listener
3354    Running	    team debugger
3374    Running	    w>/Data/returnvaluetest (3349)

[phoudoin]

I was wondering if in fact it could be in some more structure container (BMessage) format, so that one could write a tool to display such report but allowing user to change sort order for instance.

Or that the textual *.report export be moved to the Debugger UI part, and for the GUI made available under a Save As button. The report itself then could be displayed in UI tables, where user/developer could easily change sort order, copy/paste addresses, and differents states could be displayed with icons (running, locked, etc).

[anevilyak]

Replying to phoudoin:

Or that the textual *.report form, export be moved to the Debugger UI part, and for the GUI made available under a Save As button. The report itself then could be displayed in UI tables, where user/developer could easily change sort order, copy/paste addresses, and differents states could be displayed with icons (running, locked, etc).

It's already possible to save them via the UI, in the GUI: Tools -> Save Report, and the respective CLI save-report command. That having been said, I'd personally find that to be a bit overkill for the Debugger itself, though it'd certainly be an interesting possibility to export the data in a format that could be consumed by e.g. DebugAnalyzer for the kind of display/manipulation you're referring to.

As far as the Threads go, the only problem there is that they have two potentially highly variable width columns, since in the case of either an actual crash or a debugger call you have the variable length messages from those in addition to the variable length thread name, which makes formatting a bit less fun without potentially wasting a lot of space. Can look into it though.

[bonefish]

Looks much better already. A few more suggestions:

• Images:
  • Type column:
    • Is not so important, so move it farther right, just before the name column.
    • Should be left-aligned.
    • The long upper-case words look somewhat weird in a table. I would use "app", "lib", "add-on", "system".
• Areas:
  • The RAM Size can be omitted, I think.
  • I guess an additional Size column would be nice after all. Base/End have all the information, but it is hard to spot at a quick glance whether an area is unusually large. The unit could be KiB to make the column a bit more compact.
  • Protection column:
    • Should be left-aligned.
    • I'd reserve space for the standard protection flags and display them in the following manner:

      r-x   s
      r--rw-
      r-x   Sk
      

    • An index for the abbreviations would be nice. Just one ore two lines below the table.
  • Locking column: Should be left-aligned. I'd also use lower case.
• I agree with phoudoin that the Active Threads section look a bit untidy now. Due to the additional information printed for crashed threads a table might not be the best choice, though. Maybe something like this:

      thread 3321: team 3320 debug task
      thread 3351: worker
      thread 3352: DebugReportGenerator
      thread 3353: team 3349 debug listener
      thread 3354: team debugger
          state: Exception (Segment violation)
  
          Frame       IP          Function Name
          -----------------------------------------------
          ...
      thread 3374: w>/Data/returnvaluetest (3349)
  

  That is, omit the state for running threads and print other states in the next line(s). Maybe also sort the list such that the boring threads come first (or last).

@phoudoin: I don't think there's a lot to be gained from a GUI application displaying the data. The crash reports are stack traces on steroids with a lot of nice information that help with a first assessment of the problem and, in some cases, even to understand it sufficiently. But I don't think it's worth to develop them into something that allows sophisticated post mortem debugging. The best tool for that purpose are core dumps. It shouldn't be that much work to implement the required kernel support nor should the support in Debugger be particularly complicated to do either (mostly an alternate DebuggerInterface implementation).

[anevilyak]

Thanks for the feedback! Will see what I can do with those additional ideas tonight.

[anevilyak]

Implemented in hrev45548, example of (hopefully) finalized format attached.

[phoudoin]

Now that's great, the detailed stack crawl of the debugged thread(s) is really nice. In the top header, could it be possible to show which gcc2/4 flavor is it?

[bonefish]

Yep, looks great already. Some more nitpicking, nonetheless:

• The Image:Type column seems to be wider than necessary.
• Areas:Locking is rather wide for the values actually in the column. I suppose it's just wide enough for the longest possible value. Maybe iterate once through the area list before printing it to determine the column width actually needed. That would also be useful for the ID columns.
• Semaphores: There's also a bit more spacing than usual between the Count and the Last Holder and between the Last Holder and the Name column. Though, maybe that was intentional.

[anevilyak]

Replying to phoudoin:

Now that's great, the detailed stack crawl of the debugged thread(s) is really nice.

Note that's only available if we have the requisite debug info to dump variables though.

In the top header, could it be possible to show which gcc2/4 flavor is it?

Currently we're only aware of that in the DWARF subsystem since we have to detect that to account for various differences in the format of .eh_frame. Will need to think about how to propagate that information back up to a level where the report generator could consume it, unless there's an alternative heuristic which can be used to detect that differently.

Replying to bonefish:

• The Image:Type column seems to be wider than necessary.

Indeed, that one simply has an extraneous tab. Will deal with that.

• Areas:Locking is rather wide for the values actually in the column. I suppose it's just wide enough for the longest possible value. Maybe iterate once through the area list before printing it to determine the column width actually needed. That would also be useful for the ID columns.

Indeed, its current width is based on the widest possible locking flag, namely "32-bit contiguous". Will see what I can do with that, though I suppose that one could also simply be shortened to "32-bit contig." or something in that vein.

• Semaphores: There's also a bit more spacing than usual between the Count and the Last Holder and between the Last Holder and the Name column. Though, maybe that was intentional.

The space between Last Holder and Name is indeed due to another extra tab that can be eliminated. In the case of Count vs Last Holder though, there's only a single tab separator. The perceived additional space is simply due to being right aligned, the padding width there is actually based on the width of the column header though, so without shortening the latter in some way there'd be extra space regardless.

[anevilyak]

Tweaked as discussed in hrev45552.