[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: labath.
labath added a comment.

Forgive my ignorance, but what makes getenv unportable? llvm uses in a bunch of 
places so it can't be that bad... Is it some wide string thingy?

Regardless, using the lldb function for that seems fine, but if I remember 
correctly, each GetEnvironment call causes us to reparse the entire environment 
block. So it would be nice to just call it once in this function and then query 
that object for what we need.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56230



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


[Lldb-commits] [PATCH] D56231: [lldb-server] Improve support on Windows

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: labath.
labath added a comment.

Nice to see some action on the lldb-server for windows front. It looks like it 
should be easy to add a unit test for the File::Write bug you fixed. Can you 
add something like that?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56231



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


[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added reviewers: labath, jingham.
labath added a comment.

All of these functions seem identical to their PlatformPOSIX counterparts. Is 
that right? And I seem to remember already seeing a lot of code duplication 
between PlatformPOSIX and PlatformWindows.

It sounds to me like we should create a new common base class for 
PlatformWindows and PlatformPOSIX (`RemoteAwarePlatform`?), and put the common 
code there.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56232



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


[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: labath.
labath added inline comments.



Comment at: include/lldb/Host/windows/PosixApi.h:79
+#define WNOHANG 1
+#define WUNTRACED 2
+

I don't see `WUNTRACED` being used anywhere within lldb. Why did you need this?



Comment at: include/lldb/Host/windows/PosixApi.h:108-111
+inline pid_t waitpid(pid_t pid, int *status, int options) {
+  // To be implemented.
+  return pid_t(-1);
+}

What are your plans for this?

I assume you needed this for the fork-server in lldb-platform.cpp. I think the 
best way to address that would be to change the code from spawning a separate 
process for each connection to having a new thread for each connection. That 
should be much more portable and will avoid the need to mock posix apis on 
windows. I can help you with that if you run into any problems there.



Comment at: source/Host/common/SocketAddress.cpp:268
+#endif
+printf("SocketAddress::GetAddressInfo - %s\n", error.AsCString());
   }

Writing to stdout like this is very rude. If there isn't a suitable way to 
return this info, then I suggest writing this to the log instead.



Comment at: tools/lldb-server/SystemInitializerLLGS.cpp:37-52
+#if defined(_WIN32)
+  if (g_ref_count++ == 0) {
+// Require Windows Sockets version 2.2.
+auto wVersion = MAKEWORD(2, 2);
+WSADATA wsaData;
+auto err = WSAStartup(wVersion, &wsaData);
+if (err == 0) {

I think a better option here would be to move this code to something like 
`Socket::Initialize`. Then we could call this from 
`SystemInitializerCommon::Initialize`, and it would be available to everyone. 
This would allow us to remove the copy of this code in PlatformWindows, and 
replace the `WSAStartup` calls in various unittests with `Socket::Initialize()`



Comment at: tools/lldb-server/lldb-server.cpp:40
 
-static void initialize() {
+namespace llsvr {
+void initialize() {

In other places we use `llgs` (for `LL`db `G`db `S`erver), so I suggest 
sticking to that.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56233



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


[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I think there is something wrong here. Normally, it should be the job of the 
dynamic loader plugin to figure out where a module got loaded (and populate the 
target section load list). `Process::GetFileLoadAddress` is there to help it 
with this job. So, the fact that `ProcessWindows::GetFileLoadAddress` goes back 
to the target section load list to get the data creates a weird loop. The only 
way I can see this working is if this implementation happens to return the 
module base address, as specified in the object file, and that base address 
happens to be correct (no ASLR). The kind of code I would expect to see here is 
a call to some windows API to determine the load address straight from the OS 
(if such a thing is possible). For example, on Linux, we parse the 
`/proc//maps` file to determine this information.




Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:878
+  section_sp->GetFileAddress();
+  load_addr += module->GetObjectFile()->GetFileOffset();
+  break;

I think FileOffset will always be zero here. The only case where this is not 
zero is for fat macho binaries. And even in that case, I don't think adding 
that offset to the load address is the right thing to do.



Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:886
+  }
+  return Status("Moudle is not loaded");
+}

typo


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56237



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


[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I would definitely encourage using something better than the file checksum as 
UUID, if at all possible.




Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:139
+uuid =
+UUID::fromOptionalData(llvm::ArrayRef(Result->Bytes));
+}

I think this should be `fromData`. The `Optional` is there to give special 
meaning to an all-zero checksum, but you don't need that here. Just because the 
md5 checksum comes out as all-zero, it doesn't mean it is not valid.

PS: I am responsible for the existence of this function, so I am to blame for 
any confusion. If you have any idea, how to make this api more clear, I'd like 
to hear it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56229



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


Re: [Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

2019-01-03 Thread Pavel Labath via lldb-commits

On 03/01/2019 02:08, Zachary Turner via lldb-commits wrote:
Very excited to see this.  I'm technically on vacation so I might not be 
able to review it immediately, but I'm looking forward to getting to it 
soon.




I am also very excited. Unlike PDBs, I think I actually know a thing or 
two about lldb-server :), so I've added myself as a reviewer to these 
patches too.

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


[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath requested review of this revision.
labath added a comment.

Greg, given the intended usage of these sections as demonstrated in D56173 
, do you agree with representing the sections 
of the breakpad object file in this way?


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

https://reviews.llvm.org/D55434



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


[Lldb-commits] [PATCH] D56129: Simplify ObjectFile::GetArchitecture

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 180008.
labath added a comment.

Add unit test for ArchSpec::operator bool


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

https://reviews.llvm.org/D56129

Files:
  include/lldb/Core/Module.h
  include/lldb/Expression/IRExecutionUnit.h
  include/lldb/Symbol/ObjectFile.h
  include/lldb/Symbol/UnwindTable.h
  include/lldb/Utility/ArchSpec.h
  source/Core/Module.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/CompactUnwindInfo.cpp
  source/Symbol/DWARFCallFrameInfo.cpp
  source/Symbol/FuncUnwinders.cpp
  source/Symbol/Type.cpp
  source/Symbol/UnwindTable.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -228,3 +228,8 @@
 ASSERT_TRUE(A.IsCompatibleMatch(B));
   }
 }
+
+TEST(ArchSpecTest, OperatorBool) {
+  EXPECT_FALSE(ArchSpec());
+  EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
+}
Index: source/Symbol/UnwindTable.cpp
===
--- source/Symbol/UnwindTable.cpp
+++ source/Symbol/UnwindTable.cpp
@@ -180,8 +180,8 @@
   return m_arm_unwind_up.get();
 }
 
-bool UnwindTable::GetArchitecture(lldb_private::ArchSpec &arch) {
-  return m_object_file.GetArchitecture(arch);
+ArchSpec UnwindTable::GetArchitecture() {
+  return m_object_file.GetArchitecture();
 }
 
 bool UnwindTable::GetAllowAssemblyEmulationUnwindPlans() {
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -328,8 +328,7 @@
 case eEncodingIsPointerUID:
 case eEncodingIsLValueReferenceUID:
 case eEncodingIsRValueReferenceUID: {
-  ArchSpec arch;
-  if (m_symbol_file->GetObjectFile()->GetArchitecture(arch))
+  if (ArchSpec arch = m_symbol_file->GetObjectFile()->GetArchitecture())
 m_byte_size = arch.GetAddressByteSize();
 } break;
 }
Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -443,8 +443,7 @@
 lldb::UnwindAssemblySP
 FuncUnwinders::GetUnwindAssemblyProfiler(Target &target) {
   UnwindAssemblySP assembly_profiler_sp;
-  ArchSpec arch;
-  if (m_unwind_table.GetArchitecture(arch)) {
+  if (ArchSpec arch = m_unwind_table.GetArchitecture()) {
 arch.MergeFrom(target.GetArchitecture());
 assembly_profiler_sp = UnwindAssembly::FindPlugin(arch);
   }
Index: source/Symbol/DWARFCallFrameInfo.cpp
===
--- source/Symbol/DWARFCallFrameInfo.cpp
+++ source/Symbol/DWARFCallFrameInfo.cpp
@@ -423,8 +423,7 @@
  m_objfile.GetFileSpec().GetFilename().AsCString(""));
 
   bool clear_address_zeroth_bit = false;
-  ArchSpec arch;
-  if (m_objfile.GetArchitecture(arch)) {
+  if (ArchSpec arch = m_objfile.GetArchitecture()) {
 if (arch.GetTriple().getArch() == llvm::Triple::arm ||
 arch.GetTriple().getArch() == llvm::Triple::thumb)
   clear_address_zeroth_bit = true;
Index: source/Symbol/CompactUnwindInfo.cpp
===
--- source/Symbol/CompactUnwindInfo.cpp
+++ source/Symbol/CompactUnwindInfo.cpp
@@ -183,8 +183,7 @@
 if (function_info.encoding == 0)
   return false;
 
-ArchSpec arch;
-if (m_objfile.GetArchitecture(arch)) {
+if (ArchSpec arch = m_objfile.GetArchitecture()) {
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
   if (log && log->GetVerbose()) {
@@ -338,8 +337,7 @@
 // };
 
 bool clear_address_zeroth_bit = false;
-ArchSpec arch;
-if (m_objfile.GetArchitecture(arch)) {
+if (ArchSpec arch = m_objfile.GetArchitecture()) {
   if (arch.GetTriple().getArch() == llvm::Triple::arm ||
   arch.GetTriple().getArch() == llvm::Triple::thumb)
 clear_address_zeroth_bit = true;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1084,9 +1084,7 @@
 

[Lldb-commits] [PATCH] D56129: Simplify ObjectFile::GetArchitecture

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350291: Simplify ObjectFile::GetArchitecture (authored by 
labath, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56129?vs=180008&id=180009#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56129

Files:
  include/lldb/Core/Module.h
  include/lldb/Expression/IRExecutionUnit.h
  include/lldb/Symbol/ObjectFile.h
  include/lldb/Symbol/UnwindTable.h
  include/lldb/Utility/ArchSpec.h
  source/Core/Module.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/CompactUnwindInfo.cpp
  source/Symbol/DWARFCallFrameInfo.cpp
  source/Symbol/FuncUnwinders.cpp
  source/Symbol/Type.cpp
  source/Symbol/UnwindTable.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -228,3 +228,8 @@
 ASSERT_TRUE(A.IsCompatibleMatch(B));
   }
 }
+
+TEST(ArchSpecTest, OperatorBool) {
+  EXPECT_FALSE(ArchSpec());
+  EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
+}
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -309,7 +309,7 @@
   // Once we get the object file, update our module with the object
   // file's architecture since it might differ in vendor/os if some
   // parts were unknown.
-  m_objfile_sp->GetArchitecture(m_arch);
+  m_arch = m_objfile_sp->GetArchitecture();
 } else {
   error.SetErrorString("unable to find suitable object file plug-in");
 }
@@ -1265,9 +1265,7 @@
   // parts were unknown.  But since the matching arch might already be
   // more specific than the generic COFF architecture, only merge in
   // those values that overwrite unspecified unknown values.
-  ArchSpec new_arch;
-  m_objfile_sp->GetArchitecture(new_arch);
-  m_arch.MergeFrom(new_arch);
+  m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
 } else {
   ReportError("failed to load objfile for %s",
   GetFileSpec().GetPath().c_str());
Index: source/Symbol/UnwindTable.cpp
===
--- source/Symbol/UnwindTable.cpp
+++ source/Symbol/UnwindTable.cpp
@@ -180,8 +180,8 @@
   return m_arm_unwind_up.get();
 }
 
-bool UnwindTable::GetArchitecture(lldb_private::ArchSpec &arch) {
-  return m_object_file.GetArchitecture(arch);
+ArchSpec UnwindTable::GetArchitecture() {
+  return m_object_file.GetArchitecture();
 }
 
 bool UnwindTable::GetAllowAssemblyEmulationUnwindPlans() {
Index: source/Symbol/DWARFCallFrameInfo.cpp
===
--- source/Symbol/DWARFCallFrameInfo.cpp
+++ source/Symbol/DWARFCallFrameInfo.cpp
@@ -423,8 +423,7 @@
  m_objfile.GetFileSpec().GetFilename().AsCString(""));
 
   bool clear_address_zeroth_bit = false;
-  ArchSpec arch;
-  if (m_objfile.GetArchitecture(arch)) {
+  if (ArchSpec arch = m_objfile.GetArchitecture()) {
 if (arch.GetTriple().getArch() == llvm::Triple::arm ||
 arch.GetTriple().getArch() == llvm::Triple::thumb)
   clear_address_zeroth_bit = true;
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -328,8 +328,7 @@
 case eEncodingIsPointerUID:
 case eEncodingIsLValueReferenceUID:
 case eEncodingIsRValueReferenceUID: {
-  ArchSpec arch;
-  if (m_symbol_file->GetObjectFile()->GetArchitecture(arch))
+  if (ArchSpec arch = m_symbol_file->GetObjectFile()->GetArchitecture())
 m_byte_size = arch.GetAddressByteSize();
 } break;
 }
Index: source/Symbol/FuncUnwinders.cpp
===
--- source/Symbol/FuncUnwinders.cpp
+++ source/Symbol/FuncUnwinders.cpp
@@ -443,8 +443,7 @@
 lldb::UnwindAssemblySP
 FuncUnwinders::GetUnwindAssemblyProfiler(Target &target) {
   UnwindAssemblySP assembly_profiler_sp;
-  ArchSpec arch;
-  if (m_unwind_table.GetArchitec

[Lldb-commits] [lldb] r350291 - Simplify ObjectFile::GetArchitecture

2019-01-03 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Jan  3 02:37:19 2019
New Revision: 350291

URL: http://llvm.org/viewvc/llvm-project?rev=350291&view=rev
Log:
Simplify ObjectFile::GetArchitecture

Summary:
instead of returning the architecture through by-ref argument and a
boolean value indicating success, we can just return the ArchSpec
directly. Since the ArchSpec already has an invalid state, it can be
used to denote the failure without the additional bool.

Reviewers: clayborg, zturner, espindola

Subscribers: emaste, arichardson, JDevlieghere, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
lldb/trunk/include/lldb/Symbol/ObjectFile.h
lldb/trunk/include/lldb/Symbol/UnwindTable.h
lldb/trunk/include/lldb/Utility/ArchSpec.h
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Expression/IRExecutionUnit.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp
lldb/trunk/source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
lldb/trunk/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Symbol/CompactUnwindInfo.cpp
lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
lldb/trunk/source/Symbol/FuncUnwinders.cpp
lldb/trunk/source/Symbol/Type.cpp
lldb/trunk/source/Symbol/UnwindTable.cpp
lldb/trunk/unittests/Utility/ArchSpecTest.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=350291&r1=350290&r2=350291&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Thu Jan  3 02:37:19 2019
@@ -168,10 +168,11 @@ public:
 // Once we get the object file, update our module with the object file's
 // architecture since it might differ in vendor/os if some parts were
 // unknown.
-if (!module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch))
-  return nullptr;
-
-return module_sp;
+if (ArchSpec arch = module_sp->m_objfile_sp->GetArchitecture()) {
+  module_sp->m_arch = arch;
+  return module_sp;
+}
+return nullptr;
   }
 
   //--

Modified: lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/IRExecutionUnit.h?rev=350291&r1=350290&r2=350291&view=diff
==
--- lldb/trunk/include/lldb/Expression/IRExecutionUnit.h (original)
+++ lldb/trunk/include/lldb/Expression/IRExecutionUnit.h Thu Jan  3 02:37:19 
2019
@@ -108,7 +108,7 @@ public:
   void PopulateSectionList(lldb_private::ObjectFile *obj_file,
lldb_private::SectionList §ion_list) override;
 
-  bool GetArchitecture(lldb_private::ArchSpec &arch) override;
+  ArchSpec GetArchitecture() override;
 
   lldb::ModuleSP GetJITModule();
 

Modified: lldb/trunk/include/lldb/Symbol/ObjectFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ObjectFile.h?rev=350291&r1=350290&r2=350291&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ObjectFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/ObjectFile.h Thu Jan  3 02:37:19 2019
@@ -40,7 +40,7 @@ public:
   virtual void PopulateSectionList(lldb_private::ObjectFile *obj_file,
lldb_private::SectionList §ion_list) = 
0;
 
-  virtual bool GetArchitecture(lldb_private::ArchSpec &arch) = 0;
+  virtual ArchSpec GetArchitecture() = 0;
 };
 
 //--
@@ -305,19 +305,13 @@ public:
   virtual const FileSpec &GetFileSpec() const { return m_file; }
 
   //--
-  /// Get the name of the cpu, vendor and OS for this object file.
-  ///
-  /// This value is a string that represents the target triple where the cpu
-  /// type, the vendor and the OS are encoded into a string.
-  ///
-  /// @param[out] target_triple
-  /// The string value of the target triple.
+  /// Get the ArchSpec for this object file.
   ///
   /// @return
-  /// 

Re: [Lldb-commits] [PATCH] D56173: Introduce SymbolFileBreakpad and use it to fill symtab

2019-01-03 Thread Zachary Turner via lldb-commits
Ok, lgtm then
On Wed, Jan 2, 2019 at 11:53 PM Pavel Labath  wrote:

> On 02/01/2019 18:32, Zachary Turner wrote:
> > Just to be sure, this new test will fail without your Symtab changes
> > right?  I'm not in a state where I can look at code right now, and you
> > say anything that symbolicates an address *can* use the symtab, but I
> > don't know if you really meant *must* use the symtab.
> >
>
> Yes, the test will fail without these changes. The reason I said "can"
> is because symtab is one of the sources of symbolication (the other is
> debug info for instance). However, in this case there is no other source
> of symbol information, so this information "must" come from the symtab
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 180027.
labath added a comment.

Set N=0 default parameter, and remove the size specification from all usages
(including the one in DWARFDebugArranges, where N was 1).


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

https://reviews.llvm.org/D56170

Files:
  include/lldb/Core/RangeMap.h
  source/Plugins/Process/elf-core/ProcessElfCore.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
  unittests/Core/CMakeLists.txt
  unittests/Core/RangeMapTest.cpp

Index: unittests/Core/RangeMapTest.cpp
===
--- /dev/null
+++ unittests/Core/RangeMapTest.cpp
@@ -0,0 +1,55 @@
+//===-- RangeTest.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/RangeMap.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+using RangeDataVectorT = RangeDataVector;
+using EntryT = RangeDataVectorT::Entry;
+
+static testing::Matcher EntryIs(uint32_t ID) {
+  return testing::Pointee(testing::Field(&EntryT::data, ID));
+}
+
+TEST(RangeDataVector, FindEntryThatContains) {
+  RangeDataVectorT Map;
+  uint32_t NextID = 0;
+  Map.Append(EntryT(0, 10, NextID++));
+  Map.Append(EntryT(10, 10, NextID++));
+  Map.Append(EntryT(20, 10, NextID++));
+  Map.Sort();
+
+  EXPECT_THAT(Map.FindEntryThatContains(0), EntryIs(0));
+  EXPECT_THAT(Map.FindEntryThatContains(9), EntryIs(0));
+  EXPECT_THAT(Map.FindEntryThatContains(10), EntryIs(1));
+  EXPECT_THAT(Map.FindEntryThatContains(19), EntryIs(1));
+  EXPECT_THAT(Map.FindEntryThatContains(20), EntryIs(2));
+  EXPECT_THAT(Map.FindEntryThatContains(29), EntryIs(2));
+  EXPECT_THAT(Map.FindEntryThatContains(30), nullptr);
+}
+
+TEST(RangeDataVector, FindEntryThatContains_Overlap) {
+  RangeDataVectorT Map;
+  uint32_t NextID = 0;
+  Map.Append(EntryT(0, 40, NextID++));
+  Map.Append(EntryT(10, 20, NextID++));
+  Map.Append(EntryT(20, 10, NextID++));
+  Map.Sort();
+
+  // With overlapping intervals, the intention seems to be to return the first
+  // interval which contains the address.
+  EXPECT_THAT(Map.FindEntryThatContains(25), EntryIs(0));
+
+  // However, this does not always succeed.
+  // TODO: This should probably return the range (0, 40) as well.
+  EXPECT_THAT(Map.FindEntryThatContains(35), nullptr);
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
+  RangeMapTest.cpp
   RangeTest.cpp
   RichManglingContextTest.cpp
   StreamCallbackTest.cpp
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
@@ -19,7 +19,7 @@
 
 class DWARFDebugAranges {
 protected:
-  typedef lldb_private::RangeDataArray
+  typedef lldb_private::RangeDataVector
   RangeToDIE;
 
 public:
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -136,7 +136,7 @@
   // For ProcessElfCore only
   //--
   typedef lldb_private::Range FileRange;
-  typedef lldb_private::RangeDataArray
+  typedef lldb_private::RangeDataVector
   VMRangeToFileOffset;
   typedef lldb_private::RangeDataVector
   VMRangeToPermissions;
Index: include/lldb/Core/RangeMap.h
===
--- include/lldb/Core/RangeMap.h
+++ include/lldb/Core/RangeMap.h
@@ -633,201 +633,12 @@
   }
 };
 
-template  class RangeDataArray {
+template 
+class RangeDataVector {
 public:
   typedef RangeData Entry;
   typedef llvm::SmallVector Collection;
 
-  RangeDataArray() = default;
-
-  ~RangeDataArray() = default;
-
-  void Append(const Entry &entry) { m_entries.push_back(entry); }
-
-  void Sort() {
-if (m_entries.size() > 1)
-  std::stable_sort(m_entries.begin(), m_entries.end());
-  }
-
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-  bool IsSorted() const {
-typename Collection::const_iterator pos, end, prev;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos != end;
- prev = pos++) {
-  if (prev != end && *pos < *prev)
-return false;
-}
-return true;
-  

[Lldb-commits] [lldb] r350294 - Fix some -Wreorder warnings introduced in r350274

2019-01-03 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Jan  3 03:31:50 2019
New Revision: 350294

URL: http://llvm.org/viewvc/llvm-project?rev=350294&view=rev
Log:
Fix some -Wreorder warnings introduced in r350274

Modified:
lldb/trunk/include/lldb/Symbol/LineTable.h

Modified: lldb/trunk/include/lldb/Symbol/LineTable.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/LineTable.h?rev=350294&r1=350293&r2=350294&view=diff
==
--- lldb/trunk/include/lldb/Symbol/LineTable.h (original)
+++ lldb/trunk/include/lldb/Symbol/LineTable.h Thu Jan  3 03:31:50 2019
@@ -238,21 +238,22 @@ public:
 protected:
   struct Entry {
 Entry()
-: file_addr(LLDB_INVALID_ADDRESS), line(0), column(0), file_idx(0),
+: file_addr(LLDB_INVALID_ADDRESS), line(0),
   is_start_of_statement(false), is_start_of_basic_block(false),
   is_prologue_end(false), is_epilogue_begin(false),
-  is_terminal_entry(false) {}
+  is_terminal_entry(false), column(0), file_idx(0) {}
 
 Entry(lldb::addr_t _file_addr, uint32_t _line, uint16_t _column,
   uint16_t _file_idx, bool _is_start_of_statement,
   bool _is_start_of_basic_block, bool _is_prologue_end,
   bool _is_epilogue_begin, bool _is_terminal_entry)
-: file_addr(_file_addr), line(_line), column(_column),
-  file_idx(_file_idx), is_start_of_statement(_is_start_of_statement),
+: file_addr(_file_addr), line(_line),
+  is_start_of_statement(_is_start_of_statement),
   is_start_of_basic_block(_is_start_of_basic_block),
   is_prologue_end(_is_prologue_end),
   is_epilogue_begin(_is_epilogue_begin),
-  is_terminal_entry(_is_terminal_entry) {}
+  is_terminal_entry(_is_terminal_entry), column(_column),
+  file_idx(_file_idx) {}
 
 int bsearch_compare(const void *key, const void *arrmem);
 


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


[Lldb-commits] [lldb] r350298 - PECOFF: Remove tabs introduced accidentally in r350094

2019-01-03 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Jan  3 04:07:38 2019
New Revision: 350298

URL: http://llvm.org/viewvc/llvm-project?rev=350298&view=rev
Log:
PECOFF: Remove tabs introduced accidentally in r350094

Modified:
lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp?rev=350298&r1=350297&r2=350298&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Thu Jan  3 
04:07:38 2019
@@ -743,96 +743,96 @@ void ObjectFilePECOFF::CreateSections(Se
   static ConstString g_sect_name_go_symtab(".gosymtab");
   SectionType section_type = eSectionTypeOther;
   if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
- ((const_sect_name == g_code_sect_name) ||
-  (const_sect_name == g_CODE_sect_name))) {
-   section_type = eSectionTypeCode;
+  ((const_sect_name == g_code_sect_name) ||
+   (const_sect_name == g_CODE_sect_name))) {
+section_type = eSectionTypeCode;
   } else if (m_sect_headers[idx].flags &
-llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
-((const_sect_name == g_data_sect_name) ||
- (const_sect_name == g_DATA_sect_name))) {
-   if (m_sect_headers[idx].size == 0 && m_sect_headers[idx].offset == 0)
- section_type = eSectionTypeZeroFill;
-   else
- section_type = eSectionTypeData;
+ llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
+ ((const_sect_name == g_data_sect_name) ||
+  (const_sect_name == g_DATA_sect_name))) {
+if (m_sect_headers[idx].size == 0 && m_sect_headers[idx].offset == 0)
+  section_type = eSectionTypeZeroFill;
+else
+  section_type = eSectionTypeData;
   } else if (m_sect_headers[idx].flags &
-llvm::COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA &&
-((const_sect_name == g_bss_sect_name) ||
- (const_sect_name == g_BSS_sect_name))) {
-   if (m_sect_headers[idx].size == 0)
- section_type = eSectionTypeZeroFill;
-   else
- section_type = eSectionTypeData;
+ llvm::COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA &&
+ ((const_sect_name == g_bss_sect_name) ||
+  (const_sect_name == g_BSS_sect_name))) {
+if (m_sect_headers[idx].size == 0)
+  section_type = eSectionTypeZeroFill;
+else
+  section_type = eSectionTypeData;
   } else if (const_sect_name == g_debug_sect_name) {
-   section_type = eSectionTypeDebug;
+section_type = eSectionTypeDebug;
   } else if (const_sect_name == g_stabstr_sect_name) {
-   section_type = eSectionTypeDataCString;
+section_type = eSectionTypeDataCString;
   } else if (const_sect_name == g_reloc_sect_name) {
-   section_type = eSectionTypeOther;
+section_type = eSectionTypeOther;
   } else if (const_sect_name == g_sect_name_dwarf_debug_abbrev)
-   section_type = eSectionTypeDWARFDebugAbbrev;
+section_type = eSectionTypeDWARFDebugAbbrev;
   else if (const_sect_name == g_sect_name_dwarf_debug_aranges)
-   section_type = eSectionTypeDWARFDebugAranges;
+section_type = eSectionTypeDWARFDebugAranges;
   else if (const_sect_name == g_sect_name_dwarf_debug_frame)
-   section_type = eSectionTypeDWARFDebugFrame;
+section_type = eSectionTypeDWARFDebugFrame;
   else if (const_sect_name == g_sect_name_dwarf_debug_info)
-   section_type = eSectionTypeDWARFDebugInfo;
+section_type = eSectionTypeDWARFDebugInfo;
   else if (const_sect_name == g_sect_name_dwarf_debug_line)
-   section_type = eSectionTypeDWARFDebugLine;
+section_type = eSectionTypeDWARFDebugLine;
   else if (const_sect_name == g_sect_name_dwarf_debug_loc)
-   section_type = eSectionTypeDWARFDebugLoc;
+section_type = eSectionTypeDWARFDebugLoc;
   else if (const_sect_name == g_sect_name_dwarf_debug_loclists)
-   section_type = eSectionTypeDWARFDebugLocLists;
+section_type = eSectionTypeDWARFDebugLocLists;
   else if (const_sect_name == g_sect_name_dwarf_debug_macinfo)
-   section_type = eSectionTypeDWARFDebugMacInfo;
+section_type = eSectionTypeDWARFDebugMacInfo;
   else if (const_sect_name == g_sect_name_dwarf_debug_names)
-   section_type = eSectionTypeDWARFDebugNames;
+section_type = eSectionTypeDWARFDebugNames;
   else if (const_sect_name == g_sect_name_dwarf_debug_pubnames)
-   section_type = eSectionTypeDWARFDebugPubNames;
+section_type = eSectionTypeDWARFDebugPubNames;
   else if (const_sect

[Lldb-commits] [PATCH] D56234: [lldb-server] Add unnamed pipe support to PipeWindows

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: source/Host/windows/PipeWindows.cpp:28
 std::atomic g_pipe_serial(0);
-}
+static std::string g_pipe_name_prefix = ".\\Pipe\\";
+} // namespace

This would be better as llvm::StringLiteral (or just a char array) to avoid 
global constructors. Also, `static` in an anonymous namespace is overkill (anon 
namespace implies static linkage).



Comment at: source/Host/windows/PipeWindows.cpp:47
+  // in the current process and usually crashes the program.  Pass handles
+  // instead since the handle can be inheritated.
+  m_read = read_handle;

inherited



Comment at: source/Host/windows/PipeWindows.cpp:120
 
-  // Open the write end of the pipe.
+  // FIXME: For server end pipe, it's not applicable to use CreateFile to get
+  // read or write end of the pipe for the reason that closing of the file will

I find it strange reading about "servers" in a low-level utility class that 
seemingly has nothing to do with serving. I think this will confuse future 
readers as they will not look at this in the context of this patch. Could you 
rephrase this?



Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1074
+}
+auto write_handle = socket_pipe.GetWriteNativeHandle();
+debugserver_args.AppendArgument(llvm::StringRef("--pipe"));

What do you think about adding `GetWriteNativeHandle` to the generic Pipe 
interface? In posix the file descriptor _is_ the native handle, so PipePosix 
could just return the fd, but it would allow you to write this bit of code 
generically without ifdefs (well, maybe except the `AppendCloseFileAction` 
below). Similarly, we could have a generic Pipe factory function(*) which takes 
the handles as input, which would allow you to implement the lldb-server side 
of things without ifdefs.

(*) I think a factory function (`Pipe::FromNativeHandles`?) would be better 
than a constructor to avoid ambiguities between descriptors and handles.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56234



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


[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

2019-01-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

So is this done as one section per function? Or one section for contiguous 
functions? What about if there are only symbols? I tried to read the code but 
wasn't able to decipher everything clearly in my head.


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

https://reviews.llvm.org/D55434



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


[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

2019-01-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Just read the original description again and now code makes sense. Main 
questions for me: is there a benefit to creating multiple sections? Can we just 
create one section and name it ".breakpad"? Should we not try to find a section 
that contains the address from the FUNC, line entry or PUBLIC and then avoid 
creating a section? That way we only create breakpad sections of there are not 
backing sections from a real object  or symbol file?


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

https://reviews.llvm.org/D55434



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


[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

2019-01-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

excited to see this starting as well!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56233



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


[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

2019-01-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Seems like DynamicLoaderWindowsDYLD isn't doing its job correctly here and 
DynamicLoaderWindowsDYLD should be fixed. We shouldn't have to go to the 
process at all in order to set the section load addresses correctly. Why? As 
you might have seen lldb-server.exe is being worked on now and then we will 
have two clients for DynamicLoaderWindowsDYLD: the native ProcessWindows 
plug-in and ProcessGDBRemote. The




Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:858
 
+Status ProcessWindows::GetFileLoadAddress(const FileSpec &file, bool 
&is_loaded,
+  lldb::addr_t &load_addr) {

This entire function is doing the job of the dynamic loader and should be moved 
into the dynamic loader.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56237



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


[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

2019-01-03 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.

So it seems like DynamicLoaderWindowsDYLD is not doings its job correctly and 
it needs to be fixed. DynamicLoaderWindowsDYLD is responsible for setting all 
of the section load addresses correctly using an abstract lldb_private::Process 
plug-in. No special calls should be added if that is possible. We have someone 
working on lldb-server.exe and then we will have two clients for 
DynamicLoaderWindowsDYLD: ProcessWindows and ProcessGDBRemote. 
DynamicLoaderWindowsDYLD should work for any lldb_private::Process subclass 
without the need to call back into a specific lldb_private::Process subclass 
method.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56237



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


[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

2019-01-03 Thread Kamil Rytarowski via Phabricator via lldb-commits
krytarowski added inline comments.



Comment at: include/lldb/Host/windows/PosixApi.h:78
 
+#define WNOHANG 1
+#define WUNTRACED 2

I think that these symbols should not be leaked here in the first place.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56233



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


[Lldb-commits] [lldb] r350360 - TestQueues: Move the synchronisation code into the binary itself.

2019-01-03 Thread Adrian Prantl via lldb-commits
Author: adrian
Date: Thu Jan  3 14:34:48 2019
New Revision: 350360

URL: http://llvm.org/viewvc/llvm-project?rev=350360&view=rev
Log:
TestQueues: Move the synchronisation code into the binary itself.

Thanks to Pavel Labath for the suggestion!

Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py?rev=350360&r1=350359&r2=350360&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py Thu 
Jan  3 14:34:48 2019
@@ -30,16 +30,6 @@ class TestQueues(TestBase):
 # Find the line numbers that we will step to in main:
 self.main_source = "main.c"
 
-def remove_token(self, name):
-for i in range(7):
-token = name+'.token.%d'%i
-if os.path.exists(token):
-os.remove(token)
-
-def await_token(self, name):
-for i in range(7):
-lldbutil.wait_for_file_on_target(self, name+'.token.%d'%i)
-
 def check_queue_for_valid_queue_id(self, queue):
 self.assertTrue(
 queue.GetQueueID() != 0, "Check queue %s for valid QueueID (got 
0x%x)" %
@@ -122,14 +112,12 @@ class TestQueues(TestBase):
 self.main_source_spec = lldb.SBFileSpec(self.main_source)
 break1 = target.BreakpointCreateByName("stopper", 'a.out')
 self.assertTrue(break1, VALID_BREAKPOINT)
-self.remove_token(exe)
 process = target.LaunchSimple(
-[exe+'.token.'], None, self.get_process_working_directory())
+[], None, self.get_process_working_directory())
 self.assertTrue(process, PROCESS_IS_VALID)
 threads = lldbutil.get_threads_stopped_at_breakpoint(process, break1)
 if len(threads) != 1:
 self.fail("Failed to stop at breakpoint 1.")
-self.await_token(exe)
 
 queue_submittor_1 = lldb.SBQueue()
 queue_performer_1 = lldb.SBQueue()
@@ -283,9 +271,8 @@ class TestQueues(TestBase):
 if self.getArchitecture() in ['arm', 'arm64', 'arm64e', 'arm64_32', 
'armv7', 'armv7k']:
 libbtr_path = "/Developer/usr/lib/libBacktraceRecording.dylib"
 
-self.remove_token(exe)
 process = target.LaunchSimple(
-[exe+'.token.'],
+[],
 [
 'DYLD_INSERT_LIBRARIES=%s' % (libbtr_path),
 'DYLD_LIBRARY_PATH=/usr/lib/system/introspection'],
@@ -297,7 +284,6 @@ class TestQueues(TestBase):
 threads = lldbutil.get_threads_stopped_at_breakpoint(process, break1)
 if len(threads) != 1:
 self.fail("Failed to stop at breakpoint 1.")
-self.await_token(exe)
 
 libbtr_module_filespec = lldb.SBFileSpec("libBacktraceRecording.dylib")
 libbtr_module = target.FindModule(libbtr_module_filespec)

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c?rev=350360&r1=350359&r2=350360&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/main.c Thu Jan  3 
14:34:48 2019
@@ -1,30 +1,16 @@
-#include 
-#include 
+#include 
 #include 
 #include 
 #include 
 #include 
 
 int finished_enqueueing_work = 0;
-char *name = NULL;
-
-void
-touch (const char *name, unsigned n)
-{
-char token[2048];
-snprintf (token, 2048, "%s%d", name, n);
-FILE *f = fopen (token, "wx");
-if (!f)
-exit (1);
-fputs ("\n", f);
-fflush (f);
-fclose (f);
-}
+atomic_int thread_count = 0;
 
 void
 doing_the_work_1(void *in)
 {
-touch (name, 0);
+atomic_fetch_add(&thread_count, 1);
 while (1)
 sleep (1);
 }
@@ -96,9 +82,6 @@ stopper ()
 
 int main (int argc, const char **argv)
 {
-if (argc != 2)
-  return 2;
-name = argv[1];
 dispatch_queue_t work_submittor_1 = dispatch_queue_create 
("com.apple.work_submittor_1", DISPATCH_QUEUE_SERIAL);
 dispatch_queue_t work_submittor_2 = dispatch_queue_create 
("com.apple.work_submittor_and_quit_2", DISPATCH_QUEUE_SERIAL);
 dispatch_queue_t work_submittor_3 = dispatch_queue_create 
("com.apple.work_submittor_3", DISPATCH_QUEUE_SERIAL);
@@ -117,44 +100,46 @@ int main (int argc, const char **argv)
 
 
 // Spin up threads with each of the different libdispatch QoS values.
-
 dispatch_async (dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{
 pthread_setname_np ("user initiated

[Lldb-commits] [PATCH] D56208: Add file-based synchronization to flaky test

2019-01-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.
Herald added a reviewer: serge-sans-paille.

I implemented a poor man's version of that in r350360.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56208



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


[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

2019-01-03 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo created this revision.
lemo added reviewers: labath, zturner.
lemo added a project: LLDB.
Herald added subscribers: jfb, abidh.

If the minidump contains a saved exception record use it automatically.

(This patch is cherry picked from the larger https://reviews.llvm.org/D55142)


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D56293

Files:
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.exe
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb
  lit/Minidump/Windows/Sigsegv/sigsegv.test
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/Threading.h"
 
 #include "Plugins/Process/Utility/StopInfoMachException.h"
+
 // C includes
 // C++ includes
 
@@ -80,7 +81,7 @@
 section_sp, module->base_of_image);
   }
 
-  ObjectFile *GetObjectFile() override { return nullptr; }
+ObjectFile *GetObjectFile() override { return nullptr; }
 
   SectionList *GetSectionList() override {
 return Module::GetUnifiedSectionList();
@@ -309,15 +310,23 @@
   if (m_thread_list.size() > 0)
 num_threads = m_thread_list.size();
 
-  for (lldb::tid_t tid = 0; tid < num_threads; ++tid) {
+  for (size_t t_index = 0; t_index < num_threads; ++t_index) {
 llvm::ArrayRef context;
+const auto& thread = m_thread_list[t_index];
+auto context_location = thread.thread_context;
+
+// If the minidump contains an exception context, use it
+if (m_active_exception != nullptr &&
+m_active_exception->thread_id == thread.thread_id) {
+  context_location = m_active_exception->thread_context;
+}
+
 if (!m_is_wow64)
-  context = m_minidump_parser.GetThreadContext(m_thread_list[tid]);
+  context = m_minidump_parser.GetThreadContext(context_location);
 else
-  context = m_minidump_parser.GetThreadContextWow64(m_thread_list[tid]);
+  context = m_minidump_parser.GetThreadContextWow64(thread);
 
-lldb::ThreadSP thread_sp(
-new ThreadMinidump(*this, m_thread_list[tid], context));
+lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
 new_thread_list.AddThread(thread_sp);
   }
   return new_thread_list.GetSize(false) > 0;
@@ -549,9 +558,9 @@
 APPEND_OPT(m_dump_linux_all);
 m_option_group.Finalize();
   }
-  
+
   ~CommandObjectProcessMinidumpDump() {}
-  
+
   Options *GetOptions() override { return &m_option_group; }
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
@@ -563,7 +572,7 @@
   return false;
 }
 SetDefaultOptionsIfNoneAreSet();
-
+
 ProcessMinidump *process = static_cast(
 m_interpreter.GetExecutionContext().GetProcessPtr());
 result.SetStatus(eReturnStatusSuccessFinishResult);
@@ -635,7 +644,7 @@
 LoadSubCommand("dump",
 CommandObjectSP(new CommandObjectProcessMinidumpDump(interpreter)));
   }
-  
+
   ~CommandObjectMultiwordProcessMinidump() {}
 };
 
Index: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -58,6 +58,9 @@
 
   llvm::ArrayRef GetThreads();
 
+  llvm::ArrayRef
+  GetThreadContext(const MinidumpLocationDescriptor &location);
+
   llvm::ArrayRef GetThreadContext(const MinidumpThread &td);
 
   llvm::ArrayRef GetThreadContextWow64(const MinidumpThread &td);
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -107,11 +107,15 @@
 }
 
 llvm::ArrayRef
-MinidumpParser::GetThreadContext(const MinidumpThread &td) {
-  if (td.thread_context.rva + td.thread_context.data_size > GetData().size())
+MinidumpParser::GetThreadContext(const MinidumpLocationDescriptor &location) {
+  if (location.rva + location.data_size > GetData().size())
 return {};
+  return GetData().slice(location.rva, location.data_size);
+}
 
-  return GetData().slice(td.thread_context.rva, td.thread_context.data_size);
+llvm::ArrayRef
+MinidumpParser::GetThreadContext(const MinidumpThread &td) {
+  return GetThreadContext(td.thread_context);
 }
 
 llvm::ArrayRef
Index: lit/Minidump/Windows/Sigsegv/sigsegv.test
===
--- lit/Minidump/Windows/Sigsegv/sigsegv.test
+++ lit/Minidump/Windows/Sigsegv/sigsegv.test
@@ -0,0 +1,13 @@
+// RUN: cd %p

[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

2019-01-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

A testcase that triggers this situation would be nice, too.




Comment at: 
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:558
+  if (!thread_sp->SafeToCallFunctions())
+return ValueObjectSP();
+

I recently started writing these early-exit returns as `return {};`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56115



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


[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

2019-01-03 Thread Zachary Turner via Phabricator via lldb-commits
zturner requested changes to this revision.
zturner added a comment.
This revision now requires changes to proceed.

I don't think we can check in an executable file, we should try to compile it 
on the spot.  We have 1-2 existing unit tests that check in an exe and we 
occasionally get reports that peoples' virus scanners flag them as trojans, 
even though they obviously aren't.  In any case, I've been meaning to remove 
those tests, so I think we should set a precedent that executable binaries are 
never checked in.

Is there something about this executable that makes it impractical to compile 
on the fly using the `%build` substitution?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56293



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


[Lldb-commits] [lldb] r350368 - symbols.enable-external-lookup=false on all hosts (not just OSX)

2019-01-03 Thread Jan Kratochvil via lldb-commits
Author: jankratochvil
Date: Thu Jan  3 15:11:06 2019
New Revision: 350368

URL: http://llvm.org/viewvc/llvm-project?rev=350368&view=rev
Log:
symbols.enable-external-lookup=false on all hosts (not just OSX)

There is already in use:
lit/lit-lldb-init:
settings set symbols.enable-external-lookup false
packages/Python/lldbsuite/test/lldbtest.py:
self.runCmd('settings set symbols.enable-external-lookup false')

But those are not in effect during MI part of the testsuite. Another problem is
that symbols.enable-external-lookup (read by GetEnableExternalLookup) has been
currently read only by LocateMacOSXFilesUsingDebugSymbols and therefore it had
no effect on Linux.

On Red Hat platforms (Fedoras, RHEL-7) there is DWZ in use and so
MiSyntaxTestCase-test_lldbmi_output_grammar FAILs due to:
AssertionError: error: inconsistent pattern ''^.+?\n'' for state 0x5f
(matched string: warning: (x86_64) /lib64/libstdc++.so.6 unsupported
DW_FORM values: 0x1f20 0x1f21
It is the only testcase with this error. It happens due to:
(lldb) target create "/lib64/libstdc++.so.6"
Current executable set to '/lib64/libstdc++.so.6' (x86_64).
(lldb) b main
warning: (x86_64) /lib64/libstdc++.so.6 unsupported DW_FORM values: 
0x1f20 0x1f21
Breakpoint 1: no locations (pending).
WARNING:  Unable to resolve breakpoint to any actual locations.
which happens only with gcc-base-debuginfo rpm installed (similarly for other 
packages).

It should also speed up the testsuite as it no longer needs to read
/usr/lib/debug symbols which have no effect (and should not have any effect) on
the testsuite results.

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/start_script_error
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Host/common/Symbols.cpp
lldb/trunk/source/Target/Target.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py?rev=350368&r1=350367&r2=350368&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py 
Thu Jan  3 15:11:06 2019
@@ -40,7 +40,7 @@ class MiTestCaseBase(Base):
 pass
 Base.tearDown(self)
 
-def spawnLldbMi(self, exe=None, args=None):
+def spawnLldbMi(self, exe=None, args=None, preconfig=True):
 import pexpect
 self.child = pexpect.spawn("%s --interpreter %s" % (
 self.lldbMiExec, args if args else ""), cwd=self.getBuildDir())
@@ -49,6 +49,10 @@ class MiTestCaseBase(Base):
 self.child.logfile_read = open(self.mylog, "w")
 # wait until lldb-mi has started up and is ready to go
 self.expect(self.child_prompt, exactly=True)
+if preconfig:
+self.runCmd("settings set symbols.enable-external-lookup false")
+self.expect("\^done")
+self.expect(self.child_prompt, exactly=True)
 if exe:
 self.runCmd("-file-exec-and-symbols \"%s\"" % exe)
 # Testcases expect to be able to match output of this command,

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py?rev=350368&r1=350367&r2=350368&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
 Thu Jan  3 15:11:06 2019
@@ -237,7 +237,11 @@ class MiStartupOptionsTestCase(lldbmi_te
 
 # Prepared source file
 sourceFile = self.copyScript("start_script_error")
-self.spawnLldbMi(args="--source %s" % sourceFile)
+self.spawnLldbMi(args="--source %s" % sourceFile, preconfig=False)
+
+# After 'settings set symbols.enable-external-lookup false'
+self.expect("settings set symbols.enable-external-lookup false")
+self.expect("\^done")
 
 # After '-file-exec-and-symbols a.out'
 self.expect("-file-exec-and-symbols %s" % self.myexe)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/start_script_error
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

2019-01-03 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
jankratochvil marked an inline comment as done.
Closed by commit rLLDB350368: symbols.enable-external-lookup=false on all hosts 
(not just OSX) (authored by jankratochvil, committed by ).
Herald added a reviewer: serge-sans-paille.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55859

Files:
  packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
  
packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
  
packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/start_script_error
  source/Core/ModuleList.cpp
  source/Host/common/Symbols.cpp
  source/Target/Target.cpp

Index: packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
@@ -40,7 +40,7 @@
 pass
 Base.tearDown(self)
 
-def spawnLldbMi(self, exe=None, args=None):
+def spawnLldbMi(self, exe=None, args=None, preconfig=True):
 import pexpect
 self.child = pexpect.spawn("%s --interpreter %s" % (
 self.lldbMiExec, args if args else ""), cwd=self.getBuildDir())
@@ -49,6 +49,10 @@
 self.child.logfile_read = open(self.mylog, "w")
 # wait until lldb-mi has started up and is ready to go
 self.expect(self.child_prompt, exactly=True)
+if preconfig:
+self.runCmd("settings set symbols.enable-external-lookup false")
+self.expect("\^done")
+self.expect(self.child_prompt, exactly=True)
 if exe:
 self.runCmd("-file-exec-and-symbols \"%s\"" % exe)
 # Testcases expect to be able to match output of this command,
Index: packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/TestMiStartupOptions.py
@@ -237,7 +237,11 @@
 
 # Prepared source file
 sourceFile = self.copyScript("start_script_error")
-self.spawnLldbMi(args="--source %s" % sourceFile)
+self.spawnLldbMi(args="--source %s" % sourceFile, preconfig=False)
+
+# After 'settings set symbols.enable-external-lookup false'
+self.expect("settings set symbols.enable-external-lookup false")
+self.expect("\^done")
 
 # After '-file-exec-and-symbols a.out'
 self.expect("-file-exec-and-symbols %s" % self.myexe)
Index: packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/start_script_error
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/start_script_error
+++ packages/Python/lldbsuite/test/tools/lldb-mi/startup_options/start_script_error
@@ -1,2 +1,3 @@
+settings set symbols.enable-external-lookup false
 -file-exec-and-symbols a.out
 -break-ins -f main
Index: source/Host/common/Symbols.cpp
===
--- source/Host/common/Symbols.cpp
+++ source/Host/common/Symbols.cpp
@@ -247,6 +247,8 @@
   return result;
 }
 
+// Keep "symbols.enable-external-lookup" description in sync with this function.
+
 FileSpec Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec) {
   FileSpec symbol_file_spec = module_spec.GetSymbolFileSpec();
   if (symbol_file_spec.IsAbsolute() &&
@@ -270,30 +272,33 @@
   debug_file_search_paths.AppendIfUnique(file_spec);
 }
 
-// Add current working directory.
-{
-  FileSpec file_spec(".");
-  FileSystem::Instance().Resolve(file_spec);
-  debug_file_search_paths.AppendIfUnique(file_spec);
-}
+if (ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) {
+
+  // Add current working directory.
+  {
+FileSpec file_spec(".");
+FileSystem::Instance().Resolve(file_spec);
+debug_file_search_paths.AppendIfUnique(file_spec);
+  }
 
 #ifndef _WIN32
 #if defined(__NetBSD__)
-// Add /usr/libdata/debug directory.
-{
-  FileSpec file_spec("/usr/libdata/debug");
-  FileSystem::Instance().Resolve(file_spec);
-  debug_file_search_paths.AppendIfUnique(file_spec);
-}
+  // Add /usr/libdata/debug directory.
+  {
+FileSpec file_spec("/usr/libdata/debug");
+FileSystem::Instance().Resolve(file_spec);
+debug_file_search_paths.AppendIfUnique(file_spec);
+  }
 #else
-// Add /usr/lib/debug directory.
-{
-  FileSpec file_spec("/usr/lib/debug");
-  FileSystem::Instance().Resolve(file_spec);
-  debug_file_search_paths.AppendIfUnique(file_spec);
-

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

2019-01-03 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: source/Core/ModuleList.cpp:72
  {},
  "Control the use of external tools or libraries to locate symbol files. "
+ "Directories listed in target.debug-file-search-paths and directory of "

jankratochvil wrote:
> labath wrote:
> > My main issue here was with the "tools or libraries" part of this 
> > description -- we're not using any special tools or libraries when looking 
> > in /usr/lib/debug.
> > 
> > Maybe just say that this is controls whether we use "external sources" when 
> > searching for symbols. The rest of the description is fine (though I would 
> > just drop the ifdefs and print everything everywhere).
> The Spotlight on OSX was a bit unclear to me what it really does.  Doesn't it 
> use some those "tools or libraries" to access the dSYM files downloaded from 
> internet? But I have put there the "sources" word now as you wish.
> Also rather added `See also symbols.enable-external-lookup.` to 
> `target.debug-file-search-paths`.
> I will check it in if there are no more replies in some time. Thanks for the 
> approval.
> 
The original wording is meant for tools that download debug symbols from 
centralized repositories like the dsym download scripts mentioned on 
http://lldb.llvm.org/symbols.html + the Spotlight metadata search engine.

The wording "sources" is problematic because it could be confused with source 
code. Does "/usr/lib..." count as an external library? Alternatively, what 
about "tools and repositories"?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55859



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


[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

2019-01-03 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I have a minidump generator if you need me to make any specific minidump files 
for you.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56293



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


[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

2019-01-03 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked an inline comment as done.
jankratochvil added inline comments.



Comment at: source/Core/ModuleList.cpp:72
  {},
  "Control the use of external tools or libraries to locate symbol files. "
+ "Directories listed in target.debug-file-search-paths and directory of "

aprantl wrote:
> jankratochvil wrote:
> > labath wrote:
> > > My main issue here was with the "tools or libraries" part of this 
> > > description -- we're not using any special tools or libraries when 
> > > looking in /usr/lib/debug.
> > > 
> > > Maybe just say that this is controls whether we use "external sources" 
> > > when searching for symbols. The rest of the description is fine (though I 
> > > would just drop the ifdefs and print everything everywhere).
> > The Spotlight on OSX was a bit unclear to me what it really does.  Doesn't 
> > it use some those "tools or libraries" to access the dSYM files downloaded 
> > from internet? But I have put there the "sources" word now as you wish.
> > Also rather added `See also symbols.enable-external-lookup.` to 
> > `target.debug-file-search-paths`.
> > I will check it in if there are no more replies in some time. Thanks for 
> > the approval.
> > 
> The original wording is meant for tools that download debug symbols from 
> centralized repositories like the dsym download scripts mentioned on 
> http://lldb.llvm.org/symbols.html + the Spotlight metadata search engine.
> 
> The wording "sources" is problematic because it could be confused with source 
> code. Does "/usr/lib..." count as an external library? Alternatively, what 
> about "tools and repositories"?
GDB calls "**/usr/lib/debug**" as "**debug-file-directory**". I definitely 
would not call it a "**library**". Would be OK for everyone to use "**external 
tools or directories**"?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55859



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


[Lldb-commits] [lldb] r350375 - [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

2019-01-03 Thread Kuba Mracek via lldb-commits
Author: kuba.brecka
Date: Thu Jan  3 16:20:52 2019
New Revision: 350375

URL: http://llvm.org/viewvc/llvm-project?rev=350375&view=rev
Log:
[lldb] Check SafeToCallFunctions before calling functions in 
GetExceptionObjectForThread

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


Modified:

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

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=350375&r1=350374&r2=350375&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
 Thu Jan  3 16:20:52 2019
@@ -554,6 +554,9 @@ bool ItaniumABILanguageRuntime::Exceptio
 
 ValueObjectSP ItaniumABILanguageRuntime::GetExceptionObjectForThread(
 ThreadSP thread_sp) {
+  if (!thread_sp->SafeToCallFunctions())
+return {};
+
   ClangASTContext *clang_ast_context =
   m_process->GetTarget().GetScratchClangASTContext();
   CompilerType voidstar =


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


[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

2019-01-03 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350375: [lldb] Check SafeToCallFunctions before calling 
functions in… (authored by kuba.brecka, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56115?vs=179589&id=180173#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56115

Files:
  
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp


Index: 
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
===
--- 
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++ 
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -554,6 +554,9 @@
 
 ValueObjectSP ItaniumABILanguageRuntime::GetExceptionObjectForThread(
 ThreadSP thread_sp) {
+  if (!thread_sp->SafeToCallFunctions())
+return {};
+
   ClangASTContext *clang_ast_context =
   m_process->GetTarget().GetScratchClangASTContext();
   CompilerType voidstar =


Index: source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
===
--- source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++ source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -554,6 +554,9 @@
 
 ValueObjectSP ItaniumABILanguageRuntime::GetExceptionObjectForThread(
 ThreadSP thread_sp) {
+  if (!thread_sp->SafeToCallFunctions())
+return {};
+
   ClangASTContext *clang_ast_context =
   m_process->GetTarget().GetScratchClangASTContext();
   CompilerType voidstar =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

2019-01-03 Thread Kuba (Brecka) Mracek via Phabricator via lldb-commits
kubamracek added a comment.

> I recently started writing these early-exit returns as return {};

Good suggestion!

> A testcase that triggers this situation would be nice, too.

I think this is too tricky. If I understand correctly, an example of an 
unsafe-to-call thread state is when we're parked inside a `__select` system 
call, and if we do invoke other code on that thread, sometimes, some kernel 
data structures can get out of sync from the thread state and the `__select` 
system call can sometimes return wrong results. Sounds quite hard to write a 
reliable test case for this, since the exact behavior also depends on OS 
version. I hope you'll forgive me for not providing this test :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56115



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


[Lldb-commits] [lldb] r350376 - [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized arguments

2019-01-03 Thread Kuba Mracek via lldb-commits
Author: kuba.brecka
Date: Thu Jan  3 16:25:08 2019
New Revision: 350376

URL: http://llvm.org/viewvc/llvm-project?rev=350376&view=rev
Log:
[lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized 
arguments

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


Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

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

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py?rev=350376&r1=350375&r2=350376&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
 Thu Jan  3 16:25:08 2019
@@ -34,6 +34,17 @@ class ObjCExceptionsTestCase(TestBase):
 'name: "ThrownException" - reason: "SomeReason"',
 ])
 
+target = self.dbg.GetSelectedTarget()
+thread = target.GetProcess().GetSelectedThread()
+frame = thread.GetSelectedFrame()
+
+opts = lldb.SBVariablesOptions()
+opts.SetIncludeRecognizedArguments(True)
+variables = frame.GetVariables(opts)
+
+self.assertEqual(variables.GetSize(), 1)
+self.assertEqual(variables.GetValueAtIndex(0).name, "exception")
+
 lldbutil.run_to_source_breakpoint(self, "// Set break point at this 
line.", lldb.SBFileSpec("main.mm"), launch_info=launch_info)
 
 self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,

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=350376&r1=350375&r2=350376&view=diff
==
--- 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 Thu Jan  3 16:25:08 2019
@@ -2630,6 +2630,9 @@ class ObjCExceptionRecognizedStackFrame
 exception = ValueObjectConstResult::Create(frame_sp.get(), value,
ConstString("exception"));
 exception = exception->GetDynamicValue(eDynamicDontRunTarget);
+  
+m_arguments = ValueObjectListSP(new ValueObjectList());
+m_arguments->Append(exception);
   }
 
   ValueObjectSP exception;


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


[Lldb-commits] [PATCH] D56027: [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized arguments

2019-01-03 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB350376: [lldb] Fix ObjCExceptionRecognizedStackFrame to 
populate the list of recognized… (authored by kuba.brecka, committed by ).

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56027

Files:
  packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2630,6 +2630,9 @@
 exception = ValueObjectConstResult::Create(frame_sp.get(), value,
ConstString("exception"));
 exception = exception->GetDynamicValue(eDynamicDontRunTarget);
+  
+m_arguments = ValueObjectListSP(new ValueObjectList());
+m_arguments->Append(exception);
   }
 
   ValueObjectSP exception;
Index: packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
===
--- packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
+++ packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
@@ -34,6 +34,17 @@
 'name: "ThrownException" - reason: "SomeReason"',
 ])
 
+target = self.dbg.GetSelectedTarget()
+thread = target.GetProcess().GetSelectedThread()
+frame = thread.GetSelectedFrame()
+
+opts = lldb.SBVariablesOptions()
+opts.SetIncludeRecognizedArguments(True)
+variables = frame.GetVariables(opts)
+
+self.assertEqual(variables.GetSize(), 1)
+self.assertEqual(variables.GetValueAtIndex(0).name, "exception")
+
 lldbutil.run_to_source_breakpoint(self, "// Set break point at this 
line.", lldb.SBFileSpec("main.mm"), launch_info=launch_info)
 
 self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,


Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2630,6 +2630,9 @@
 exception = ValueObjectConstResult::Create(frame_sp.get(), value,
ConstString("exception"));
 exception = exception->GetDynamicValue(eDynamicDontRunTarget);
+  
+m_arguments = ValueObjectListSP(new ValueObjectList());
+m_arguments->Append(exception);
   }
 
   ValueObjectSP exception;
Index: packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
===
--- packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
+++ packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
@@ -34,6 +34,17 @@
 'name: "ThrownException" - reason: "SomeReason"',
 ])
 
+target = self.dbg.GetSelectedTarget()
+thread = target.GetProcess().GetSelectedThread()
+frame = thread.GetSelectedFrame()
+
+opts = lldb.SBVariablesOptions()
+opts.SetIncludeRecognizedArguments(True)
+variables = frame.GetVariables(opts)
+
+self.assertEqual(variables.GetSize(), 1)
+self.assertEqual(variables.GetValueAtIndex(0).name, "exception")
+
 lldbutil.run_to_source_breakpoint(self, "// Set break point at this line.", lldb.SBFileSpec("main.mm"), launch_info=launch_info)
 
 self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r350380 - RangeMap.h: merge RangeDataArray and RangeDataVector

2019-01-03 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Jan  3 23:14:17 2019
New Revision: 350380

URL: http://llvm.org/viewvc/llvm-project?rev=350380&view=rev
Log:
RangeMap.h: merge RangeDataArray and RangeDataVector

Summary:
The main difference between the classes was supposed to be the fact that
one is backed by llvm::SmallVector, and the other by std::vector.
However, over the years, they have accumulated various other differences
too.

This essentially removes the std::vector version, as that is pretty much
identical to llvm::SmallVector, and combines their interfaces. It
does not attempt to do a more significant refactoring, even though there
is still a lot of duplication in this file, as it is hard to tell which
quirk of some API is depended on by somebody (and, a previous, more
ambitious attempt at this in D16769 has failed).

I also add some tests, including one which demonstrates one of the
quirks/bugs of the API I have noticed in the process.

Reviewers: clayborg, teemperor, tberghammer

Subscribers: mgorny, JDevlieghere, lldb-commits

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

Added:
lldb/trunk/unittests/Core/RangeMapTest.cpp
Modified:
lldb/trunk/include/lldb/Core/RangeMap.h
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
lldb/trunk/unittests/Core/CMakeLists.txt

Modified: lldb/trunk/include/lldb/Core/RangeMap.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RangeMap.h?rev=350380&r1=350379&r2=350380&view=diff
==
--- lldb/trunk/include/lldb/Core/RangeMap.h (original)
+++ lldb/trunk/include/lldb/Core/RangeMap.h Thu Jan  3 23:14:17 2019
@@ -633,201 +633,12 @@ struct RangeData : public Range {
   }
 };
 
-template  class RangeDataArray 
{
+template 
+class RangeDataVector {
 public:
   typedef RangeData Entry;
   typedef llvm::SmallVector Collection;
 
-  RangeDataArray() = default;
-
-  ~RangeDataArray() = default;
-
-  void Append(const Entry &entry) { m_entries.push_back(entry); }
-
-  void Sort() {
-if (m_entries.size() > 1)
-  std::stable_sort(m_entries.begin(), m_entries.end());
-  }
-
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-  bool IsSorted() const {
-typename Collection::const_iterator pos, end, prev;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos != 
end;
- prev = pos++) {
-  if (prev != end && *pos < *prev)
-return false;
-}
-return true;
-  }
-#endif
-
-  void CombineConsecutiveEntriesWithEqualData() {
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-assert(IsSorted());
-#endif
-typename Collection::iterator pos;
-typename Collection::iterator end;
-typename Collection::iterator prev;
-bool can_combine = false;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos != 
end;
- prev = pos++) {
-  if (prev != end && prev->data == pos->data) {
-can_combine = true;
-break;
-  }
-}
-
-// We we can combine at least one entry, then we make a new collection and
-// populate it accordingly, and then swap it into place.
-if (can_combine) {
-  Collection minimal_ranges;
-  for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
-   pos != end; prev = pos++) {
-if (prev != end && prev->data == pos->data)
-  minimal_ranges.back().SetRangeEnd(pos->GetRangeEnd());
-else
-  minimal_ranges.push_back(*pos);
-  }
-  // Use the swap technique in case our new vector is much smaller. We must
-  // swap when using the STL because std::vector objects never release or
-  // reduce the memory once it has been allocated/reserved.
-  m_entries.swap(minimal_ranges);
-}
-  }
-
-  void Clear() { m_entries.clear(); }
-
-  bool IsEmpty() const { return m_entries.empty(); }
-
-  size_t GetSize() const { return m_entries.size(); }
-
-  const Entry *GetEntryAtIndex(size_t i) const {
-return ((i < m_entries.size()) ? &m_entries[i] : nullptr);
-  }
-
-  // Clients must ensure that "i" is a valid index prior to calling this
-  // function
-  const Entry &GetEntryRef(size_t i) const { return m_entries[i]; }
-
-  static bool BaseLessThan(const Entry &lhs, const Entry &rhs) {
-return lhs.GetRangeBase() < rhs.GetRangeBase();
-  }
-
-  uint32_t FindEntryIndexThatContains(B addr) const {
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-assert(IsSorted());
-#endif
-if (!m_entries.empty()) {
-  Entry entry(addr, 1);
-  typename Collection::const_iterator begin = m_entries.begin();
-  typename Collection::const_iterator end = m_entries.end

[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL350380: RangeMap.h: merge RangeDataArray and RangeDataVector 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56170?vs=180027&id=180199#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56170

Files:
  lldb/trunk/include/lldb/Core/RangeMap.h
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
  lldb/trunk/unittests/Core/CMakeLists.txt
  lldb/trunk/unittests/Core/RangeMapTest.cpp

Index: lldb/trunk/include/lldb/Core/RangeMap.h
===
--- lldb/trunk/include/lldb/Core/RangeMap.h
+++ lldb/trunk/include/lldb/Core/RangeMap.h
@@ -633,201 +633,12 @@
   }
 };
 
-template  class RangeDataArray {
+template 
+class RangeDataVector {
 public:
   typedef RangeData Entry;
   typedef llvm::SmallVector Collection;
 
-  RangeDataArray() = default;
-
-  ~RangeDataArray() = default;
-
-  void Append(const Entry &entry) { m_entries.push_back(entry); }
-
-  void Sort() {
-if (m_entries.size() > 1)
-  std::stable_sort(m_entries.begin(), m_entries.end());
-  }
-
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-  bool IsSorted() const {
-typename Collection::const_iterator pos, end, prev;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos != end;
- prev = pos++) {
-  if (prev != end && *pos < *prev)
-return false;
-}
-return true;
-  }
-#endif
-
-  void CombineConsecutiveEntriesWithEqualData() {
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-assert(IsSorted());
-#endif
-typename Collection::iterator pos;
-typename Collection::iterator end;
-typename Collection::iterator prev;
-bool can_combine = false;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos != end;
- prev = pos++) {
-  if (prev != end && prev->data == pos->data) {
-can_combine = true;
-break;
-  }
-}
-
-// We we can combine at least one entry, then we make a new collection and
-// populate it accordingly, and then swap it into place.
-if (can_combine) {
-  Collection minimal_ranges;
-  for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
-   pos != end; prev = pos++) {
-if (prev != end && prev->data == pos->data)
-  minimal_ranges.back().SetRangeEnd(pos->GetRangeEnd());
-else
-  minimal_ranges.push_back(*pos);
-  }
-  // Use the swap technique in case our new vector is much smaller. We must
-  // swap when using the STL because std::vector objects never release or
-  // reduce the memory once it has been allocated/reserved.
-  m_entries.swap(minimal_ranges);
-}
-  }
-
-  void Clear() { m_entries.clear(); }
-
-  bool IsEmpty() const { return m_entries.empty(); }
-
-  size_t GetSize() const { return m_entries.size(); }
-
-  const Entry *GetEntryAtIndex(size_t i) const {
-return ((i < m_entries.size()) ? &m_entries[i] : nullptr);
-  }
-
-  // Clients must ensure that "i" is a valid index prior to calling this
-  // function
-  const Entry &GetEntryRef(size_t i) const { return m_entries[i]; }
-
-  static bool BaseLessThan(const Entry &lhs, const Entry &rhs) {
-return lhs.GetRangeBase() < rhs.GetRangeBase();
-  }
-
-  uint32_t FindEntryIndexThatContains(B addr) const {
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-assert(IsSorted());
-#endif
-if (!m_entries.empty()) {
-  Entry entry(addr, 1);
-  typename Collection::const_iterator begin = m_entries.begin();
-  typename Collection::const_iterator end = m_entries.end();
-  typename Collection::const_iterator pos =
-  std::lower_bound(begin, end, entry, BaseLessThan);
-
-  if (pos != end && pos->Contains(addr)) {
-return std::distance(begin, pos);
-  } else if (pos != begin) {
---pos;
-if (pos->Contains(addr))
-  return std::distance(begin, pos);
-  }
-}
-return UINT32_MAX;
-  }
-
-  Entry *FindEntryThatContains(B addr) {
-#ifdef ASSERT_RANGEMAP_ARE_SORTED
-assert(IsSorted());
-#endif
-if (!m_entries.empty()) {
-  Entry entry;
-  entry.SetRangeBase(addr);
-  entry.SetByteSize(1);
-  typename Collection::iterator begin = m_entries.begin();
-  typename Collection::iterator end = m_entries.end();
-  typename Collection::iterator pos =
-  std::lower_bound(begin, end, entry, BaseLessThan);
-
-  if (pos != end && pos->Contains(ad

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: source/Core/ModuleList.cpp:72
  {},
  "Control the use of external tools or libraries to locate symbol files. "
+ "Directories listed in target.debug-file-search-paths and directory of "

jankratochvil wrote:
> aprantl wrote:
> > jankratochvil wrote:
> > > labath wrote:
> > > > My main issue here was with the "tools or libraries" part of this 
> > > > description -- we're not using any special tools or libraries when 
> > > > looking in /usr/lib/debug.
> > > > 
> > > > Maybe just say that this is controls whether we use "external sources" 
> > > > when searching for symbols. The rest of the description is fine (though 
> > > > I would just drop the ifdefs and print everything everywhere).
> > > The Spotlight on OSX was a bit unclear to me what it really does.  
> > > Doesn't it use some those "tools or libraries" to access the dSYM files 
> > > downloaded from internet? But I have put there the "sources" word now as 
> > > you wish.
> > > Also rather added `See also symbols.enable-external-lookup.` to 
> > > `target.debug-file-search-paths`.
> > > I will check it in if there are no more replies in some time. Thanks for 
> > > the approval.
> > > 
> > The original wording is meant for tools that download debug symbols from 
> > centralized repositories like the dsym download scripts mentioned on 
> > http://lldb.llvm.org/symbols.html + the Spotlight metadata search engine.
> > 
> > The wording "sources" is problematic because it could be confused with 
> > source code. Does "/usr/lib..." count as an external library? 
> > Alternatively, what about "tools and repositories"?
> GDB calls "**/usr/lib/debug**" as "**debug-file-directory**". I definitely 
> would not call it a "**library**". Would be OK for everyone to use 
> "**external tools or directories**"?
Both "repositories" and "directories" sound fine to me. Another option might be 
to use something really generic like "facilities"? 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55859



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


[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

2019-01-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D56293#1345790 , @zturner wrote:

> Is there something about this executable that makes it impractical to compile 
> on the fly using the `%build` substitution?


I think the impractical part comes when you need to use that compiled binary to 
generate a minidump.

That said, I am not sure if you actually need the .exe for this test. Lldb 
should be able to open a core file/minidump without the matching executable 
file. It won't be terribly useful (for example, you won't get the disassembly 
around the crashing instructions), but you should be able to get the stop 
reason and the topmost PC for instance.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56293



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