date:20210824

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

2021-08-24 Thread Greg Clayton via Phabricator via lldb-commits

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

Thanks for moving over into object file. See inlined comments, we should be 
able to get this down to just the SaveCore and other static functions that just 
return nothing. Let me know if the inlined comments don't make sense.




Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1235
 SaveCoreStyle m_requested_save_core_style;
+std::string m_requested_plugin_name;
   };

Make this a ConstString



Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1242
 if (process_sp) {
-  if (command.GetArgumentCount() == 1) {
+  ConstString plugin_name(m_options.m_requested_plugin_name.c_str());
+  if (command.GetArgumentCount() <= 2) {

Remove this if we switch m_requested_plugin_name to be a ConstString. Also a 
quick FYI: ConstString has a std::string constructor, so there is no need to 
add ".c_str()" here if this code was going to stay.



Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1243
+  ConstString plugin_name(m_options.m_requested_plugin_name.c_str());
+  if (command.GetArgumentCount() <= 2) {
 FileSpec output_file(command.GetArgumentAtIndex(0));

We haven't changed the number of arguments here right? Options and their option 
values to no count towards the argument count. So all of these have just one 
argument:
```
(lldb) process save-core 
(lldb) process save-core -p  
(lldb) proicess save-core -s

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

2021-08-24 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D108545#2962017 , @OmarEmaraDev 
wrote:

> @clayborg I was planning on getting to field completion later as part of a 
> global "context window" feature. There are reasons why I need this as a full 
> separate window for now. Imminently, I am creating an operator that changes 
> the file in the sources window so that breakpoints can be inserted manually. 
> So once the user press Ctrl+F, a search window will appear where the user can 
> choose a source file to navigate to.

Pressing CTRL+F can and should bring up a dialog window right? The user will 
search for something, and finalize with some action. So this seems like perfect 
use for dialog windows and the fields and forms you have already created.

> This will require a custom searcher, and not the common searcher implemented 
> above, which I will add in a following patch. What do you think?

See comments above, but I do think this is a perfect usage for dialog windows:

- user presses CTRL+F and dialog comes up
  - user selects a file and presses OPT+ENTER to finalize the action and source 
view updates
  - user presses escape and dismisses the dialog




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3592
+m_selected_match(0), m_first_visible_match(0) {
+;
+  }

OmarEmaraDev wrote:
> OmarEmaraDev wrote:
> > clayborg wrote:
> > > Should we be doing just like any other dialog that we have created and be 
> > > constructing these items on the fly?
> > > 
> > > ```
> > > m_text_field = AddTextField("Search", "", true);
> > > ```
> > > It seems the matches could use a modified ChoicesField for the matches? 
> > > Are you currently drawing the choices (matches) manually?
> > `AddTextField` is part of form delegates, I don't think implementing this 
> > as a form is a good idea as they are functionally distinct.
> > 
> > I am drawing choices manually because the duplicated code is not really a 
> > lot and I like to be able to control the style of the drawing. But I guess 
> > we can reimplemented that using a choices field.
> One thing to note is that text representation of matches are lazily computed 
> in this window but not in the choices field, which gives an advantage. We can 
> probably add support for lazy computation in the choice field, but it may not 
> be worth it at the moment.
But this does seem like the perfect use for a dialog window: user presses 
CTRL+F, dialog shows up and allows user to select something, and then press 
OPT+ENTER to update the source view. Is there a reason you think this should be 
a window and not a dialog?

If we re-use the ChoicesField, you can just update the choices with a delegate 
any time the choices should change and repaint the windows right?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3731
+  SearcherDelegateSP m_delegate_sp;
+  TextFieldDelegate m_text_field;
+  // The index of the currently selected match.

OmarEmaraDev wrote:
> clayborg wrote:
> > Is this a new text field itself, or designed to be a reference to an 
> > existing text field? 
> A new instance of a text field.
Yeah, the main questions is if this should be a FormDelegate or not. See other 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

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

[Lldb-commits] [lldb] 120d97b - Revert "[lldb] Add support for debugging via the dynamic linker."

2021-08-24 Thread Rumeet Dhindsa via lldb-commits


Author: Rumeet Dhindsa
Date: 2021-08-24T15:20:52-07:00
New Revision: 120d97b1a7a81561bb02fc7cd5dc304ef5cbe1cf

URL: 
https://github.com/llvm/llvm-project/commit/120d97b1a7a81561bb02fc7cd5dc304ef5cbe1cf
DIFF: 
https://github.com/llvm/llvm-project/commit/120d97b1a7a81561bb02fc7cd5dc304ef5cbe1cf.diff

LOG: Revert "[lldb] Add support for debugging via the dynamic linker."

This reverts commit 1cbdc07ec015d83038c08dc562d55eccfd808591.

Buildbot failure started after this patch with failure in
api/multithreaded/TestMultithreaded.py:
https://lab.llvm.org/buildbot/#/builders/68/builds/17556

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Removed: 
lldb/test/API/functionalities/dyld-launch-linux/Makefile
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
lldb/test/API/functionalities/dyld-launch-linux/main.cpp
lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
lldb/test/API/functionalities/dyld-launch-linux/signal_file.h



diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index e385b583cef9a..866acbddbdc8a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -56,19 +56,6 @@ static addr_t ResolveRendezvousAddress(Process *process) {
   "%s resolved via direct object file approach to 0x%" PRIx64,
   __FUNCTION__, info_location);
   } else {
-const Symbol *_r_debug =
-target->GetExecutableModule()->FindFirstSymbolWithNameAndType(
-ConstString("_r_debug"));
-if (_r_debug) {
-  info_addr = _r_debug->GetAddress().GetLoadAddress(target);
-  if (info_addr != LLDB_INVALID_ADDRESS) {
-LLDB_LOGF(log,
-  "%s resolved by finding symbol '_r_debug' whose value is 
"
-  "0x%" PRIx64,
-  __FUNCTION__, info_addr);
-return info_addr;
-  }
-}
 LLDB_LOGF(log,
   "%s FAILED - direct object file approach did not yield a "
   "valid address",
@@ -289,14 +276,6 @@ bool DYLDRendezvous::FillSOEntryFromModuleInfo(
   entry.base_addr = base_addr;
   entry.dyn_addr = dyn_addr;
 
-  // ld.so saves empty file name for the executable file in the link map.
-  // When executable is run using ld.so, we need to be update executable path.
-  if (name.empty()) {
-MemoryRegionInfo region;
-Status region_status =
-m_process->GetMemoryRegionInfo(entry.dyn_addr, region);
-name = region.GetName().AsCString();
-  }
   entry.file_spec.SetFile(name, FileSpec::Style::native);
 
   UpdateBaseAddrIfNecessary(entry, name);
@@ -568,15 +547,6 @@ bool DYLDRendezvous::ReadSOEntryFromMemory(lldb::addr_t 
addr, SOEntry ) {
 return false;
 
   std::string file_path = ReadStringFromMemory(entry.path_addr);
-
-  // ld.so saves empty file name for the executable file in the link map.
-  // When executable is run using ld.so, we need to be update executable path.
-  if (file_path.empty()) {
-MemoryRegionInfo region;
-Status region_status =
-m_process->GetMemoryRegionInfo(entry.dyn_addr, region);
-file_path = region.GetName().AsCString();
-  }
   entry.file_spec.SetFile(file_path, FileSpec::Style::native);
 
   UpdateBaseAddrIfNecessary(entry, file_path);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 09d0523f676cf..160faa74af239 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -333,48 +333,28 @@ bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
 LLDB_LOG(log, "Rendezvous structure is not set up yet. "
   "Trying to locate rendezvous breakpoint in the interpreter "
   "by symbol name.");
-// Function names from 
diff erent dynamic loaders that are known to be
-// used as rendezvous between the loader and debuggers.
+ModuleSP interpreter = LoadInterpreterModule();
+if (!interpreter) {
+  LLDB_LOG(log, "Can't find interpreter, rendezvous breakpoint isn't 
set.");
+  return false;
+}
+
+// Function names from 
diff erent dynamic loaders that are known to be used
+// as rendezvous between the loader and debuggers.
 static std::vector DebugStateCandidates{
 "_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlactivity",
 "r_debug_state",   "_r_debug_state", "_rtld_debug_state",
 };
 
-ModuleSP interpreter =

[Lldb-commits] [lldb] d7e2e97 - [LLDB] Remove typos from NativeRegisterContextLinux_arm*

2021-08-24 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2021-08-25T02:55:38+05:00
New Revision: d7e2e9794a3e353e60a956140dcab93980a2a989

URL: 
https://github.com/llvm/llvm-project/commit/d7e2e9794a3e353e60a956140dcab93980a2a989
DIFF: 
https://github.com/llvm/llvm-project/commit/d7e2e9794a3e353e60a956140dcab93980a2a989.diff

LOG: [LLDB] Remove typos from NativeRegisterContextLinux_arm*

This patch removed some typos from NativeRegisterContextLinux_arm and
NativeRegisterContextLinux_arm64. Some of the log/error messages were
being reported as x86_64.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index 91f88280145d2..dcd68ddd279dd 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -236,14 +236,14 @@ Status 
NativeRegisterContextLinux_arm::WriteAllRegisterValues(
 
   if (!data_sp) {
 error.SetErrorStringWithFormat(
-"NativeRegisterContextLinux_x86_64::%s invalid data_sp provided",
+"NativeRegisterContextLinux_arm::%s invalid data_sp provided",
 __FUNCTION__);
 return error;
   }
 
   if (data_sp->GetByteSize() != REG_CONTEXT_SIZE) {
 error.SetErrorStringWithFormat(
-"NativeRegisterContextLinux_x86_64::%s data_sp contained mismatched "
+"NativeRegisterContextLinux_arm::%s data_sp contained mismatched "
 "data size, expected %" PRIu64 ", actual %" PRIu64,
 __FUNCTION__, (uint64_t)REG_CONTEXT_SIZE, data_sp->GetByteSize());
 return error;
@@ -251,7 +251,7 @@ Status 
NativeRegisterContextLinux_arm::WriteAllRegisterValues(
 
   uint8_t *src = data_sp->GetBytes();
   if (src == nullptr) {
-error.SetErrorStringWithFormat("NativeRegisterContextLinux_x86_64::%s "
+error.SetErrorStringWithFormat("NativeRegisterContextLinux_arm::%s "
"DataBuffer::GetBytes() returned a null "
"pointer",
__FUNCTION__);
@@ -401,7 +401,7 @@ Status 
NativeRegisterContextLinux_arm::GetHardwareBreakHitIndex(
 uint32_t _index, lldb::addr_t trap_addr) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_BREAKPOINTS));
 
-  LLDB_LOGF(log, "NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+  LLDB_LOGF(log, "NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
 
   lldb::addr_t break_addr;
 

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 34a520edb6937..ebde0a499acf7 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -478,14 +478,14 @@ Status 
NativeRegisterContextLinux_arm64::WriteAllRegisterValues(
 
   if (!data_sp) {
 error.SetErrorStringWithFormat(
-"NativeRegisterContextLinux_x86_64::%s invalid data_sp provided",
+"NativeRegisterContextLinux_arm64::%s invalid data_sp provided",
 __FUNCTION__);
 return error;
   }
 
   if (data_sp->GetByteSize() != REG_CONTEXT_SIZE) {
 error.SetErrorStringWithFormat(
-"NativeRegisterContextLinux_x86_64::%s data_sp contained mismatched "
+"NativeRegisterContextLinux_arm64::%s data_sp contained mismatched "
 "data size, expected %" PRIu64 ", actual %" PRIu64,
 __FUNCTION__, REG_CONTEXT_SIZE, data_sp->GetByteSize());
 return error;
@@ -493,7 +493,7 @@ Status 
NativeRegisterContextLinux_arm64::WriteAllRegisterValues(
 
   uint8_t *src = data_sp->GetBytes();
   if (src == nullptr) {
-error.SetErrorStringWithFormat("NativeRegisterContextLinux_x86_64::%s "
+error.SetErrorStringWithFormat("NativeRegisterContextLinux_arm64::%s "
"DataBuffer::GetBytes() returned a null "
"pointer",
__FUNCTION__);



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

[Lldb-commits] [lldb] ce512d5 - Revert "[lldb] Refactor Module::LookupInfo constructor"

2021-08-24 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2021-08-24T14:52:17-07:00
New Revision: ce512d5c2af58515378f5ecd989cdc0ccea8c997

URL: 
https://github.com/llvm/llvm-project/commit/ce512d5c2af58515378f5ecd989cdc0ccea8c997
DIFF: 
https://github.com/llvm/llvm-project/commit/ce512d5c2af58515378f5ecd989cdc0ccea8c997.diff

LOG: Revert "[lldb] Refactor Module::LookupInfo constructor"

This reverts commit cd2134e42aa7d1168a3ed54e41793b022f961b1f.

Seems like this broke some tests on arm and aarch64 boxes. Will
investigate before re-landing.

Added: 


Modified: 
lldb/include/lldb/Core/Module.h
lldb/include/lldb/Target/Language.h
lldb/source/Core/CMakeLists.txt
lldb/source/Core/Module.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 07f75191ca74d..767f2d232947c 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -896,7 +896,7 @@ class Module : public std::enable_shared_from_this,
 LookupInfo() : m_name(), m_lookup_name() {}
 
 LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
-   lldb::LanguageType language_type);
+   lldb::LanguageType language);
 
 ConstString GetName() const { return m_name; }
 
@@ -922,7 +922,7 @@ class Module : public std::enable_shared_from_this,
 ConstString m_lookup_name;
 
 /// Limit matches to only be for this language
-lldb::LanguageType m_language_type = lldb::eLanguageTypeUnknown;
+lldb::LanguageType m_language = lldb::eLanguageTypeUnknown;
 
 /// One or more bits from lldb::FunctionNameType that indicate what kind of
 /// names we are looking for

diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 73cba0726be64..2ad9677ea3f8c 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -202,20 +202,6 @@ class Language : public PluginInterface {
 return std::vector();
   };
 
-  class FunctionNameInfo {
-  public:
-llvm::StringRef basename;
-llvm::StringRef context;
-lldb::FunctionNameType func_name_type;
-  };
-
-  virtual Language::FunctionNameInfo
-  GetFunctionNameInfo(ConstString name) const {
-FunctionNameInfo ret;
-ret.func_name_type = lldb::eFunctionNameTypeNone;
-return ret;
-  };
-
   /// Returns true iff the given symbol name is compatible with the mangling
   /// scheme of this language.
   ///

diff  --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 2257b883fc403..65be803addf5f 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -82,6 +82,7 @@ add_lldb_library(lldbCore
 lldbTarget
 lldbUtility
 lldbPluginCPlusPlusLanguage
+lldbPluginObjCLanguage
 ${LLDB_CURSES_LIBS}
 
   CLANG_LIBS

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 16308d9dd0b88..c7538db7dd240 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -51,6 +51,7 @@
 #endif
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
@@ -634,81 +635,101 @@ void Module::FindCompileUnits(const FileSpec ,
 
 Module::LookupInfo::LookupInfo(ConstString name,
FunctionNameType name_type_mask,
-   LanguageType language_type)
-: m_name(name), m_lookup_name(name), m_language_type(language_type),
+   LanguageType language)
+: m_name(name), m_lookup_name(), m_language(language),
   m_name_type_mask(eFunctionNameTypeNone),
   m_match_name_after_lookup(false) {
+  const char *name_cstr = name.GetCString();
   llvm::StringRef basename;
   llvm::StringRef context;
 
-  std::vector languages;
-  Language::ForEach([](Language *l) {
-languages.push_back(l);
-return true;
-  });
-
   if (name_type_mask & eFunctionNameTypeAuto) {
-if (language_type == eLanguageTypeUnknown) {
-  for (Language *lang : languages) {
-Language::FunctionNameInfo info = lang->GetFunctionNameInfo(name);
-if (info.func_name_type != eFunctionNameTypeNone) {
-  m_name_type_mask = info.func_name_type;
-  basename = info.basename;
-  context = info.context;
-  break;
-}
-  }
+if (CPlusPlusLanguage::IsCPPMangledName(name_cstr))
+  m_name_type_mask = eFunctionNameTypeFull;
+else if ((language == eLanguageTypeUnknown ||
+  Language::LanguageIsObjC(language)) &&
+ ObjCLanguage::IsPossibleObjCMethodName(name_cstr))
+

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

2021-08-24 Thread Alex Langford via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcd2134e42aa7: [lldb] Refactor Module::LookupInfo constructor 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108229

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Target/Language.h
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -104,6 +104,9 @@
   std::vector
   GetMethodNameVariants(ConstString method_name) const override;
 
+  Language::FunctionNameInfo
+  GetFunctionNameInfo(ConstString name) const override;
+
   bool SymbolNameFitsToLanguage(Mangled mangled) const override;
 
   lldb::TypeCategoryImplSP GetFormatters() override;
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -275,6 +275,22 @@
   return variant_names;
 }
 
+Language::FunctionNameInfo
+ObjCLanguage::GetFunctionNameInfo(ConstString name) const {
+  Language::FunctionNameInfo info;
+  info.func_name_type = lldb::eFunctionNameTypeNone;
+
+  if (ObjCLanguage::IsPossibleObjCMethodName(name.GetCString())) {
+info.func_name_type = lldb::eFunctionNameTypeFull;
+  }
+
+  if (ObjCLanguage::IsPossibleObjCSelector(name.GetCString())) {
+info.func_name_type |= lldb::eFunctionNameTypeSelector;
+  }
+
+  return info;
+}
+
 bool ObjCLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
   ConstString demangled_name = mangled.GetDemangledName();
   if (!demangled_name)
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -104,6 +104,9 @@
 
   static lldb_private::ConstString GetPluginNameStatic();
 
+  Language::FunctionNameInfo
+  GetFunctionNameInfo(ConstString name) const override;
+
   bool SymbolNameFitsToLanguage(Mangled mangled) const override;
 
   ConstString
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -59,6 +59,39 @@
   return g_name;
 }
 
+Language::FunctionNameInfo
+CPlusPlusLanguage::GetFunctionNameInfo(ConstString name) const {
+  FunctionNameInfo info;
+  info.func_name_type = lldb::eFunctionNameTypeNone;
+
+  if (IsCPPMangledName(name.GetCString())) {
+info.func_name_type = lldb::eFunctionNameTypeFull;
+  }
+
+  CPlusPlusLanguage::MethodName method(name);
+  llvm::StringRef basename = method.GetBasename();
+  if (basename.empty()) {
+if (CPlusPlusLanguage::ExtractContextAndIdentifier(
+name.GetCString(), info.context, info.basename)) {
+  info.func_name_type |=
+  (lldb::eFunctionNameTypeMethod | eFunctionNameTypeBase);
+} else {
+  info.func_name_type |= lldb::eFunctionNameTypeFull;
+}
+  } else {
+info.func_name_type |=
+(lldb::eFunctionNameTypeMethod | eFunctionNameTypeBase);
+  }
+
+  if (!method.GetQualifiers().empty()) {
+// There is a 'const' or other qualifier following the end of the function
+// parens, this can't be a eFunctionNameTypeBase.
+info.func_name_type &= ~(lldb::eFunctionNameTypeBase);
+  }
+
+  return info;
+}
+
 bool CPlusPlusLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
   const char *mangled_name = mangled.GetMangledName().GetCString();
   return mangled_name && CPlusPlusLanguage::IsCPPMangledName(mangled_name);
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -51,7 +51,6 @@
 #endif
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
@@ -635,101 +634,81 @@
 
 Module::LookupInfo::LookupInfo(ConstString name,
FunctionNameType name_type_mask,
-   LanguageType language)
-: m_name(name), m_lookup_name(), m_language(language),
+

[Lldb-commits] [lldb] cd2134e - [lldb] Refactor Module::LookupInfo constructor

2021-08-24 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2021-08-24T13:53:49-07:00
New Revision: cd2134e42aa7d1168a3ed54e41793b022f961b1f

URL: 
https://github.com/llvm/llvm-project/commit/cd2134e42aa7d1168a3ed54e41793b022f961b1f
DIFF: 
https://github.com/llvm/llvm-project/commit/cd2134e42aa7d1168a3ed54e41793b022f961b1f.diff

LOG: [lldb] Refactor Module::LookupInfo constructor

Module::LookupInfo's constructor currently goes over supported languages
trying to figure out the best way to search for a symbol name. This
seems like a great candidate for refactoring. Specifically, this is work
that can be delegated to language plugins.

Once again, the goal here is to further decouple plugins from
non-plugins. The idea is to have each language plugin take a name and
give you back some information about the name from the perspective of
the language. Specifically, each language now implements a
`GetFunctionNameInfo` method which returns an object of type
`Language::FunctionNameInfo`. Right now, it consists of a basename,
a context, and a FunctionNameType. Module::LookupInfo's constructor will
call `GetFunctionNameInfo` with the appropriate language plugin(s) and
then decide what to do with that information. I have attempted to maintain
existing behavior as best as possible.

A nice side effect of this change is that lldbCore no longer links
against the ObjC Language plugin.

Differential Revision: https://reviews.llvm.org/D108229

Added: 


Modified: 
lldb/include/lldb/Core/Module.h
lldb/include/lldb/Target/Language.h
lldb/source/Core/CMakeLists.txt
lldb/source/Core/Module.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 767f2d232947c..07f75191ca74d 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -896,7 +896,7 @@ class Module : public std::enable_shared_from_this,
 LookupInfo() : m_name(), m_lookup_name() {}
 
 LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask,
-   lldb::LanguageType language);
+   lldb::LanguageType language_type);
 
 ConstString GetName() const { return m_name; }
 
@@ -922,7 +922,7 @@ class Module : public std::enable_shared_from_this,
 ConstString m_lookup_name;
 
 /// Limit matches to only be for this language
-lldb::LanguageType m_language = lldb::eLanguageTypeUnknown;
+lldb::LanguageType m_language_type = lldb::eLanguageTypeUnknown;
 
 /// One or more bits from lldb::FunctionNameType that indicate what kind of
 /// names we are looking for

diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 2ad9677ea3f8c..73cba0726be64 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -202,6 +202,20 @@ class Language : public PluginInterface {
 return std::vector();
   };
 
+  class FunctionNameInfo {
+  public:
+llvm::StringRef basename;
+llvm::StringRef context;
+lldb::FunctionNameType func_name_type;
+  };
+
+  virtual Language::FunctionNameInfo
+  GetFunctionNameInfo(ConstString name) const {
+FunctionNameInfo ret;
+ret.func_name_type = lldb::eFunctionNameTypeNone;
+return ret;
+  };
+
   /// Returns true iff the given symbol name is compatible with the mangling
   /// scheme of this language.
   ///

diff  --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 65be803addf5f..2257b883fc403 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -82,7 +82,6 @@ add_lldb_library(lldbCore
 lldbTarget
 lldbUtility
 lldbPluginCPlusPlusLanguage
-lldbPluginObjCLanguage
 ${LLDB_CURSES_LIBS}
 
   CLANG_LIBS

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index c7538db7dd240..16308d9dd0b88 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -51,7 +51,6 @@
 #endif
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
@@ -635,101 +634,81 @@ void Module::FindCompileUnits(const FileSpec ,
 
 Module::LookupInfo::LookupInfo(ConstString name,
FunctionNameType name_type_mask,
-   LanguageType language)
-: m_name(name), m_lookup_name(), m_language(language),
+   LanguageType language_type)
+: m_name(name), m_lookup_name(name), m_language_type(language_type),
   m_name_type_mask(eFunctionNameTypeNone),
   m_match_name_after_lookup(false) {
-  const char *name_cstr = name.GetCString();

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-24 Thread Rumeet Dhindsa via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1cbdc07ec015: [lldb] Add support for debugging via the 
dynamic linker. (authored by rdhindsa).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/dyld-launch-linux/Makefile
  lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
  lldb/test/API/functionalities/dyld-launch-linux/main.cpp
  lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
  lldb/test/API/functionalities/dyld-launch-linux/signal_file.h

Index: lldb/test/API/functionalities/dyld-launch-linux/signal_file.h
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/signal_file.h
@@ -0,0 +1 @@
+int get_signal_crash(void);
Index: lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
@@ -0,0 +1,7 @@
+#include "signal_file.h"
+#include 
+
+int get_signal_crash(void) {
+  raise(SIGSEGV);
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-launch-linux/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/main.cpp
@@ -0,0 +1,6 @@
+#include "signal_file.h"
+
+int main() {
+  // Break here
+  return get_signal_crash();
+}
Index: lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
@@ -0,0 +1,46 @@
+"""
+Test that LLDB can launch a linux executable through the dynamic loader and still hit a breakpoint.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test.lldbtest import *
+
+class TestLinux64LaunchingViaDynamicLoader(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+exe = "/lib64/ld-linux-x86-64.so.2"
+if(os.path.exists(exe)):
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set breakpoints both on shared library function as well as on
+# main. Both of them will be pending breakpoints.
+breakpoint_main = target.BreakpointCreateBySourceRegex("// Break here", lldb.SBFileSpec("main.cpp"))
+breakpoint_shared_library = target.BreakpointCreateBySourceRegex("get_signal_crash", lldb.SBFileSpec("signal_file.cpp"))
+launch_info = lldb.SBLaunchInfo([ "--library-path",self.get_process_working_directory(),self.getBuildArtifact("a.out")])
+launch_info.SetWorkingDirectory(self.get_process_working_directory())
+error = lldb.SBError()
+process = target.Launch(launch_info,error)
+self.assertTrue(error.Success())
+
+# Stopped on main here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+thread = process.GetSelectedThread()
+self.assertIn("main",thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()
+
+# Stopped on get_signal_crash function here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+self.assertIn("get_signal_crash",thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()
+
+# Stopped because of generated signal.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+self.assertIn("raise",thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+self.assertIn("get_signal_crash",thread.GetFrameAtIndex(1).GetDisplayFunctionName())
+
+
Index: lldb/test/API/functionalities/dyld-launch-linux/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/Makefile
@@ -0,0 +1,15 @@
+CXX_SOURCES := main.cpp
+LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
+USE_LIBDL := 1
+
+include Makefile.rules
+
+# The following shared library will be used to test breakpoints under dynamic loading
+libsignal_file.so:  signal_file.cpp
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_ONLY=YES DYLIB_CXX_SOURCES=signal_file.cpp DYLIB_NAME=signal_file
+
+a.out: libsignal_file.so main.cpp
+	$(MAKE) -f $(MAKEFILE_RULES) \
+CXX_SOURCES=main.cpp LD_EXTRAS=libsignal_file.so
+
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
---

[Lldb-commits] [lldb] 1cbdc07 - [lldb] Add support for debugging via the dynamic linker.

2021-08-24 Thread Rumeet Dhindsa via lldb-commits


Author: Rumeet Dhindsa
Date: 2021-08-24T13:41:22-07:00
New Revision: 1cbdc07ec015d83038c08dc562d55eccfd808591

URL: 
https://github.com/llvm/llvm-project/commit/1cbdc07ec015d83038c08dc562d55eccfd808591
DIFF: 
https://github.com/llvm/llvm-project/commit/1cbdc07ec015d83038c08dc562d55eccfd808591.diff

LOG: [lldb] Add support for debugging via the dynamic linker.

This patch adds support for shared library load when the executable is
called through ld.so.

Differential Revision:https://reviews.llvm.org/D108061

Added: 
lldb/test/API/functionalities/dyld-launch-linux/Makefile
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
lldb/test/API/functionalities/dyld-launch-linux/main.cpp
lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
lldb/test/API/functionalities/dyld-launch-linux/signal_file.h

Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index 866acbddbdc8..e385b583cef9 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -56,6 +56,19 @@ static addr_t ResolveRendezvousAddress(Process *process) {
   "%s resolved via direct object file approach to 0x%" PRIx64,
   __FUNCTION__, info_location);
   } else {
+const Symbol *_r_debug =
+target->GetExecutableModule()->FindFirstSymbolWithNameAndType(
+ConstString("_r_debug"));
+if (_r_debug) {
+  info_addr = _r_debug->GetAddress().GetLoadAddress(target);
+  if (info_addr != LLDB_INVALID_ADDRESS) {
+LLDB_LOGF(log,
+  "%s resolved by finding symbol '_r_debug' whose value is 
"
+  "0x%" PRIx64,
+  __FUNCTION__, info_addr);
+return info_addr;
+  }
+}
 LLDB_LOGF(log,
   "%s FAILED - direct object file approach did not yield a "
   "valid address",
@@ -276,6 +289,14 @@ bool DYLDRendezvous::FillSOEntryFromModuleInfo(
   entry.base_addr = base_addr;
   entry.dyn_addr = dyn_addr;
 
+  // ld.so saves empty file name for the executable file in the link map.
+  // When executable is run using ld.so, we need to be update executable path.
+  if (name.empty()) {
+MemoryRegionInfo region;
+Status region_status =
+m_process->GetMemoryRegionInfo(entry.dyn_addr, region);
+name = region.GetName().AsCString();
+  }
   entry.file_spec.SetFile(name, FileSpec::Style::native);
 
   UpdateBaseAddrIfNecessary(entry, name);
@@ -547,6 +568,15 @@ bool DYLDRendezvous::ReadSOEntryFromMemory(lldb::addr_t 
addr, SOEntry ) {
 return false;
 
   std::string file_path = ReadStringFromMemory(entry.path_addr);
+
+  // ld.so saves empty file name for the executable file in the link map.
+  // When executable is run using ld.so, we need to be update executable path.
+  if (file_path.empty()) {
+MemoryRegionInfo region;
+Status region_status =
+m_process->GetMemoryRegionInfo(entry.dyn_addr, region);
+file_path = region.GetName().AsCString();
+  }
   entry.file_spec.SetFile(file_path, FileSpec::Style::native);
 
   UpdateBaseAddrIfNecessary(entry, file_path);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 160faa74af23..09d0523f676c 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -333,28 +333,48 @@ bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
 LLDB_LOG(log, "Rendezvous structure is not set up yet. "
   "Trying to locate rendezvous breakpoint in the interpreter "
   "by symbol name.");
-ModuleSP interpreter = LoadInterpreterModule();
-if (!interpreter) {
-  LLDB_LOG(log, "Can't find interpreter, rendezvous breakpoint isn't 
set.");
-  return false;
-}
-
-// Function names from 
diff erent dynamic loaders that are known to be used
-// as rendezvous between the loader and debuggers.
+// Function names from 
diff erent dynamic loaders that are known to be
+// used as rendezvous between the loader and debuggers.
 static std::vector DebugStateCandidates{
 "_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlactivity",
 "r_debug_state",   "_r_debug_state", "_rtld_debug_state",
 };
 
-FileSpecList containingModules;
-containingModules.Append(interpreter->GetFileSpec());
-dyld_break =

Re: [Lldb-commits] [PATCH] D108510: [lldb] Allow to register frame recognizers applied beyond the first instruction

2021-08-24 Thread Jim Ingham via lldb-commits



> On Aug 24, 2021, at 11:57 AM, Roman Podoliaka via Phabricator 
>  wrote:
> 
> malor added a comment.
> 
> In D108510#2961332 , @jingham wrote:
> 
>> One of the key reasons to use "frame recognizers" is to provide argument 
>> values to functions that don't have debug information.  That generally only 
>> works when stopped at the first instruction, since otherwise you have to 
>> follow the value as it moves through the code, which in the absence of debug 
>> information isn't entirely trivial.
>> 
>> A naive user might think reporting `expr (SomeType *) $arg1` is going to 
>> always work, but in fact it only works reliably on the first instruction.
>> 
>> I don't think that possible incorrect usage means we should only allow frame 
>> recognizers on the first instruction.  But I do think we should say 
>> something more in the help for this flag, like "remember that $arg is 
>> only reliable on the first instruction" to warn folks in advance of this 
>> pitfall.
> 
> Thank you, Jim! That makes sense. I figured it was something along those 
> lines.
> 
> Funnily enough, the way //I// ran into this issue is that I tried to add a 
> frame recognizer for a binary that //had// debug information. In that case, 
> doing something like `breakpoint set -n foo` actually sets the breakpoint 
> *right after* the prologue of a function, so execution won't stop at the 
> first instruction but at something like `foo+4`, and thus the frame 
> recognizer won't be applied.

If you are relying on "first instruction" behavior for breakpoints you can pass 
"--skip-prologue 0" when you set the breakpoint.  For source debugging, you 
generally want to stop after the prologue because debug info doesn't generally 
describe variable locations in the prologue, so the locals variable values will 
likely be wrong.


> 
> I think we should give users a choice and keep the current behavior of only 
> applying recognizers to the first instruction as the default. I'll upload a 
> new revision with the updated documentation.
> 

SGTM

Jim

> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D108510/new/
> 
> https://reviews.llvm.org/D108510
> 

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

[Lldb-commits] [PATCH] D108510: [lldb] Allow to register frame recognizers applied beyond the first instruction

2021-08-24 Thread Roman Podoliaka via Phabricator via lldb-commits

malor added a comment.

In D108510#2961332 , @jingham wrote:

> One of the key reasons to use "frame recognizers" is to provide argument 
> values to functions that don't have debug information.  That generally only 
> works when stopped at the first instruction, since otherwise you have to 
> follow the value as it moves through the code, which in the absence of debug 
> information isn't entirely trivial.
>
> A naive user might think reporting `expr (SomeType *) $arg1` is going to 
> always work, but in fact it only works reliably on the first instruction.
>
> I don't think that possible incorrect usage means we should only allow frame 
> recognizers on the first instruction.  But I do think we should say something 
> more in the help for this flag, like "remember that $arg is only reliable 
> on the first instruction" to warn folks in advance of this pitfall.

Thank you, Jim! That makes sense. I figured it was something along those lines.

Funnily enough, the way //I// ran into this issue is that I tried to add a 
frame recognizer for a binary that //had// debug information. In that case, 
doing something like `breakpoint set -n foo` actually sets the breakpoint 
*right after* the prologue of a function, so execution won't stop at the first 
instruction but at something like `foo+4`, and thus the frame recognizer won't 
be applied.

I think we should give users a choice and keep the current behavior of only 
applying recognizers to the first instruction as the default. I'll upload a new 
revision with the updated documentation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108510

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

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

2021-08-24 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:73
+  llvm::StringRef basename = method.GetBasename();
+  if (basename.empty()) {
+if (CPlusPlusLanguage::ExtractContextAndIdentifier(

jingham wrote:
> The old code checked method.IsValid before pulling out the base name.  It 
> seems sensible that an invalid MethodName would return an empty base name, 
> but that does seem to rely a little on implicit behavior?  Not sure that's a 
> big deal.
Yep, this does rely on implicit behavior. I don't think it's a huge deal, but 
it's easily changeable. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108229

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-24 Thread Rumeet Dhindsa via Phabricator via lldb-commits

rdhindsa added a comment.

Thank you for the review and suggestion to check memory region.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

2021-08-24 Thread Omar Emara via Phabricator via lldb-commits

OmarEmaraDev added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3592
+m_selected_match(0), m_first_visible_match(0) {
+;
+  }

OmarEmaraDev wrote:
> clayborg wrote:
> > Should we be doing just like any other dialog that we have created and be 
> > constructing these items on the fly?
> > 
> > ```
> > m_text_field = AddTextField("Search", "", true);
> > ```
> > It seems the matches could use a modified ChoicesField for the matches? Are 
> > you currently drawing the choices (matches) manually?
> `AddTextField` is part of form delegates, I don't think implementing this as 
> a form is a good idea as they are functionally distinct.
> 
> I am drawing choices manually because the duplicated code is not really a lot 
> and I like to be able to control the style of the drawing. But I guess we can 
> reimplemented that using a choices field.
One thing to note is that text representation of matches are lazily computed in 
this window but not in the choices field, which gives an advantage. We can 
probably add support for lazy computation in the choice field, but it may not 
be worth it at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

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

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

2021-08-24 Thread Omar Emara via Phabricator via lldb-commits

OmarEmaraDev added a comment.

@clayborg I was planning on getting to field completion later as part of a 
global "context window" feature. There are reasons why I need this as a full 
separate window for now. Imminently, I am creating an operator that changes the 
file in the sources window so that breakpoints can be inserted manually. So 
once the user press Ctrl+F, a search window will appear where the user can 
choose a source file to navigate to. This will require a custom searcher, and 
not the common searcher implemented above, which I will add in a following 
patch. What do you think?

Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3592
+m_selected_match(0), m_first_visible_match(0) {
+;
+  }

clayborg wrote:
> Should we be doing just like any other dialog that we have created and be 
> constructing these items on the fly?
> 
> ```
> m_text_field = AddTextField("Search", "", true);
> ```
> It seems the matches could use a modified ChoicesField for the matches? Are 
> you currently drawing the choices (matches) manually?
`AddTextField` is part of form delegates, I don't think implementing this as a 
form is a good idea as they are functionally distinct.

I am drawing choices manually because the duplicated code is not really a lot 
and I like to be able to control the style of the drawing. But I guess we can 
reimplemented that using a choices field.

Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3731
+  SearcherDelegateSP m_delegate_sp;
+  TextFieldDelegate m_text_field;
+  // The index of the currently selected match.

clayborg wrote:
> Is this a new text field itself, or designed to be a reference to an existing 
> text field? 
A new instance of a text field.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

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

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

2021-08-24 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

I like the look of this. It seems we would want this kind of auto complete in 
many fields that we already have, like the file or directory dialogs to start 
with.

My main question is do we need a full window for this, or can be paint a window 
that is just the search results? My thinking is that the right arrow key could 
be used to auto complete a text field if we are at the end of the input. If we 
have a file dialog and you type "~/" and then RIGHT arrow, it would be great if 
this window could appear right at the correct location and not look like a 
complete new window, but just a popup window that augments an existing Field 
(not necessarily a text field!) right below with minimal bordering so that it 
blends into the current form. You can select things using the up and down 
arrows, press enter to update the text, or keep typing to filter more.

Maybe we can:

- Maybe we add a "SearcherWindowDelegate *m_search_window;" member to 
FormDelegate so that this helper UI can appear when needed for any 
FieldDelegate that requests/needs it
  - "m_search_window" can be set to a non NULL value when one of the 
FieldDelegates in the FormDelegate asks for auto completion and provides a 
SearcherDelegate to use and a pointer to the FieldDelegate itself so that the 
SearcherWindowDelegate can be positioned correctly
  - If "m_search_window" is NULL, then never show this UI
  - If "m_search_window" is not NULL, then some keys get sent to the searcher 
delegate before or after the field gets them so it can do the choices UI
- if the user hits ENTER, then the Field is asked to update its value via 
some API in FieldDelegate, and the GUI for the matches disappears and 
m_search_window is freed and set to NULL again

We should hook up at least one of the FieldDelegate objects so that it uses 
this functionality so we can play with it in this patch to make sure it works.

Let me know if you had other plans for this window and how it would be used.




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3592
+m_selected_match(0), m_first_visible_match(0) {
+;
+  }

Should we be doing just like any other dialog that we have created and be 
constructing these items on the fly?

```
m_text_field = AddTextField("Search", "", true);
```
It seems the matches could use a modified ChoicesField for the matches? Are you 
currently drawing the choices (matches) manually?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3635
+
+  void DrawMatches(Surface ) {
+if (m_delegate_sp->GetNumberOfMatches() == 0)

Seems like this could be done using a ChoicesField no instead of drawing 
manually?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3731
+  SearcherDelegateSP m_delegate_sp;
+  TextFieldDelegate m_text_field;
+  // The index of the currently selected match.

Is this a new text field itself, or designed to be a reference to an existing 
text field? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108545

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

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Lldb-commits] [lldb] 120d97b - Revert "[lldb] Add support for debugging via the dynamic linker."

[Lldb-commits] [lldb] d7e2e97 - [LLDB] Remove typos from NativeRegisterContextLinux_arm*

[Lldb-commits] [lldb] ce512d5 - Revert "[lldb] Refactor Module::LookupInfo constructor"

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

[Lldb-commits] [lldb] cd2134e - [lldb] Refactor Module::LookupInfo constructor

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Lldb-commits] [lldb] 1cbdc07 - [lldb] Add support for debugging via the dynamic linker.

Re: [Lldb-commits] [PATCH] D108510: [lldb] Allow to register frame recognizers applied beyond the first instruction

[Lldb-commits] [PATCH] D108510: [lldb] Allow to register frame recognizers applied beyond the first instruction

[Lldb-commits] [PATCH] D108229: [lldb] Refactor Module::LookupInfo constructor

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

[Lldb-commits] [PATCH] D108545: [LLDB][GUI] Add initial searcher support

16 matches

Site Navigation

Mail list logo

Footer information