Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Works only with .crash, not with .hang #7

Open
fletcher opened this issue Dec 3, 2017 · 2 comments
Open

Works only with .crash, not with .hang #7

fletcher opened this issue Dec 3, 2017 · 2 comments

Comments

@fletcher
Copy link

fletcher commented Dec 3, 2017

The program only works to symbolicate crash reports, not hang reports that have a slightly different syntax.

The style of the program is somewhat "hardcoded" in that the matches are stored as an array of NSTextCheckingResults in a fixed order, rather stored by function. In other words match.groups[2] is used rather than something like match.binary.

I did not feel like rewriting all of the code to make it more flexible, so instead did something quick and dirty. Rather than a pull request, as this code is messy, I'll show you what I did and you can decide what to do with it. (Also, I normally use Objective-C, not Swift, so I am sure the code style can be dramatically improved.)

  1. The architecture is stored in an Architecture: field, rather than Code Type:. So I used this instead (using a single regexp caused false positives in .crash reports, which is why I used the if/then model instead):

     var optionalArchitectureMatch = "^Code Type:\\s+([^ \\r\\n]+)".toRx(options: NSRegularExpression.Options.anchorsMatchLines)!.firstMatch(withDetails: contents);
     if (optionalArchitectureMatch == nil) {
     	optionalArchitectureMatch = "^Architecture:\\s+([^ \\r\\n]+)".toRx(options: NSRegularExpression.Options.anchorsMatchLines)!.firstMatch(withDetails: contents);
     	if (optionalArchitectureMatch == nil) {
     		print("ERROR: Process architecture value is missing!")
     		return nil
     	}
     }
    
  2. The lines containing symbols needing symbolication have a different format:

     10  ??? (MultiMarkdown Composer + 345055) [0x1015b73df]
    

    or

             10  ??? (MultiMarkdown Composer + 365149) [0x1015bc25d] 1-10
    

(with widely varying amounts of leading whitespace).

There did not seem to be an immediately obvious way of combining the necessary regular expressions, so instead I created a separate subroutine to match the lines and to reorder the matches to the fit the order your program expects. Again, this is "quick and dirty", and doesn't use the same result checking(partially because we have to split one field to become two, so by definition the symbolOrAddress will contain the binary):

fileprivate func hangLinesToSymbolicate(_ contents: NSString) -> [RxMatch] {
	let pattern = "^\\s+[0-9]+\\s+[?]*\\s+\\(((.*?) \\+ \\d+)\\)\\s+\\[(0x[0-9a-fA-F]*)\\]$"
	let regex = pattern.toRx(options: .anchorsMatchLines)

	let matches = contents.matches(withDetails: regex) as! [RxMatch]

	return matches.filter { match in
		let newGroups: [RxMatchGroup] = [match.groups[0] as! RxMatchGroup, match.groups[2] as! RxMatchGroup, match.groups[3] as! RxMatchGroup, match.groups[1] as! RxMatchGroup]

		match.groups = newGroups

		return true
	}
}

The array results of this function are simply concatenated to the results of this line, creating a single listing of all matches that can then be handled by symbolicateString():

let matches = linesToSymbolicate(contents as NSString)

With this tweak, I have now been able to automate symbolicating the most recent crash/hang reports that I have received.

Hope this helps, and thanks again for your work in this project!!

Fletcher

@tomaz
Copy link
Owner

tomaz commented Dec 3, 2017

Thanks!

For this I'd rather create separate class - more modular code and easier maintenance. Ideally the app would detect the type of file and parse is automatically but otherwise cmd line switch would be needed.

Just writting my thoughts here for future reference when I (or anyone else ;) will pick it up.

@fletcher
Copy link
Author

fletcher commented Dec 3, 2017

I think a more modular approach would be good.

After writing this, I also realized a user had sent a .diag file as well, which has yet another slightly different format. In this case, copying in the Identifier: and Process: lines from the hang report were sufficient.

So technically, there are at least 3 different variations that need supporting.

(And BTW I hardcoded the --fuzzy switch in my build and haven't noticed any problems with it so far. But I only had a couple of crash reports to deal with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants