[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

2018-01-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D42281#989793, @aprantl wrote:

> I am now working on building each test variant (dwarf,dwo,dsym,...) in its 
> own build directory so they can run in parallel and we can get rid of the 
> lockfile.


Are you planning to merge that into this patch? I am hoping that can be done in 
a separate pass, once dust settles down from landing this batch.

I think this is in a pretty good shape now, and we should land it soon. The 
only thing i'd like to wait for is confirmation that this runs on windows (i.e. 
does not run into any fundamental make limitations on that platform). I'm going 
to see if I can get around to that today.




Comment at: packages/Python/lldbsuite/test/functionalities/exec/TestExec.py:47
 if self.getArchitecture() == 'x86_64':
-source = os.path.join(os.getcwd(), "main.cpp")
-o_file = os.path.join(os.getcwd(), "main.o")
+source = self.getSourcPath("main.cpp")
+o_file = self.getBuildArtifact("main.o")

s/Sourc/Source/



Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:65
+(not os.path.isabs(rel_testdir))):
+#import pdb; pdb.set_trace()
+raise Exception("Could not derive test directories")

delete



Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5139-5142
-if (file_spec.Exists() && files.AppendIfUnique(file_spec)) {
+if (file_spec.Exists() && files.AppendIfUnique(file_spec))
   count++;
-  break;
-}

I think it makes sense to separate the single change in actual code from the 
gigantuous code refactor.


https://reviews.llvm.org/D42281



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


[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

2018-01-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for the clarification. It seems nobody was calling that function, so 
maybe it never actually worked.

I'll try committing this for Owen with the clang paths modified and check 
whether the bots like it.


https://reviews.llvm.org/D42195



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


[Lldb-commits] [lldb] r323636 - [lldb] Generic base for testing gdb-remote behavior

2018-01-29 Thread Pavel Labath via lldb-commits
Author: labath
Date: Mon Jan 29 02:02:40 2018
New Revision: 323636

URL: http://llvm.org/viewvc/llvm-project?rev=323636&view=rev
Log:
[lldb] Generic base for testing gdb-remote behavior

Summary:
Adds new utilities that make it easier to write test cases for lldb acting as a 
client over a gdb-remote connection.

- A GDBRemoteTestBase class that starts a mock GDB server and provides an easy 
way to check client packets
- A MockGDBServer that, via MockGDBServerResponder, can be made to issue server 
responses that test client behavior.
- Utility functions for handling common data encoding/decoding
- Utility functions for creating dummy targets from YAML files



Split from the review at https://reviews.llvm.org/D42145, which was a new 
feature that necessitated the new testing capabilities.

Reviewers: clayborg, labath

Reviewed By: clayborg, labath

Subscribers: hintonda, davide, jingham, krytarowski, mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D42195
Patch by Owen Shaw 

Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/a.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/test/CMakeLists.txt

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py?rev=323636&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
 Mon Jan 29 02:02:40 2018
@@ -0,0 +1,13 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestGDBRemoteClient(GDBRemoteTestBase):
+
+def test_connect(self):
+"""Test connecting to a remote gdb server"""
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertPacketLogContains(["qProcessInfo", "qfThreadInfo"])

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/a.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/a.yaml?rev=323636&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/a.yaml
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/a.yaml
 Mon Jan 29 02:02:40 2018
@@ -0,0 +1,34 @@
+!ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_ARM
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x1000
+AddressAlign:0x4
+Content: "c3c3c3c3"
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC ]
+Address: 0x2000
+AddressAlign:0x4
+Content: "3232"
+ProgramHeaders:
+  - Type: PT_LOAD
+Flags: [ PF_X, PF_R ]
+VAddr: 0x1000
+PAddr: 0x1000
+Align: 0x4
+Sections:
+  - Section: .text
+  - Type: PT_LOAD
+Flags: [ PF_R, PF_W ]
+VAddr: 0x2000
+PAddr: 0x1004
+Align: 0x4
+Sections:
+  - Section: .data

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py?rev=323636&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
 Mon Jan 29 02:02:40 2018
