[Lldb-commits] [PATCH] D74168: [CMake] Make EXCLUDE_FROM_ALL an argument to add_lit_testsuite

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: llvm/test/CMakeLists.txt:171
 
+if(LLVM_BUILD_TOOLS)
+  set(exclude_from_check_all "EXCLUDE_FROM_CHECK_ALL")

kschwarz wrote:
> Hi @JDevlieghere, we've noticed that with this patch check-llvm isn't added 
> to check-all anymore by default. Is this the intended behaviour?
Nope, I unintentionally inverted the condition. Thanks for pointing this out! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74168



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

[Lldb-commits] [PATCH] D74168: [CMake] Make EXCLUDE_FROM_ALL an argument to add_lit_testsuite

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: llvm/test/CMakeLists.txt:171
 
+if(LLVM_BUILD_TOOLS)
+  set(exclude_from_check_all "EXCLUDE_FROM_CHECK_ALL")

JDevlieghere wrote:
> kschwarz wrote:
> > Hi @JDevlieghere, we've noticed that with this patch check-llvm isn't added 
> > to check-all anymore by default. Is this the intended behaviour?
> Nope, I unintentionally inverted the condition. Thanks for pointing this out! 
Fixed in `c10b9f0bde2666abd3e2d0845dee16ac9db1ab6f`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74168



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

[Lldb-commits] [PATCH] D74245: [lldb/Plugins] Use external functions to (de)initialize plugins

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rGfbb4d1e43d0d: [lldb/Plugins] Use external functions to 
(de)initialize plugins (authored by JDevlieghere).
Herald added subscribers: jsji, atanasyan, MaskRay, jrtc27, kbarton, aheejin, 
krytarowski, sbc100, nemanjai, sdardis, emaste.
Herald added a reviewer: espindola.

Changed prior to commit:
  https://reviews.llvm.org/D74245?vs=243247&id=243302#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74245

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
  lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
  lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp
  lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
  lldb/source/Plugins/ABI/X86/ABISysV_i386.cpp
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
  lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source

[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 243385.
clayborg added a comment.

Fixed Pavel's issues. I thoroughly tested this and found that we should be only 
sending "changed" events for breakpoints since breakpoints never go away.

Changed in this patch:

- add a "vscode" name to each breakpoint so we know which breakpoints were set 
in the IDE UI. Any breakpoints that don't have this won't get updates to the 
IDE since the IDE doesn't know about them. This is in case people set 
breakpoints in the debugger console. I tried reporting such breakpoints as 
"new" breakpoints, but the IDE UI doesn't do anything with new breakpoint that 
weren't created in the UI.
- Fixed logic to always report "changed" as the breakpoint event reason. LLDB 
breakpoints never go away, so they are never "new" or "removed".
- If a breakpoint has no resolved locations, then the breakpoint is marked as 
"verified": false in the breakpoint responses and events. When a breakpoint 
resolves then it will get updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665

Files:
  lldb/tools/lldb-vscode/BreakpointBase.cpp
  lldb/tools/lldb-vscode/BreakpointBase.h
  lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp
  lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/LLDBUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.h
  lldb/tools/lldb-vscode/SourceBreakpoint.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -379,27 +379,20 @@
 if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
   auto event_type =
   lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
-  const auto num_locs =
-  lldb::SBBreakpoint::GetNumBreakpointLocationsFromEvent(event);
   auto bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event);
