JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

In several places you have `SourceLocationSpec::Create` followed by a 
`lldbassert`. That's not sufficient, because in a non-assert build, the 
lldbassert is going to print a message but not halt the program and we'll crash 
when trying to dereference the expected. You'll need to add proper error 
handling to those places to bail out if the location spec is not valid.



================
Comment at: lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h:63
   bool m_skip_prologue;
-  bool m_exact_match;
+  SourceLocationSpec m_location_spec;
 
----------------
Move this before the bool to reduce padding. 


================
Comment at: lldb/source/API/SBThread.cpp:856
     SymbolContextList sc_list;
-    frame_sc.comp_unit->ResolveSymbolContext(step_file_spec, line,
-                                             check_inlines, exact,
+    frame_sc.comp_unit->ResolveSymbolContext(*location_spec,
                                              eSymbolContextLineEntry, sc_list);
----------------
This will crash in a non-assert build. You can't rely on the assert to make 
this unreachable. 


================
Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:274-275
+            m_location_spec.GetLine());
+  if (m_location_spec.HasColumn())
+    s->Printf("column = %u, ", *m_location_spec.GetColumn());
+  s->Printf("exact_match = %d", m_location_spec.GetExactMatch());
----------------
I'd get the column, check if it's valid, and use it, instead of calling 
`HasColumn` before `GetColumn`. 


================
Comment at: lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp:116
+    lldbassert(location_spec && "Invalid SourceLocationSpec.");
+    cu->ResolveSymbolContext(*location_spec, eSymbolContextEverything, 
sc_list);
     // Find all the function names:
----------------
Same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to