@@ -0,0 +1,442 @@
+import os
+import os.path
+import subprocess
+import threading
+import socket
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbtest_config
+
+
+def checksum(message):
+"""
+Calculate the GDB server protocol checksum of the message.
+
+The GDB server protocol uses a simple modulo 256 sum.
+"""
+check = 0
+for c in message:
+check += ord(c)
+return check % 256
+
+
+def frame_packet(message):
+"""
+Create a framed packet that's re

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

2018-01-29 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323636: [lldb] Generic base for testing gdb-remote behavior 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42195?vs=131310&id=131754#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42195

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/a.yaml
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
  lldb/trunk/test/CMakeLists.txt

Index: lldb/trunk/test/CMakeLists.txt
===
--- lldb/trunk/test/CMakeLists.txt
+++ lldb/trunk/test/CMakeLists.txt
@@ -34,6 +34,10 @@
   list(APPEND LLDB_TEST_DEPS lldb-mi)
 endif()
 
+if(NOT LLDB_BUILT_STANDALONE)
+  list(APPEND LLDB_TEST_DEPS yaml2obj)
+endif()
+
 # The default architecture with which to compile test executables is the default LLVM target
 # architecture, which itself defaults to the host architecture.
 string(TOLOWER "${LLVM_TARGET_ARCH}" LLDB_DEFAULT_TEST_ARCH)
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -1589,10 +1589,10 @@
 def findBuiltClang(self):
 """Tries to find and use Clang from the build directory as the compiler (instead of the system compiler)."""
 paths_to_try = [
-"llvm-build/Release+Asserts/x86_64/Release+Asserts/bin/clang",
-"llvm-build/Debug+Asserts/x86_64/Debug+Asserts/bin/clang",
-"llvm-build/Release/x86_64/Release/bin/clang",
-"llvm-build/Debug/x86_64/Debug/bin/clang",
+"llvm-build/Release+Asserts/x86_64/bin/clang",
+"llvm-build/Debug+Asserts/x86_64/bin/clang",
+"llvm-build/Release/x86_64/bin/clang",
+"llvm-build/Debug/x86_64/bin/clang",
 ]
 lldb_root_path = os.path.join(
 os.path.dirname(__file__), "..", "..", "..", "..")
@@ -1608,6 +1608,31 @@
 
 return os.environ["CC"]
 
+def findYaml2obj(self):
+"""
+Get the path to the yaml2obj executable, which can be used to create
+test object files from easy to write yaml instructions.
+
+Throws an Exception if the executable cannot be found.
+"""
+# Tries to find yaml2obj at the same folder as clang
+clang_dir = os.path.dirname(self.findBuiltClang())
+path = os.path.join(clang_dir, "yaml2obj")
+if os.path.exists(path):
+return path
+raise Exception("yaml2obj executable not found")
+
+
+def yaml2obj(self, yaml_path, obj_path):
+"""
+Create an object file at the given path from a yaml file.
+
+Throws subprocess.CalledProcessError if the object could not be created.
+"""
+yaml2obj = self.findYaml2obj()
+command = [yaml2obj, "-o=%s" % obj_path, yaml_path]
+system([command])
+
 def getBuildFlags(
 self,
 use_cpp11=True,
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -0,0 +1,13 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestGDBRemoteClient(GDBRemoteTestBase):
+
+def test_connect(self):
+"""Test connecting to a remote gdb server"""
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertPacketLogContains(["qProcessInfo", "qfThreadInfo"])
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
@@ -0,0 +1,442 @@
+import os
+import os.path
+import subprocess
+import threading
+import socket
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbtest_config
+
+
+def checksum(message):
+"""
+Calculate the GDB server protocol checksum of the message.
+
+The GDB server protocol uses a simple modulo 256 sum.
+"""
+check = 0
+for c in message:
+check += ord(c)
+  

[Lldb-commits] [lldb] r323637 - Remove ObjectFile usage from HostLinux::GetProcessInfo

2018-01-29 Thread Pavel Labath via lldb-commits
Author: labath
Date: Mon Jan 29 02:46:00 2018
New Revision: 323637

URL: http://llvm.org/viewvc/llvm-project?rev=323637&view=rev
Log:
Remove ObjectFile usage from HostLinux::GetProcessInfo

Summary:
The ObjectFile class was used to determine the architecture of a running
process by inspecting it's main executable. There were two issues with
this:
- it's in the wrong layer
- the call can be very expensive (it can end up computing the crc of the
  whole file).

Since the process is running on the host, ideally we would be able to
just query the data straight from the OS like darwin does, but there
doesn't seem to be a reasonable way to do that. So, this fixes the
layering issue by using the llvm object library to inspect the file.
Since we know the process is already running on the host, we just need
to peek at a few bytes of the elf header to determine whether it's 32-
or 64-bit (which should make this faster as well).

Pretty much the same logic was implemented in
NativeProcessProtocol::ResolveProcessArchitecture, so I delete this
logic and replace calls with GetProcessInfo.

Reviewers: eugene, krytarowski

Subscribers: mgorny, hintonda, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
lldb/trunk/include/lldb/Utility/ArchSpec.h
lldb/trunk/source/Host/CMakeLists.txt
lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
lldb/trunk/source/Host/linux/Host.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/trunk/source/Utility/ArchSpec.cpp
lldb/trunk/unittests/Host/linux/HostTest.cpp

Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=323637&r1=323636&r2=323637&view=diff
==
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Mon Jan 29 
02:46:00 2018
@@ -466,11 +466,6 @@ protected:
 
   NativeThreadProtocol *GetThreadByIDUnlocked(lldb::tid_t tid);
 
-  // ---
-  // Static helper methods for derived classes.
-  // ---
-  static Status ResolveProcessArchitecture(lldb::pid_t pid, ArchSpec &arch);
-
 private:
   void SynchronouslyNotifyProcessStateChanged(lldb::StateType state);
 };

Modified: lldb/trunk/include/lldb/Utility/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ArchSpec.h?rev=323637&r1=323636&r2=323637&view=diff
==
--- lldb/trunk/include/lldb/Utility/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Utility/ArchSpec.h Mon Jan 29 02:46:00 2018
@@ -617,6 +617,7 @@ protected:
 /// @return true if \a lhs is less than \a rhs
 //--
 bool operator<(const ArchSpec &lhs, const ArchSpec &rhs);
+bool operator==(const ArchSpec &lhs, const ArchSpec &rhs);
 
 bool ParseMachCPUDashSubtypeTriple(llvm::StringRef triple_str, ArchSpec &arch);
 

Modified: lldb/trunk/source/Host/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/CMakeLists.txt?rev=323637&r1=323636&r2=323637&view=diff
==
--- lldb/trunk/source/Host/CMakeLists.txt (original)
+++ lldb/trunk/source/Host/CMakeLists.txt Mon Jan 29 02:46:00 2018
@@ -188,5 +188,6 @@ add_lldb_library(lldbHost
 ${EXTRA_LIBS}
   
   LINK_COMPONENTS
+Object
 Support
   )

Modified: lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeProcessProtocol.cpp?rev=323637&r1=323636&r2=323637&view=diff
==
--- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Mon Jan 29 02:46:00 
2018
@@ -8,13 +8,11 @@
 
//===--===//
 
 #include "lldb/Host/common/NativeProcessProtocol.h"
-#include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/State.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
 #include "lldb/Host/common/SoftwareBreakpoint.h"
-#include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -431,26 +429,4 @@ void NativeProcessProtocol::DoStopIDBump
   // Default implementation does nothing.
 }
 
-Status NativeProcessProtocol::ResolveProcessArch

[Lldb-commits] [PATCH] D42488: Remove ObjectFile usage from HostLinux::GetProcessInfo

2018-01-29 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323637: Remove ObjectFile usage from 
HostLinux::GetProcessInfo (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42488?vs=131401&id=131757#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42488

Files:
  lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/trunk/include/lldb/Utility/ArchSpec.h
  lldb/trunk/source/Host/CMakeLists.txt
  lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
  lldb/trunk/source/Host/linux/Host.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/trunk/source/Utility/ArchSpec.cpp
  lldb/trunk/unittests/Host/linux/HostTest.cpp

Index: lldb/trunk/include/lldb/Utility/ArchSpec.h
===
--- lldb/trunk/include/lldb/Utility/ArchSpec.h
+++ lldb/trunk/include/lldb/Utility/ArchSpec.h
@@ -617,6 +617,7 @@
 /// @return true if \a lhs is less than \a rhs
 //--
 bool operator<(const ArchSpec &lhs, const ArchSpec &rhs);
+bool operator==(const ArchSpec &lhs, const ArchSpec &rhs);
 
 bool ParseMachCPUDashSubtypeTriple(llvm::StringRef triple_str, ArchSpec &arch);
 
Index: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
===
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
@@ -466,11 +466,6 @@
 
   NativeThreadProtocol *GetThreadByIDUnlocked(lldb::tid_t tid);
 
-  // ---
-  // Static helper methods for derived classes.
-  // ---
-  static Status ResolveProcessArchitecture(lldb::pid_t pid, ArchSpec &arch);
-
 private:
   void SynchronouslyNotifyProcessStateChanged(lldb::StateType state);
 };
Index: lldb/trunk/unittests/Host/linux/HostTest.cpp
===
--- lldb/trunk/unittests/Host/linux/HostTest.cpp
+++ lldb/trunk/unittests/Host/linux/HostTest.cpp
@@ -45,4 +45,8 @@
 
   ASSERT_TRUE(Info.GroupIDIsValid());
   EXPECT_EQ(getegid(), Info.GetGroupID());
+
+  EXPECT_TRUE(Info.GetArchitecture().IsValid());
+  EXPECT_EQ(HostInfo::GetArchitecture(HostInfo::eArchKindDefault),
+Info.GetArchitecture());
 }
Index: lldb/trunk/source/Utility/ArchSpec.cpp
===
--- lldb/trunk/source/Utility/ArchSpec.cpp
+++ lldb/trunk/source/Utility/ArchSpec.cpp
@@ -1416,6 +1416,11 @@
   return lhs_core < rhs_core;
 }
 
+
+bool lldb_private::operator==(const ArchSpec &lhs, const ArchSpec &rhs) {
+  return lhs.GetCore() == rhs.GetCore();
+}
+
 bool ArchSpec::IsFullySpecifiedTriple() const {
   const auto &user_specified_triple = GetTriple();
 
Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -93,17 +93,19 @@
   }
   LLDB_LOG(log, "inferior started, now in stopped state");
 
-  ArchSpec arch;
-  if ((status = ResolveProcessArchitecture(pid, arch)).Fail())
-return status.ToError();
+  ProcessInstanceInfo Info;
+  if (!Host::GetProcessInfo(pid, Info)) {
+return llvm::make_error("Cannot get process architecture",
+ llvm::inconvertibleErrorCode());
+  }
 
   // Set the architecture to the exe architecture.
   LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
-   arch.GetArchitectureName());
+   Info.GetArchitecture().GetArchitectureName());
 
   std::unique_ptr process_up(new NativeProcessNetBSD(
   pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate,
-  arch, mainloop));
+  Info.GetArchitecture(), mainloop));
 
   status = process_up->ReinitializeThreads();
   if (status.Fail())
@@ -124,13 +126,14 @@
   LLDB_LOG(log, "pid = {0:x}", pid);
 
   // Retrieve the architecture for the running process.
-  ArchSpec arch;
-  Status status = ResolveProcessArchitecture(pid, arch);
-  if (!status.Success())
-return status.ToError();
+  ProcessInstanceInfo Info;
+  if (!Host::GetProcessInfo(pid, Info)) {
+return llvm::make_error("Cannot get process architecture",
+ llvm::inconvertibleErrorCode());
+  }
 
-  std::unique_ptr process_up(
-  new NativeProcessNetBSD(pid, -1, native_delegate, arch, mainloop));
+  std::unique_ptr process_up(new NativeProcessNetBSD(
+  pid, -1, native_delegate, Info.GetArchitecture(), mainloop));
 
   status = process_up->Attach();
   if 

[Lldb-commits] [lldb] r323639 - Fix NetBsd build broken by r323637

2018-01-29 Thread Pavel Labath via lldb-commits
Author: labath
Date: Mon Jan 29 03:10:21 2018
New Revision: 323639

URL: http://llvm.org/viewvc/llvm-project?rev=323639&view=rev
Log:
Fix NetBsd build broken by r323637

Modified:
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

Modified: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp?rev=323639&r1=323638&r2=323639&view=diff
==
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Mon Jan 29 
03:10:21 2018
@@ -135,7 +135,7 @@ NativeProcessNetBSD::Factory::Attach(
   std::unique_ptr process_up(new NativeProcessNetBSD(
   pid, -1, native_delegate, Info.GetArchitecture(), mainloop));
 
-  status = process_up->Attach();
+  Status status = process_up->Attach();
   if (!status.Success())
 return status.ToError();
 


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


[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-29 Thread Tamas Berghammer via Phabricator via lldb-commits
tberghammer added a comment.

Thanks for the explanation, sorry I haven't read your commit message carefully.

In case of non-split-dwarf a die_offset is sufficient to uniquely identify a 
DIE because it is an offset from the beginning of the debug_info section (we 
assume we have at most 1 debug_info section everywhere in LLDB) and we are 
already relying on this as we don't store the compile unit offset in the 
non-split dwarf case. Looking at the code setting the cu_offset to 
DW_INVALID_OFFSET will be more correct then what we are doing at the moment 
because DWARFDebugInfo::GetDIE will call DWARFDebugInfo::GetCompileUnit and 
that function will look for a compile unit based on the cu_offset if cu_offset 
!= DW_INVALID_OFFSET what is currently the case so it will return an incorrect 
compile unit in the non-split-dwarf case. Setting cu_offset to 
DW_INVALID_OFFSET would solve the issue as it will look for a compile unit 
based on the DIE offset what is correct in the non-split dwarf case (I think 
this change of behavior is the one fixing your test case).

My only concern with setting the SymbolFileUID to DW_INVALID_OFFSET is how will 
it work in case of MachO (I have very little knowledge about that code path) 
but I really hope we don't implicitly depend on the offset being set to 0 
instead of DW_INVALID_OFFSET what is meant to represent unused/invalid values 
so I expect it to work.


Repository:
  rL LLVM

https://reviews.llvm.org/D42563



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


[Lldb-commits] [PATCH] D42620: [lldb] Silence signed <-> unsigned integer comparison warning

2018-01-29 Thread Kirill Bobyrev via Phabricator via lldb-commits
omtcyfz added a comment.

@asmith Thanks!

@davide Thanks for the input!
I'll be able to do land it since I have commit access, thanks!


https://reviews.llvm.org/D42620



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


[Lldb-commits] [PATCH] D42620: [lldb] Silence signed <-> unsigned integer comparison warning

2018-01-29 Thread Kirill Bobyrev via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323645: [lldb] Silence signed <-> unsigned integer 
comparison warning (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42620?vs=131704&id=131773#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42620

Files:
  lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp


Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -270,7 +270,7 @@
 // Drop last variadic argument.
 if (is_variadic)
   --num_args;
-for (int arg_idx = 0; arg_idx < num_args; arg_idx++) {
+for (uint32_t arg_idx = 0; arg_idx < num_args; arg_idx++) {
   auto arg = arg_enum->getChildAtIndex(arg_idx);
   if (!arg)
 break;


Index: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -270,7 +270,7 @@
 // Drop last variadic argument.
 if (is_variadic)
   --num_args;
-for (int arg_idx = 0; arg_idx < num_args; arg_idx++) {
+for (uint32_t arg_idx = 0; arg_idx < num_args; arg_idx++) {
   auto arg = arg_enum->getChildAtIndex(arg_idx);
   if (!arg)
 break;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

2018-01-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I haven't gotten around to trying this out on windows yet, but I have tried 
running the tests remotely. I've updated https://reviews.llvm.org/D42572 with 
the two fixes necessary to make the remote tests pass (for android).


https://reviews.llvm.org/D42281



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


[Lldb-commits] [PATCH] D42317: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

We have one plugin in-tree (tools/intel-features), but I'm not sure if anyone 
uses/installs it.

In any case, the change seems like the right thing to do.


https://reviews.llvm.org/D42317



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


[Lldb-commits] [PATCH] D42317: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: source/Host/linux/HostInfoLinux.cpp:208
 bool HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec) {
-  FileSpec temp_file("/usr/lib/lldb/plugins", true);
+  FileSpec temp_file("/usr/lib" LLDB_LIBDIR_SUFFIX "/lldb/plugins", true);
   file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());

Is there no setting that sets the start of the path here ("/usr/lib")? Not sure 
how open source projects handle this, but it seems that this should be a build 
setting that defaults to "/usr/lib" and can be modified?


https://reviews.llvm.org/D42317



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


[Lldb-commits] [PATCH] D42317: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

See inlined comment


https://reviews.llvm.org/D42317



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


[Lldb-commits] [PATCH] D42317: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: source/Host/linux/HostInfoLinux.cpp:208
 bool HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec) {
-  FileSpec temp_file("/usr/lib/lldb/plugins", true);
+  FileSpec temp_file("/usr/lib" LLDB_LIBDIR_SUFFIX "/lldb/plugins", true);
   file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());

clayborg wrote:
> Is there no setting that sets the start of the path here ("/usr/lib")? Not 
> sure how open source projects handle this, but it seems that this should be a 
> build setting that defaults to "/usr/lib" and can be modified?
Well, yes, this is something that could be possibly be done but TBH I don't 
really know how exactly it should be done. For autoconf projects, you generally 
do something like `-Dbindir=$(bindir)` in the Makefile. For CMake projects, I 
suppose this could be `CMAKE_INSTALL_PREFIX`, except I don't know if it has any 
corner cases. AFAIK clang uses path relative to executable to find compiler-rt, 
so maybe that would be revelant here as well.

In other words, I went for fixing the most obvious bug, and leaving the harder 
(yet less frequent) problem to somebody else.


https://reviews.llvm.org/D42317



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


Re: [Lldb-commits] [PATCH] D42386: Fix memory leak in TestClangASTContext.TestRecordHasFields

2018-01-29 Thread David Blaikie via lldb-commits
Any chance of using unique_ptr or other RAII/etc ownership to make this API
safer by default?

On Mon, Jan 22, 2018 at 10:58 AM Raphael Isemann via Phabricator via
llvm-commits  wrote:

> This revision was not accepted when it landed; it landed in state "Needs
> Review".
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL323138: Fix memory leak in
> TestClangASTContext.TestRecordHasFields (authored by teemperor, committed
> by ).
> Herald added a subscriber: llvm-commits.
>
> Changed prior to commit:
>   https://reviews.llvm.org/D42386?vs=130926&id=130928#toc
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D42386
>
> Files:
>   lldb/trunk/unittests/Symbol/TestClangASTContext.cpp
>
>
> Index: lldb/trunk/unittests/Symbol/TestClangASTContext.cpp
> ===
> --- lldb/trunk/unittests/Symbol/TestClangASTContext.cpp
> +++ lldb/trunk/unittests/Symbol/TestClangASTContext.cpp
> @@ -11,6 +11,8 @@
>
>  #include "gtest/gtest.h"
>
> +#include "clang/AST/DeclCXX.h"
> +
>  #include "lldb/Host/HostInfo.h"
>  #include "lldb/Symbol/ClangASTContext.h"
>  #include "lldb/Symbol/ClangUtil.h"
> @@ -375,6 +377,9 @@
> empty_derived_non_empty_vbase_cxx_decl, false));
>EXPECT_TRUE(
>
>  ClangASTContext::RecordHasFields(empty_derived_non_empty_vbase_decl));
> +
> +  delete non_empty_base_spec;
> +  delete non_empty_vbase_spec;
>  }
>
>  TEST_F(TestClangASTContext, TemplateArguments) {
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D42386: Fix memory leak in TestClangASTContext.TestRecordHasFields

2018-01-29 Thread Davide Italiano via lldb-commits
On Mon, Jan 29, 2018 at 9:42 AM, David Blaikie via lldb-commits
 wrote:
> Any chance of using unique_ptr or other RAII/etc ownership to make this API
> safer by default?
>

I agree that would be a more robust way of dealing with the problem.

Thanks,

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


Re: [Lldb-commits] [lldb] r322339 - When parsing the target.xml register file, if no architecture has

2018-01-29 Thread Jason Molenda via lldb-commits


> On Jan 28, 2018, at 9:59 PM, Davide Italiano  wrote:
> 
> On Thu, Jan 11, 2018 at 5:26 PM, Davide Italiano  
> wrote:
>> On Thu, Jan 11, 2018 at 5:16 PM, Jason Molenda via lldb-commits
>>  wrote:
>>> Author: jmolenda
>>> Date: Thu Jan 11 17:16:13 2018
>>> New Revision: 322339
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=322339&view=rev
>>> Log:
>>> When parsing the target.xml register file, if no architecture has
>>> been specified yet (either by the user, or by one of the lldb
>>> extensions like qHostInfo or qProcessInfo), and the target.xml
>>> includes a  tag specifying x86_64, set the architecture
>>> appropriately.
>>> 
>>> I'm not sure what we can expect to see in the  tag, so
>>> I'm only doing this for x86_64 right now where I've seen "i386:x86_64"
>>> used.  I've seen a target.xml from a jtag board that sends just "arm"
>>> because it doesn't know more specifically what type of board it is
>>> connected to...
>>> 
>>> 
>>> 
>> 
>> Jason,
>> is there a way to test this change?
>> 
> 
> ping? Any news on this front?

Thanks for the reminder.  I need to look at doing this - I'm in the middle of 
another patch right now but I'll return to this after that.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D42317: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.

Looks good.


https://reviews.llvm.org/D42317



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


[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

2018-01-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

In https://reviews.llvm.org/D42281#990296, @labath wrote:

> In https://reviews.llvm.org/D42281#989793, @aprantl wrote:
>
> > I am now working on building each test variant (dwarf,dwo,dsym,...) in its 
> > own build directory so they can run in parallel and we can get rid of the 
> > lockfile.
>
>
> Are you planning to merge that into this patch? I am hoping that can be done 
> in a separate pass, once dust settles down from landing this batch.
>
> I think this is in a pretty good shape now, and we should land it soon. The 
> only thing i'd like to wait for is confirmation that this runs on windows 
> (i.e. does not run into any fundamental make limitations on that platform). 
> I'm going to see if I can get around to that today.


Sure that makes total sense. "Smaller" patches are always better. I'm mostly 
waiting for someone to greenlight Windows now.




Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5139-5142
-if (file_spec.Exists() && files.AppendIfUnique(file_spec)) {
+if (file_spec.Exists() && files.AppendIfUnique(file_spec))
   count++;
-  break;
-}

labath wrote:
> I think it makes sense to separate the single change in actual code from the 
> gigantuous code refactor.
Oh right. This is an actual bug that I found because my refactoring of 
makefiles happened to change the relative order of dylibs in the Mach-O headers.


https://reviews.llvm.org/D42281



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


[Lldb-commits] [PATCH] D42317: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323673: [Host] Respect LLVM_LIBDIR_SUFFIX when looking for 
LLDB plugins on Linux (authored by mgorny, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42317?vs=130695&id=131826#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42317

Files:
  lldb/trunk/include/lldb/Host/Config.h.cmake
  lldb/trunk/source/Host/linux/HostInfoLinux.cpp


Index: lldb/trunk/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/trunk/source/Host/linux/HostInfoLinux.cpp
+++ lldb/trunk/source/Host/linux/HostInfoLinux.cpp
@@ -7,6 +7,7 @@
 //
 
//===--===//
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/linux/HostInfoLinux.h"
 #include "lldb/Utility/Log.h"
 
@@ -204,7 +205,7 @@
 }
 
 bool HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec) {
-  FileSpec temp_file("/usr/lib/lldb/plugins", true);
+  FileSpec temp_file("/usr/lib" LLDB_LIBDIR_SUFFIX "/lldb/plugins", true);
   file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());
   return true;
 }
Index: lldb/trunk/include/lldb/Host/Config.h.cmake
===
--- lldb/trunk/include/lldb/Host/Config.h.cmake
+++ lldb/trunk/include/lldb/Host/Config.h.cmake
@@ -14,6 +14,8 @@
 
 #cmakedefine LLDB_DISABLE_POSIX
 
+#define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
+
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL


Index: lldb/trunk/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/trunk/source/Host/linux/HostInfoLinux.cpp
+++ lldb/trunk/source/Host/linux/HostInfoLinux.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/linux/HostInfoLinux.h"
 #include "lldb/Utility/Log.h"
 
@@ -204,7 +205,7 @@
 }
 
 bool HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec) {
-  FileSpec temp_file("/usr/lib/lldb/plugins", true);
+  FileSpec temp_file("/usr/lib" LLDB_LIBDIR_SUFFIX "/lldb/plugins", true);
   file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());
   return true;
 }
Index: lldb/trunk/include/lldb/Host/Config.h.cmake
===
--- lldb/trunk/include/lldb/Host/Config.h.cmake
+++ lldb/trunk/include/lldb/Host/Config.h.cmake
@@ -14,6 +14,8 @@
 
 #cmakedefine LLDB_DISABLE_POSIX
 
+#define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
+
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r323673 - [Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

2018-01-29 Thread Michal Gorny via lldb-commits
Author: mgorny
Date: Mon Jan 29 10:25:06 2018
New Revision: 323673

URL: http://llvm.org/viewvc/llvm-project?rev=323673&view=rev
Log:
[Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux

Fix the Linux plugin lookup path to include appropriate libdir suffix
for the system. To accomplish this, store the value of
LLVM_LIBDIR_SUFFIX in lldb/Host/Config.h as LLDB_LIBDIR_SUFFIX,
and use this variable when defining the plugin path.

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

Modified:
lldb/trunk/include/lldb/Host/Config.h.cmake
lldb/trunk/source/Host/linux/HostInfoLinux.cpp

Modified: lldb/trunk/include/lldb/Host/Config.h.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Config.h.cmake?rev=323673&r1=323672&r2=323673&view=diff
==
--- lldb/trunk/include/lldb/Host/Config.h.cmake (original)
+++ lldb/trunk/include/lldb/Host/Config.h.cmake Mon Jan 29 10:25:06 2018
@@ -14,6 +14,8 @@
 
 #cmakedefine LLDB_DISABLE_POSIX
 
+#define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
+
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL

Modified: lldb/trunk/source/Host/linux/HostInfoLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/HostInfoLinux.cpp?rev=323673&r1=323672&r2=323673&view=diff
==
--- lldb/trunk/source/Host/linux/HostInfoLinux.cpp (original)
+++ lldb/trunk/source/Host/linux/HostInfoLinux.cpp Mon Jan 29 10:25:06 2018
@@ -7,6 +7,7 @@
 //
 
//===--===//
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/linux/HostInfoLinux.h"
 #include "lldb/Utility/Log.h"
 
@@ -204,7 +205,7 @@ bool HostInfoLinux::ComputeSupportExeDir
 }
 
 bool HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec) {
-  FileSpec temp_file("/usr/lib/lldb/plugins", true);
+  FileSpec temp_file("/usr/lib" LLDB_LIBDIR_SUFFIX "/lldb/plugins", true);
   file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());
   return true;
 }


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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added a comment.

What's the failure mode? Have we had any issues with this on the bots?

Generally I'm all for removing flaky tests, I'd like to understand what makes 
this flaky so we can avoid whatever it is in the future. In this case, we 
should be able test this like we test clang code completion, a la c-index-test.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

There are a whole bunch of other tests that test completion in this file that 
use the exact same mechanism but don't seem to be flakey.  Why is this one test 
flakey?

If for instance it's because "Fo" ends up being ambiguous because we chose too 
common a start string, then you could trivially fix the test by choosing a more 
uncommon name to complete on.  But I'd want to know why this test is flakey 
first.

I also don't see why you say this test doesn't test something important.  The 
ability to auto-complete symbol names it pretty important to command-line lldb 
users.  If anything we should have more tests of the symbol completer...


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

In https://reviews.llvm.org/D42656#991065, @jingham wrote:

> There are a whole bunch of other tests that test completion in this file that 
> use the exact same mechanism but don't seem to be flakey.  Why is this one 
> test flakey?
>
> If for instance it's because "Fo" ends up being ambiguous because we chose 
> too common a start string, then you could trivially fix the test by choosing 
> a more uncommon name to complete on.  But I'd want to know why this test is 
> flakey first.


OK, I'll take a look at this further.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

In https://reviews.llvm.org/D42656#991065, @jingham wrote:

> There are a whole bunch of other tests that test completion in this file that 
> use the exact same mechanism but don't seem to be flakey.  Why is this one 
> test flakey?
>
> If for instance it's because "Fo" ends up being ambiguous because we chose 
> too common a start string, then you could trivially fix the test by choosing 
> a more uncommon name to complete on.  But I'd want to know why this test is 
> flakey first.
>
> I also don't see why you say this test doesn't test something important.  The 
> ability to auto-complete symbol names it pretty important to command-line 
> lldb users.  If anything we should have more tests of the symbol completer...


Apologies, I was thinking of another test (happens when there are multiple 
`UNEXPECTED SUCCESSES` ;)

This seems to pass pretty reliably locally. I'd go for UNXFAILING this unless 
there are objections.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

2018-01-29 Thread Owen Shaw via Phabricator via lldb-commits
owenpshaw added a comment.

Hi Greg, I got distracted from this one for a bit.  Maybe I missed it, but do 
you have any thoughts on my previous comment/question about the batch API vs 
other options?

Thanks,
Owen


https://reviews.llvm.org/D42145



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

In https://reviews.llvm.org/D42656#991065, @jingham wrote:

> There are a whole bunch of other tests that test completion in this file that 
> use the exact same mechanism but don't seem to be flakey.  Why is this one 
> test flakey?


So, I take a look at this to reply to your question. They're all flakey.
Most of them fails non-deterministically depending on the load of the system. I 
just wrote a driver that spawns randomly threads that spin loops & checked out 
multiple copies of lldb and ran test suites in parallel.
I see many of the failures testing completion randomly failing.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

In https://reviews.llvm.org/D42656#991239, @davide wrote:

> In https://reviews.llvm.org/D42656#991065, @jingham wrote:
>
> > There are a whole bunch of other tests that test completion in this file 
> > that use the exact same mechanism but don't seem to be flakey.  Why is this 
> > one test flakey?
>
>
> So, I take a look at this to reply to your question. They're all flakey.
>  Most of them fails non-deterministically depending on the load of the 
> system. I just wrote a driver that spawns randomly threads that spin loops & 
> checked out multiple copies of lldb and ran test suites in parallel.
>  I see many of the failures testing completion randomly failing.


As an example of a test which just failed on my machine because of a timeout 
(non-deterministic, happens around ~1/50 times on my machine)

  TIMEOUT: test_step_over_3_times_dsym (python_api/thread/TestThreadAPI.py)



  [TestThreadAPI.py FAILED] (TIMEOUT)
  Command invoked: 
/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
 /Users/dcci/work/llvm/llvm/tools/lldb/test/dotest.py -q --arch=x86_64 
--executable /Users/dcci/work/llvm/build/bin/lldb -s 
/Users/dcci/work/llvm/build/lldb-test-traces -S nm -u CXXFLAGS -u CFLAGS -C 
/Users/dcci/work/llvm/build/bin/clang --codesign-identity lldb_codesign 
--server /Users/dcci/work/llvm/build/bin/debugserver --results-port 52049 -S nm 
--inferior -p TestThreadAPI.py 
/Users/dcci/work/llvm/llvm/tools/lldb/packages/Python/lldbsuite/test 
--event-add-entries worker_index=6:int

I'll probably go ahead and either unXFAIL or SKIP the tests that I find being 
non-reliable cause of `pexpect`. Having tests that pass most of the time under 
load but not always is quite unfortunate, in particular now that we want to 
enable continuous integration for downstream projects using lldb.

Best,

-

Davide


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

So that sounds like a pexpect problem.  We have seen that pexpect based tests 
tend to be flakey, and particular will time out on loaded systems as you are 
seeing.  But I really don't think there are very many cases where we need to 
use pexpect.

For instance, all the completion tests using the 
SBCommandInterpreter::HandleCompletion.  This is a pretty direct translation to 
the code that the CommandInterpreter uses to handle tabs, so you wouldn't 
change what you are testing, and the tests would be much more stable because 
you're just calling an API rather than scraping stdout from a sub-launched lldb.

We do need to add more command-line parsing tests as well, but it would be 
straightforward to write a little unit test harness that feeds in command lines 
and checks the parsed version of the command (before it gets passed to 
DoExecute) without actually executing the command.

I would rather not just xfail all these pexpect tests, I worry they will just 
stay that way.  Be better if we took the time to write them using more stable 
testing methods.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

If we just need to test completion, write a lit-style test that uses lldb-test 
that looks like this:

  RUN: lldb-test complete --target=%T/foo --complete_str=MyPrefix | FileCheck %s
  
  CHECK: Foo::Bar
  CHECK: Foo::Baz
  etc

Simple and not flaky


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

There are SB API's that call into the completion mechanism, so you could also 
just change the TestCompletion.complete_from_to test method to call the SB 
completion call.  Given how the test is written, it looks like all you would 
have to do is reimplement that method, and all the tests should be on a good 
footing.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

Spinning up a process just to test that auto-completion works though seems a 
little bit unnecessary, and introduces the possibility that flakiness and bugs 
in the process spawn mechanism (if any exist) will manifest in the test 
failure.  It would be nice, if and when this test fails, to have some 
confidence that the failure is related to auto completing symbol names.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [lldb] r323707 - [test-suite] UNXfail several tests that now pass locally.

2018-01-29 Thread Davide Italiano via lldb-commits
Author: davide
Date: Mon Jan 29 15:24:50 2018
New Revision: 323707

URL: http://llvm.org/viewvc/llvm-project?rev=323707&view=rev
Log:
[test-suite] UNXfail several tests that now pass locally.

Another step towards enabling unexpected successes as failures
by default.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=323707&r1=323706&r2=323707&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
 Mon Jan 29 15:24:50 2018
@@ -298,9 +298,6 @@ class CommandLineCompletionTestCase(Test
 self.complete_from_to('target va', 'target variable ')
 
 @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
-@expectedFailureAll(
-oslist=lldbplatform.darwin_all,
-bugnumber="llvm.org/pr25485,")
 def test_symbol_name(self):
 self.build()
 self.complete_from_to('''file %s

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py?rev=323707&r1=323706&r2=323707&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
 Mon Jan 29 15:24:50 2018
@@ -261,7 +261,6 @@ class LldbGdbServerTestCase(gdbremote_te
 lldbgdbserverutils.parse_reg_info_response(reg_info_packet))
 
 @debugserver_test
-@expectedFailureDarwin("llvm.org/pr25486")
 @skipIfDarwinEmbedded #  lldb-server tests not 
updated to work on ios etc yet
 def test_qRegisterInfo_returns_one_valid_result_debugserver(self):
 self.init_debugserver_test()
@@ -294,7 +293,6 @@ class LldbGdbServerTestCase(gdbremote_te
 self.assert_valid_reg_info(reg_info)
 
 @debugserver_test
-@expectedFailureDarwin("llvm.org/pr25486")
 @skipIfDarwinEmbedded #  lldb-server tests not 
updated to work on ios etc yet
 def test_qRegisterInfo_returns_all_valid_results_debugserver(self):
 self.init_debugserver_test()


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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment.

In https://reviews.llvm.org/D42656#991284, @zturner wrote:

> If we just need to test completion, write a lit-style test that uses 
> lldb-test that looks like this:
>
>   RUN: lldb-test complete --target=%T/foo --complete_str=MyPrefix | FileCheck 
> %s
>  
>   CHECK: Foo::Bar
>   CHECK: Foo::Baz
>   etc
>
>
> Simple and not flaky


That sound a great idea. I'll work on something like that in case this turns 
out to be unreliable (or, independently, when I get a chance :)

In the meanwhile, I reenabled the tests on https://reviews.llvm.org/rL323707 to 
see whether they fail on the bot.
FWIW, I agree that's a sledgehammer spinning an instance to test autocompletion.

Best,

-

Davide


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

There would be no spinning an instance, it would be call the API in python. No 
extra process, done in process.


https://reviews.llvm.org/D42656



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


[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

2018-01-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In https://reviews.llvm.org/D42145#979629, @owenpshaw wrote:

> I'm not envisioning that anything else needs to change to use begin/end or 
> care it's there.  I guess the way I look at it, having 
> ObjectFile::LoadInMemory do begin/end is basically the same as what you're 
> saying about having it be more process aware.
>
> If Process is going to introduce new concepts for ObjectFile to use either 
> way, isn't a high level of abstraction (batched writes) preferable to 
> ObjectFile needing to know the fine details of flash memory blocks or that 
> flash is even used?  And doesn't keeping the heavy lifting in ProcessGDB make 
> it reusable should another case come along?
>
> Hope you don't mind the pushback.  I think we're looking at this from 
> different angles, and I'm genuinely asking to help my understanding of your 
> thinking so hopefully we can converge on the same view.


I really would like users to not have to know how things must happen and that 
they must use a batch start and batch end. I strongly believe we should have 
Process::WriteMemory just do the right thing. We can make ObjectFile::Load() be 
smart and make sure to batch all consecutive writes to avoid unnecessary 
erases. I just really want memory write to just work. No questions.


https://reviews.llvm.org/D42145



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


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Yes.  We do need to have symbols to do symbol completion, which does require a 
binary, but you don't need to run it.  Most of the other tests in there (e.g. 
simple command completion) should be able to work without even a binary.  It 
seems weird to add a potentially fallible lldb-test layer on top of 
SBCommandInterpreter.HandleCompletion just so you can send text input through 
lldb-test rather than directly sending text input to 
SBCommandInterpreter.HandleCommand.


https://reviews.llvm.org/D42656



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


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Zachary Turner via lldb-commits
dotest is also a potentially fallible layer on top of the SB Api call, but
one that involves *more* behind-the-scenes code between the test and code
being tested.

An lldb-test test would consist, in its entirety, of about 10 lines of
text. I don’t see how it’s possible to beat that from a simplicity
standpoint (and you get the added bonus that the thing running the test is
known to not be flaky)
On Mon, Jan 29, 2018 at 4:33 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:

> jingham added a comment.
>
> Yes.  We do need to have symbols to do symbol completion, which does
> require a binary, but you don't need to run it.  Most of the other tests in
> there (e.g. simple command completion) should be able to work without even
> a binary.  It seems weird to add a potentially fallible lldb-test layer on
> top of SBCommandInterpreter.HandleCompletion just so you can send text
> input through lldb-test rather than directly sending text input to
> SBCommandInterpreter.HandleCommand.
>
>
> https://reviews.llvm.org/D42656
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

lldb testcases are know not to be flakey if they don't use pexpect, which these 
wouldn't.  The setup machinery for running a dotest based test is pretty well 
tested at this point.

And the lldb-test test would not just magically come into being by writing the 
lit-form text you suggested.  You will have to write a lldb-test func that 
marshals the input (both --complete-string and you'll also need a cursor 
position to test completion inside a line).  That's new and not tested code.  
Whereas the dotest test relies on the API it is directly testing, and trust 
that the basic machinery of dotest is going to continue to function.


https://reviews.llvm.org/D42656



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


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Zachary Turner via lldb-commits
We’ve had many instances of flakiness in non pexpect tests (on all
platforms). There’s no obvious pattern to when a test will be flaky.
Whether those are due to dotest or liblldb is an open question, but one
good way of answering those types of questions is to replace one source of
unknown-flakiness with a source of known-not-flakiness and seeing if the
flakiness goes away.

The new-and-not-tested code you’re referring to would be about 5 lines of
c++ that also directly calls the api, just like your dotest example. So
that aspect doesn’t feel like a convincing argument
On Mon, Jan 29, 2018 at 5:28 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:

> jingham added a comment.
>
> lldb testcases are know not to be flakey if they don't use pexpect, which
> these wouldn't.  The setup machinery for running a dotest based test is
> pretty well tested at this point.
>
> And the lldb-test test would not just magically come into being by writing
> the lit-form text you suggested.  You will have to write a lldb-test func
> that marshals the input (both --complete-string and you'll also need a
> cursor position to test completion inside a line).  That's new and not
> tested code.  Whereas the dotest test relies on the API it is directly
> testing, and trust that the basic machinery of dotest is going to continue
> to function.
>
>
> https://reviews.llvm.org/D42656
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Zachary Turner via lldb-commits
Also, I can think of at least 3 different companies/people who are
investing in LLDB for their downstream needs (who haven't publicly
announced this, so this isn't widely known), which involves bringing LLDB
up on currently unsupported platforms.  It's easy to lose sight of what
that entails when you've had a supported platform for 10+ years, but
suffice it to say that the less a test does, the better.  For these people,
when a test fails, you want as close to an absolute guarantee as possible
that the failure points immediately to the underlying cause as possible.
This drastically reduces the amount of work people have to do.

We can have another bi-monthly centithread about this if you want, but at
the end of the day if we want the test situation to improve, this is the
way to go and I believe there's pretty wide concensus in the larger LLVM
community about this.

On Mon, Jan 29, 2018 at 5:51 PM Zachary Turner  wrote:

> We’ve had many instances of flakiness in non pexpect tests (on all
> platforms). There’s no obvious pattern to when a test will be flaky.
> Whether those are due to dotest or liblldb is an open question, but one
> good way of answering those types of questions is to replace one source of
> unknown-flakiness with a source of known-not-flakiness and seeing if the
> flakiness goes away.
>
> The new-and-not-tested code you’re referring to would be about 5 lines of
> c++ that also directly calls the api, just like your dotest example. So
> that aspect doesn’t feel like a convincing argument
> On Mon, Jan 29, 2018 at 5:28 PM Jim Ingham via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> jingham added a comment.
>>
>> lldb testcases are know not to be flakey if they don't use pexpect, which
>> these wouldn't.  The setup machinery for running a dotest based test is
>> pretty well tested at this point.
>>
>> And the lldb-test test would not just magically come into being by
>> writing the lit-form text you suggested.  You will have to write a
>> lldb-test func that marshals the input (both --complete-string and you'll
>> also need a cursor position to test completion inside a line).  That's new
>> and not tested code.  Whereas the dotest test relies on the API it is
>> directly testing, and trust that the basic machinery of dotest is going to
>> continue to function.
>>
>>
>> https://reviews.llvm.org/D42656
>>
>>
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via lldb-commits
So far, all the "flakey" tests I've analyzed - of which there have been not a 
few over the years - have either been:

1) Why was pexpect so hard to get right when expect has been rock solid for 
decades...
2) Tests whose setup conditions are hard to get right, or hard to have happen 
on whatever machine we have to run them on in a timely fashion
3) Badly written tests - very often these are command-scraping tests that test 
for incidental accidents of the output
4) Actual flakiness in liblldb.

Of these, #2 is the most common, followed (sadly for our pride but to the 
credit of the testsuite) were real non-deterministic bugs in lldb.

So I'm not much convinced that the supposed flakiness of dotest provides a 
substantial reason to go write a whole parallel test harness.

Jim


> On Jan 29, 2018, at 5:51 PM, Zachary Turner  wrote:
> 
> We’ve had many instances of flakiness in non pexpect tests (on all 
> platforms). There’s no obvious pattern to when a test will be flaky. Whether 
> those are due to dotest or liblldb is an open question, but one good way of 
> answering those types of questions is to replace one source of 
> unknown-flakiness with a source of known-not-flakiness and seeing if the 
> flakiness goes away.
> 
> The new-and-not-tested code you’re referring to would be about 5 lines of c++ 
> that also directly calls the api, just like your dotest example. So that 
> aspect doesn’t feel like a convincing argument 
> On Mon, Jan 29, 2018 at 5:28 PM Jim Ingham via Phabricator 
>  wrote:
> jingham added a comment.
> 
> lldb testcases are know not to be flakey if they don't use pexpect, which 
> these wouldn't.  The setup machinery for running a dotest based test is 
> pretty well tested at this point.
> 
> And the lldb-test test would not just magically come into being by writing 
> the lit-form text you suggested.  You will have to write a lldb-test func 
> that marshals the input (both --complete-string and you'll also need a cursor 
> position to test completion inside a line).  That's new and not tested code.  
> Whereas the dotest test relies on the API it is directly testing, and trust 
> that the basic machinery of dotest is going to continue to function.
> 
> 
> https://reviews.llvm.org/D42656
> 
> 
> 

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


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via lldb-commits
I guess I don't see how having a test dive into lldb-test and do a bunch of 
work opaque work that I can't really annotate makes for an easier debugging 
scenario than a test were I can trivially insert code to query the state of the 
test as it goes along.  In the current testsuite, the progress of the test 
moves pretty clearly through the stages of getting the situation up to the 
point that you can test the thing you want to test.  That makes it, to my mind, 
very easy to understand and debug, and you end up dealing with the core pieces 
of lldb functionality, not whatever odd bits of business the lldb command line 
or the lldb-test utility do.  Moreover, you can pretty much type the test 
script of a dotest test directly into the script interpreter, so you can also 
interactively recreate the test patterns.  I find this makes it very convenient 
to debug using these tests.  You of course have to learn the SB API to some 
extent, but I can't see why that's a gating factor if you're going to take the 
time to do work like port lldb to a new platform.

The wider LLVM community tests a very different kind of tool than lldb, which 
leaves me less moved by the argumentum ad verecundiam than I might otherwise be.

We can have another centithread and discuss this again.  I'm not so sure we'll 
convince one another, however.

Jim


> On Jan 29, 2018, at 5:56 PM, Zachary Turner  wrote:
> 
> Also, I can think of at least 3 different companies/people who are investing 
> in LLDB for their downstream needs (who haven't publicly announced this, so 
> this isn't widely known), which involves bringing LLDB up on currently 
> unsupported platforms.  It's easy to lose sight of what that entails when 
> you've had a supported platform for 10+ years, but suffice it to say that the 
> less a test does, the better.  For these people, when a test fails, you want 
> as close to an absolute guarantee as possible that the failure points 
> immediately to the underlying cause as possible.  This drastically reduces 
> the amount of work people have to do.  
> 
> We can have another bi-monthly centithread about this if you want, but at the 
> end of the day if we want the test situation to improve, this is the way to 
> go and I believe there's pretty wide concensus in the larger LLVM community 
> about this.
> 
> On Mon, Jan 29, 2018 at 5:51 PM Zachary Turner  wrote:
> We’ve had many instances of flakiness in non pexpect tests (on all 
> platforms). There’s no obvious pattern to when a test will be flaky. Whether 
> those are due to dotest or liblldb is an open question, but one good way of 
> answering those types of questions is to replace one source of 
> unknown-flakiness with a source of known-not-flakiness and seeing if the 
> flakiness goes away.
> 
> The new-and-not-tested code you’re referring to would be about 5 lines of c++ 
> that also directly calls the api, just like your dotest example. So that 
> aspect doesn’t feel like a convincing argument 
> On Mon, Jan 29, 2018 at 5:28 PM Jim Ingham via Phabricator 
>  wrote:
> jingham added a comment.
> 
> lldb testcases are know not to be flakey if they don't use pexpect, which 
> these wouldn't.  The setup machinery for running a dotest based test is 
> pretty well tested at this point.
> 
> And the lldb-test test would not just magically come into being by writing 
> the lit-form text you suggested.  You will have to write a lldb-test func 
> that marshals the input (both --complete-string and you'll also need a cursor 
> position to test completion inside a line).  That's new and not tested code.  
> Whereas the dotest test relies on the API it is directly testing, and trust 
> that the basic machinery of dotest is going to continue to function.
> 
> 
> https://reviews.llvm.org/D42656
> 
> 
> 

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


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Zachary Turner via lldb-commits
On Mon, Jan 29, 2018 at 6:11 PM Jim Ingham  wrote:

>
> The wider LLVM community tests a very different kind of tool than lldb,
> which leaves me less moved by the argumentum ad verecundiam than I might
> otherwise be.
>

But it's not a different kind of test than this particular test, which is
why I suggested it.  In fact, it's exactly the same kind of test as this.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Jim Ingham via lldb-commits
That doesn't seem to me a strong enough argument to me to justify devising a 
parallel mechanism to the one we have to use for our more complex tests when it 
also happens to serve this purpose perfectly well.  Every time we make folks 
learn to diagnose a different mode of failure we are putting a burden on 
developers.  I don't see that this one has proved to be worth that burden yet.  
It seems arbitrary and inexpressive to me.

But I'm certainly willing to be surprised as you go along.

Jim


> On Jan 29, 2018, at 6:12 PM, Zachary Turner  wrote:
> 
> 
> 
> On Mon, Jan 29, 2018 at 6:11 PM Jim Ingham  wrote:
> 
> The wider LLVM community tests a very different kind of tool than lldb, which 
> leaves me less moved by the argumentum ad verecundiam than I might otherwise 
> be.
> 
> But it's not a different kind of test than this particular test, which is why 
> I suggested it.  In fact, it's exactly the same kind of test as this. 

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


[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

2018-01-29 Thread Owen Shaw via Phabricator via lldb-commits
owenpshaw added a comment.

Thanks.  What I'm struggling to reconcile are your statements that users should 
not have to know how things must happen, but then that we should make 
ObjectFile::Load smart so it doesn't result in an unnecessary amount of 
reads/writes.  If writing to flash memory effectively requires some smarts on 
the caller's part, then how have we made things easier for the callers?  And at 
that point isn't start/end a lot simpler than requiring callers to worry about 
block sizes themselves?


https://reviews.llvm.org/D42145



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


[Lldb-commits] [lldb] r323723 - dotest: Apply --skip-categories to debug info categories

2018-01-29 Thread Vedant Kumar via lldb-commits
Author: vedantk
Date: Mon Jan 29 19:36:00 2018
New Revision: 323723

URL: http://llvm.org/viewvc/llvm-project?rev=323723&view=rev
Log:
dotest: Apply --skip-categories to debug info categories

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=323723&r1=323722&r2=323723&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Mon Jan 29 19:36:00 
2018
@@ -1733,7 +1733,7 @@ class LLDBTestCaseFactory(type):
 # authoritative.  If none were specified, try with all debug
 # info formats.
 all_dbginfo_categories = set(
-test_categories.debug_info_categories)
+test_categories.debug_info_categories) - 
set(configuration.skipCategories)
 categories = set(
 getattr(
 attrvalue,


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


[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-29 Thread Alexander Shaposhnikov via Phabricator via lldb-commits
alexshap updated this revision to Diff 131920.
alexshap added a comment.

Update, rerun the tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D42563

Files:
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
  
packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -392,9 +392,10 @@
 }
 
 SymbolFileDWARF::SymbolFileDWARF(ObjectFile *objfile)
-: SymbolFile(objfile), UserID(0), // Used by SymbolFileDWARFDebugMap to when
-  // this class parses .o files to contain
-  // the .o file index/ID
+: SymbolFile(objfile),
+  UserID(uint64_t(DW_INVALID_OFFSET) << 32), // Used by SymbolFileDWARFDebugMap to when
+ // this class parses .o files to contain
+ // the .o file index/ID
   m_debug_map_module_wp(), m_debug_map_symfile(NULL), m_data_debug_abbrev(),
   m_data_debug_aranges(), m_data_debug_frame(), m_data_debug_info(),
   m_data_debug_line(), m_data_debug_macro(), m_data_debug_loc(),
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
@@ -0,0 +1,11 @@
+extern int f();
+
+void g() {
+  int y = 14;
+  int x = f();
+}
+
+int main() {
+  g();
+  return 0;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
@@ -0,0 +1,3 @@
+int f() {
+  return 1;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
@@ -0,0 +1,46 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestMixedDwarfBinary(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+@add_test_categories(["dwo"])
+@skipUnlessPlatform(['linux'])
+def test_target_symbols_add(self):
+"""Test that 'frame variable' works
+for the executable built from two source files compiled
+with/whithout -gsplit-dwarf correspondingly."""
+
+self.build()
+exe = os.path.join(os.getcwd(), "main")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("g", "main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+frame = self.process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+x = frame.FindVariable("x")
+self.assertTrue(x.IsValid(), "x is not valid")
+y = frame.FindVariable("y")
+self.assertTrue(y.IsValid(), "y is not valid")
+
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
@@ -0,0 +1,17 @@
+.PHONY : all
+.PHONY : clean
+all: main
+
+CCFLAGS=-O0 -g
+
+a.o : a.c
+	"$(CC)" $(CCFLAGS) -gsplit-dwarf -c "$<" -o "$@"
+
+b.o : b.c
+	"$(CC)" $(CCFLAGS) -c "$<" -o "$@"
+
+main : a.o b.o
+	"$(CC)" $^ -o "$@"
+
+clean:
+	rm -f a.dwo a.o b.o main
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-29 Thread Alexander Shaposhnikov via Phabricator via lldb-commits
alexshap updated this revision to Diff 131921.
alexshap added a comment.

fix typo


Repository:
  rL LLVM

https://reviews.llvm.org/D42563

Files:
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
  
packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -392,9 +392,10 @@
 }
 
 SymbolFileDWARF::SymbolFileDWARF(ObjectFile *objfile)
-: SymbolFile(objfile), UserID(0), // Used by SymbolFileDWARFDebugMap to when
-  // this class parses .o files to contain
-  // the .o file index/ID
+: SymbolFile(objfile),
+  UserID(uint64_t(DW_INVALID_OFFSET) << 32), // Used by SymbolFileDWARFDebugMap to when
+ // this class parses .o files to contain
+ // the .o file index/ID
   m_debug_map_module_wp(), m_debug_map_symfile(NULL), m_data_debug_abbrev(),
   m_data_debug_aranges(), m_data_debug_frame(), m_data_debug_info(),
   m_data_debug_line(), m_data_debug_macro(), m_data_debug_loc(),
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
@@ -0,0 +1,11 @@
+extern int f();
+
+void g() {
+  int y = 14;
+  int x = f();
+}
+
+int main() {
+  g();
+  return 0;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
@@ -0,0 +1,3 @@
+int f() {
+  return 1;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
@@ -0,0 +1,46 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestMixedDwarfBinary(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+@add_test_categories(["dwo"])
+@skipUnlessPlatform(['linux'])
+def test_mixed_dwarf(self):
+"""Test that 'frame variable' works
+for the executable built from two source files compiled
+with/whithout -gsplit-dwarf correspondingly."""
+
+self.build()
+exe = os.path.join(os.getcwd(), "main")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("g", "main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+frame = self.process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+x = frame.FindVariable("x")
+self.assertTrue(x.IsValid(), "x is not valid")
+y = frame.FindVariable("y")
+self.assertTrue(y.IsValid(), "y is not valid")
+
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
@@ -0,0 +1,17 @@
+.PHONY : all
+.PHONY : clean
+all: main
+
+CCFLAGS=-O0 -g
+
+a.o : a.c
+	"$(CC)" $(CCFLAGS) -gsplit-dwarf -c "$<" -o "$@"
+
+b.o : b.c
+	"$(CC)" $(CCFLAGS) -c "$<" -o "$@"
+
+main : a.o b.o
+	"$(CC)" $^ -o "$@"
+
+clean:
+	rm -f a.dwo a.o b.o main
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-29 Thread Alexander Shaposhnikov via Phabricator via lldb-commits
alexshap updated this revision to Diff 131922.
alexshap added a comment.

one more update


Repository:
  rL LLVM

https://reviews.llvm.org/D42563

Files:
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
  
packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -392,9 +392,10 @@
 }
 
 SymbolFileDWARF::SymbolFileDWARF(ObjectFile *objfile)
-: SymbolFile(objfile), UserID(0), // Used by SymbolFileDWARFDebugMap to when
-  // this class parses .o files to contain
-  // the .o file index/ID
+: SymbolFile(objfile),
+  UserID(uint64_t(DW_INVALID_OFFSET) << 32), // Used by SymbolFileDWARFDebugMap to when
+ // this class parses .o files to contain
+ // the .o file index/ID
   m_debug_map_module_wp(), m_debug_map_symfile(NULL), m_data_debug_abbrev(),
   m_data_debug_aranges(), m_data_debug_frame(), m_data_debug_info(),
   m_data_debug_line(), m_data_debug_macro(), m_data_debug_loc(),
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
@@ -0,0 +1,11 @@
+extern int f();
+
+void g() {
+  int y = 14;
+  int x = f();
+}
+
+int main() {
+  g();
+  return 0;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
@@ -0,0 +1,3 @@
+int f() {
+  return 1;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
@@ -0,0 +1,46 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestMixedDwarfBinary(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+@add_test_categories(["dwo"])
+@skipUnlessPlatform(['linux'])
+def test_mixed_dwarf(self):
+"""Test that 'frame variable' works
+for the executable built from two source files compiled
+with/whithout -gsplit-dwarf correspondingly."""
+
+self.build()
+exe = os.path.join(os.getcwd(), "main")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("g", "main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+frame = self.process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+x = frame.FindVariable("x")
+self.assertTrue(x.IsValid(), "x is not valid")
+y = frame.FindVariable("y")
+self.assertTrue(y.IsValid(), "y is not valid")
+
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
@@ -0,0 +1,17 @@
+.PHONY : all
+.PHONY : clean
+all: main
+
+CCFLAGS=-O0 -g
+
+a.o : a.c
+	"$(CC)" $(CCFLAGS) -gsplit-dwarf -c "$<" -o "$@"
+
+b.o : b.c
+	"$(CC)" $(CCFLAGS) -c "$<" -o "$@"
+
+main : a.o b.o
+	"$(CC)" $^ -o "$@"
+
+clean:
+	rm -f a.dwo a.o b.o main
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-29 Thread Alexander Shaposhnikov via Phabricator via lldb-commits
alexshap updated this revision to Diff 131924.
alexshap added a comment.

fix comment


https://reviews.llvm.org/D42563

Files:
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
  
packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
  packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -392,9 +392,10 @@
 }
 
 SymbolFileDWARF::SymbolFileDWARF(ObjectFile *objfile)
-: SymbolFile(objfile), UserID(0), // Used by SymbolFileDWARFDebugMap to when
-  // this class parses .o files to contain
-  // the .o file index/ID
+: SymbolFile(objfile),
+  UserID(uint64_t(DW_INVALID_OFFSET) << 32), // Used by SymbolFileDWARFDebugMap to when
+ // this class parses .o files to contain
+ // the .o file index/ID
   m_debug_map_module_wp(), m_debug_map_symfile(NULL), m_data_debug_abbrev(),
   m_data_debug_aranges(), m_data_debug_frame(), m_data_debug_info(),
   m_data_debug_line(), m_data_debug_macro(), m_data_debug_loc(),
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
@@ -0,0 +1,11 @@
+extern int f();
+
+void g() {
+  int y = 14;
+  int x = f();
+}
+
+int main() {
+  g();
+  return 0;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
@@ -0,0 +1,3 @@
+int f() {
+  return 1;
+}
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
@@ -0,0 +1,46 @@
+""" Testing debugging of a binary with "mixed" dwarf (with/without fission). """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestMixedDwarfBinary(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+@add_test_categories(["dwo"])
+@skipUnlessPlatform(['linux'])
+def test_mixed_dwarf(self):
+"""Test that 'frame variable' works
+for the executable built from two source files compiled
+with/whithout -gsplit-dwarf correspondingly."""
+
+self.build()
+exe = os.path.join(os.getcwd(), "main")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("g", "main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+frame = self.process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+x = frame.FindVariable("x")
+self.assertTrue(x.IsValid(), "x is not valid")
+y = frame.FindVariable("y")
+self.assertTrue(y.IsValid(), "y is not valid")
+
Index: packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
@@ -0,0 +1,17 @@
+.PHONY : all
+.PHONY : clean
+all: main
+
+CCFLAGS=-O0 -g
+
+a.o : a.c
+	"$(CC)" $(CCFLAGS) -gsplit-dwarf -c "$<" -o "$@"
+
+b.o : b.c
+	"$(CC)" $(CCFLAGS) -c "$<" -o "$@"
+
+main : a.o b.o
+	"$(CC)" $^ -o "$@"
+
+clean:
+	rm -f a.dwo a.o b.o main
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits