Page MenuHomePhabricator

crashlog.py: Improve regular expressions
ClosedPublic

Authored by aprantl on Tue, Nov 5, 4:40 PM.

Details

Summary

This is yet another change to the regular expressions in crashlog.py that fix a few edge cases, and attempt to improve the readability quite a bit in the process. My last change to support spaces in filenames introduced a bug that caused the version/archspec field to be parsed as part of the image name.

So in 0x1111111 - 0x22222 +MyApp Pro arm64 <01234>, the name of the image was recognized as MyApp Pro arm64 instead of MyApp Pro with a "version" of arm64.

The bugfix makes the space following an optional field mandatory *inside* the optional group.

rdar://problem/56883435

Diff Detail

Event Timeline

aprantl created this revision.Tue, Nov 5, 4:40 PM
aprantl edited the summary of this revision. (Show Details)Tue, Nov 5, 4:41 PM

I'll be honest I'm having trouble thinking of all the possible problems these regexes might have just by looking at them, but overall this looks good to me.

Should we add a couple of extra file-check tests for variations in the version number part of the binary list in a crash report? Looking around some random crash reports, I see number-number "(6.9 - 1674.102)" I see one where the framework name is in there "(1.11 - FrameworkName 1.11)" and hah a random hand built lldb crash I have on my system has "(7.0.0svn - 7.0.0svn)".

aprantl updated this revision to Diff 228139.Wed, Nov 6, 2:08 PM

Added some more tests as suggested by Jason and made the regex slightly more robust by requiring a / at the beginning of img_path. All crashlogs I found were meeting this requirement.

aprantl updated this revision to Diff 228141.Wed, Nov 6, 2:10 PM
This revision was not accepted when it landed; it landed in state Needs Review.Thu, Nov 7, 10:56 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptThu, Nov 7, 10:56 AM