-  bool added = event_type & lldb::eBreakpointEventTypeLocationsAdded;
-  bool removed =
-  event_type & lldb::eBreakpointEventTypeLocationsRemoved;
-  if (added || removed) {
-for (size_t i = 0; i < num_locs; ++i) {
-  auto bp_loc =
-  lldb::SBBreakpoint::GetBreakpointLocationAtIndexFromEvent(
-  event, i);
-  auto bp_event = CreateEventObject("breakpoint");
-  llvm::json::Object body;
-  body.try_emplace("breakpoint", CreateBreakpoint(bp_loc));
-  if (added)
-body.try_emplace("reason", "new");
-  else
-body.try_emplace("reason", "removed");
-  bp_event.try_emplace("body", std::move(body));
-  g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
-}
+  // If the breakpoint was originated from the IDE, it will have the
+  // BreakpointBase::GetBreakpointLabel() label attached. Regardless
+  // of wether the locations were added or removed, the breakpoint
+  // ins't going away, so we the reason is always "changed".
+  if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
+   event_type & lldb::eBreakpointEventTypeLocationsRemoved) &&
+  bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
+auto bp_event = CreateEventObject("breakpoint");
+llvm::json::Object body;
+body.try_emplace("breakpoint", CreateBreakpoint(bp));
+body.try_emplace("reason", "changed");
+bp_event.try_emplace("body", std::move(body));
+g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
Index: lldb/tools/lldb-vscode/SourceBreakpoint.cpp
===
--- lldb/tools/lldb-vscode/SourceBreakpoint.cpp
+++ lldb/tools/lldb-vscode/SourceBreakpoint.cpp
@@ -17,6 +17,9 @@
 
 void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) {
   bp = g_vsc.target.BreakpointCreateByLocation(source_path.str().c_str(), line);
+  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+  // we add a label to our breakpoints.
+  bp.AddName(GetBreakpointLabel());
   if (!condition.empty())
 SetCondition();
   if (!hitCondition.empty())
Index: lldb/tools/lldb-vscode/LLDBUtils.h
===
--- lldb/tools/lldb-vscode/LLDBUtils.h
+++ lldb/tools/lldb-vscode/LLDBUtils.h
@@ -106,46 +106,6 @@
 /// The LLDB frame index ID.
 uint32_t GetLLDBFrameID(uint64_t dap_frame_id);
 
-/// Given a LLDB breakpoint, make a breakpoint ID that is unique to a
-/// 

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D74252#1865504 , @mib wrote:

> but let's leave symbol as is and add a `ConstString alternate_symbol`


FYI this had a purpose. Any code touching `symbol` should be rechecked if it 
should not handle also `alternate_symbol` now. Both in existing codebase and 
also in any 3rd party patches we do not know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74187: [lldb] Add method Language::IsMangledName

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:96-98
+  bool IsMangledName(llvm::StringRef name) const override {
+return false;
+  }

friss wrote:
> shafik wrote:
> > xiaobai wrote:
> > > friss wrote:
> > > > The original code was calling `IsPossibleObjCMethodName` and it looks 
> > > > like you completely lose this codepath with this rewrite. Unclear to me 
> > > > if it's the right thing to return here, but that's definitely a change 
> > > > in behavior.
> > > If I understand correctly, Objective-C names aren't mangled in general, 
> > > and are usually stored in the `m_demangled` name of `Mangled`, so that 
> > > code path should have been bogus to begin with. Maybe I'm missing a 
> > > detail though?
> > > 
> > > I can add a comment to this to clarify that Objective-C names aren't 
> > > mangled (if I am correct in my understanding).
> > I believe this is true, I have not verified it myself. I believe what 
> > happens is that `m_mangled` is just set directly in the Objective-C but it 
> > probably would be nice to verify this via a test in `MangledTest.cpp` I 
> > actually though @mib had added a test here for Objective-C the other day.
> > If I understand correctly, Objective-C names aren't mangled in general, and 
> > are usually stored in the m_demangled name of Mangled, so that code path 
> > should have been bogus to begin with. Maybe I'm missing a detail though?
> 
> I'm just pointing out a change in behavior. If a `Mangled` object had 
> `m_mangled` set to an Obj-C method name, `GuessLanguage()` would have 
> returned `eLanguageTypeObjC` before your patch. It won't do it after. I don't 
> actually know whether we ever put a method name in `m_mangled`, but I bet we 
> do.
> 
> Whether Obj-C symbols are actually mangled or not is debatable too. The name 
> of the method symbol tells you whether it is a class or instance method and 
> the number and names of arguments. Looks pretty close to the information the 
> C++ mangling gives you for a symbol.
In D71237, I only added a test for the `function.mangled-name` frame-format 
entity with a C++ example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74187



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 243358.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252

Files:
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/source/Commands/CommandObjectFrame.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  lldb/test/Shell/Recognizer/assert.test
  lldb/unittests/Target/StackFrameRecognizerTest.cpp

Index: lldb/unittests/Target/StackFrameRecognizerTest.cpp
===
--- lldb/unittests/Target/StackFrameRecognizerTest.cpp
+++ lldb/unittests/Target/StackFrameRecognizerTest.cpp
@@ -77,6 +77,7 @@
   StackFrameRecognizerManager::ForEach(
   [&any_printed](uint32_t recognizer_id, std::string name,
  std::string function, std::string symbol,
+ std::string alternate_symbol,
  bool regexp) { any_printed = true; });
 
   EXPECT_TRUE(any_printed);
Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -1,4 +1,4 @@
-# UNSUPPORTED: system-windows, system-linux
+# UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
Index: lldb/source/Target/StackFrameRecognizer.cpp
===
--- lldb/source/Target/StackFrameRecognizer.cpp
+++ lldb/source/Target/StackFrameRecognizer.cpp
@@ -50,24 +50,28 @@
 
 class StackFrameRecognizerManagerImpl {
 public:
-  void AddRecognizer(StackFrameRecognizerSP recognizer,
- ConstString module, ConstString symbol,
+  void AddRecognizer(StackFrameRecognizerSP recognizer, ConstString module,
+ ConstString symbol, ConstString alternate_symbol,
  bool first_instruction_only) {
-m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer, false, module, RegularExpressionSP(),
-  symbol, RegularExpressionSP(),
+m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
+  false, module, RegularExpressionSP(), symbol,
+  alternate_symbol, RegularExpressionSP(),
   first_instruction_only});
   }
 
   void AddRecognizer(StackFrameRecognizerSP recognizer,
  RegularExpressionSP module, RegularExpressionSP symbol,
  bool first_instruction_only) {
-m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer, true, ConstString(), module,
+m_recognizers.push_front({(uint32_t)m_recognizers.size(), false, recognizer,
+  true, ConstString(), module, ConstString(),
   ConstString(), symbol, first_instruction_only});
   }
 
   void ForEach(
-  std::function const &callback) {
+  std::function const
+  &callback) {
 for (auto entry : m_recognizers) {
   if (entry.is_regexp) {
 std::string module_name;
@@ -79,11 +83,12 @@
   symbol_name = entry.symbol_regexp->GetText().str();
 
 callback(entry.recognizer_id, entry.recognizer->GetName(), module_name,
- symbol_name, true);
+ symbol_name, {}, true);
 
   } else {
-callback(entry.recognizer_id, entry.recognizer->GetName(), entry.module.GetCString(),
- entry.symbol.GetCString(), false);
+callback(entry.recognizer_id, entry.recognizer->GetName(),
+ entry.module.GetCString(), entry.symbol.GetCString(),
+ entry.alternate_symbol.GetCString(), false);
   }
 }
   }
