[Lldb-commits] [PATCH] D60268: Breakpad: Parse Stack CFI records

[Pavel Labath via Phabricator via lldb-commits]

labath marked 5 inline comments as done.
labath added inline comments.


================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:393
+  llvm::StringRef LHS, RHS;
+  while (std::tie(Str, Line) = getToken(Line), !Str.empty()) {
+    if (Str.back() == ':') { // regN
----------------
clayborg wrote:
> clayborg wrote:
> > Do we really need to pull the content apart into separate strings for each register? Seems like a lot of work and 99% of these we will never accessed. Maybe just store the entire string for all registers and be done? 
> You can add an iterator method to the StackCFIRecord record maybe for when you do want to parse each register?
That's a good point. I'll remove the register parsing code from this patch and just store the entire expression as one string(ref). I'll see what's the best api for splitting these up when I get to connecting this with the postfix expression parser.


================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:394
+  while (std::tie(Str, Line) = getToken(Line), !Str.empty()) {
+    if (Str.back() == ':') { // regN
+      // Flush the previous expression, if there is one.
----------------
clayborg wrote:
> Does the format specify no space between the register name and the colon?
That is how I interpret the format "spec" <https://chromium.googlesource.com/breakpad/breakpad/+/master/docs/symbol_files.md>
```
Each registeri is the name of a register or pseudoregister. Each expression is a Breakpad postfix expression, which may contain spaces, but never ends with a colon.
```
That's not a very good definition because there are many ways one can split up these strings such they "don't end with a colon", but I chose this interpretation, as that's what made most sense to me (and it works for the example files I have lying around). We can always change that if we find a producer doing things slightly differently.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60268/new/

https://reviews.llvm.org/D60268