[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-11 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

Ok, so given that both NetBSD and FreeBSD implement this feature (and the 
current DynamicLoaderPOSIXDYLD plugin reads it), I think we should move this 
bit of code into NativeProcessELF -- after all, that's what we've created it 
for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



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

[Lldb-commits] [PATCH] D62852: Ignore DIEs in the skeleton unit in a DWO scenario

2019-06-11 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits 
kwk added a comment.

> This patch fixes a couple of existing tests,

@labath can you please tell which tests are fixed by this commit exactly? I try 
to reproduce an issue with a test that (potentially under load) used tp 
sometimes hang. Now it does no longer do that and I wonder if your change has 
anything to do with that.

Here's the test call I'm talking about:

  cd ~/llvm/lldb/test/
  
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:~/llvm-builds/relwithdebinfo-ninja-clang-gold-ccache-distcc/lib
 python dotest.py \
-v \
--executable 
~/llvm-builds/relwithdebinfo-ninja-clang-gold-ccache-distcc/bin/lldb \
-f TestVSCode_setBreakpoints.test_functionality \
../../lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62852



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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

2019-06-11 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

Given that you're improving the linux implementation (which is the only one 
that benefits from chunked reads) of ReadMemory in 
https://reviews.llvm.org/D62715, does it make sense to do the chunking here? 
After all, if an implementation (like NetBSD) has an efficient ptrace interface 
for reading memory, then the chunked reads might actually be pessimizing it. 
Theoretically, the linux implementation could be improved further to use the 
chunking ("If I am using ptrace, and have just crossed a page boundary, try 
process_vm_readv again in case the new page happens to be readable"), but I'm 
not sure if that is really necessary.




Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:686
+
+auto str_end = std::memchr(curr_buffer, '\0', bytes_read);
+if (str_end != NULL) {

s/auto/void */



Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:687
+auto str_end = std::memchr(curr_buffer, '\0', bytes_read);
+if (str_end != NULL) {
+  total_bytes_read =

s/NULL/nullptr/



Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:696-697
+curr_buffer += bytes_read;
+curr_addr = reinterpret_cast(reinterpret_cast(curr_addr) +
+ bytes_read);
+bytes_left -= bytes_read;

curr_addr+=bytes_read



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2093
+  error = ReadCStringFromMemory(link_map.l_name,
+reinterpret_cast(&name_buffer),
+sizeof(name_buffer), bytes_read);

It doesn't look like the reinterpret_cast should be needed here.



Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:128-133
+  EXPECT_THAT_ERROR(
+  Process.ReadCStringFromMemory(0x0, &string[0], sizeof(string), 
bytes_read)
+  .ToError(),
+  llvm::Succeeded());
+  EXPECT_STREQ(string, "hel");
+  EXPECT_EQ(bytes_read, 3UL);

I'm wondering how will the caller know that the read has been truncated. Given 
that ReadMemory already returns an error in case of partial reads, maybe we 
could do the same here ? (return an error, but still set bytes_read and the 
string as they are now). What do you think ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D62852: Ignore DIEs in the skeleton unit in a DWO scenario

2019-06-11 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D62852#1537529 , @kwk wrote:

> > This patch fixes a couple of existing tests,
>
> @labath can you please tell which tests are fixed by this commit exactly? I 
> try to reproduce an issue with a test that (potentially under load) used tp 
> sometimes hang. Now it does no longer do that and I wonder if your change has 
> anything to do with that.
>
> Here's the test call I'm talking about:
>
>   cd ~/llvm/lldb/test/
>   
> LD_LIBRARY_PATH=$LD_LIBRARY_PATH:~/llvm-builds/relwithdebinfo-ninja-clang-gold-ccache-distcc/lib
>  python dotest.py \
> -v \
> --executable 
> ~/llvm-builds/relwithdebinfo-ninja-clang-gold-ccache-distcc/bin/lldb \
> -f TestVSCode_setBreakpoints.test_functionality \
> ../../lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint
>


I don't remember which tests exactly were broken by this, but I know that all  
of them were using `__attribute__((always_inline))`, and only  the dwo variant 
was broken.

`TestVSCode_setBreakpoints` does not fit either of the two criteria, and was 
flaky even before the change which "broke" the dwo+inline combo, so I would be 
very surprised if this patch had any effect on that test...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62852



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

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

2019-06-11 Thread Pavel Labath via Phabricator via lldb-commits 
labath marked 11 inline comments as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:309
+  decl.SetFile(
+  die.GetDWARF()->GetFile(*die.GetCU(), form_value.Unsigned()));
   break;

clayborg wrote:
> At the very least we should be asking the unit for the file? That is why I 
> wanted to be able to ask the DWARFDIE for the file because it contains the 
> right unit. If we just put the API on the unit, then we have the chance 
> someone will use the wrong unit for the file. Also, as the DWARF gets fancier 
> as time goes on (DWO, DWZ, etc), the unit might refer to another unit. But at 
> the very least here I would feel better if we ask the DWARFUnit for the file. 
> The GetDWARF() will ask the DWARFUnit in the DIE for the is DWARF file, then 
> we will call the DWARF file class (SymbolFileDWARF or DWARFContext to get a 
> file from the unit by passing a reference? Seems convoluted. Fine not adding 
> the API as a FileSpec if we are trying to keep the API the same as LLVM.
I think asking the unit for the file is fine. I'm not sure why llvm does not do 
that (it does the "get the context, pass the unit to get the line table, fetch 
the file" dance), but it sounds like a utility function doing this could be 
added there. Eventually our DWARFUnit will start using DWARFContext instead of 
SymbolFileDWARF for the other operations, which will make the two approaches 
pretty similar.

I've kept the function as returning FileSpecs. Returning any other type would 
be a pessimization, as the values are already parsed as FileSpecs.


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

https://reviews.llvm.org/D62894



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

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

2019-06-11 Thread Pavel Labath via Phabricator via lldb-commits 
labath updated this revision to Diff 203992.
labath marked an inline comment as done.
labath added a comment.

- add DWARFUnit::GetFile helper function


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

https://reviews.llvm.org/D62894

Files:
  lit/SymbolFile/DWARF/debug-types-line-tables.s
  lit/SymbolFile/DWARF/forward-declarations.s
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -49,6 +49,7 @@
 class DWARFDebugRangesBase;
 class DWARFDeclContext;
 class DWARFFormValue;
+class DWARFTypeUnit;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
@@ -299,6 +300,8 @@
 
   lldb_private::DWARFContext &GetDWARFContext() { return m_context; }
 
+  lldb_private::FileSpec GetFile(DWARFUnit &unit, size_t file_idx);
+
 protected:
   typedef llvm::DenseMap
   DIEToTypePtr;
@@ -438,6 +441,8 @@
 
   SymbolFileDWARFDwp *GetDwpSymbolFile();
 
+  const lldb_private::FileSpecList &GetTypeUnitSupportFiles(DWARFTypeUnit &tu);
+
   lldb::ModuleWP m_debug_map_module_wp;
   SymbolFileDWARFDebugMap *m_debug_map_symfile;
 
@@ -476,6 +481,8 @@
   DIEToVariableSP m_die_to_variable_sp;
   DIEToClangType m_forward_decl_die_to_clang_type;
   ClangTypeToDIE m_forward_decl_clang_type_to_die;
+  llvm::DenseMap
+  m_type_unit_support_files;
 };
 
 #endif // SymbolFileDWARF_SymbolFileDWARF_h_
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -54,6 +54,7 @@
 #include "AppleDWARFIndex.h"
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
+#include "DWARFCompileUnit.h"
 #include "DWARFDebugAbbrev.h"
 #include "DWARFDebugAranges.h"
 #include "DWARFDebugInfo.h"
@@ -62,6 +63,7 @@
 #include "DWARFDebugRanges.h"
 #include "DWARFDeclContext.h"
 #include "DWARFFormValue.h"
+#include "DWARFTypeUnit.h"
 #include "DWARFUnit.h"
 #include "DebugNamesDWARFIndex.h"
 #include "LogChannelDWARF.h"
@@ -775,26 +777,55 @@
 bool SymbolFileDWARF::ParseSupportFiles(CompileUnit &comp_unit,
 FileSpecList &support_files) {
   ASSERT_MODULE_LOCK(this);
-  DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
-  if (dwarf_cu) {
-const DWARFBaseDIE cu_die = dwarf_cu->GetUnitDIEOnly();
-
-if (cu_die) {
-  const dw_offset_t stmt_list = cu_die.GetAttributeValueAsUnsigned(
-  DW_AT_stmt_list, DW_INVALID_OFFSET);
-  if (stmt_list != DW_INVALID_OFFSET) {
-// All file indexes in DWARF are one based and a file of index zero is
-// supposed to be the compile unit itself.
-support_files.Append(comp_unit);
-return DWARFDebugLine::ParseSupportFiles(
-comp_unit.GetModule(), m_context.getOrLoadLineData(), stmt_list,
-support_files, dwarf_cu);
-  }
+  DWARFUnit *unit = GetDWARFCompileUnit(&comp_unit);
+  if (auto *tu = llvm::dyn_cast_or_null(unit)) {
+support_files = GetTypeUnitSupportFiles(*tu);
+return true;
+  }
+
+  if (unit) {
+const dw_offset_t stmt_list = unit->GetLineTableOffset();
+if (stmt_list != DW_INVALID_OFFSET) {
+  // All file indexes in DWARF are one based and a file of index zero is
+  // supposed to be the compile unit itself.
+  support_files.Append(comp_unit);
+  return DWARFDebugLine::ParseSupportFiles(comp_unit.GetModule(),
+   m_context.getOrLoadLineData(),
+   stmt_list, support_files, unit);
 }
   }
   return false;
 }
 
+FileSpec SymbolFileDWARF::GetFile(DWARFUnit &unit, size_t file_idx) {
+  if (CompileUnit *lldb_cu = GetCompUnitForDWARFCompUnit(&unit))
+return lldb_cu->GetSupportFiles().GetFileSpecAtIndex(file_idx);
+  return FileSpec();
+}
+
+const FileSpecList &
+SymbolFileDWARF::GetTypeUnitSupportFiles(DWARFTypeUnit &tu) {
+  static FileSpecList empty_list;
+
+  dw_offset_t offset = tu.GetLineTableOffset();
+  if (offset == DW_INVALID_OFFSET ||
+  offset == llvm::DenseMapInfo::getEmptyKey() ||
+  offset == llvm::DenseMapInfo::getTombstoneKey())
+return empty_list;
+
+  // Many type units can share a line table, so parse the support file list
+

[Lldb-commits] [PATCH] D63110: Fix a crash in option parsing.

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

nice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63110



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

[Lldb-commits] [PATCH] D62756: Be consistent when adding names and mangled names to the index

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D62756#1526996 , @labath wrote:

> Being consistent definitely sounds like a good idea. Since this does change 
> behavior somewhat, I'm wondering whether it would make sense to add a test 
> here. The thing that's not clear to me is whether this change in behavior is 
> only theoretical, or if it can also happen in a real-world scenario. We can 
> always hand-construct an DIE which will have a linkage name, but no "normal" 
> name, but I don't see how would this ever happen in practice. How did you 
> initially discover this inconsistency?


I added asserts in the else clause if code like:

  if (name) {
...
  } else {
assert(mangled_str == nullptr);
  }

And I got the assert. So this is a real issue.

> 
> 
>> We might think about adding a feature where if there is a mangled name and 
>> no simple name, chop up the demangled name like we do in the ObjectFile 
>> plug-ins when we see mangled names so we extract the basename out and then 
>> could use this as the simple name. This might help expressions find things 
>> correctly by basename
> 
> My worry about that is two-fold:
> 
> - it adds a demangling step (a pretty expensive operation) to something that 
> is already the biggest bottleneck in loading DWARF
> - this isn't something that we would do when using accelerator tables, so 
> this will result in inconsistent behavior in these two cases To understand 
> the tradeoffs here, it would be good to understand under what circumstances 
> can this situation occur...

Fine not doing this yet.

I will post a new diff with a simpler solution that maintains performance to 
address Adrian's question of using StringRef.


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

https://reviews.llvm.org/D62756



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

[Lldb-commits] [PATCH] D62756: Be consistent when adding names and mangled names to the index

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg updated this revision to Diff 204104.
clayborg added a comment.

Changed to use ConstString in AddMangled which simplifies the logic quite a 
bit. I verified performance didn't regress.


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

https://reviews.llvm.org/D62756

Files:
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -110,6 +110,21 @@
   }
 }
 
+static inline bool AddMangled(ConstString name, ConstString mangled) {
+  return mangled && name != mangled;
+}
+
+//
+//static inline bool AddMangled(const char *name, const char *mangled) {
+//  if (mangled == nullptr || name == mangled)
+//return false;
+//  if (name == nullptr)
+//return true;
+//  if (mangled[0] != name[0])
+//return true;
+//  return ::strcmp(name, mangled) != 0;
+//}
+
 void ManualDWARFIndex::IndexUnitImpl(
 DWARFUnit &unit, const LanguageType cu_language,
 const dw_offset_t cu_offset, IndexSet &set) {
@@ -139,7 +154,7 @@
 }
 
 DWARFAttributes attributes;
-const char *name = nullptr;
+const char *name_cstr = nullptr;
 const char *mangled_cstr = nullptr;
 bool is_declaration = false;
 // bool is_artificial = false;
@@ -156,7 +171,7 @@
 switch (attr) {
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
-name = form_value.AsCString();
+name_cstr = form_value.AsCString();
   break;
 
 case DW_AT_declaration:
@@ -247,8 +262,9 @@
 case DW_TAG_inlined_subroutine:
 case DW_TAG_subprogram:
   if (has_address) {
+ConstString name(name_cstr);
 if (name) {
-  ObjCLanguage::MethodName objc_method(name, true);
+  ObjCLanguage::MethodName objc_method(name.GetStringRef(), true);
   if (objc_method.IsValid(true)) {
 ConstString objc_class_name_with_category(
 objc_method.GetClassNameWithCategory());
@@ -256,7 +272,7 @@
 ConstString objc_fullname_no_category_name(
 objc_method.GetFullNameWithoutCategory(true));
 ConstString objc_class_name_no_category(objc_method.GetClassName());
-set.function_fullnames.Insert(ConstString(name), ref);
+set.function_fullnames.Insert(name, ref);
 if (objc_class_name_with_category)
   set.objc_class_selectors.Insert(objc_class_name_with_category,
   ref);
@@ -274,24 +290,17 @@
   bool is_method = DWARFDIE(&unit, &die).IsMethod();
 
   if (is_method)
-set.function_methods.Insert(ConstString(name), ref);
+set.function_methods.Insert(name, ref);
   else
-set.function_basenames.Insert(ConstString(name), ref);
+set.function_basenames.Insert(name, ref);
 
   if (!is_method && !mangled_cstr && !objc_method.IsValid(true))
-set.function_fullnames.Insert(ConstString(name), ref);
+set.function_fullnames.Insert(name, ref);
 }
-if (mangled_cstr) {
-  // Make sure our mangled name isn't the same string table entry as
-  // our name. If it starts with '_', then it is ok, else compare the
-  // string to make sure it isn't the same and we don't end up with
-  // duplicate entries
-  if (name && name != mangled_cstr &&
-  ((mangled_cstr[0] == '_') ||
-   (::strcmp(name, mangled_cstr) != 0))) {
-set.function_fullnames.Insert(ConstString(mangled_cstr), ref);
-  }
-}
+// Make sure our mangled name isn't the same as our name.
+ConstString mangled(mangled_cstr);
+if (AddMangled(name, mangled))
+ set.function_fullnames.Insert(mangled, ref);
   }
   break;
 
@@ -306,33 +315,36 @@
 case DW_TAG_typedef:
 case DW_TAG_union_type:
 case DW_TAG_unspecified_type:
-  if (name && !is_declaration)
-set.types.Insert(ConstString(name), ref);
-  if (mangled_cstr && !is_declaration)
-set.types.Insert(ConstString(mangled_cstr), ref);
+  if (!is_declaration) {
+ConstString name(name_cstr);
+if (name)
+  set.types.Insert(name, ref);
+// Make sure our mangled name isn't the same as our name.
+ConstString mangled(mangled_cstr);
+if (AddMangled(name, mangled))
+  set.types.Insert(mangled, ref);
+  }
   break;
 
 case DW_TAG_namespace:
-  if (name)
-set.namespaces.Insert(ConstString(name), ref);
+  if (name_cstr)
+set.namespaces.Insert(ConstString(name_cstr), ref);
   break;
 
 case DW_TAG_variable:
-  if (name && has_location_or_const_value && is_global_or_stati

[Lldb-commits] [PATCH] D62756: Be consistent when adding names and mangled names to the index

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg updated this revision to Diff 204116.
clayborg added a comment.

Remove commented out code.


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

https://reviews.llvm.org/D62756

Files:
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -110,6 +110,10 @@
   }
 }
 
+static inline bool AddMangled(ConstString name, ConstString mangled) {
+  return mangled && name != mangled;
+}
+
 void ManualDWARFIndex::IndexUnitImpl(
 DWARFUnit &unit, const LanguageType cu_language,
 const dw_offset_t cu_offset, IndexSet &set) {
@@ -139,7 +143,7 @@
 }
 
 DWARFAttributes attributes;
-const char *name = nullptr;
+const char *name_cstr = nullptr;
 const char *mangled_cstr = nullptr;
 bool is_declaration = false;
 // bool is_artificial = false;
@@ -156,7 +160,7 @@
 switch (attr) {
 case DW_AT_name:
   if (attributes.ExtractFormValueAtIndex(i, form_value))
-name = form_value.AsCString();
+name_cstr = form_value.AsCString();
   break;
 
 case DW_AT_declaration:
@@ -247,8 +251,9 @@
 case DW_TAG_inlined_subroutine:
 case DW_TAG_subprogram:
   if (has_address) {
+ConstString name(name_cstr);
 if (name) {
-  ObjCLanguage::MethodName objc_method(name, true);
+  ObjCLanguage::MethodName objc_method(name.GetStringRef(), true);
   if (objc_method.IsValid(true)) {
 ConstString objc_class_name_with_category(
 objc_method.GetClassNameWithCategory());
@@ -256,7 +261,7 @@
 ConstString objc_fullname_no_category_name(
 objc_method.GetFullNameWithoutCategory(true));
 ConstString objc_class_name_no_category(objc_method.GetClassName());
-set.function_fullnames.Insert(ConstString(name), ref);
+set.function_fullnames.Insert(name, ref);
 if (objc_class_name_with_category)
   set.objc_class_selectors.Insert(objc_class_name_with_category,
   ref);
@@ -274,24 +279,17 @@
   bool is_method = DWARFDIE(&unit, &die).IsMethod();
 
   if (is_method)
-set.function_methods.Insert(ConstString(name), ref);
+set.function_methods.Insert(name, ref);
   else
-set.function_basenames.Insert(ConstString(name), ref);
+set.function_basenames.Insert(name, ref);
 
   if (!is_method && !mangled_cstr && !objc_method.IsValid(true))
-set.function_fullnames.Insert(ConstString(name), ref);
+set.function_fullnames.Insert(name, ref);
 }
-if (mangled_cstr) {
-  // Make sure our mangled name isn't the same string table entry as
-  // our name. If it starts with '_', then it is ok, else compare the
-  // string to make sure it isn't the same and we don't end up with
-  // duplicate entries
-  if (name && name != mangled_cstr &&
-  ((mangled_cstr[0] == '_') ||
-   (::strcmp(name, mangled_cstr) != 0))) {
-set.function_fullnames.Insert(ConstString(mangled_cstr), ref);
-  }
-}
+// Make sure our mangled name isn't the same as our name.
+ConstString mangled(mangled_cstr);
+if (AddMangled(name, mangled))
+  set.function_fullnames.Insert(mangled, ref);
   }
   break;
 
@@ -306,33 +304,36 @@
 case DW_TAG_typedef:
 case DW_TAG_union_type:
 case DW_TAG_unspecified_type:
-  if (name && !is_declaration)
-set.types.Insert(ConstString(name), ref);
-  if (mangled_cstr && !is_declaration)
-set.types.Insert(ConstString(mangled_cstr), ref);
+  if (!is_declaration) {
+ConstString name(name_cstr);
+if (name)
+  set.types.Insert(name, ref);
+// Make sure our mangled name isn't the same as our name.
+ConstString mangled(mangled_cstr);
+if (AddMangled(name, mangled))
+  set.types.Insert(mangled, ref);
+  }
   break;
 
 case DW_TAG_namespace:
-  if (name)
-set.namespaces.Insert(ConstString(name), ref);
+  if (name_cstr)
+set.namespaces.Insert(ConstString(name_cstr), ref);
   break;
 
 case DW_TAG_variable:
-  if (name && has_location_or_const_value && is_global_or_static_variable) {
-set.globals.Insert(ConstString(name), ref);
+  if (has_location_or_const_value && is_global_or_static_variable) {
+ConstString name(name_cstr);
+if (name)
+  set.globals.Insert(name, ref);
 // Be sure to include variables by their mangled and demangled names if
 // they have any since a variable can have a basename "i"

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

2019-06-11 Thread António Afonso via Phabricator via lldb-commits 
aadsm added a comment.

Yeah, that makes sense. I was thinking about this yesterday after checking the 
freebsd code. I was concerned if there's something different that I'm not aware 
of but we do have tests and I guess we can also override these functions if 
needed in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



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

[Lldb-commits] [PATCH] D62500: Add support to read aux vector values

2019-06-11 Thread António Afonso via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363098: Add support to read aux vector values (authored by 
aadsm, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62500?vs=202853&id=204144#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62500

Files:
  lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/trunk/include/lldb/Target/Process.h
  lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.cpp
  lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/AuxVector.h
  lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
  lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
  lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/trunk/source/Plugins/Process/Utility/AuxVector.cpp
  lldb/trunk/source/Plugins/Process/Utility/AuxVector.h
  lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/trunk/source/Target/Process.cpp

Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
===
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
@@ -127,7 +127,7 @@
   size_t PutSTDIN(const char *buf, size_t len,
   lldb_private::Status &error) override;
 
-  const lldb::DataBufferSP GetAuxvData() override;
+  const lldb_private::DataExtractor GetAuxvData() override;
 
   // ProcessFreeBSD internal API.
 
Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
===
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
@@ -876,7 +876,7 @@
   return is_running;
 }
 
-const DataBufferSP ProcessFreeBSD::GetAuxvData() {
+lldb_private::DataExtractor ProcessFreeBSD::GetAuxvData() {
   // If we're the local platform, we can ask the host for auxv data.
   PlatformSP platform_sp = GetTarget().GetPlatform();
   assert(platform_sp && platform_sp->IsHost());
@@ -890,7 +890,7 @@
 buf_sp.reset();
   }
 
-  return buf_sp;
+  return DataExtractor(buf_sp, GetByteOrder(), GetAddressByteSize());
 }
 
 struct EmulatorBaton {
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4046,7 +4046,7 @@
   return error;
 }
 
-const DataBufferSP ProcessGDBRemote::GetAuxvData() {
+DataExtractor ProcessGDBRemote::GetAuxvData() {
   DataBufferSP buf;
   if (m_gdb_comm.GetQXferAuxvReadSupported()) {
 std::string response_string;
@@ -4056,7 +4056,7 @@
   buf = std::make_shared(response_string.c_str(),
  response_string.length());
   }
-  return buf;
+  return DataExtractor(buf, GetByteOrder(), GetAddressByteSize());
 }
 
 StructuredData::ObjectSP
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -324,7 +324,7 @@
 
   bool ParsePythonTargetDefinition(const FileSpec &target_definition_fspec);
 
-  const lldb::DataBufferSP GetAuxvData() override;
+  DataExtractor GetAuxvData() override;
 
   StructuredData::ObjectSP GetExtendedInfoForThread(lldb::tid_t tid);
 
Index: lldb/trunk/source/Plugins/Process/Utility/AuxVector.h
===
--- lldb/trunk/source/Plugins/Process/Utility/AuxVector.h
+++ lldb/trunk/source/Plugins/Process/Utility/AuxVector.h
@@ -0,0 +1,73 @@
+//===-- AuxVector.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_AuxVector_H_
+#define liblldb_AuxVector_H

[Lldb-commits] [PATCH] D63166: Move common functionality from processwindows into processdebugger

2019-06-11 Thread Aaron Smith via Phabricator via lldb-commits 
asmith created this revision.
asmith added reviewers: labath, Hui.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.
asmith added reviewers: jfb, clayborg.
Herald added a subscriber: dexonsmith.

This change extracts functionalities from processwindows into a
introduced processdebugger that can be reused in native process
debugging.

The main reason is that the native process debugging
can't directly be based on processwindows or be implemented
as a pass-through to this plugin since the plugin has ties to
Target and Process classes that are needed in host debugging but
not necessary in native debugging.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D63166

Files:
  source/Plugins/Process/Windows/Common/CMakeLists.txt
  source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  source/Plugins/Process/Windows/Common/ProcessDebugger.h
  source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  source/Plugins/Process/Windows/Common/ProcessWindows.h

Index: source/Plugins/Process/Windows/Common/ProcessWindows.h
===
--- source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -13,17 +13,14 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-forward.h"
 
-#include "llvm/Support/Mutex.h"
-
-#include "IDebugDelegate.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "ProcessDebugger.h"
 
 namespace lldb_private {
 
 class HostProcess;
-class ProcessWindowsData;
 
-class ProcessWindows : public Process, public IDebugDelegate {
+class ProcessWindows : public Process, public ProcessDebugger {
 public:
   // Static functions.
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
@@ -101,19 +98,7 @@
   void OnUnloadDll(lldb::addr_t module_addr) override;
   void OnDebugString(const std::string &string) override;
   void OnDebuggerError(const Status &error, uint32_t type) override;
-
-private:
-  Status WaitForDebuggerConnection(DebuggerThreadSP debugger,
-   HostProcess &process);
-
-  // These decode the page protection bits.
-  static bool IsPageReadable(uint32_t protect);
-  static bool IsPageWritable(uint32_t protect);
-  static bool IsPageExecutable(uint32_t protect);
-
-  llvm::sys::Mutex m_mutex;
-  std::unique_ptr m_session_data;
 };
-}
+} // namespace lldb_private
 
 #endif // liblldb_Plugins_Process_Windows_Common_ProcessWindows_H_
Index: source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -70,46 +70,10 @@
   }
   return file_name;
 }
-
-DWORD ConvertLldbToWinApiProtect(uint32_t protect) {
-  // We also can process a read / write permissions here, but if the debugger
-  // will make later a write into the allocated memory, it will fail. To get
-  // around it is possible inside DoWriteMemory to remember memory permissions,
-  // allow write, write and restore permissions, but for now we process only
-  // the executable permission.
-  //
-  // TODO: Process permissions other than executable
-  if (protect & ePermissionsExecutable)
-return PAGE_EXECUTE_READWRITE;
-
-  return PAGE_READWRITE;
-}
-
 } // anonymous namespace
 
 namespace lldb_private {
 
-// We store a pointer to this class in the ProcessWindows, so that we don't
-// expose Windows-specific types and implementation details from a public
-// header file.
-class ProcessWindowsData {
-public:
-  ProcessWindowsData(bool stop_at_entry) : m_stop_at_entry(stop_at_entry) {
-m_initial_stop_event = ::CreateEvent(nullptr, TRUE, FALSE, nullptr);
-  }
-
-  ~ProcessWindowsData() { ::CloseHandle(m_initial_stop_event); }
-
-  Status m_launch_error;
-  DebuggerThreadSP m_debugger;
-  StopInfoSP m_pending_stop_info;
-  HANDLE m_initial_stop_event = nullptr;
-  bool m_initial_stop_received = false;
-  bool m_stop_at_entry;
-  std::map m_new_threads;
-  std::set m_exited_threads;
-};
-
 ProcessSP ProcessWindows::CreateInstance(lldb::TargetSP target_sp,
  lldb::ListenerSP listener_sp,
  const FileSpec *) {
@@ -192,159 +156,41 @@
 }
 
 Status ProcessWindows::DoDetach(bool keep_stopped) {
-  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
-  DebuggerThreadSP debugger_thread;
-  StateType private_state;
-  {
-// Acquire the lock only long enough to get the DebuggerThread.
-// StopDebugging() will trigger a call back into ProcessWindows which will
-// also acquire the lock.  Thus we have to release the lock before calling
-// StopDebugging().
-llvm::sys::ScopedLock lock(m_mutex);
-
-private_state = GetPrivateState();
-
-if (!m_session_data) {
-  LLDB_LOG(log, "state = {0}, but there is no active sessio

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

I don't see much I would change here as long as this works and gets tested by 
the generic GDB remote protocol testing? Any others have comments?




Comment at: source/Plugins/Process/Windows/Common/DebuggerThread.cpp:350
+(info.ExceptionRecord.ExceptionCode == EXCEPTION_BREAKPOINT ||
+ info.ExceptionRecord.ExceptionCode == 0x401FL /* WOW64 
STATUS_WX86_BREAKPOINT */)) {
   LLDB_LOG(log, "Breakpoint exception is cue to detach from process {0:x}",

Define STATUS_WX86_BREAKPOINT somewhere and don't use a magic number?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63165



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

[Lldb-commits] [lldb] r363101 - Fix a crash in option parsing.

2019-06-11 Thread Adrian Prantl via lldb-commits 
Author: adrian
Date: Tue Jun 11 14:14:02 2019
New Revision: 363101

URL: http://llvm.org/viewvc/llvm-project?rev=363101&view=rev
Log:
Fix a crash in option parsing.

The call to getopt_long didn't handle the case where the *last* option
had an argument missing.



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

Added:
lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
lldb/trunk/lit/Driver/TestProcessAttach.test
Modified:
lldb/trunk/source/Interpreter/Options.cpp

Added: lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Driver/Inputs/process_attach_pid.in?rev=363101&view=auto
==
--- lldb/trunk/lit/Driver/Inputs/process_attach_pid.in (added)
+++ lldb/trunk/lit/Driver/Inputs/process_attach_pid.in Tue Jun 11 14:14:02 2019
@@ -0,0 +1 @@
+process attach --pid

Added: lldb/trunk/lit/Driver/TestProcessAttach.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Driver/TestProcessAttach.test?rev=363101&view=auto
==
--- lldb/trunk/lit/Driver/TestProcessAttach.test (added)
+++ lldb/trunk/lit/Driver/TestProcessAttach.test Tue Jun 11 14:14:02 2019
@@ -0,0 +1,2 @@
+# RUN: %lldb -x -b -S %S/Inputs/process_attach_pid.in 2>&1 | FileCheck %s
+# CHECK: requires an argument

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=363101&r1=363100&r2=363101&view=diff
==
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Tue Jun 11 14:14:02 2019
@@ -1362,6 +1362,12 @@ llvm::Expected Options::Parse(cons
 int long_options_index = -1;
 val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
   long_options, &long_options_index);
+
+if ((size_t)OptionParser::GetOptionIndex() > argv.size()) {
+  error.SetErrorStringWithFormat("option requires an argument");
+  break;
+}
+
 if (val == -1)
   break;
 


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

[Lldb-commits] [PATCH] D63110: Fix a crash in option parsing.

2019-06-11 Thread Phabricator via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363101: Fix a crash in option parsing. (authored by adrian, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63110?vs=203940&id=204164#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63110

Files:
  lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
  lldb/trunk/lit/Driver/TestProcessAttach.test
  lldb/trunk/source/Interpreter/Options.cpp


Index: lldb/trunk/source/Interpreter/Options.cpp
===
--- lldb/trunk/source/Interpreter/Options.cpp
+++ lldb/trunk/source/Interpreter/Options.cpp
@@ -1362,6 +1362,12 @@
 int long_options_index = -1;
 val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
   long_options, &long_options_index);
+
+if ((size_t)OptionParser::GetOptionIndex() > argv.size()) {
+  error.SetErrorStringWithFormat("option requires an argument");
+  break;
+}
+
 if (val == -1)
   break;
 
Index: lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
===
--- lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
+++ lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
@@ -0,0 +1 @@
+process attach --pid
Index: lldb/trunk/lit/Driver/TestProcessAttach.test
===
--- lldb/trunk/lit/Driver/TestProcessAttach.test
+++ lldb/trunk/lit/Driver/TestProcessAttach.test
@@ -0,0 +1,2 @@
+# RUN: %lldb -x -b -S %S/Inputs/process_attach_pid.in 2>&1 | FileCheck %s
+# CHECK: requires an argument


Index: lldb/trunk/source/Interpreter/Options.cpp
===
--- lldb/trunk/source/Interpreter/Options.cpp
+++ lldb/trunk/source/Interpreter/Options.cpp
@@ -1362,6 +1362,12 @@
 int long_options_index = -1;
 val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
   long_options, &long_options_index);
+
+if ((size_t)OptionParser::GetOptionIndex() > argv.size()) {
+  error.SetErrorStringWithFormat("option requires an argument");
+  break;
+}
+
 if (val == -1)
   break;
 
Index: lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
===
--- lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
+++ lldb/trunk/lit/Driver/Inputs/process_attach_pid.in
@@ -0,0 +1 @@
+process attach --pid
Index: lldb/trunk/lit/Driver/TestProcessAttach.test
===
--- lldb/trunk/lit/Driver/TestProcessAttach.test
+++ lldb/trunk/lit/Driver/TestProcessAttach.test
@@ -0,0 +1,2 @@
+# RUN: %lldb -x -b -S %S/Inputs/process_attach_pid.in 2>&1 | FileCheck %s
+# CHECK: requires an argument
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r363102 - Update AuxVector.cpp

2019-06-11 Thread Jason Molenda via lldb-commits 
Author: jmolenda
Date: Tue Jun 11 14:20:34 2019
New Revision: 363102

URL: http://llvm.org/viewvc/llvm-project?rev=363102&view=rev
Log:
Update AuxVector.cpp

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=363102&r1=363101&r2=363102&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Jun 11 14:20:34 2019
@@ -129,7 +129,7 @@
2689007D13353E2200698AC0 /* Args.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 26BC7E6C10F1B85900F91463 /* Args.cpp */; };
23CB153C1D66DA9300EDDDE1 /* ArgsTest.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 2321F93E1BDD33CE00BA9A93 /* ArgsTest.cpp */; };
6D99A3631BBC2F3200979793 /* ArmUnwindInfo.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 6D99A3621BBC2F3200979793 /* ArmUnwindInfo.cpp 
*/; };
-   26FFC19914FC072100087D58 /* AuxVector.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = 26FFC19314FC072100087D58 /* AuxVector.cpp */; };
+   AF7BD81B22B04E20008E78D1 /* AuxVector.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = AF7BD81A22B04E20008E78D1 /* AuxVector.cpp */; };
AFC2DCE91E6E2F2C00283714 /* Baton.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = AFC2DCE81E6E2F2C00283714 /* Baton.cpp */; };
268900D013353E6F00698AC0 /* Block.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 26BC7F1310F1B8EC00F91463 /* Block.cpp */; };
49DEF1251CD7C6DF006A7C7D /* BlockPointer.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 49DEF11F1CD7BD90006A7C7D /* BlockPointer.cpp */; 
};
@@ -1493,8 +1493,7 @@
6D99A3621BBC2F3200979793 /* ArmUnwindInfo.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = ArmUnwindInfo.cpp; path = source/Symbol/ArmUnwindInfo.cpp; sourceTree = 
""; };
6D99A3611BBC2F1600979793 /* ArmUnwindInfo.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ArmUnwindInfo.h; 
path = include/lldb/Symbol/ArmUnwindInfo.h; sourceTree = ""; };
3FDFE54719A2946B009756A7 /* AutoHandle.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = AutoHandle.h; path 
= include/lldb/Host/windows/AutoHandle.h; sourceTree = ""; };
-   26FFC19314FC072100087D58 /* AuxVector.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = AuxVector.cpp; sourceTree = ""; };
-   26FFC19414FC072100087D58 /* AuxVector.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
AuxVector.h; sourceTree = ""; };
+   AF7BD81A22B04E20008E78D1 /* AuxVector.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = AuxVector.cpp; path = Utility/AuxVector.cpp; sourceTree = ""; };
AFC2DCE81E6E2F2C00283714 /* Baton.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = Baton.cpp; path = source/Utility/Baton.cpp; sourceTree = ""; };
AFC2DCEE1E6E2FA300283714 /* Baton.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = Baton.h; path = 
include/lldb/Utility/Baton.h; sourceTree = ""; };
26BC7F1310F1B8EC00F91463 /* Block.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = Block.cpp; path = source/Symbol/Block.cpp; sourceTree = ""; };
@@ -5009,6 +5008,7 @@
26B4666E11A2080F00CF6220 /* Utility */ = {
isa = PBXGroup;
children = (
+   AF7BD81A22B04E20008E78D1 /* AuxVector.cpp */,
9A0FDE951E8EF5010086B2F5 /* 
RegisterContext_mips.h */,
9A0FDE961E8EF5010086B2F5 /* 
RegisterContext_s390x.h */,
9A0FDE971E8EF5010086B2F5 /* 
RegisterContextLinux_mips.cpp */,
@@ -6069,8 +6069,6 @@
26FFC19214FC072100087D58 /* POSIX-DYLD */ = {
isa = PBXGroup;
children = (
-   26FFC19314FC072100087D58 /* AuxVector.cpp */,
-   26FFC19414FC072100087D58 /* AuxVector.h */,
26FFC19514FC072100087D58 /* DYLDRendezvous.cpp 
*/,
26FFC19614FC072100087D58 /* DYLDRendezvous.h */,
26FFC19714FC072100087D58 /* 
DynamicLoaderPOSIXDYLD.cpp */,
@@ -8314,6 +8312,7 @@
268900D913353E6F00698AC0 /* FuncUnwinders.cpp 
in Sources */,
268900DA13353E6F0069

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

2019-06-11 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added inline comments.



Comment at: 
source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp:66
+const_cast(tdecl->getTypeForDecl(;
+return compiler_type;
+  }

I think there's a matching constructor, which would allow you to get rid of the 
temporary and do 
```
return 
CompilerType(ClangASTContext::GetASTContext(&tdecl->getASTContext()),reinterpret_cast(const_cast(tdecl->getTypeForDecl(;
```


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

https://reviews.llvm.org/D62797



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

[Lldb-commits] [lldb] r363103 - When reading ObjC class table, use new SPI if it is avail

2019-06-11 Thread Jason Molenda via lldb-commits 
Author: jmolenda
Date: Tue Jun 11 14:31:19 2019
New Revision: 363103

URL: http://llvm.org/viewvc/llvm-project?rev=363103&view=rev
Log:
When reading ObjC class table, use new SPI if it is avail

In the latest OS betas, the objc runtime has a special interface
for the debugger, class_getNameRaw(), instead of the existing
class_getName(), which will return class names in their raw, unmangled
(in the case of swift) form.  When lldb can access the unmangled
names of classes, it won't need to fetch them out of the inferior
process after we run our "get the objc class table" expression.

If the new interface is absent (debugging a process on an older
target), lldb will fall back to class_getName and reading any class
names that it got back in demangled form, at a bit of a performance
cost on the first expression.

 

Modified:

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp?rev=363103&r1=363102&r2=363103&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 Tue Jun 11 14:31:19 2019
@@ -157,6 +157,16 @@ __lldb_apple_objc_v2_get_dynamic_class_i
 
 )";
 
+// We'll substitute in class_getName or class_getNameRaw depending
+// on which is present.
+static const char *g_shared_cache_class_name_funcptr = R"(
+extern "C"
+{
+const char *%s(void *objc_class);
+const char *(*class_name_lookup_func)(void *) = %s;
+}
+)";
+
 static const char *g_get_shared_cache_class_info_name =
 "__lldb_apple_objc_v2_get_shared_cache_class_info";
 // Testing using the new C++11 raw string literals. If this breaks GCC then we
@@ -165,7 +175,6 @@ static const char *g_get_shared_cache_cl
 
 extern "C"
 {
-const char *class_getName(void *objc_class);
 size_t strlen(const char *);
 char *strncpy (char * s1, const char * s2, size_t n);
 int printf(const char * format, ...);
@@ -286,7 +295,7 @@ __lldb_apple_objc_v2_get_shared_cache_cl
 if (class_infos && idx < max_class_infos)
 {
 class_infos[idx].isa = (Class)((uint8_t *)clsopt + 
clsOffset);
-const char *name = class_getName (class_infos[idx].isa);
+const char *name = class_name_lookup_func 
(class_infos[idx].isa);
 DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, 
class_infos[idx].isa, name);
 // Hash the class name so we don't have to read it
 const char *s = name;
@@ -329,7 +338,7 @@ __lldb_apple_objc_v2_get_shared_cache_cl
 if (class_infos && idx < max_class_infos)
 {
 class_infos[idx].isa = (Class)((uint8_t *)clsopt + 
clsOffset);
-const char *name = class_getName (class_infos[idx].isa);
+const char *name = class_name_lookup_func 
(class_infos[idx].isa);
 DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, 
class_infos[idx].isa, name);
 // Hash the class name so we don't have to read it
 const char *s = name;
@@ -1589,9 +1598,46 @@ AppleObjCRuntimeV2::UpdateISAToDescripto
 
   if (!m_get_shared_cache_class_info_code) {
 Status error;
+
+// If the inferior objc.dylib has the class_getNameRaw function,
+// use that in our jitted expression.  Else fall back to the old
+// class_getName.
+static ConstString g_class_getName_symbol_name("class_getName");
+static ConstString g_class_getNameRaw_symbol_name("class_getNameRaw");
+ConstString class_name_getter_function_name = g_class_getName_symbol_name;
+
+ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
+if (objc_runtime) {
+  const ModuleList &images = process->GetTarget().GetImages();
+  std::lock_guard guard(images.GetMutex());
+  for (size_t i = 0; i < images.GetSize(); ++i) {
+lldb::ModuleSP mod_sp = images.GetModuleAtIndexUnlocked(i);
+if (objc_runtime->IsModuleObjCLibrary(mod_sp)) {
+  const Symbol *symbol =
+  
mod_sp->FindFirstSymbolWithNameAndType(g_class_getNameRaw_symbol_name, 
+lldb::eSymbolTypeCode);
+  if (symbol && 
+  (symbol->ValueIsAddress() || symbol->GetAddressRef().IsValid())) 
{
+class_name_getter_function_name = g_class_getNameRaw_symbol_name;
+  }
+}
+  }
+}
+
+// Substitute in the correct class_getName / class_getNameRaw function 
name,
+// concatenate the two parts of our expression text.  The format st

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg created this revision.
clayborg added reviewers: labath, aprantl, jingham.
Herald added a subscriber: arphaman.

Improve manual indexing performance when indexing non objective C code. One 
question I have is all Darwin compilers currently support the apple DWARF 
indexes, so do we even need the objective C parsing code here?


https://reviews.llvm.org/D63171

Files:
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -113,6 +113,9 @@
 void ManualDWARFIndex::IndexUnitImpl(
 DWARFUnit &unit, const LanguageType cu_language,
 const dw_offset_t cu_offset, IndexSet &set) {
+
+  const bool check_objc = cu_language == eLanguageTypeObjC ||
+  cu_language == eLanguageTypeObjC_plus_plus;
   for (const DWARFDebugInfoEntry &die : unit.dies()) {
 const dw_tag_t tag = die.Tag();
 
@@ -248,26 +251,29 @@
 case DW_TAG_subprogram:
   if (has_address) {
 if (name) {
-  ObjCLanguage::MethodName objc_method(name, true);
-  if (objc_method.IsValid(true)) {
-ConstString objc_class_name_with_category(
-objc_method.GetClassNameWithCategory());
-ConstString objc_selector_name(objc_method.GetSelector());
-ConstString objc_fullname_no_category_name(
-objc_method.GetFullNameWithoutCategory(true));
-ConstString 
objc_class_name_no_category(objc_method.GetClassName());
-set.function_fullnames.Insert(ConstString(name), ref);
-if (objc_class_name_with_category)
-  set.objc_class_selectors.Insert(objc_class_name_with_category,
+  bool is_objc_method = false;
+  if (check_objc) {
+ObjCLanguage::MethodName objc_method(name, true);
+if (objc_method.IsValid(true)) {
+  is_objc_method = true;
+  ConstString class_name_with_category(
+  objc_method.GetClassNameWithCategory());
+  ConstString objc_selector_name(objc_method.GetSelector());
+  ConstString objc_fullname_no_category_name(
+  objc_method.GetFullNameWithoutCategory(true));
+  ConstString class_name_no_category(objc_method.GetClassName());
+  set.function_fullnames.Insert(ConstString(name), ref);
+  if (class_name_with_category)
+set.objc_class_selectors.Insert(class_name_with_category, ref);
+  if (class_name_no_category &&
+  class_name_no_category != class_name_with_category)
+set.objc_class_selectors.Insert(class_name_no_category, ref);
+  if (objc_selector_name)
+set.function_selectors.Insert(objc_selector_name, ref);
+  if (objc_fullname_no_category_name)
+set.function_fullnames.Insert(objc_fullname_no_category_name,
   ref);
-if (objc_class_name_no_category &&
-objc_class_name_no_category != objc_class_name_with_category)
-  set.objc_class_selectors.Insert(objc_class_name_no_category, 
ref);
-if (objc_selector_name)
-  set.function_selectors.Insert(objc_selector_name, ref);
-if (objc_fullname_no_category_name)
-  set.function_fullnames.Insert(objc_fullname_no_category_name,
-ref);
+}
   }
   // If we have a mangled name, then the DW_AT_name attribute is
   // usually the method name without the class or any parameters
@@ -278,7 +284,7 @@
   else
 set.function_basenames.Insert(ConstString(name), ref);
 
-  if (!is_method && !mangled_cstr && !objc_method.IsValid(true))
+  if (!is_method && !mangled_cstr && !is_objc_method)
 set.function_fullnames.Insert(ConstString(name), ref);
 }
 if (mangled_cstr) {


Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -113,6 +113,9 @@
 void ManualDWARFIndex::IndexUnitImpl(
 DWARFUnit &unit, const LanguageType cu_language,
 const dw_offset_t cu_offset, IndexSet &set) {
+
+  const bool check_objc = cu_language == eLanguageTypeObjC ||
+  cu_language == eLanguageTypeObjC_plus_plus;
   for (const DWARFDebugInfoEntry &die : unit.dies()) {
 const dw_tag_t tag = die.Tag();
 
@@ -248,26 +251,29 @@
 case DW_TAG_subprogram:
   if (has_address) {
 if (name) {
-  ObjCLanguage::MethodName objc_method(name, true);
-  if (objc_method

[Lldb-commits] [lldb] r363109 - [LanguageRuntime] Simplify CreateExceptionSearchFilter in derived classes

2019-06-11 Thread Alex Langford via lldb-commits 
Author: xiaobai
Date: Tue Jun 11 15:52:08 2019
New Revision: 363109

URL: http://llvm.org/viewvc/llvm-project?rev=363109&view=rev
Log:
[LanguageRuntime] Simplify CreateExceptionSearchFilter in derived classes

Modified:
lldb/trunk/include/lldb/Target/LanguageRuntime.h

lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/trunk/source/Target/LanguageRuntime.cpp

Modified: lldb/trunk/include/lldb/Target/LanguageRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/LanguageRuntime.h?rev=363109&r1=363108&r2=363109&view=diff
==
--- lldb/trunk/include/lldb/Target/LanguageRuntime.h (original)
+++ lldb/trunk/include/lldb/Target/LanguageRuntime.h Tue Jun 11 15:52:08 2019
@@ -136,7 +136,9 @@ public:
   virtual lldb::BreakpointResolverSP
   CreateExceptionResolver(Breakpoint *bkpt, bool catch_bp, bool throw_bp) = 0;
 
-  virtual lldb::SearchFilterSP CreateExceptionSearchFilter();
+  virtual lldb::SearchFilterSP CreateExceptionSearchFilter() {
+return m_process->GetTarget().GetSearchFilterForModule(nullptr);
+  }
 
   virtual bool GetTypeBitSize(const CompilerType &compiler_type,
   uint64_t &size) {

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp?rev=363109&r1=363108&r2=363109&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
 Tue Jun 11 15:52:08 2019
@@ -475,16 +475,14 @@ BreakpointResolverSP ItaniumABILanguageR
 lldb::SearchFilterSP ItaniumABILanguageRuntime::CreateExceptionSearchFilter() {
   Target &target = m_process->GetTarget();
 
+  FileSpecList filter_modules;
   if (target.GetArchitecture().GetTriple().getVendor() == llvm::Triple::Apple) 
{
 // Limit the number of modules that are searched for these breakpoints for
 // Apple binaries.
-FileSpecList filter_modules;
 filter_modules.Append(FileSpec("libc++abi.dylib"));
 filter_modules.Append(FileSpec("libSystem.B.dylib"));
-return target.GetSearchFilterForModuleList(&filter_modules);
-  } else {
-return LanguageRuntime::CreateExceptionSearchFilter();
   }
+  return target.GetSearchFilterForModuleList(&filter_modules);
 }
 
 lldb::BreakpointSP ItaniumABILanguageRuntime::CreateExceptionBreakpoint(

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp?rev=363109&r1=363108&r2=363109&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
 Tue Jun 11 15:52:08 2019
@@ -453,13 +453,11 @@ bool AppleObjCRuntime::CalculateHasNewLi
 lldb::SearchFilterSP AppleObjCRuntime::CreateExceptionSearchFilter() {
   Target &target = m_process->GetTarget();
 
+  FileSpecList filter_modules;
   if (target.GetArchitecture().GetTriple().getVendor() == llvm::Triple::Apple) 
{
-FileSpecList filter_modules;
 filter_modules.Append(std::get<0>(GetExceptionThrowLocation()));
-return target.GetSearchFilterForModuleList(&filter_modules);
-  } else {
-return LanguageRuntime::CreateExceptionSearchFilter();
   }
+  return target.GetSearchFilterForModuleList(&filter_modules);
 }
 
 ValueObjectSP AppleObjCRuntime::GetExceptionObjectForThread(

Modified: lldb/trunk/source/Target/LanguageRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/LanguageRuntime.cpp?rev=363109&r1=363108&r2=363109&view=diff
==
--- lldb/trunk/source/Target/LanguageRuntime.cpp (original)
+++ lldb/trunk/source/Target/LanguageRuntime.cpp Tue Jun 11 15:52:08 2019
@@ -293,6 +293,3 @@ void LanguageRuntime::InitializeCommands
   }
 }
 
-lldb::SearchFilterSP LanguageRuntime::CreateExceptionSearchFilter() {
-  return m_process->GetTarget().GetSearchFilterForModule(nullptr);
-}


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

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-11 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

I suppose one could compile Objective-C code on Linux using GCC.




Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:255
+  bool is_objc_method = false;
+  if (check_objc) {
+ObjCLanguage::MethodName objc_method(name, true);

Since check_objc is only used here, I think it would be better for readability 
to say 
```
if (cu_language == eLanguageTypeObjC ||
cu_language == eLanguageTypeObjC_plus_plus)
```
here


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

https://reviews.llvm.org/D63171



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

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

2019-06-11 Thread Saleem Abdulrasool via Phabricator via lldb-commits 
compnerd added inline comments.



Comment at: source/Plugins/Process/Utility/RegisterContextWindows_i386.cpp:40
+
+// clang-format off
+#define DEFINE_GPR(reg, alt, kind1, kind2, kind3, kind4)   
\

I believe that this bounds the range, and needs to be re-enabled.  Why not 
permit clang-format to reflow `DEFINE_GPR`?



Comment at: source/Plugins/Process/Utility/RegisterContextWindows_i386.cpp:76
+  default:
+assert(false && "Unhandled target architecture.");
+return nullptr;

Could you use `llvm_unreachable` instead please?  Same throughout.



Comment at: source/Plugins/Process/Utility/RegisterContextWindows_i386.cpp:84
+  case llvm::Triple::x86:
+return static_cast(sizeof(g_register_infos_i386) /
+ sizeof(g_register_infos_i386[0]));

Why not `llvm::array_lengthof`?



Comment at: source/Plugins/Process/Utility/RegisterContextWindows_wow64.h:14
+
+class RegisterContextWindows_wow64
+: public lldb_private::RegisterInfoInterface {

Could you spell this `WoW64` to match the Microsoft naming scheme for the 
"proper" noun?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63165



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

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg marked an inline comment as done.
clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:255
+  bool is_objc_method = false;
+  if (check_objc) {
+ObjCLanguage::MethodName objc_method(name, true);

aprantl wrote:
> Since check_objc is only used here, I think it would be better for 
> readability to say 
> ```
> if (cu_language == eLanguageTypeObjC ||
> cu_language == eLanguageTypeObjC_plus_plus)
> ```
> here
This is a hot loop. As long as the compiler will compute this once when 
optimizations are enabled, I am fine with inlining it into the if statement. 
But I pulled it out of the loop to ensure it only gets calculated once.


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

https://reviews.llvm.org/D63171



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

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-11 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D63171#1539049 , @aprantl wrote:

> I suppose one could compile Objective-C code on Linux using GCC.


Will GCC not set the language to ObjC or ObjC++?


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

https://reviews.llvm.org/D63171



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

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

2019-06-11 Thread Adrian McCarthy via Phabricator via lldb-commits 
amccarth added a comment.

Sorry for the stupid question, but ...

What exactly is meant here by "Native"?  How is a NativeProcessWindows 
different from ProcessWindows?




Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:16
+#include "IDebugDelegate.h"
+#include "ProcessDebugger.h"
+

Where is ProcessDebugger.h?  Should that be part of this patch?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63165



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

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

2019-06-11 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

In D63171#1539070 , @clayborg wrote:

> In D63171#1539049 , @aprantl wrote:
>
> > I suppose one could compile Objective-C code on Linux using GCC.
>
>
> Will GCC not set the language to ObjC or ObjC++?


This was intended as a reply to:

> One question I have is all Darwin compilers currently support the apple DWARF 
> indexes, so do we even need the objective C parsing code here?




Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:255
+  bool is_objc_method = false;
+  if (check_objc) {
+ObjCLanguage::MethodName objc_method(name, true);

clayborg wrote:
> aprantl wrote:
> > Since check_objc is only used here, I think it would be better for 
> > readability to say 
> > ```
> > if (cu_language == eLanguageTypeObjC ||
> > cu_language == eLanguageTypeObjC_plus_plus)
> > ```
> > here
> This is a hot loop. As long as the compiler will compute this once when 
> optimizations are enabled, I am fine with inlining it into the if statement. 
> But I pulled it out of the loop to ensure it only gets calculated once.
cu_language is a constant, so it should be safe for the compiler to hoist the 
computation. Also, this computation is so cheap that I'd rather optimize for 
readability.


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

https://reviews.llvm.org/D63171



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

[Lldb-commits] [lldb] r363115 - Back out r363103 ("When reading ObjC class table, use new SPI if it is avail")

2019-06-11 Thread Jason Molenda via lldb-commits 
Author: jmolenda
Date: Tue Jun 11 18:01:34 2019
New Revision: 363115

URL: http://llvm.org/viewvc/llvm-project?rev=363115&view=rev
Log:
Back out r363103 ("When reading ObjC class table, use new SPI if it is avail")
because it breaks the windows bot - asprintf() is not available.


Modified:

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp?rev=363115&r1=363114&r2=363115&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 Tue Jun 11 18:01:34 2019
@@ -157,16 +157,6 @@ __lldb_apple_objc_v2_get_dynamic_class_i
 
 )";
 
-// We'll substitute in class_getName or class_getNameRaw depending
-// on which is present.
-static const char *g_shared_cache_class_name_funcptr = R"(
-extern "C"
-{
-const char *%s(void *objc_class);
-const char *(*class_name_lookup_func)(void *) = %s;
-}
-)";
-
 static const char *g_get_shared_cache_class_info_name =
 "__lldb_apple_objc_v2_get_shared_cache_class_info";
 // Testing using the new C++11 raw string literals. If this breaks GCC then we
@@ -175,6 +165,7 @@ static const char *g_get_shared_cache_cl
 
 extern "C"
 {
+const char *class_getName(void *objc_class);
 size_t strlen(const char *);
 char *strncpy (char * s1, const char * s2, size_t n);
 int printf(const char * format, ...);
@@ -295,7 +286,7 @@ __lldb_apple_objc_v2_get_shared_cache_cl
 if (class_infos && idx < max_class_infos)
 {
 class_infos[idx].isa = (Class)((uint8_t *)clsopt + 
clsOffset);
-const char *name = class_name_lookup_func 
(class_infos[idx].isa);
+const char *name = class_getName (class_infos[idx].isa);
 DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, 
class_infos[idx].isa, name);
 // Hash the class name so we don't have to read it
 const char *s = name;
@@ -338,7 +329,7 @@ __lldb_apple_objc_v2_get_shared_cache_cl
 if (class_infos && idx < max_class_infos)
 {
 class_infos[idx].isa = (Class)((uint8_t *)clsopt + 
clsOffset);
-const char *name = class_name_lookup_func 
(class_infos[idx].isa);
+const char *name = class_getName (class_infos[idx].isa);
 DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, 
class_infos[idx].isa, name);
 // Hash the class name so we don't have to read it
 const char *s = name;
@@ -1598,46 +1589,9 @@ AppleObjCRuntimeV2::UpdateISAToDescripto
 
   if (!m_get_shared_cache_class_info_code) {
 Status error;
-
-// If the inferior objc.dylib has the class_getNameRaw function,
-// use that in our jitted expression.  Else fall back to the old
-// class_getName.
-static ConstString g_class_getName_symbol_name("class_getName");
-static ConstString g_class_getNameRaw_symbol_name("class_getNameRaw");
-ConstString class_name_getter_function_name = g_class_getName_symbol_name;
-
-ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-if (objc_runtime) {
-  const ModuleList &images = process->GetTarget().GetImages();
-  std::lock_guard guard(images.GetMutex());
-  for (size_t i = 0; i < images.GetSize(); ++i) {
-lldb::ModuleSP mod_sp = images.GetModuleAtIndexUnlocked(i);
-if (objc_runtime->IsModuleObjCLibrary(mod_sp)) {
-  const Symbol *symbol =
-  
mod_sp->FindFirstSymbolWithNameAndType(g_class_getNameRaw_symbol_name, 
-lldb::eSymbolTypeCode);
-  if (symbol && 
-  (symbol->ValueIsAddress() || symbol->GetAddressRef().IsValid())) 
{
-class_name_getter_function_name = g_class_getNameRaw_symbol_name;
-  }
-}
-  }
-}
-
-// Substitute in the correct class_getName / class_getNameRaw function 
name,
-// concatenate the two parts of our expression text.  The format string
-// has two %s's, so provide the name twice.
-char *class_name_func_ptr_expr = nullptr;
-asprintf (&class_name_func_ptr_expr, g_shared_cache_class_name_funcptr,
-  class_name_getter_function_name.AsCString(),
-  class_name_getter_function_name.AsCString());
-std::string shared_class_expression = class_name_func_ptr_expr;
-shared_class_expression += g_get_shared_cache_class_info_body;
-free (class_name_func_ptr_expr);
-
 m_get_shared_cache_class_info_code.reset(
 GetTa

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

2019-06-11 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai created this revision.
xiaobai added reviewers: labath, JDevlieghere, davide, compnerd.

ObjCLanguageRuntime was being pulled into LanguageRuntime because of
Breakpoint Preconditions. I tried a few ideas, but I felt like this was the last
invasive.

Other ideas I had and reasons I rejected them:

- Make "LanguageRuntime::CreateExceptionBreakpoint" a virtual method and having 
each language runtime implement them. I didn't do this because 
LanguageRuntime::CreateExceptionBreakpoint would look extremely similar (if not 
the same) in each of the LanguageRuntime derived classes. It makes sense to me 
to keep it as-is, and try to change just the BreakpointPrecondition parts.

- Add a callback to return a BreakpointPreconditionSP instead of having the 
callback add one to the breakpoint. I didn't do this because that would mean 
that the typedef for the callback (lldb-private-interfaces.h) would have to 
have visibility into the Breakpoint class to see BreakpointPrecondition. Either 
that, or we would move BreakpointPrecondition out of Breakpoint, and I'm not 
entirely sure that makes sense to do.

If you have another idea or think one of the ideas I rejected is actually better
than this solution, please let me know.


https://reviews.llvm.org/D63181

Files:
  include/lldb/Core/PluginManager.h
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  include/lldb/lldb-private-interfaces.h
  source/Core/PluginManager.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  source/Target/LanguageRuntime.cpp
  source/Target/ObjCLanguageRuntime.cpp

Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -375,6 +375,17 @@
   return found;
 }
 
+void ObjCLanguageRuntime::SetBreakpointExceptionPrecondition(
+BreakpointSP breakpoint, LanguageType language, bool throw_bp) {
+  if (!throw_bp)
+return;
+  Breakpoint::BreakpointPreconditionSP precondition_sp(
+  new ObjCLanguageRuntime::ObjCExceptionPrecondition());
+  if (!precondition_sp)
+return;
+  breakpoint->SetPrecondition(precondition_sp);
+}
+
 // Exception breakpoint Precondition class for ObjC:
 void ObjCLanguageRuntime::ObjCExceptionPrecondition::AddClassName(
 const char *class_name) {
Index: source/Target/LanguageRuntime.cpp
===
--- source/Target/LanguageRuntime.cpp
+++ source/Target/LanguageRuntime.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/Language.h"
-#include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Target.h"
 
 using namespace lldb;
@@ -224,19 +223,23 @@
 
 LanguageRuntime::~LanguageRuntime() = default;
 
-Breakpoint::BreakpointPreconditionSP
-LanguageRuntime::CreateExceptionPrecondition(lldb::LanguageType language,
- bool catch_bp, bool throw_bp) {
-  switch (language) {
-  case eLanguageTypeObjC:
-if (throw_bp)
-  return Breakpoint::BreakpointPreconditionSP(
-  new ObjCLanguageRuntime::ObjCExceptionPrecondition());
-break;
-  default:
-break;
+void LanguageRuntime::AddExceptionPrecondition(BreakpointSP breakpoint,
+   LanguageType language,
+   bool throw_bp) {
+  LanguageRuntimeCreateInstance create_callback;
+  for (uint32_t idx = 0;
+   (create_callback =
+PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx)) !=
+   nullptr;
+   idx++) {
+if (auto precondition_callback =
+PluginManager::GetLanguageRuntimeAddBreakpointPreconditionAtIndex(
+idx)) {
+  precondition_callback(breakpoint, language, throw_bp);
+  if (breakpoint->GetPrecondition())
+break;
+}
   }
-  return Breakpoint::BreakpointPreconditionSP();
 }
 
 BreakpointSP LanguageRuntime::CreateExceptionBreakpoint(
@@ -252,10 +255,7 @@
   target.CreateBreakpoint(filter_sp, resolver_sp, is_internal, hardware,
   resolve_indirect_functions));
   if (exc_breakpt_sp) {
-Breakpoint::BreakpointPreconditionSP precondition_sp =
-CreateExceptionPrecondition(language, catch_bp, throw_bp);
-if (precondition_sp)
-  exc_breakpt_sp->SetPrecondition(precondition_sp);
+AddExceptionPrecondition(exc_breakpt_sp, language, throw_bp);
 
 if (is_internal)
   exc_breakpt_sp->SetBreakpointKind("exception");
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/Lang

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

2019-06-11 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

Have you considered making just `AddExceptionPrecondition` virtual? Wouldn't 
that solve the problem too, without the code duplication of making 
`CreateExceptionBreakpoint` virtual? Also, I think it's totally reasonable to 
hoist `BreakpointPrecondition` out of `Breakpoint`. After having had to deal 
with the nuisance of not being able to forward declare nested classes, I try to 
avoid them in general.


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

https://reviews.llvm.org/D63181



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

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

2019-06-11 Thread Alex Langford via Phabricator via lldb-commits 
xiaobai added a comment.

In D63181#1539291 , @JDevlieghere 
wrote:

> Have you considered making just `AddExceptionPrecondition` virtual? Wouldn't 
> that solve the problem too, without the code duplication of making 
> `CreateExceptionBreakpoint` virtual?


I don't believe so. As I understand it, you might not have a process yet when 
you call `LanguageRuntime::CreateExceptionBreakpoint` from `Target`, so you 
won't have an instance of LanguageRuntime. You could create an instance of a 
LanguageRuntime object with a nullptr process just to invoke it as an instance 
method, but that seems like the wrong choice to me. All this combined has led 
me to believe that `AddExceptionPrecondition` needs to get a callback to a 
static function from PluginManager.

> Also, I think it's totally reasonable to hoist `BreakpointPrecondition` out 
> of `Breakpoint`. After having had to deal with the nuisance of not being able 
> to forward declare nested classes, I try to avoid them in general.

Yeah, I was kinda iffy on this, but if you (and possibly others) think this is 
reasonable, I might go with this option. The only thing that would change is 
the callback would return a `BreakpointPreconditionSP` instead of `void`, and 
`AddExceptionPrecondition` would be the one to modify the breakpoint. I don't 
think the core idea would change much but the implementation might be slightly 
nicer. Let me know if you prefer that.


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

https://reviews.llvm.org/D63181



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