@@ -120,7 +125,10 @@
 if (!entry.module_regexp->Execute(module_name.GetStringRef())) continue;
 
   if (entry.symbol)
-if (entry.symbol != function_name) continue;
+if (entry.symbol != function_name &&
+(!entry.alternate_symbol ||
+ entry.alternate_symbol != function_name))
+  continue;
 
   if (entry.symbol_regexp)
 if (!entry.symbol_regexp->Execute(function_name.GetStringRef()))
@@ -149,6 +157,7 @@
 ConstString module;
 RegularExpressionSP module_regexp;
 ConstString symbol;
+ConstString alternate_symbol;
 RegularExpressionSP symbol_regexp;
 bool first_instruction_only;
   };
@@ -163,10 +172,10 @@
 }
 
 void StackFrameRecognizerManager::AddRecognizer(
-StackFrameRecognizerSP recognizer, ConstString module,
-ConstString symbol, bool first_instruction_only) {
-  GetStackFrameRecognizerManagerImpl().AddRecognizer(recognizer, module, symbol,
-   

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D74252#1865501 , @jankratochvil 
wrote:

> In D74252#1865500 , @mib wrote:
>
> > Knowing that this will be called every time a thread stops, it would be 
> > better if we could avoid processing a regex every time we try to recognise 
> > a frame.
>
>
> Or `StackFrameRecognizerManagerImpl::AddRecognizer` and its `m_recognizers` 
> can have `ConstString symbol1, symbol2;`. What is less bad?


This sounds like a good idea, but let's leave symbol as is and add a 
`ConstString alternate_symbol` defaulting to an empty `ConstString`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D74252#1865500 , @mib wrote:

> Knowing that this will be called every time a thread stops, it would be 
> better if we could avoid processing a regex every time we try to recognise a 
> frame.


Or `StackFrameRecognizerManagerImpl::AddRecognizer` and its `m_recognizers` can 
have `ConstString symbol1, symbol2;`. What is less bad?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D74252#1865496 , @jankratochvil 
wrote:

> OK, then it could be changed to a regex. Is it fine afterwards?


Knowing that this will be called every time a thread stops, it would be better 
if we could avoid processing a regex every time we try to recognise a frame. 
But if that's the only way to solve the issue ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D74252#1865493 , @mib wrote:

> We'd like to avoid registering recognizers multiple times for the same 
> purpose ...


OK, then it could be changed to a regex. Is it fine afterwards?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

We'd like to avoid registering recognizers multiple times for the same purpose 
...

This could cause some issues down the road, since those recognizers can - 
indirectly - perform an action (change the current frame or the stop reason).

Also, from a usability stand-point, this could be confusing for the user when 
listing recognizers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252



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

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 243353.
jankratochvil retitled this revision from "Fix+re-enable Assert StackFrame 
Recognizer" to "Fix+re-enable Assert StackFrame Recognizer on Linux".
jankratochvil edited the summary of this revision.
Herald added a subscriber: jfb.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74252

Files:
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/test/Shell/Recognizer/assert.test

Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -1,4 +1,4 @@
-# UNSUPPORTED: system-windows, system-linux
+# UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -16,26 +16,6 @@
 using namespace lldb_private;
 
 namespace lldb_private {
-/// Checkes if the module containing a symbol has debug info.
-///
-/// \param[in] target
-///The target containing the module.
-/// \param[in] module_spec
-///The module spec that should contain the symbol.
-/// \param[in] symbol_name
-///The symbol's name that should be contained in the debug info.
-/// \return
-///If  \b true the symbol was found, \b false otherwise.
-bool ModuleHasDebugInfo(Target &target, FileSpec &module_spec,
-StringRef symbol_name) {
-  ModuleSP module_sp = target.GetImages().FindFirstModule(module_spec);
-
-  if (!module_sp)
-return false;
-
-  return module_sp->FindFirstSymbolWithNameAndType(ConstString(symbol_name));
-}
-
 /// Fetches the abort frame location depending on the current platform.
 ///
 /// \param[in] process_sp
@@ -43,26 +23,26 @@
 ///the target and the platform.
 /// \return
 ///If the platform is supported, returns an optional tuple containing
-///the abort module as a \a FileSpec and the symbol name as a \a StringRef.
+///the abort module as a \a FileSpec and two symbol names as two \a
+///StringRef. The second \a StringRef may be empty.
 ///Otherwise, returns \a llvm::None.
-llvm::Optional>
+llvm::Optional>
 GetAbortLocation(Process *process) {
   Target &target = process->GetTarget();
 
   FileSpec module_spec;
-  StringRef symbol_name;
+  StringRef symbol_name1, symbol_name2;
 
   switch (target.GetArchitecture().GetTriple().getOS()) {
   case llvm::Triple::Darwin:
   case llvm::Triple::MacOSX:
 module_spec = FileSpec("libsystem_kernel.dylib");
-symbol_name = "__pthread_kill";
+symbol_name1 = "__pthread_kill";
 break;
   case llvm::Triple::Linux:
 module_spec = FileSpec("libc.so.6");
-symbol_name = "__GI_raise";
-if (!ModuleHasDebugInfo(target, module_spec, symbol_name))
-  symbol_name = "raise";
+symbol_name1 = "raise";
+symbol_name2 = "__GI_raise";
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -70,7 +50,7 @@
 return llvm::None;
   }
 
-  return std::make_tuple(module_spec, symbol_name);
+  return std::make_tuple(module_spec, symbol_name1, symbol_name2);
 }
 
 /// Fetches the assert frame location depending on the current platform.
@@ -80,27 +60,26 @@
 ///the target and the platform.
 /// \return
 ///If the platform is supported, returns an optional tuple containing
-///the asserting frame module as  a \a FileSpec and the symbol name as a \a
-///StringRef.
+///the asserting frame module as a \a FileSpec and two possible symbol
+///names as two \a StringRef. The second \a StringRef may be empty.
 ///Otherwise, returns \a llvm::None.
-llvm::Optional>
+llvm::Optional>
 GetAssertLocation(Process *process) {
   Target &target = process->GetTarget();
 
   FileSpec module_spec;
-  StringRef symbol_name;
+  StringRef symbol_name1, symbol_name2;
 
   switch (target.GetArchitecture().GetTriple().getOS()) {
   case llvm::Triple::Darwin:
   case llvm::Triple::MacOSX:
 module_spec = FileSpec("libsystem_c.dylib");
-symbol_name = "__assert_rtn";
+symbol_name1 = "__assert_rtn";
 break;
   case llvm::Triple::Linux:
 module_spec = FileSpec("libc.so.6");
-symbol_name = "__GI___assert_fail";
-if (!ModuleHasDebugInfo(target, module_spec, symbol_name))
-  symbol_name = "__assert_fail";
+symbol_name1 = "__assert_fail";
+symbol_name2 = "__GI___assert_fail";
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -108,7 +87,7 @@
 return llvm::None;
   }
 
-  return std::make_tuple(module_spec, symbol_name);
+  return std::make_tuple(module_spec, symbol_name1, symbol_name2);
 }
 
 void RegisterAssertFrameRecognizer(