date:20210809

[Lldb-commits] [lldb] 8813bc0 - [LLDB] Skip random fails on Arm/AArch64 Linux buildbot

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


Author: Muhammad Omair Javaid
Date: 2021-08-09T13:53:48+05:00
New Revision: 8813bc02b40c90eeca0b30ec46aa898bd49e1522

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

LOG: [LLDB] Skip random fails on Arm/AArch64 Linux buildbot

Following tests fail on Arm/AArch64 randomly with timeouts:

TestMultilineNavigation.py
TestBatchMode.py
TestUnicode.py
TestGdbRemote_vContThreads.py

I am marking them as skipped until we find a away make to pass reliably.

Added: 


Modified: 

lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
lldb/test/API/driver/batch_mode/TestBatchMode.py
lldb/test/API/iohandler/unicode/TestUnicode.py
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
 
b/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
index 0cd0a5d447b56..369ae3fb57a4a 100644
--- 
a/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
+++ 
b/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
@@ -19,7 +19,7 @@ class TestCase(PExpectTest):
 @skipIfAsan
 @skipIfEditlineSupportMissing
 @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr48316')
-@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 def test_nav_arrow_up(self):
 """Tests that we can navigate back to the previous line with the up 
arrow"""
 self.launch()
@@ -43,6 +43,7 @@ def test_nav_arrow_up(self):
 @skipIfAsan
 @skipIfEditlineSupportMissing
 @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr48316')
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 def test_nav_arrow_down(self):
 """Tests that we can navigate to the next line with the down arrow"""
 self.launch()
@@ -74,6 +75,7 @@ def test_nav_arrow_down(self):
 @skipIfAsan
 @skipIfEditlineSupportMissing
 @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr48316')
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 def test_nav_arrow_up_empty_pr49845(self):
 """Tests that navigating with the up arrow doesn't crash."""
 self.launch()

diff  --git a/lldb/test/API/driver/batch_mode/TestBatchMode.py 
b/lldb/test/API/driver/batch_mode/TestBatchMode.py
index e5364a460f9ce..d9d75f71de21a 100644
--- a/lldb/test/API/driver/batch_mode/TestBatchMode.py
+++ b/lldb/test/API/driver/batch_mode/TestBatchMode.py
@@ -16,6 +16,7 @@ class DriverBatchModeTest(PExpectTest):
 mydir = TestBase.compute_mydir(__file__)
 source = 'main.c'
 
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
 def test_batch_mode_run_crash(self):
 """Test that the lldb driver's batch mode works correctly."""
@@ -46,6 +47,7 @@ def test_batch_mode_run_crash(self):
 self.expect_prompt()
 self.expect("frame variable touch_me_not", substrs=['(char *) 
touch_me_not'])
 
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
 def test_batch_mode_run_exit(self):
 """Test that the lldb driver's batch mode works correctly."""
@@ -75,6 +77,7 @@ def test_batch_mode_run_exit(self):
 import pexpect
 child.expect(pexpect.EOF)
 
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
 def test_batch_mode_launch_stop_at_entry(self):
 """Test that the lldb driver's batch mode works correctly for process 
launch."""
@@ -109,6 +112,7 @@ def closeVictim(self):
 self.victim.close()
 self.victim = None
 
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
 @expectedFailureNetBSD
 def test_batch_mode_attach_exit(self):

diff  --git a/lldb/test/API/iohandler/unicode/TestUnicode.py 
b/lldb/test/API/iohandler/unicode/TestUnicode.py
index c8ff9a6ab32dd..7d239341d59c9 100644
--- a/lldb/test/API/iohandler/unicode/TestUnicode.py
+++ b/lldb/test/API/iohandler/unicode/TestUnicode.py
@@ -17,6 +17,7 @@ class TestCase(PExpectTest):
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
 @skipIfAsan
+@skipIf(osli

[Lldb-commits] [lldb] 14735ca - [lldb] [gdb-remote] Add eOpenOptionReadWrite for future gdb compat

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T12:06:59+02:00
New Revision: 14735cab655441ba45c4b88ad82f11267e5fe916

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

LOG: [lldb] [gdb-remote] Add eOpenOptionReadWrite for future gdb compat

Modify OpenOptions enum to open the future path into synchronizing
vFile:open bits with GDB.  Currently, LLDB and GDB use different flag
models effectively making it impossible to match bits.  Notably, LLDB
uses two bits to indicate read and write status, and uses union of both
for read/write.  GDB uses a value of 0 for read-only, 1 for write-only
and 2 for read/write.

In order to future-proof the code for the GDB variant:

1. Add a distinct eOpenOptionReadWrite constant to be used instead
   of (eOpenOptionRead | eOpenOptionWrite) when R/W access is required.

2. Rename eOpenOptionRead and eOpenOptionWrite to eOpenOptionReadOnly
   and eOpenOptionWriteOnly respectively, to make it clear that they
   do not mean to be combined and require update to all call sites.

3. Use the intersection of all three flags when matching against
   the three possible values.

This commit does not change the actual bits used by LLDB.

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

Added: 


Modified: 
lldb/docs/lldb-platform-packets.txt
lldb/include/lldb/Host/File.h
lldb/source/API/SBStream.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/StreamFile.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Host/common/File.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Host/windows/Host.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Target/ModuleCache.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/unittests/Host/FileSystemTest.cpp
lldb/unittests/Host/FileTest.cpp
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Removed: 




diff  --git a/lldb/docs/lldb-platform-packets.txt 
b/lldb/docs/lldb-platform-packets.txt
index 9a1444afef05c..5d0a399b800f8 100644
--- a/lldb/docs/lldb-platform-packets.txt
+++ b/lldb/docs/lldb-platform-packets.txt
@@ -375,7 +375,7 @@ incompatible with the flags that gdb specifies.
 //  COMPATIBILITY
 //The gdb-remote serial protocol documentatio defines a vFile:open:
 //packet which uses incompatible flag values, e.g. 1 means O_WRONLY
-//in gdb's vFile:open:, but it means eOpenOptionRead to lldb's
+//in gdb's vFile:open:, but it means eOpenOptionReadOnly to lldb's
 //implementation.
 
 //--

diff  --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index d364d954a1c16..b928222b3fb6b 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -44,8 +44,11 @@ class File : public IOObject {
   // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
   // * rdar://problem/46788934
   enum OpenOptions : uint32_t {
-eOpenOptionRead = (1u << 0),  // Open file for reading
-eOpenOptionWrite = (1u << 1), // Open file for writing
+eOpenOptionReadOnly = (1u << 0),  // Open file for reading (only)
+eOpenOptionWriteOnly = (1u << 1), // Open file for writing (only)
+eOpenOptionReadWrite =
+eOpenOptionReadOnly |
+eOpenOptionWriteOnly, // Open file for both reading and writing
 eOpenOptionAppend =
 (1u << 2), // Don't truncate file when opening, append to end of file
 eOpenOptionTruncate = (1u << 3),// Truncate file when opening
@@ -303,8 +306,8 @@ class File : public IOObject {
   /// Some options like eOpenOptionDontFollowSymlinks only make
   /// sense when a file is being opened (or not at all)
   /// and may not be preserved for this method.  But any valid
-  /// File should return either or both of eOpenOptionRead and
-  ///

[Lldb-commits] [lldb] 8bbef4f - [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T12:07:18+02:00
New Revision: 8bbef4f9afd8b5f6b4435d5bab48b75f048f353c

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

LOG: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

Sync the mode constants used to drive vFile:open requests with these
used by GDB and defined for the gdb remote protocol.  This makes it
possible to use 'platform file open' after connecting to gdbremote
server (and to some degree to operate on the open file modulo other
incompatibilities).

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

Added: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Modified: 
lldb/docs/lldb-platform-packets.txt
lldb/include/lldb/Host/File.h
lldb/source/Host/common/File.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 




diff  --git a/lldb/docs/lldb-platform-packets.txt 
b/lldb/docs/lldb-platform-packets.txt
index 5d0a399b800f8..0b72c657b86bb 100644
--- a/lldb/docs/lldb-platform-packets.txt
+++ b/lldb/docs/lldb-platform-packets.txt
@@ -372,12 +372,6 @@ incompatible with the flags that gdb specifies.
 //  response is F followed by the opened file descriptor in base 10.
 //  "F-1,errno" with the errno if an error occurs.
 //
-//  COMPATIBILITY
-//The gdb-remote serial protocol documentatio defines a vFile:open:
-//packet which uses incompatible flag values, e.g. 1 means O_WRONLY
-//in gdb's vFile:open:, but it means eOpenOptionReadOnly to lldb's
-//implementation.
-
 //--
 // vFile:close:
 //

diff  --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index b928222b3fb6b..b0bc7d9535f6c 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -39,27 +39,29 @@ class File : public IOObject {
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
   //
-  // FIXME
-  // These values do not match the values used by GDB
+  // The first set of values is defined by gdb headers and can be found
+  // in the documentation at:
   // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
+  //
+  // The second half are LLDB extensions and use the highest uint32_t bits
+  // to avoid risk of collisions with future gdb remote protocol changes.
   enum OpenOptions : uint32_t {
-eOpenOptionReadOnly = (1u << 0),  // Open file for reading (only)
-eOpenOptionWriteOnly = (1u << 1), // Open file for writing (only)
-eOpenOptionReadWrite =
-eOpenOptionReadOnly |
-eOpenOptionWriteOnly, // Open file for both reading and writing
+eOpenOptionReadOnly = 0x0,  // Open file for reading (only)
+eOpenOptionWriteOnly = 0x1, // Open file for writing (only)
+eOpenOptionReadWrite = 0x2, // Open file for both reading and writing
 eOpenOptionAppend =
-(1u << 2), // Don't truncate file when opening, append to end of file
-eOpenOptionTruncate = (1u << 3),// Truncate file when opening
-eOpenOptionNonBlocking = (1u << 4), // File reads
-eOpenOptionCanCreate = (1u << 5),   // Create file if doesn't already exist
+0x8, // Don't truncate file when opening, append to end of file
+eOpenOptionCanCreate = 0x200, // Create file if doesn't already exist
+eOpenOptionTruncate = 0x400,  // Truncate file when opening
 eOpenOptionCanCreateNewOnly =
-(1u << 6), // Can create file only if it doesn't already exist
-eOpenOptionDontFollowSymlinks = (1u << 7),
+0x800, // Can create file only if it doesn't already exist
+
+eOpenOptionNonBlocking = (1u << 28), // File reads
+eOpenOptionDontFollowSymlinks = (1u << 29),
 eOpenOptionCloseOnExec =
-(1u << 8), // Close the file when executing a new process
-LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionCloseOnExec)
+(1u << 30), // Close the file when executing a new process
+eOpenOptionInvalid = (1u << 31), // Used as invalid value
+LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionInvalid)
   };
 
   static mode_t ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options);

diff  --git a/lldb/source/Host/common/File.cpp 
b/lldb/source/Host/common/File.cpp
index b92b4c1cf928d..87ed405c4428a 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -90,8 +90,8 @@ Expected 
File::GetOptionsFromMode(llvm::StringRef mode) {
   .Cases("a+", "ab+", "a+b",
  eOpenOptionReadWrite | eOpenOptionAppend |
  eOpenOptionCanCre

[Lldb-commits] [lldb] 9929cfb - [lldb] [gdb-remote] Use hexadecimal numbers in vFile packats for GDB compliance

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T12:07:33+02:00
New Revision: 9929cfbcd5bf5402fbb180e5bb3f917f50bbdf94

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

LOG: [lldb] [gdb-remote] Use hexadecimal numbers in vFile packats for GDB 
compliance

Use hexadecimal numbers rather than decimal in various vFile packets
in order to fix compatibility with gdbserver.  This also changes the few
custom LLDB packets -- while technically they do not have to be changed,
it is easier to use the same syntax consistently across LLDB.

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

Added: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Modified: 
lldb/docs/lldb-platform-packets.txt
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git a/lldb/docs/lldb-platform-packets.txt 
b/lldb/docs/lldb-platform-packets.txt
index 0b72c657b86bb..691a4412ea630 100644
--- a/lldb/docs/lldb-platform-packets.txt
+++ b/lldb/docs/lldb-platform-packets.txt
@@ -369,7 +369,7 @@ incompatible with the flags that gdb specifies.
 //are the constant values in enum OpenOptions from lldb's File.h
 // 3. mode bits, base 16
 //  
-//  response is F followed by the opened file descriptor in base 10.
+//  response is F followed by the opened file descriptor in base 16.
 //  "F-1,errno" with the errno if an error occurs.
 //
 //--
@@ -383,7 +383,7 @@ incompatible with the flags that gdb specifies.
 //  receive: vFile:close:7
 //  send:F0
 //
-//  File descriptor is in base 10.
+//  File descriptor is in base 16.
 //  "F-1,errno" with the errno if an error occurs.
 
 
@@ -399,17 +399,12 @@ incompatible with the flags that gdb specifies.
 //  send:F4;a'b\00
 //
 //  request packet has the fields:
-// 1. file descriptor, base 10
-// 2. number of bytes to be read, base 10
-// 3. offset into file to start from, base 10
+// 1. file descriptor, base 16
+// 2. number of bytes to be read, base 16
+// 3. offset into file to start from, base 16
 //
-//  Response is F, followed by the number of bytes read (base 10), a
+//  Response is F, followed by the number of bytes read (base 16), a
 //  semicolon, followed by the data in the binary-escaped-data encoding.
-//
-//  COMPATIBILITY
-//The gdb-remote serial protocol documentation says that numbers
-//in "vFile:" packets should be hexadecimal. Instead lldb uses
-//decimal.
 
 
 //--
@@ -424,16 +419,11 @@ incompatible with the flags that gdb specifies.
 //  send:F1024
 //
 //  request packet has the fields:
-// 1. file descriptor, base 10
-// 2. offset into file to start from, base 10
+// 1. file descriptor, base 16
+// 2. offset into file to start from, base 16
 // 3. binary-escaped-data to be written
 //
-//  Response is F, followed by the number of bytes written (base 10)
-//
-//  COMPATIBILITY
-//The gdb-remote serial protocol documentation says that numbers
-//in "vFile:" packets should be hexadecimal. Instead lldb uses
-//decimal.
+//  Response is F, followed by the number of bytes written (base 16)
 
 
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index b16aed4f5c90f..ee8c7c66573d4 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2959,7 +2959,7 @@ Status GDBRemoteCommunicationClient::MakeDirectory(const 
FileSpec &file_spec,
   if (response.GetChar() != 'F')
 return Status("invalid response to '%s' packet", packet.str().c_str());
 
-  return Status(response.GetU32(UINT32_MAX), eErrorTypePOSIX);
+  return Status(response.GetHexMaxU32(false, UINT32_MAX), eErrorTypePOSIX);
 }
 
 Status
@@ -2980,7 +2980,7 @@ GDBRemoteCommunicationClient::SetFilePermissions(const 
FileSpec &file_spec,
   if (response.GetChar() != 'F')
 return Status("invalid response to '%s' packet", stream.GetData());
 
-  return Status(response.GetU32(UINT32_MAX), eErrorTypePOSIX);
+  return Status(response.GetHexMaxU32(false, UINT32_MAX), eErrorTypePOSIX);
 }
 
 static uint64_t ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
@@ -2988,11 +2988,11 @@ static uint64_t 
ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
   response.SetFilePos(0);
   if (response.GetChar() != 'F')
 return fail_result;
-  i

[Lldb-commits] [PATCH] D106985: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bbef4f9afd8: [lldb] [gdb-remote] Sync vFile:open mode 
constants with GDB (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106985

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -181,6 +181,8 @@
 return self.qfProcessInfo(packet)
 if packet.startswith("qPathComplete:"):
 return self.qPathComplete()
+if packet.startswith("vFile:"):
+return self.vFile(packet)
 
 return self.other(packet)
 
@@ -288,6 +290,9 @@
 def qPathComplete(self):
 return ""
 
+def vFile(self, packet):
+return ""
+
 """
 Raised when we receive a packet for which there is no default action.
 Override the responder class to implement behavior suitable for the test at
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -0,0 +1,25 @@
+from gdbclientutils import *
+
+class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+
+def test_file_open(self):
+"""Test mock-opening a remote file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F10"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.runCmd("platform file open /some/file.txt -v 0755")
+self.assertPacketLogContains([
+"vFile:open:2f736f6d652f66696c652e747874,020a,01ed"
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -501,10 +501,6 @@
   packet.GetHexByteStringTerminatedBy(path, ',');
   if (!path.empty()) {
 if (packet.GetChar() == ',') {
-  // FIXME
-  // The flag values for OpenOptions do not match the values used by GDB
-  // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
   auto flags = File::OpenOptions(packet.GetHexMaxU32(false, 0));
   if (packet.GetChar() == ',') {
 mode_t mode = packet.GetHexMaxU32(false, 0600);
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -90,8 +90,8 @@
   .Cases("a+", "ab+", "a+b",
  eOpenOptionReadWrite | eOpenOptionAppend |
  eOpenOptionCanCreate)
-  .Default(OpenOptions());
-  if (opts)
+  .Default(eOpenOptionInvalid);
+  if (opts != eOpenOptionInvalid)
 return opts;
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -39,27 +39,29 @@
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
   //
-  // FIXME
-  // These values do not match the values used by GDB
+  // The first set of values is defined by gdb headers and can be found
+  // in the documentation at:
   // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
+  //
+  // The second half are LLDB extensions and use the highest uint32_t bits
+  // to avoid risk of collisions with future gdb remote protocol changes.
   enum OpenOptions : uint32_t {
-eOpenOptionReadOnly = (1u << 0),  // Open file for read

[Lldb-commits] [PATCH] D106984: [lldb] [gdb-remote] Add eOpenOptionReadWrite for future gdb compat

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG14735cab6554: [lldb] [gdb-remote] Add eOpenOptionReadWrite 
for future gdb compat (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106984

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/include/lldb/Host/File.h
  lldb/source/API/SBStream.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/StreamFile.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/windows/Host.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Target/ModuleCache.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Host/FileSystemTest.cpp
  lldb/unittests/Host/FileTest.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -583,7 +583,7 @@
 
 TEST_F(PythonDataObjectsTest, TestPythonFile) {
   auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
-  File::eOpenOptionRead);
+  File::eOpenOptionReadOnly);
   ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
   auto py_file = PythonFile::FromFile(*file.get(), "r");
   ASSERT_THAT_EXPECTED(py_file, llvm::Succeeded());
@@ -858,4 +858,4 @@
   ASSERT_THAT_EXPECTED(r, llvm::Succeeded());
   auto g = As(globals.GetItem("g"));
   ASSERT_THAT_EXPECTED(g, llvm::HasValue("foobarbaz"));
-}
\ No newline at end of file
+}
Index: lldb/unittests/Host/FileTest.cpp
===
--- lldb/unittests/Host/FileTest.cpp
+++ lldb/unittests/Host/FileTest.cpp
@@ -46,7 +46,7 @@
   llvm::FileRemover remover(name);
   ASSERT_GE(fd, 0);
 
-  NativeFile file(fd, File::eOpenOptionWrite, true);
+  NativeFile file(fd, File::eOpenOptionWriteOnly, true);
   ASSERT_TRUE(file.IsValid());
 
   FILE *stream = file.GetStream();
Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -296,7 +296,7 @@
   FileSpec spec("/file/that/does/not/exist.txt");
 #endif
   FileSystem fs;
-  auto file = fs.Open(spec, File::eOpenOptionRead, 0, true);
+  auto file = fs.Open(spec, File::eOpenOptionReadOnly, 0, true);
   ASSERT_FALSE(file);
   std::error_code code = errorToErrorCode(file.takeError());
   EXPECT_EQ(code.category(), std::system_category());
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1022,7 +1022,7 @@
   }
 
   StreamFile out_file(path.c_str(),
-  File::eOpenOptionTruncate | File::eOpenOptionWrite |
+  File::eOpenOptionTruncate | File::eOpenOptionWriteOnly |
   File::eOpenOptionCanCreate |
   File::eOpenOptionCloseOnExec,
   lldb::eFilePermissionsFileDefault);
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4310,8 +4310,8 @@
   : IOHandler(process->GetTarget().GetDebugger(),
   IOHandler::Type::ProcessIO),
 m_process(process),
-m_read_file(GetInputFD(), File::eOpenOptionRead, false),
-m_write_file(write_fd, File::eOpenOptionWrite, false) {
+m_read_file(GetInputFD(), File::eOpenOptionReadOnly, false),
+m_write_file(write_fd, F

[Lldb-commits] [PATCH] D107475: [lldb] [gdb-remote] Use hexadecimal numbers in vFile packets for GDB compliance

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9929cfbcd5bf: [lldb] [gdb-remote] Use hexadecimal numbers in 
vFile packats for GDB compliance (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107475

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -0,0 +1,215 @@
+from __future__ import print_function
+
+# lldb test suite imports
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import TestBase
+
+# gdb-remote-specific imports
+import lldbgdbserverutils
+from gdbremote_testcase import GdbRemoteTestCaseBase
+
+import binascii
+import stat
+import tempfile
+
+
+class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_platform_file_rdonly(self):
+self.vFile_test(read=True)
+
+def test_platform_file_wronly(self):
+self.vFile_test(write=True)
+
+def test_platform_file_rdwr(self):
+self.vFile_test(read=True, write=True)
+
+def test_platform_file_wronly_append(self):
+self.vFile_test(write=True, append=True)
+
+def test_platform_file_rdwr_append(self):
+self.vFile_test(read=True, write=True, append=True)
+
+def test_platform_file_wronly_trunc(self):
+self.vFile_test(write=True, trunc=True)
+
+def test_platform_file_rdwr_trunc(self):
+self.vFile_test(read=True, write=True, trunc=True)
+
+def test_platform_file_wronly_creat(self):
+self.vFile_test(write=True, creat=True)
+
+def test_platform_file_wronly_creat_excl(self):
+self.vFile_test(write=True, creat=True, excl=True)
+
+def test_platform_file_wronly_fail(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+# create a temporary directory
+with tempfile.TemporaryDirectory() as temp_dir:
+temp_path = os.path.join(temp_dir, "test")
+self.assertFalse(os.path.exists(temp_path))
+
+# attempt to open the file without O_CREAT
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,1,0#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
+def test_platform_file_wronly_creat_excl_fail(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+# attempt to open the file with O_CREAT|O_EXCL
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,a01,0#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
+def expect_error(self):
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
+def vFile_test(self, read=False, write=False, append=False, trunc=False,
+   creat=False, excl=False):
+if read and write:
+mode = 2
+elif write:
+mode = 1
+else:  # read
+mode = 0
+if append:
+mode |= 8
+if creat:
+mode |= 0x200
+if trunc:
+mode |= 0x400
+if excl:
+mode |= 0x800
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+# create a temporary file with some data
+test_data = 'some test data longer than 16 bytes\n'
+if creat:
+temp_dir = tempfile.TemporaryDirectory()
+else:
+temp_file = tempfile.NamedTemporaryFile()
+
+try:
+if creat:
+temp_path = os.path.join(temp_dir.name, "test")
+self.assertFalse(os.path.exists(temp_path))
+else:
+

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

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

OmarEmaraDev updated this revision to Diff 365145.
OmarEmaraDev added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

- Address review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107386

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/VariableList.h"
@@ -3772,9 +3773,14 @@
TreeItem *&selected_item) {
 return;
   }
-  virtual bool TreeDelegateItemSelected(
-  TreeItem &item) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem &item) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -3964,6 +3970,10 @@
 
   void SetIdentifier(uint64_t identifier) { m_identifier = identifier; }
 
+  const std::string &GetText() const { return m_text; }
+
+  void SetText(const char *text) { m_text = text; }
+
   void SetMightHaveChildren(bool b) { m_might_have_children = b; }
 
 protected:
@@ -3971,6 +3981,7 @@
   TreeDelegate &m_delegate;
   void *m_user_data;
   uint64_t m_identifier;
+  std::string m_text;
   int m_row_idx; // Zero based visible row index, -1 if not visible or for the
  // root item
   std::vector m_children;
@@ -3989,21 +4000,6 @@
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window &window, bool force) override {
-ExecutionContext exe_ctx(
-m_debugger.GetCommandInterpreter().GetExecutionContext());
-Process *process = exe_ctx.GetProcessPtr();
-
-bool display_content = false;
-if (process) {
-  StateType state = process->GetState();
-  if (StateIsStoppedState(state, true)) {
-// We are stopped, so it is ok to
-display_content = true;
-  } else if (StateIsRunningState(state)) {
-return true; // Don't do any updating when we are running
-  }
-}
-
 m_min_x = 2;
 m_min_y = 1;
 m_max_x = window.GetWidth() - 1;
@@ -4012,35 +4008,36 @@
 window.Erase();
 window.DrawTitleBox(window.GetName());
 
-if (display_content) {
-  const int num_visible_rows = NumVisibleRows();
-  m_num_rows = 0;
-  m_root.CalculateRowIndexes(m_num_rows);
-  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
- m_selected_item);
-
-  // If we unexpanded while having something selected our total number of
-  // rows is less than the num visible rows, then make sure we show all the
-  // rows by setting the first visible row accordingly.
-  if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-m_first_visible_row = 0;
-
-  // Make sure the selected row is always visible
-  if (m_selected_row_idx < m_first_visible_row)
-m_first_visible_row = m_selected_row_idx;
-  else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
-
-  int row_idx = 0;
-  int num_rows_left = num_visible_rows;
-  m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-  num_rows_left);
-  // Get the selected row
-  m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-} else {
+if (!m_delegate_sp->TreeDelegateShouldDraw()) {
   m_selected_item = nullptr;
+  return true;
 }
 
+const int num_visible_rows = NumVisibleRows();
+m_num_rows = 0;
+m_root.CalculateRowIndexes(m_num_rows);
+m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+   m_selected_item);
+
+// If we unexpanded while having something selected our total number of
+// rows is less than the num visible rows, then make sure we show all the
+// rows by setting the first visible row accordingly.
+if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
+  m_first_visible_row = 0;
+
+// Make sure the selected row is always visible
+i

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

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

OmarEmaraDev added a comment.

@jingham I see. Thanks!

Here is how it looks now:

F18450643: 20210809-130123.png <https://reviews.llvm.org/F18450643>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107386

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

[Lldb-commits] [PATCH] D106270: [DWARF5] Fix offset check when using .debug_names

2021-08-09 Thread Kim-Anh Tran via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0dda5425318a: [DWARF5] Fix offset check when using 
.debug_names (authored by kimanh).

Changed prior to commit:
  https://reviews.llvm.org/D106270?vs=359763&id=365154#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106270

Files:
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp

Index: lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
@@ -27,6 +27,17 @@
 // RUN: lldb-test symbols --file=find-variable-file-2.cpp --find=variable %t | \
 // RUN:   FileCheck --check-prefix=TWO %s
 
+// Run the same test with split dwarf and pubnames to check whether we can find
+// the compile unit using the name index if it is split.
+// RUN: %clang -c -o %t-1.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %s
+// RUN: %clang -c -o %t-2.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %S/Inputs/find-variable-file-2.cpp
+// RUN: ld.lld %t-1.o %t-2.o -o %t
+// RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix NAMES
+// RUN: lldb-test symbols --file=find-variable-file.cpp --find=variable %t | \
+// RUN:   FileCheck --check-prefix=ONE %s
+// RUN: lldb-test symbols --file=find-variable-file-2.cpp --find=variable %t | \
+// RUN:   FileCheck --check-prefix=TWO %s
+
 // NAMES: Name: .debug_names
 
 // ONE: Found 1 variables:
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3069,16 +3069,14 @@
 variables = std::make_shared();
 sc.comp_unit->SetVariableList(variables);
 
-m_index->GetGlobalVariables(
-dwarf_cu->GetNonSkeletonUnit(), [&](DWARFDIE die) {
-  VariableSP var_sp(
-  ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS));
-  if (var_sp) {
-variables->AddVariableIfUnique(var_sp);
-++vars_added;
-  }
-  return true;
-});
+m_index->GetGlobalVariables(*dwarf_cu, [&](DWARFDIE die) {
+  VariableSP var_sp(ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS));
+  if (var_sp) {
+variables->AddVariableIfUnique(var_sp);
+++vars_added;
+  }
+  return true;
+});
   }
   return vars_added;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -33,7 +33,7 @@
   GetGlobalVariables(const RegularExpression ®ex,
  llvm::function_ref callback) override;
   void
-  GetGlobalVariables(const DWARFUnit &unit,
+  GetGlobalVariables(DWARFUnit &unit,
  llvm::function_ref callback) override;
   void GetObjCMethods(ConstString class_name,
   llvm::function_ref callback) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -358,9 +358,10 @@
 }
 
 void ManualDWARFIndex::GetGlobalVariables(
-const DWARFUnit &unit, llvm::function_ref callback) {
+DWARFUnit &unit, llvm::function_ref callback) {
   Index();
-  m_set.globals.FindAllEntriesForUnit(unit, DIERefCallback(callback));
+  m_set.globals.FindAllEntriesForUnit(unit.GetNonSkeletonUnit(),
+  DIERefCallback(callback));
 }
 
 void ManualDWARFIndex::GetObjCMethods(
Index: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -32,7 +32,7 @@
   GetGlobalVariables(const RegularExpression ®ex,
  llvm::function_r

[Lldb-commits] [PATCH] D106270: [DWARF5] Fix offset check when using .debug_names

2021-08-09 Thread Kim-Anh Tran via Phabricator via lldb-commits

kimanh added a comment.

Thanks for the review! Landing now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106270

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

[Lldb-commits] [lldb] 0dda542 - [DWARF5] Fix offset check when using .debug_names

2021-08-09 Thread Kim-Anh Tran via lldb-commits


Author: Kim-Anh Tran
Date: 2021-08-09T13:15:14+02:00
New Revision: 0dda5425318a5785e3e19cfe369a43b221b9642d

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

LOG: [DWARF5] Fix offset check when using .debug_names

When going through the CU entries in the name index,
make sure to compare the name entry's CU
offset against the skeleton CU's offset.

Previously there would be a mismatch, since the
wrong offset was compared, and thus no suitable
entry was found.

Reviewed By: jankratochvil

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 60b6b726f6c09..c786451a03df2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -75,12 +75,14 @@ void AppleDWARFIndex::GetGlobalVariables(
 }
 
 void AppleDWARFIndex::GetGlobalVariables(
-const DWARFUnit &cu, llvm::function_ref callback) {
+DWARFUnit &cu, llvm::function_ref callback) {
   if (!m_apple_names_up)
 return;
 
+  const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
   DWARFMappedHash::DIEInfoArray hash_data;
-  m_apple_names_up->AppendAllDIEsInRange(cu.GetOffset(), 
cu.GetNextUnitOffset(),
+  m_apple_names_up->AppendAllDIEsInRange(non_skeleton_cu.GetOffset(),
+ non_skeleton_cu.GetNextUnitOffset(),
  hash_data);
   DWARFMappedHash::ExtractDIEArray(hash_data, DIERefCallback(callback));
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
index a7032f50e590a..ef3cb5dee0356 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
@@ -39,7 +39,7 @@ class AppleDWARFIndex : public DWARFIndex {
   GetGlobalVariables(const RegularExpression ®ex,
  llvm::function_ref callback) override;
   void
-  GetGlobalVariables(const DWARFUnit &cu,
+  GetGlobalVariables(DWARFUnit &cu,
  llvm::function_ref callback) override;
   void GetObjCMethods(ConstString class_name,
   llvm::function_ref callback) 
override;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index ecf82a910b661..6f2698cc6e6fb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -35,7 +35,7 @@ class DWARFIndex {
   GetGlobalVariables(const RegularExpression ®ex,
  llvm::function_ref callback) = 0;
   virtual void
-  GetGlobalVariables(const DWARFUnit &cu,
+  GetGlobalVariables(DWARFUnit &cu,
  llvm::function_ref callback) = 0;
   virtual void
   GetObjCMethods(ConstString class_name,

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 5dfa5a176d384..2b2c13abb250b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -123,7 +123,7 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
 }
 
 void DebugNamesDWARFIndex::GetGlobalVariables(
-const DWARFUnit &cu, llvm::function_ref callback) {
+DWARFUnit &cu, llvm::function_ref callback) {
   uint64_t cu_offset = cu.GetOffset();
   bool found_entry_for_cu = false;
   for (const DebugNames::NameIndex &ni: *m_debug_names_up) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 5d041c36c8f29..c451ccd4857fa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -32,7 +32,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   GetGlobalVariables(const RegularExpression ®ex,
  llvm::function_ref callback) override;
   void
-  GetGlobalVariables(const DWARFUnit &cu,
+  GetGlobalVariables(DWAR

[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names

2021-08-09 Thread Kim-Anh Tran via Phabricator via lldb-commits

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

LGTM, I also support that assertions are added. This gives confidence in what's 
expected and also makes it easier to read/understand whether it's a 
skeleton/non-skeleton unit that we are dealing with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107659

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

[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names

2021-08-09 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

Thanks for checking it but ...

In D107659#2931836 , @clayborg wrote:

> Can we not just grab the skeleton unit when/if needed instead of asserting in 
> many places?

... I am not sure @clayborg likes the assertions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107659

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

[Lldb-commits] [lldb] d6bf9dc - [lldb] [test] Fix TestGdbRemotePlatformFile with non-022 umask

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T14:12:06+02:00
New Revision: d6bf9dcbd5d4b198ed55c311be27fee927d8721c

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

LOG: [lldb] [test] Fix TestGdbRemotePlatformFile with non-022 umask

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 5c8f5ba4ef9d..8b6bb112aa5f 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -102,7 +102,11 @@ def vFile_test(self, read=False, write=False, 
append=False, trunc=False,
 if excl:
 mode |= 0x800
 
-server = self.connect_to_debug_monitor()
+old_umask = os.umask(0o22)
+try:
+server = self.connect_to_debug_monitor()
+finally:
+os.umask(old_umask)
 self.assertIsNotNone(server)
 
 # create a temporary file with some data



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

[Lldb-commits] [PATCH] D107761: [LLDB][GUI] Refactor form drawing using subsurfaces

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

OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a new method SubSurface to the Surface class. The method
returns another surface that is a subset of this surface. This is
important to further abstract away drawing from the ncurses objects. For
instance, fields could previously be drawn on subpads only but can now
be drawn on any surface. This is needed to create the file search
dialogs and similar functionalities.

There is an opportunity to refactor window drawing in general using
surfaces, but we shall consider this separately later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107761

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -343,15 +343,30 @@
 // A surface is an abstraction for something than can be drawn on. The surface
 // have a width, a height, a cursor position, and a multitude of drawing
 // operations. This type should be sub-classed to get an actually useful ncurses
-// object, such as a Window, SubWindow, Pad, or a SubPad.
+// object, such as a Window or a Pad.
 class Surface {
 public:
-  Surface() : m_window(nullptr) {}
+  enum class Type { Window, Pad };
+
+  Surface(Surface::Type type) : m_type(type), m_window(nullptr) {}
 
   WINDOW *get() { return m_window; }
 
   operator WINDOW *() { return m_window; }
 
+  Surface SubSurface(Rect bounds) {
+Surface subSurface(m_type);
+if (m_type == Type::Pad)
+  subSurface.m_window =
+  ::subpad(m_window, bounds.size.height, bounds.size.width,
+   bounds.origin.y, bounds.origin.x);
+else
+  subSurface.m_window =
+  ::derwin(m_window, bounds.size.height, bounds.size.width,
+   bounds.origin.y, bounds.origin.x);
+return subSurface;
+  }
+
   // Copy a region of the surface to another surface.
   void CopyToSurface(Surface &target, Point source_origin, Point target_origin,
  Size size) {
@@ -535,41 +550,32 @@
   }
 
 protected:
+  Type m_type;
   WINDOW *m_window;
 };
 
 class Pad : public Surface {
 public:
-  Pad(Size size) { m_window = ::newpad(size.height, size.width); }
-
-  ~Pad() { ::delwin(m_window); }
-};
-
-class SubPad : public Surface {
-public:
-  SubPad(Pad &pad, Rect bounds) {
-m_window = ::subpad(pad.get(), bounds.size.height, bounds.size.width,
-bounds.origin.y, bounds.origin.x);
-  }
-  SubPad(SubPad &subpad, Rect bounds) {
-m_window = ::subpad(subpad.get(), bounds.size.height, bounds.size.width,
-bounds.origin.y, bounds.origin.x);
+  Pad(Size size) : Surface(Surface::Type::Pad) {
+m_window = ::newpad(size.height, size.width);
   }
 
-  ~SubPad() { ::delwin(m_window); }
+  ~Pad() { ::delwin(m_window); }
 };
 
 class Window : public Surface {
 public:
   Window(const char *name)
-  : m_name(name), m_panel(nullptr), m_parent(nullptr), m_subwindows(),
-m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
+  : Surface(Surface::Type::Window), m_name(name), m_panel(nullptr),
+m_parent(nullptr), m_subwindows(), m_delegate_sp(),
+m_curr_active_window_idx(UINT32_MAX),
 m_prev_active_window_idx(UINT32_MAX), m_delete(false),
 m_needs_update(true), m_can_activate(true), m_is_subwin(false) {}
 
   Window(const char *name, WINDOW *w, bool del = true)
-  : m_name(name), m_panel(nullptr), m_parent(nullptr), m_subwindows(),
-m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
+  : Surface(Surface::Type::Window), m_name(name), m_panel(nullptr),
+m_parent(nullptr), m_subwindows(), m_delegate_sp(),
+m_curr_active_window_idx(UINT32_MAX),
 m_prev_active_window_idx(UINT32_MAX), m_delete(del),
 m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
 if (w)
@@ -577,8 +583,8 @@
   }
 
   Window(const char *name, const Rect &bounds)
-  : m_name(name), m_parent(nullptr), m_subwindows(), m_delegate_sp(),
-m_curr_active_window_idx(UINT32_MAX),
+  : Surface(Surface::Type::Window), m_name(name), m_parent(nullptr),
+m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
 m_prev_active_window_idx(UINT32_MAX), m_delete(true),
 m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
 Reset(::newwin(bounds.size.height, bounds.size.width, bounds.origin.y,
@@ -970,20 +976,6 @@
   const Window &operator=(const Window &) = delete;
 };
 
-class DerivedWindow : public Surface {
-public:
-  DerivedWindow(Window &window, Rect bounds) {
-m_window = ::derwin(window.get(), bounds.size.height, bounds.size.width,
-

[Lldb-commits] [PATCH] D107761: [LLDB][GUI] Refactor form drawing using subsurfaces

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

OmarEmaraDev added a comment.
Herald added a subscriber: JDevlieghere.

This is a reimplementation of D107182 , which 
was committed and then reverted because the `is_pad` function is not 
universally supported. The updated patch adds an explicit type member to 
identify the backing object of the surface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107761

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

[Lldb-commits] [lldb] 0dc57a6 - [lldb] [test] Mark new vFile tests as XFAIL on Windows

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T15:43:08+02:00
New Revision: 0dc57a66a0b1aa2cac88e236f7748b022666bdc5

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

LOG: [lldb] [test] Mark new vFile tests as XFAIL on Windows

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 8b6bb112aa5f..8b58ef4c18e4 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -17,30 +17,39 @@ class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_rdonly(self):
 self.vFile_test(read=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly(self):
 self.vFile_test(write=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_rdwr(self):
 self.vFile_test(read=True, write=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly_append(self):
 self.vFile_test(write=True, append=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_rdwr_append(self):
 self.vFile_test(read=True, write=True, append=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly_trunc(self):
 self.vFile_test(write=True, trunc=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_rdwr_trunc(self):
 self.vFile_test(read=True, write=True, trunc=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly_creat(self):
 self.vFile_test(write=True, creat=True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly_creat_excl(self):
 self.vFile_test(write=True, creat=True, excl=True)
 



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

[Lldb-commits] [lldb] 116b112 - [lldb] [test] Use Windows-friendly modes in vFile O_CREAT tests

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T15:43:08+02:00
New Revision: 116b112bbfe043aba16d732c2eec783f0b817bb4

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

LOG: [lldb] [test] Use Windows-friendly modes in vFile O_CREAT tests

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 8b58ef4c18e4..54129f9420a0 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -45,11 +45,9 @@ def test_platform_file_wronly_trunc(self):
 def test_platform_file_rdwr_trunc(self):
 self.vFile_test(read=True, write=True, trunc=True)
 
-@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly_creat(self):
 self.vFile_test(write=True, creat=True)
 
-@expectedFailureAll(oslist=["windows"])
 def test_platform_file_wronly_creat_excl(self):
 self.vFile_test(write=True, creat=True, excl=True)
 
@@ -111,7 +109,7 @@ def vFile_test(self, read=False, write=False, append=False, 
trunc=False,
 if excl:
 mode |= 0x800
 
-old_umask = os.umask(0o22)
+old_umask = os.umask(0)
 try:
 server = self.connect_to_debug_monitor()
 finally:
@@ -137,7 +135,7 @@ def vFile_test(self, read=False, write=False, append=False, 
trunc=False,
 # open the file for reading
 self.do_handshake()
 self.test_sequence.add_log_lines(
-["read packet: $vFile:open:%s,%x,1a0#00" % (
+["read packet: $vFile:open:%s,%x,1b6#00" % (
 binascii.b2a_hex(temp_path.encode()).decode(),
 mode),
  {"direction": "send",
@@ -211,7 +209,7 @@ def vFile_test(self, read=False, write=False, append=False, 
trunc=False,
 if creat:
 temp_file = open(temp_path, "rb")
 self.assertEqual(os.fstat(temp_file.fileno()).st_mode & 
0o,
- 0o640)
+ 0o666)
 temp_file.seek(0)
 data = test_data.encode()
 if trunc or creat:



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

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Robin Giese via Phabricator via lldb-commits

rgiese created this revision.
rgiese requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The `File::OpenOptions` were renamed; this fixes up a callsite that breaks for 
macOS builds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107767

Files:
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm


Index: 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -426,9 +426,9 @@
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
   if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
+file_options |= File::eOpenOptionReadOnly;
   if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+file_options |= File::eOpenOptionWriteOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] 
forKey:key];
   return error; // Success


Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -426,9 +426,9 @@
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
   if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
+file_options |= File::eOpenOptionReadOnly;
   if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+file_options |= File::eOpenOptionWriteOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
   return error; // Success
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Raphael Isemann via Phabricator via lldb-commits

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

LGTM, thanks. I assume you don't have commit access so I'll land this for you 
in a minute :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107767

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

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

I'm sorry about missing this.




Comment at: 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm:428-431
   if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
+file_options |= File::eOpenOptionReadOnly;
   if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+file_options |= File::eOpenOptionWriteOnly;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107767

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

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Robin Giese via Phabricator via lldb-commits

rgiese added a comment.

@teemperor  please hold while I figure out how to update the patch with @mgorny 
's cleaner suggestion. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107767

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

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Raphael Isemann via Phabricator via lldb-commits

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

My bad, I missed that the patch isn't just renaming but actually adding a new 
option. Let me run the tests with the proposed change (feel free to update the 
review in the meantime)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107767

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

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Robin Giese via Phabricator via lldb-commits

rgiese updated this revision to Diff 365186.
rgiese added a comment.

Incorporating feedback.


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

https://reviews.llvm.org/D107767

Files:
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm


Index: 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionReadOnly;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
 file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] 
forKey:key];
   return error; // Success


Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionReadOnly;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
 file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
   return error; // Success
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Robin Giese via Phabricator via lldb-commits

rgiese updated this revision to Diff 365190.
rgiese added a comment.

Trying this patch-of-a-patch thing again.


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

https://reviews.llvm.org/D107767

Files:
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm


Index: 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
+file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] 
forKey:key];
   return error; // Success


Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
+file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
   return error; // Success
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Raphael Isemann via Phabricator via lldb-commits

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

I'm landing this to unbreak Green Dragon.

@mgorny Just a heads up, I think this also broke one test with debugserver:

  ==
  FAIL: test_platform_file_wronly_trunc_debugserver 
(TestGdbRemotePlatformFile.TestGdbRemotePlatformFile)
  --
  Traceback (most recent call last):
File 
"/Users/teemperor/1llvm/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py",
 line 52, in test_method
  return attrvalue(self)
File 
"/Users/teemperor/1llvm/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py",
 line 119, in wrapper
  func(*args, **kwargs)
File 
"/Users/teemperor/1llvm/llvm-project/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py",
 line 42, in test_platform_file_wronly_trunc
  self.vFile_test(write=True, trunc=True)
File 
"/Users/teemperor/1llvm/llvm-project/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py",
 line 146, in vFile_test
  context = self.expect_gdbremote_sequence()
File 
"/Users/teemperor/1llvm/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py",
 line 621, in expect_gdbremote_sequence
  return expect_lldb_gdbserver_replay(
File 
"/Users/teemperor/1llvm/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py",
 line 198, in expect_lldb_gdbserver_replay
  context = sequence_entry.assert_match(
File 
"/Users/teemperor/1llvm/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py",
 line 479, in assert_match
  return self._assert_regex_match(asserter, actual_packet, context)
File 
"/Users/teemperor/1llvm/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py",
 line 446, in _assert_regex_match
  asserter.fail(
  AssertionError: regex '^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$' failed to match 
against content '$#00'
  Config=x86_64-/Users/teemperor/1llvm/rel/bin/clang
  

( TestGdbRemotePlatformFile's different subtests fail with same/similar errors).


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

https://reviews.llvm.org/D107767

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

[Lldb-commits] [lldb] 875a16b - [lldb] Fix break introduced in 14735ca

2021-08-09 Thread Raphael Isemann via lldb-commits


Author: Robin Giese
Date: 2021-08-09T17:34:57+02:00
New Revision: 875a16bcfc28ba4959d67e2a4ad853f909d4ab83

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

LOG: [lldb] Fix break introduced in 14735ca

The `File::OpenOptions` were renamed; this fixes up a callsite that breaks for
macOS builds. (See
https://github.com/llvm/llvm-project/commit/14735cab655441ba45c4b88ad82f11267e5fe916)

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

Added: 


Modified: 

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
 
b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
index 91d6252aa0b16..3069214e47b84 100644
--- 
a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@ static Status HandleFileAction(ProcessLaunchInfo 
&launch_info,
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
+file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] 
forKey:key];
   return error; // Success



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

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG875a16bcfc28: [lldb] Fix break introduced in 14735ca 
(authored by rgiese, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107767

Files:
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm


Index: 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
+file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] 
forKey:key];
   return error; // Success


Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,10 +425,12 @@
 open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
 if (created_fd >= 0) {
   auto file_options = File::OpenOptions(0);
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionRead;
-  if ((oflag & O_RDWR) || (oflag & O_RDONLY))
-file_options |= File::eOpenOptionWrite;
+  if (oflag & O_RDWR)
+file_options |= File::eOpenOptionReadWrite;
+  else if (oflag & O_WRONLY)
+file_options |= File::eOpenOptionWriteOnly;
+  else if (oflag & O_RDONLY)
+file_options |= File::eOpenOptionReadOnly;
   file = std::make_shared(created_fd, file_options, true);
   [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
   return error; // Success
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107767: Fix break introduced in 14735ca

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107767

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

[Lldb-commits] [lldb] 52d89d2 - [lldb] [test] Mark vFile tests as LLGS-specific

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T17:43:37+02:00
New Revision: 52d89d26aa29d983aa7842829370be2ee5f8c76a

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

LOG: [lldb] [test] Mark vFile tests as LLGS-specific

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 54129f9420a0..432b56e49964 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -18,39 +18,49 @@ class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_rdonly(self):
 self.vFile_test(read=True)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_wronly(self):
 self.vFile_test(write=True)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_rdwr(self):
 self.vFile_test(read=True, write=True)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_wronly_append(self):
 self.vFile_test(write=True, append=True)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_rdwr_append(self):
 self.vFile_test(read=True, write=True, append=True)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_wronly_trunc(self):
 self.vFile_test(write=True, trunc=True)
 
 @expectedFailureAll(oslist=["windows"])
+@add_test_categories(["llgs"])
 def test_platform_file_rdwr_trunc(self):
 self.vFile_test(read=True, write=True, trunc=True)
 
+@add_test_categories(["llgs"])
 def test_platform_file_wronly_creat(self):
 self.vFile_test(write=True, creat=True)
 
+@add_test_categories(["llgs"])
 def test_platform_file_wronly_creat_excl(self):
 self.vFile_test(write=True, creat=True, excl=True)
 
+@add_test_categories(["llgs"])
 def test_platform_file_wronly_fail(self):
 server = self.connect_to_debug_monitor()
 self.assertIsNotNone(server)
@@ -70,6 +80,7 @@ def test_platform_file_wronly_fail(self):
 True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["llgs"])
 def test_platform_file_wronly_creat_excl_fail(self):
 server = self.connect_to_debug_monitor()
 self.assertIsNotNone(server)



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

[Lldb-commits] [lldb] 3240000 - [lldb][NFC] Remove never read member variable IOHandler::m_editing

2021-08-09 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-08-09T18:05:53+02:00
New Revision: 32454652d47467d23b73ada5c8a599087167

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

LOG: [lldb][NFC] Remove never read member variable IOHandler::m_editing

The last read access to this variable was removed in 2015 in
4446487d71c52a925c04acfcae44dec8a8d62e00 .

Added: 


Modified: 
lldb/include/lldb/Core/IOHandler.h
lldb/source/Core/IOHandler.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/IOHandler.h 
b/lldb/include/lldb/Core/IOHandler.h
index 4a3b788e3ea16..7011dd1e8e047 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -442,8 +442,6 @@ class IOHandlerEditline : public IOHandler {
   bool m_multi_line;
   bool m_color_prompts;
   bool m_interrupt_exits;
-  bool m_editing; // Set to true when fetching a line manually (not using
-  // libedit)
   std::string m_line_buffer;
 };
 

diff  --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index c6f05d43a2a70..c35b17990842f 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -251,8 +251,7 @@ IOHandlerEditline::IOHandlerEditline(
   m_delegate(delegate), m_prompt(), m_continuation_prompt(),
   m_current_lines_ptr(nullptr), m_base_line_number(line_number_start),
   m_curr_line_idx(UINT32_MAX), m_multi_line(multi_line),
-  m_color_prompts(color_prompts), m_interrupt_exits(true),
-  m_editing(false) {
+  m_color_prompts(color_prompts), m_interrupt_exits(true) {
   SetPrompt(prompt);
 
 #if LLDB_ENABLE_LIBEDIT
@@ -399,7 +398,6 @@ bool IOHandlerEditline::GetLine(std::string &line, bool 
&interrupted) {
   }
 
   if (!got_line && in) {
-m_editing = true;
 while (!got_line) {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
@@ -425,7 +423,6 @@ bool IOHandlerEditline::GetLine(std::string &line, bool 
&interrupted) {
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
 }
-m_editing = false;
   }
 
   if (got_line) {



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

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-09 Thread Raphael Isemann via Phabricator via lldb-commits

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

Thanks for the patch! I think we can actually simplify that code even more by 
just removing the `!got_line` checks in both ifs? There is really no reason to 
have them from what I can see.




Comment at: lldb/source/Core/IOHandler.cpp:402
   if (!got_line && in) {
 m_editing = true;
+do {

Just a small FYI: I removed the `m_editing` variable here in 
32454652d47467d23b73ada5c8a599087167 



Comment at: lldb/source/Core/IOHandler.cpp:435
   m_data_recorder->Record(line, true);
+return true;
   }

This version is fine too, but we could potentially do an LLVMy early return:

```
lang=c++
if (!got_line)
  return false;
[...]
return true;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107704

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

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

2021-08-09 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.

Just fix the possible crash when SetText gets a NULL string pointer, and this 
is good to go.

We also need to get the "is_pad(...)" fix in before this can be submitted right?




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3975
+
+  void SetText(const char *text) { m_text = text; }
+

This will crash if you pass in NULL. Test "text" before assignment and call 
m_text.clear() if it is NULL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107386

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

[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names

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

clayborg added a comment.

In D107659#2934245 , @jankratochvil 
wrote:

> Thanks for checking it but ...
>
> In D107659#2931836 , @clayborg 
> wrote:
>
>> Can we not just grab the skeleton unit when/if needed instead of asserting 
>> in many places?
>
> ... I am not sure @clayborg likes the assertions.

I don't mind lldbassert() calls as they are not in release builds, so it is 
nice to catch things during debug build testing. I prefer that asserts are not 
used when the program would knowingly crash immediately after the assert. I 
don't like asserts in code like:

  assert(ptr)
  ptr->foo();

This code will crash both when asserts are enabled and also when they aren't. 
And the llvm/clang/lldb code is used in a shared library that other tools link 
against. So the code really shouldn't crash when it is easy to work around and 
add code to avoid the crash. I still prefer the above code to look like:

  assert(ptr)
  if (ptr)
ptr->foo();

As long as the program continues to function correctly without the assert, feel 
free to add any asserts to help catch issues during development.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107659

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

[Lldb-commits] [PATCH] D107475: [lldb] [gdb-remote] Use hexadecimal numbers in vFile packets for GDB compliance

2021-08-09 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

This change is causing failures on the Windows lldb bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/9098


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107475

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

[Lldb-commits] [PATCH] D107475: [lldb] [gdb-remote] Use hexadecimal numbers in vFile packets for GDB compliance

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

In D107475#2934924 , 
@stella.stamenova wrote:

> This change is causing failures on the Windows lldb bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/9098

I've already attempted to fix this, I'll try more. It doesn't help that 
buildbot webUI is so shoddy it takes me around 10 minutes to get any results 
:-(.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107475

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

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

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

OmarEmaraDev updated this revision to Diff 365216.
OmarEmaraDev added a comment.

- Fix possible crash in SetText


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107386

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectRegister.h"
 #include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/VariableList.h"
@@ -3772,9 +3773,14 @@
TreeItem *&selected_item) {
 return;
   }
-  virtual bool TreeDelegateItemSelected(
-  TreeItem &item) = 0; // Return true if we need to update views
+  // This is invoked when a tree item is selected. If true is returned, the
+  // views are updated.
+  virtual bool TreeDelegateItemSelected(TreeItem &item) = 0;
   virtual bool TreeDelegateExpandRootByDefault() { return false; }
+  // This is mostly useful for root tree delegates. If false is returned,
+  // drawing will be skipped completely. This is needed, for instance, in
+  // skipping drawing of the threads tree if there is no running process.
+  virtual bool TreeDelegateShouldDraw() { return true; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -3964,6 +3970,16 @@
 
   void SetIdentifier(uint64_t identifier) { m_identifier = identifier; }
 
+  const std::string &GetText() const { return m_text; }
+
+  void SetText(const char *text) {
+if (text == nullptr) {
+  m_text.clear();
+  return;
+}
+m_text = text;
+  }
+
   void SetMightHaveChildren(bool b) { m_might_have_children = b; }
 
 protected:
@@ -3971,6 +3987,7 @@
   TreeDelegate &m_delegate;
   void *m_user_data;
   uint64_t m_identifier;
+  std::string m_text;
   int m_row_idx; // Zero based visible row index, -1 if not visible or for the
  // root item
   std::vector m_children;
@@ -3989,21 +4006,6 @@
   int NumVisibleRows() const { return m_max_y - m_min_y; }
 
   bool WindowDelegateDraw(Window &window, bool force) override {
-ExecutionContext exe_ctx(
-m_debugger.GetCommandInterpreter().GetExecutionContext());
-Process *process = exe_ctx.GetProcessPtr();
-
-bool display_content = false;
-if (process) {
-  StateType state = process->GetState();
-  if (StateIsStoppedState(state, true)) {
-// We are stopped, so it is ok to
-display_content = true;
-  } else if (StateIsRunningState(state)) {
-return true; // Don't do any updating when we are running
-  }
-}
-
 m_min_x = 2;
 m_min_y = 1;
 m_max_x = window.GetWidth() - 1;
@@ -4012,35 +4014,36 @@
 window.Erase();
 window.DrawTitleBox(window.GetName());
 
-if (display_content) {
-  const int num_visible_rows = NumVisibleRows();
-  m_num_rows = 0;
-  m_root.CalculateRowIndexes(m_num_rows);
-  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
- m_selected_item);
-
-  // If we unexpanded while having something selected our total number of
-  // rows is less than the num visible rows, then make sure we show all the
-  // rows by setting the first visible row accordingly.
-  if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
-m_first_visible_row = 0;
-
-  // Make sure the selected row is always visible
-  if (m_selected_row_idx < m_first_visible_row)
-m_first_visible_row = m_selected_row_idx;
-  else if (m_first_visible_row + num_visible_rows <= m_selected_row_idx)
-m_first_visible_row = m_selected_row_idx - num_visible_rows + 1;
-
-  int row_idx = 0;
-  int num_rows_left = num_visible_rows;
-  m_root.Draw(window, m_first_visible_row, m_selected_row_idx, row_idx,
-  num_rows_left);
-  // Get the selected row
-  m_selected_item = m_root.GetItemForRowIndex(m_selected_row_idx);
-} else {
+if (!m_delegate_sp->TreeDelegateShouldDraw()) {
   m_selected_item = nullptr;
+  return true;
 }
 
+const int num_visible_rows = NumVisibleRows();
+m_num_rows = 0;
+m_root.CalculateRowIndexes(m_num_rows);
+m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+   m_selected_item);
+
+// If we unexpanded while having something selected our total number of
+// rows is less than the num visible rows, then make sure we show all the
+// rows by setting the first visible row accordingly.
+if (m_first_visible_row > 0 && m_num_rows < num_visible_rows)
+  m_first_visible_row = 0;
+
+// Make sure the selected row i

[Lldb-commits] [PATCH] D107386: [LLDB][GUI] Add Breakpoints window

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

OmarEmaraDev added a comment.

@clayborg This is unrelated to D107761 , so 
it should be safe to commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107386

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

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-09 Thread Alf via Phabricator via lldb-commits

gAlfonso-bit updated this revision to Diff 365217.
gAlfonso-bit added a comment.

Simplify function even further


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

https://reviews.llvm.org/D107704

Files:
  lldb/source/Core/IOHandler.cpp


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -372,7 +372,11 @@
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +385,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +410,32 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }
 
-  if (got_line) {
+  if (got_line) { // Let's see if we got this right.
+gotLine:
 line = got_line.getValue();
 if (m_data_recorder)
   m_data_recorder->Record(line, true);
+return true;
   }
 
-  return (bool)got_line;
+  return false;
 }
 
 #if LLDB_ENABLE_LIBEDIT


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -372,7 +372,11 @@
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +385,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +410,32 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }
 
-  if (got_line) {
+  if (got_line) { // Let's see if we got this right.
+gotLine:
 line = got_line.getValue();
 if (m_data_recorder)
   m_data_recorder->Record(line, true);
+return true;
   }
 
-  return (bool)got_line;
+  return false;
 }
 
 #if LLDB_ENABLE_LIBEDIT
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4c830b5 - Revert "[lldb] [test] Use Windows-friendly modes in vFile O_CREAT tests"

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T19:38:05+02:00
New Revision: 4c830b5f35f0f4058d84732fafcc38c12a7969b3

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

LOG: Revert "[lldb] [test] Use Windows-friendly modes in vFile O_CREAT tests"

This reverts commit 116b112bbfe043aba16d732c2eec783f0b817bb4.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 432b56e49964..4e3ba2519da5 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -52,10 +52,12 @@ def test_platform_file_wronly_trunc(self):
 def test_platform_file_rdwr_trunc(self):
 self.vFile_test(read=True, write=True, trunc=True)
 
+@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat(self):
 self.vFile_test(write=True, creat=True)
 
+@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat_excl(self):
 self.vFile_test(write=True, creat=True, excl=True)
@@ -120,7 +122,7 @@ def vFile_test(self, read=False, write=False, append=False, 
trunc=False,
 if excl:
 mode |= 0x800
 
-old_umask = os.umask(0)
+old_umask = os.umask(0o22)
 try:
 server = self.connect_to_debug_monitor()
 finally:
@@ -146,7 +148,7 @@ def vFile_test(self, read=False, write=False, append=False, 
trunc=False,
 # open the file for reading
 self.do_handshake()
 self.test_sequence.add_log_lines(
-["read packet: $vFile:open:%s,%x,1b6#00" % (
+["read packet: $vFile:open:%s,%x,1a0#00" % (
 binascii.b2a_hex(temp_path.encode()).decode(),
 mode),
  {"direction": "send",
@@ -220,7 +222,7 @@ def vFile_test(self, read=False, write=False, append=False, 
trunc=False,
 if creat:
 temp_file = open(temp_path, "rb")
 self.assertEqual(os.fstat(temp_file.fileno()).st_mode & 
0o,
- 0o666)
+ 0o640)
 temp_file.seek(0)
 data = test_data.encode()
 if trunc or creat:



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

[Lldb-commits] [lldb] 816aa9a - Revert "[lldb] [test] Mark new vFile tests as XFAIL on Windows"

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T19:38:05+02:00
New Revision: 816aa9a5d794d0556bbe59770659aa50fbebd6c5

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

LOG: Revert "[lldb] [test] Mark new vFile tests as XFAIL on Windows"

This reverts commit 0dc57a66a0b1aa2cac88e236f7748b022666bdc5.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 4e3ba2519da5..466ead17630f 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -17,47 +17,38 @@ class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_rdonly(self):
 self.vFile_test(read=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly(self):
 self.vFile_test(write=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_rdwr(self):
 self.vFile_test(read=True, write=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_append(self):
 self.vFile_test(write=True, append=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_rdwr_append(self):
 self.vFile_test(read=True, write=True, append=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_trunc(self):
 self.vFile_test(write=True, trunc=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_rdwr_trunc(self):
 self.vFile_test(read=True, write=True, trunc=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat(self):
 self.vFile_test(write=True, creat=True)
 
-@expectedFailureAll(oslist=["windows"])
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat_excl(self):
 self.vFile_test(write=True, creat=True, excl=True)



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

[Lldb-commits] [lldb] 27b238a - [lldb] [test] Skip all vFile tests on Windows

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T19:38:05+02:00
New Revision: 27b238af163e52b9f575f854a3f391514412b25c

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

LOG: [lldb] [test] Skip all vFile tests on Windows

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 466ead17630f..4fc688deae7f 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -17,42 +17,52 @@ class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_rdonly(self):
 self.vFile_test(read=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly(self):
 self.vFile_test(write=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_rdwr(self):
 self.vFile_test(read=True, write=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_append(self):
 self.vFile_test(write=True, append=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_rdwr_append(self):
 self.vFile_test(read=True, write=True, append=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_trunc(self):
 self.vFile_test(write=True, trunc=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_rdwr_trunc(self):
 self.vFile_test(read=True, write=True, trunc=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat(self):
 self.vFile_test(write=True, creat=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat_excl(self):
 self.vFile_test(write=True, creat=True, excl=True)
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_fail(self):
 server = self.connect_to_debug_monitor()
@@ -73,6 +83,7 @@ def test_platform_file_wronly_fail(self):
 True)
 self.expect_gdbremote_sequence()
 
+@skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_wronly_creat_excl_fail(self):
 server = self.connect_to_debug_monitor()



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

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-09 Thread Alf via Phabricator via lldb-commits

gAlfonso-bit updated this revision to Diff 365218.

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

https://reviews.llvm.org/D107704

Files:
  lldb/source/Core/IOHandler.cpp


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -372,7 +372,11 @@
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +385,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +410,32 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }
 
   if (got_line) {
+  gotLine:
 line = got_line.getValue();
 if (m_data_recorder)
   m_data_recorder->Record(line, true);
+return true;
   }
 
-  return (bool)got_line;
+  return false;
 }
 
 #if LLDB_ENABLE_LIBEDIT


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -372,7 +372,11 @@
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +385,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +410,32 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }
 
   if (got_line) {
+  gotLine:
 line = got_line.getValue();
 if (m_data_recorder)
   m_data_recorder->Record(line, true);
+return true;
   }
 
-  return (bool)got_line;
+  return false;
 }
 
 #if LLDB_ENABLE_LIBEDIT
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107475: [lldb] [gdb-remote] Use hexadecimal numbers in vFile packets for GDB compliance

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

Ok, I've reverted my previous attempts and just marked the tests for skipping 
on Windows. Hopefully this'll work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107475

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

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-09 Thread Alf via Phabricator via lldb-commits

gAlfonso-bit updated this revision to Diff 365220.

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

https://reviews.llvm.org/D107704

Files:
  lldb/source/Core/IOHandler.cpp


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -362,17 +362,19 @@
 if (prompt == nullptr)
   prompt = GetPrompt();
 
-if (prompt && prompt[0]) {
-  if (m_output_sp) {
-m_output_sp->Printf("%s", prompt);
-m_output_sp->Flush();
-  }
+if (prompt && prompt[0] && m_output_sp) {
+  m_output_sp->Printf("%s", prompt);
+  m_output_sp->Flush();
 }
   }
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +383,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +408,31 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }
 
-  if (got_line) {
-line = got_line.getValue();
-if (m_data_recorder)
-  m_data_recorder->Record(line, true);
-  }
+  if (!got_line)
+return false;
 
-  return (bool)got_line;
+gotLine:
+  line = got_line.getValue();
+  if (m_data_recorder)
+m_data_recorder->Record(line, true);
+  return true;
 }
 
 #if LLDB_ENABLE_LIBEDIT


Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -362,17 +362,19 @@
 if (prompt == nullptr)
   prompt = GetPrompt();
 
-if (prompt && prompt[0]) {
-  if (m_output_sp) {
-m_output_sp->Printf("%s", prompt);
-m_output_sp->Flush();
-  }
+if (prompt && prompt[0] && m_output_sp) {
+  m_output_sp->Printf("%s", prompt);
+  m_output_sp->Flush();
 }
   }
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +383,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +408,31 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }

[Lldb-commits] [PATCH] D107701: [lldb] [test] Skip Expr/nodefaultlib.cpp test if LD_PRELOAD Is used

2021-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


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

https://reviews.llvm.org/D107701

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

[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, jasonmolenda, emaste, JDevlieghere.
mgorny requested review of this revision.

Implement a fallback to getting the file size via vFile:stat packet
when the remote server does not implement vFile:size.  This makes it
possible to query file sizes from remote gdbserver.

While at it, add a few tests for the 'platform get-size' command.


https://reviews.llvm.org/D107780

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -77,3 +77,58 @@
 ])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size(self):
+"""Test 'platform get-size'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1000"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+   [r"File size of /some/file\.txt \(remote\): 4096"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size_fallback(self):
+"""Test 'platform get-size fallback to vFile:fstat'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 28 * "\0" + "\0\0\0\0\0\1\2\3" + 28 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+   [r"File size of /some/file\.txt \(remote\): 66051"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -391,6 +391,8 @@
 
   bool CloseFile(lldb::user_id_t fd, Status &error);
 
+  llvm::Optional FStat(lldb::user_id_t fd);
+
   lldb::user_id_t GetFileSize(const FileSpec &file_spec);
 
   void AutoCompleteDiskFileOrDirectory(CompletionRequest &request,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3035,6 +3035,46 @@
   return false;
 }
 
+llvm::Optional
+GDBRemoteCommunicationClient::FStat(lldb::user_id_t fd) {
+  lldb_private::StreamString stream;
+  stream.Printf("vFile:fstat:%x", fd);
+  StringExtractorGDBRemote response;
+  if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
+  PacketResult::Success) {
+if (response.GetChar() != 'F')
+  return llvm::None;
+int64_t size = response.GetS64(-1, 16);
+if (size > 0 && response.GetChar() == ';') {
+  std::string buffer;
+  if (response.GetEscapedBinaryData(buffer)) {
+DataExtractor extractor{buffer.data(), buffer.size(),
+lldb::eByteOrderBig, sizeof(void *)};
+lldb::offset_t offset = 0;
+GDBRemoteFStatData out;
+

[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

This covers client side only, I'm going to work on the server side `vFile:stat` 
packet later.


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

https://reviews.llvm.org/D107780

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

[Lldb-commits] [PATCH] D107701: [lldb] [test] Skip Expr/nodefaultlib.cpp test if LD_PRELOAD Is used

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

Thanks!


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

https://reviews.llvm.org/D107701

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

[Lldb-commits] [lldb] 15cacab - [lldb] [test] Skip Expr/nodefaultlib.cpp test if LD_PRELOAD Is used

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T21:35:11+02:00
New Revision: 15cacab73f7e36d9dca4a5516ccb360b67bf2dbc

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

LOG: [lldb] [test] Skip Expr/nodefaultlib.cpp test if LD_PRELOAD Is used

Some LD_PRELOAD-ed libraries tend to interact badly with --nodefaultlib,
particularly Gentoo sandbox.  Do not run this test if LD_PRELOAD is
present in the running environment.

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

Added: 


Modified: 
lldb/test/Shell/Expr/nodefaultlib.cpp
lldb/test/Shell/lit.cfg.py

Removed: 




diff  --git a/lldb/test/Shell/Expr/nodefaultlib.cpp 
b/lldb/test/Shell/Expr/nodefaultlib.cpp
index 5c4f19d40efc7..fb53da7fcb08b 100644
--- a/lldb/test/Shell/Expr/nodefaultlib.cpp
+++ b/lldb/test/Shell/Expr/nodefaultlib.cpp
@@ -2,6 +2,7 @@
 // standard library (and mmap-like functions in particular).
 
 // REQUIRES: native
+// UNSUPPORTED: ld_preload-present
 // XFAIL: system-linux && !(target-x86 || target-x86_64)
 // XFAIL: system-netbsd || system-freebsd || system-darwin
 

diff  --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py
index 964efc8c3505f..aecd9d62c2144 100644
--- a/lldb/test/Shell/lit.cfg.py
+++ b/lldb/test/Shell/lit.cfg.py
@@ -140,3 +140,6 @@ def calculate_arch_features(arch_string):
 can_set_dbregs = False
 if can_set_dbregs:
 config.available_features.add('dbregs-set')
+
+if 'LD_PRELOAD' in os.environ:
+config.available_features.add('ld_preload-present')



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

[Lldb-commits] [PATCH] D107701: [lldb] [test] Skip Expr/nodefaultlib.cpp test if LD_PRELOAD Is used

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG15cacab73f7e: [lldb] [test] Skip Expr/nodefaultlib.cpp test 
if LD_PRELOAD Is used (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107701

Files:
  lldb/test/Shell/Expr/nodefaultlib.cpp
  lldb/test/Shell/lit.cfg.py


Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -140,3 +140,6 @@
 can_set_dbregs = False
 if can_set_dbregs:
 config.available_features.add('dbregs-set')
+
+if 'LD_PRELOAD' in os.environ:
+config.available_features.add('ld_preload-present')
Index: lldb/test/Shell/Expr/nodefaultlib.cpp
===
--- lldb/test/Shell/Expr/nodefaultlib.cpp
+++ lldb/test/Shell/Expr/nodefaultlib.cpp
@@ -2,6 +2,7 @@
 // standard library (and mmap-like functions in particular).
 
 // REQUIRES: native
+// UNSUPPORTED: ld_preload-present
 // XFAIL: system-linux && !(target-x86 || target-x86_64)
 // XFAIL: system-netbsd || system-freebsd || system-darwin
 


Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -140,3 +140,6 @@
 can_set_dbregs = False
 if can_set_dbregs:
 config.available_features.add('dbregs-set')
+
+if 'LD_PRELOAD' in os.environ:
+config.available_features.add('ld_preload-present')
Index: lldb/test/Shell/Expr/nodefaultlib.cpp
===
--- lldb/test/Shell/Expr/nodefaultlib.cpp
+++ lldb/test/Shell/Expr/nodefaultlib.cpp
@@ -2,6 +2,7 @@
 // standard library (and mmap-like functions in particular).
 
 // REQUIRES: native
+// UNSUPPORTED: ld_preload-present
 // XFAIL: system-linux && !(target-x86 || target-x86_64)
 // XFAIL: system-netbsd || system-freebsd || system-darwin
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-09 Thread Alf via Phabricator via lldb-commits

gAlfonso-bit updated this revision to Diff 365267.
gAlfonso-bit added a comment.

clang-tidy workaround


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

https://reviews.llvm.org/D107704

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Core/IOHandler.cpp

Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -362,17 +362,19 @@
 if (prompt == nullptr)
   prompt = GetPrompt();
 
-if (prompt && prompt[0]) {
-  if (m_output_sp) {
-m_output_sp->Printf("%s", prompt);
-m_output_sp->Flush();
-  }
+if (prompt && prompt[0] && m_output_sp) {
+  m_output_sp->Printf("%s", prompt);
+  m_output_sp->Flush();
 }
   }
 
   Optional got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+goto gotLine;
+  }
+
+  if (!m_input_sp) {
 // No more input file, we are done...
 SetIsDone(true);
 return false;
@@ -381,24 +383,8 @@
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
-  size_t bytes_read = sizeof(buffer);
-  Status error = m_input_sp->Read((void *)buffer, bytes_read);
-  if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
-  }
-  if (error.Fail())
-break;
-  m_line_buffer += StringRef(buffer, bytes_read);
-  got_line = SplitLine(m_line_buffer);
-}
-  }
-
-  if (!got_line && in) {
-while (!got_line) {
+  if (in) {
+do {
   char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
   // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +408,31 @@
   }
   m_line_buffer += buffer;
   got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+  } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+  size_t bytes_read = sizeof(buffer);
+  Status error = m_input_sp->Read((void *)buffer, bytes_read);
+  if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+  }
+  if (error.Fail())
+return false;
+  m_line_buffer += StringRef(buffer, bytes_read);
+  got_line = SplitLine(m_line_buffer);
+} while (!got_line);
   }
 
-  if (got_line) {
-line = got_line.getValue();
-if (m_data_recorder)
-  m_data_recorder->Record(line, true);
-  }
+  if (!got_line)
+return false;
 
-  return (bool)got_line;
+gotLine:
+  line = got_line.getValue();
+  if (m_data_recorder)
+m_data_recorder->Record(line, true);
+  return true;
 }
 
 #if LLDB_ENABLE_LIBEDIT
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -34,7 +34,7 @@
 namespace repro {
 class DataRecorder;
 }
-}
+} // namespace lldb_private
 
 namespace curses {
 class Application;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107700: [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

@JDevlieghere, could you also look at this one? I'm not really sure if anybody 
maintains LLDBStandalone at this point.


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

https://reviews.llvm.org/D107700

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

[Lldb-commits] [PATCH] D107700: [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

2021-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D107700#2935385 , @mgorny wrote:

> @JDevlieghere, could you also look at this one? I'm not really sure if 
> anybody maintains LLDBStandalone at this point.

I think it's pretty much just me these days. I would really like to get rid of 
it, but it's used downstream by the Swift build.


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

https://reviews.llvm.org/D107700

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

[Lldb-commits] [PATCH] D107700: [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

2021-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

A possible alternative would be to export LLVM_LIT_ARGS from the llvm build so 
that lldb inherits it. I'm pretty much on the fence: on the one hand this 
doesn't really seems like something that should be exported, but on the other 
hand it'd be nice to not have to duplicate the option and having to update the 
default if it changes in llvm.

LGTM in the sense that I have no strong opinion.


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

https://reviews.llvm.org/D107700

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

[Lldb-commits] [PATCH] D107700: [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

Well, I actually find it useful to be able to override lit args per project. 
Maybe some future solution would be to have it declared as a cache variable 
from some LLVM CMake file.


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

https://reviews.llvm.org/D107700

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

[Lldb-commits] [lldb] 614c7d0 - [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

2021-08-09 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-08-09T23:36:01+02:00
New Revision: 614c7d03877fd99c2de47429b15be3f00306a3bd

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

LOG: [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

Add a LLVM_LIT_ARGS cached variable in order to make it possible
to override lit arguments when doing standalone builds.  Without that,
the user variable is ignored and the default options are always used.

Based on a similar solution found in clang.

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

Added: 


Modified: 
lldb/cmake/modules/LLDBStandalone.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBStandalone.cmake 
b/lldb/cmake/modules/LLDBStandalone.cmake
index 94781c3583744..98d7848ce99b0 100644
--- a/lldb/cmake/modules/LLDBStandalone.cmake
+++ b/lldb/cmake/modules/LLDBStandalone.cmake
@@ -10,6 +10,8 @@ set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR} CACHE PATH 
"Path to LLVM source
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to 
llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
+
 set(lit_file_name "llvm-lit")
 if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
   set(lit_file_name "${lit_file_name}.py")



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

[Lldb-commits] [PATCH] D107700: [lldb] [cmake] Add LLVM_LIT_ARGS override support for standalone builds

2021-08-09 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG614c7d03877f: [lldb] [cmake] Add LLVM_LIT_ARGS override 
support for standalone builds (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107700

Files:
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -10,6 +10,8 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to 
llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
+
 set(lit_file_name "llvm-lit")
 if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
   set(lit_file_name "${lit_file_name}.py")


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -10,6 +10,8 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to llvm/include")
 set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR} CACHE PATH "Path to LLVM build tree")
 
+set(LLVM_LIT_ARGS "-sv" CACHE STRING "Default options for lit")
+
 set(lit_file_name "llvm-lit")
 if(CMAKE_HOST_WIN32 AND NOT CYGWIN)
   set(lit_file_name "${lit_file_name}.py")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-09 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/source/Core/IOHandler.cpp:374
+  if (got_line) {
+goto gotLine;
+  }

A `goto` is not warranted here, using `if (!got_line && !in` is perfectly fine 
and less lines. 


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

https://reviews.llvm.org/D107704

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

[Lldb-commits] [PATCH] D107625: Add a stack-memory-only style for Darwin process save-core

2021-08-09 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda updated this revision to Diff 365319.
jasonmolenda added a comment.

Have debugserver report malloc memory regions too, aka heap.  I don't know how 
to use this in lldb yet, but it was easy to add so I thought I'd add it.  Maybe 
we'll come up with an idea for it later.  On Darwin, there are multiple types 
of heap - malloc small, malloc large, malloc tiny, etc - so for these, 
debugserver will return "type:heap,malloc-small;" etc, so the generic "heap" 
word is there, but also the more darwin-specific type of heap is given.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107625

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
  lldb/test/API/macosx/stack-corefile/Makefile
  lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
  lldb/test/API/macosx/stack-corefile/main.c
  lldb/tools/debugserver/source/DNBDefs.h
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4310,6 +4310,14 @@
   }
 }
 ostrm << ";";
+if (!region_info.vm_types.empty()) {
+  for (size_t i = 0; i < region_info.vm_types.size(); i++) {
+if (i)
+  ostrm << ",";
+ostrm << region_info.vm_types[i];
+  }
+  ostrm << ";";
+}
   }
   return SendPacket(ostrm.str());
 }
Index: lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
===
--- lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
+++ lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
@@ -40,6 +40,7 @@
   vm_prot_t prot);
   bool RestoreProtections();
   bool GetRegionForAddress(nub_addr_t addr);
+  std::vector GetMemoryTypes() const;
 
   uint32_t GetDNBPermissions() const;
 
Index: lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
@@ -182,3 +182,43 @@
 dnb_permissions |= eMemoryPermissionsExecutable;
   return dnb_permissions;
 }
+
+std::vector MachVMRegion::GetMemoryTypes() const {
+  std::vector types;
+  if (m_data.user_tag == VM_MEMORY_STACK) {
+if (m_data.protection == VM_PROT_NONE) {
+  types.push_back("stack-guard");
+} else {
+  types.push_back("stack");
+}
+  }
+  if (m_data.user_tag == VM_MEMORY_MALLOC) {
+if (m_data.protection == VM_PROT_NONE)
+  types.push_back("malloc-guard");
+else if (m_data.share_mode == SM_EMPTY)
+  types.push_back("malloc-reserved");
+else
+  types.push_back("malloc-metadata");
+  }
+  if (m_data.user_tag == VM_MEMORY_MALLOC_NANO ||
+  m_data.user_tag == VM_MEMORY_MALLOC_TINY ||
+  m_data.user_tag == VM_MEMORY_MALLOC_SMALL ||
+  m_data.user_tag == VM_MEMORY_MALLOC_LARGE ||
+  m_data.user_tag == VM_MEMORY_MALLOC_LARGE_REUSED ||
+  m_data.user_tag == VM_MEMORY_MALLOC_LARGE_REUSABLE ||
+  m_data.user_tag == VM_MEMORY_MALLOC_HUGE ||
+  m_data.user_tag == VM_MEMORY_REALLOC ||
+  m_data.user_tag == VM_MEMORY_SBRK) {
+types.push_back("heap");
+if (m_data.user_tag == VM_MEMORY_MALLOC_TINY) {
+  types.push_back("malloc-tiny");
+}
+if (m_data.user_tag == VM_MEMORY_MALLOC_LARGE) {
+  types.push_back("malloc-large");
+}
+if (m_data.user_tag == VM_MEMORY_MALLOC_SMALL) {
+  types.push_back("malloc-small");
+}
+  }
+  return types;
+}
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -125,6 +125,7 @@
 region_info->permissions = vmRegion.GetDNBPermissions();
 region_info->dirty_pages =
 get_dirty_pages(task, vmRegion.StartAddress(), vmRegion.GetByteSize());
+region_info->vm_types = vmRegion.GetMemoryTypes();
   } else {
 region_info->addr = address;
 region_info->size = 0;
Index: lldb/tools/debugserver/source/DNBDefs.h
===
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include

[Lldb-commits] [PATCH] D107674: [tests] [trace] Add more comprehensive for `thread trace export ctf` command

2021-08-09 Thread walter erquinigo via Phabricator via lldb-commits

wallace added a comment.

Thank you, this is much more understandable




Comment at: lldb/docs/htr.rst:1
 Hierarchical Trace Representation (HTR)
 ==

I'd be good to put these documentation files next in 
source/Plugins/TraceExporter/docs so that the documentation can live next to 
the source code. It'll give more visibility to this doc



Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:396
 {"name", display_name},
-{"ph", "B"},
+{"ph", "X"},
 {"ts", (int64_t)i},

a little comment explaining what "ph", "X", "dur" mean would be good. Also that 
in the absence of timestamps "ts" is just an index. You can repeat the comments 
you have in the test, btw





Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:424
 
+  auto duration = metadata.GetNumInstructions();
   layers_as_json.emplace_back(llvm::json::Object{

explain why the duration is the num of instructions in the case of no timestamps



Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:433
   });
-  start_ts = end_ts;
+  start_ts += duration ;
 }

in your devserver you can run 
"arc lint"
and it will fix all these formatting issues for you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107674

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

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

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/include/lldb/Target/Trace.h:75
+  virtual llvm::Error SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) = 0;
+

This class will already have a process in "Process *m_live_process" member 
variable, so no need to pass in the process. This will be NULL if we loaded a 
trace from disk already, but that should be ok because it was already loaded 
from something on disk.



Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1687-1693
+"Save the trace of the current process in the specified directory, 
"
+"which will be created if needed."
+"This will also create a file /trace.json with the main 
"
+"properties of the trace session, along with others files which "
+"contain the actual trace data. The trace.json file can be used "
+"later as input for the \"trace load\" command to load the trace "
+"in LLDB",





Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1708
+if (llvm::Error err =
+trace_sp->SaveToDisk(m_options.m_directory, process_sp))
+  result.AppendError(toString(std::move(err)));

No need for the process as mentioned in above inline comment.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:267
 }
+llvm::Expected> PostMortemThreadDecoder::GetRawTrace() {
+  FileSpec trace_file = m_trace_thread->GetTraceFile();

llvm::ArrayRef



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:297
+
+llvm::Expected> LiveThreadDecoder::GetRawTrace() {
+  return m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());

llvm::ArrayRef



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:40
+  /// raw trace from the thread buffer as bytes.
+  virtual llvm::Expected> GetRawTrace() = 0;
+

This data is quite large, can we just get a reference to the data without 
making a copy by using llvm::ArrayRef?



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:70
 
+  llvm::Expected> GetRawTrace() override;
+

llvm::ArrayRef?



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:88
 
+  llvm::Expected> GetRawTrace() override;
+

llvm::ArrayRef



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:72
+llvm::Error TraceIntelPT::SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) {
+  RefreshLiveProcessState();

remove process_sp and use "m_live_process"



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:78
+
+llvm::Expected>
+TraceIntelPT::GetThreadBuffer(Thread &thread) {

llvm::ArrayRef



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:25
+  llvm::Error SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) override;
+

remove process



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:27
+
+  llvm::Expected> GetThreadBuffer(Thread &thread);
+

llvm::ArrayRef


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

https://reviews.llvm.org/D107669

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

[Lldb-commits] [PATCH] D107674: [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command

2021-08-09 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/test/API/commands/trace/TestTraceExport.py:68-69
+
+if os.path.exists(ctf_test_file):
+remove_file(ctf_test_file)
+self.expect(f"thread trace export ctf --file {ctf_test_file}")

you don't need this. Whenever the test runs, all previous data will be wiped out


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107674

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-09 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/include/lldb/Target/Trace.h:75
+  virtual llvm::Error SaveToDisk(FileSpec directory,
+ lldb::ProcessSP &process_sp) = 0;
+

clayborg wrote:
> This class will already have a process in "Process *m_live_process" member 
> variable, so no need to pass in the process. This will be NULL if we loaded a 
> trace from disk already, but that should be ok because it was already loaded 
> from something on disk.
Fair enough. Then better rename this method to SaveLiveProcessTraceToDisk or 
SaveLiveTraceToDisk, and make a comment mentioning that this will return an 
error if this is not tracing a live process.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:36
 
+  /// Get thr raw trace of the thread.
+  ///

thr -> the



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:40
+  /// raw trace from the thread buffer as bytes.
+  virtual llvm::Expected> GetRawTrace() = 0;
+

clayborg wrote:
> This data is quite large, can we just get a reference to the data without 
> making a copy by using llvm::ArrayRef?
Sadly this can't work. LiveThreadDecoder::GetRawTrace() gets the raw trace from 
lldb-server and puts it in a vector without any object owning it. So the actual 
vector will need to be returned because no one wants to own it. It's not that 
bad though, because Expected just moves the inner data without creating copies.



Comment at: lldb/test/API/commands/trace/TestTraceSave.py:38-43
+self.expect("process trace save -d " +
+os.path.join(self.getSourceDir(), "intelpt-trace", 
"trace_copy_dir"))
+# Load the trace just saved
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", 
"trace_copy_dir", "trace.json"),
+substrs=["intel-pt"])

use self.getBuildDir() instead of self.getSourceDir() for the files were you 
will store data



Comment at: lldb/test/API/commands/trace/TestTraceSave.py:69-70
+
+#remove 
+shutil.rmtree(os.path.join(self.getSourceDir(), "intelpt-trace", 
"trace_copy_dir"))

you don't need to do this. Every time the test runs all the previous data will 
be wiped out


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

https://reviews.llvm.org/D107669

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-09 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:267-281
+llvm::Expected> PostMortemThreadDecoder::GetRawTrace() {
+  FileSpec trace_file = m_trace_thread->GetTraceFile();
+  ErrorOr> trace_or_error =
+  MemoryBuffer::getFile(trace_file.GetPath());
+  if (std::error_code err = trace_or_error.getError())
+return errorCodeToError(err);
+  MemoryBuffer &trace = **trace_or_error;

clayborg wrote:
> llvm::ArrayRef
Given that now we will just make it work for live processes, you can revert 
these changes and just invoke TraceIntelPT::GetLiveThreadBuffer directly



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:277
+  trace.getBufferSize());
+  std::vector result;
+  for (size_t i = 0; i < trace_data.size(); i++)

result.reserve(trace_data.size())



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h:40
+  /// raw trace from the thread buffer as bytes.
+  virtual llvm::Expected> GetRawTrace() = 0;
+

wallace wrote:
> clayborg wrote:
> > This data is quite large, can we just get a reference to the data without 
> > making a copy by using llvm::ArrayRef?
> Sadly this can't work. LiveThreadDecoder::GetRawTrace() gets the raw trace 
> from lldb-server and puts it in a vector without any object owning it. So the 
> actual vector will need to be returned because no one wants to own it. It's 
> not that bad though, because Expected just moves the inner data without 
> creating copies.
Another option is to store the raw trace in the ThreadDecoder itself, though 
might not be that useful because decoding happens only once and saving to disk 
as well.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp:1
+//===-- TraceIntelPTJSONStructs.cpp -===//
+//

this shuold have the same length as line 7



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:33-48
+  if (std::error_code ec =
+  sys::fs::create_directories(directory.GetPath().c_str())) {
+return llvm::errorCodeToError(ec);
+  }
+
+  llvm::Expected json_trace_intel_pt_trace =
+  BuildTraceSection(trace_ipt);

remove braces for one-line if statements



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:50
+
+  struct JSONTraceIntelPTSchema json_trace_intel_pt_schema(
+  json_trace_intel_pt_trace.get(), json_trace_session_base.get());

remove the word struct



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:65-66
+  std::ofstream os(trace_path.GetPath());
+  os << out;
+  return;
+}

for checking errors while writing, do this:

  os << out;
  os.close();
  if (!os)
return createStringError(unconvertibleErrorCode(), formatv("couldn't write 
to the file {0}", trace_path.getPath());
  return Error::success(); 

then you have to make this method return llvm::Error



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:72-74
+  if (!cpu_info) {
+return cpu_info.takeError();
+  }

remove braces



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:76
+
+  struct JSONTraceIntelPTCPUInfo json_trace_intel_pt_cpu_info(
+  static_cast(cpu_info->family),

wallace wrote:
> remove the keyword struct. It's very old-style
it'd be better to create a constructor of JSONTraceIntelPTCPUInfo that receives 
a pt_cpu object and then does this conversion inside



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:76-82
+  struct JSONTraceIntelPTCPUInfo json_trace_intel_pt_cpu_info(
+  static_cast(cpu_info->family),
+  static_cast(cpu_info->model),
+  static_cast(cpu_info->stepping),
+  cpu_info->vendor == pcv_intel ? "intel" : "Unknown");
+
+  struct JSONTraceIntelPTTrace result("intel-pt", 
json_trace_intel_pt_cpu_info);

remove the keyword struct. It's very old-style



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:93-95
+  if (!json_threads) {
+return json_threads.takeError();
+  }

remove braces for one-liners



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:99
+
+  struct JSONProcess json_process(
+  process_sp->GetID(),

ditto



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:102
+  process_sp->GetTarget().GetArchitecture().GetTriple().getTriple(),
+  json_threads.get(), json_modules);
+  result.processes.push_back(json_process);

invoke BuildModulesSection(process_sp) here direc

[Lldb-commits] [PATCH] D107674: [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command

2021-08-09 Thread Jakob Johnson via Phabricator via lldb-commits

jj10306 updated this revision to Diff 365341.
jj10306 added a comment.

- move documentation to plugin directory
- remove unnecessary checks in tests and remove test that wasn't testing 
anything different than the others
- nits


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

https://reviews.llvm.org/D107674

Files:
  lldb/docs/htr.rst
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.h
  lldb/source/Plugins/TraceExporter/docs/htr.rst
  lldb/test/API/commands/trace/TestTraceExport.py

Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- lldb/test/API/commands/trace/TestTraceExport.py
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -34,22 +34,96 @@
 substrs=["error: Process is not being traced"],
 error=True)
 
-def testExportCreatesFile(self):
+
+def testHtrBasicSuperBlockPassFullCheck(self):
+'''
+Test the BasicSuperBlock pass of HTR.
+
+This test uses a very small trace so that the expected output is digestible and
+it's possible to manually verify the behavior of the algorithm.
+
+This test exhaustively checks that each entry
+in the output JSON is equal to the expected value.
+
+'''
+
 self.expect("trace load -v " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
 substrs=["intel-pt"])
 
 ctf_test_file = self.getBuildArtifact("ctf-test.json")
 
-if os.path.exists(ctf_test_file):
-remove_file(ctf_test_file)
 self.expect(f"thread trace export ctf --file {ctf_test_file}")
 self.assertTrue(os.path.exists(ctf_test_file))
 
+with open(ctf_test_file) as f:
+data = json.load(f)
 
-def testHtrBasicSuperBlockPass(self):
 '''
-Test the BasicSuperBlock pass of HTR
+The expected JSON contained by "ctf-test.json"
+
+dur: number of instructions in the block
+
+name: load address of the first instruction of the block and the
+name of the most frequently called function from the block (if applicable)
+
+ph: 'X' for Complete events (see link to documentation below)
+
+pid: the ID of the HTR layer the blocks belong to
+
+ts: offset from the beginning of the trace for the first instruction in the block
+
+See https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.j75x71ritcoy
+for documentation on the Trace Event Format
+'''
+# Comments on the right indicate if a block is a "head" and/or "tail"
+# See BasicSuperBlockMerge in TraceHTR.h for a description of the algorithm
+expected = [
+{"dur":1,"name":"0x400511","ph":"X","pid":0,"ts":0},
+{"dur":1,"name":"0x400518","ph":"X","pid":0,"ts":1},
+{"dur":1,"name":"0x40051f","ph":"X","pid":0,"ts":2},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":3}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":4}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":5},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":6},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":7}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":8}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":9},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":10},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":11}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":12}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":13},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":14},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":15}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":16}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":17},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":18},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":19}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":20}, # tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":3}},"dur":3,"name":"0x400511","ph":"X","pid":1,"ts":0},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":3}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":5},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":7}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":9},
+{"ar

[Lldb-commits] [PATCH] D107674: [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command

2021-08-09 Thread walter erquinigo via Phabricator via lldb-commits

wallace accepted this revision.
wallace added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:394-419
+/*
+name: load address of the first instruction of the block and the name
+of the most frequently called function from the block (if applicable)
 
-{"pid", (int64_t)layer_id},
-{"tid", (int64_t)layer_id},
-});
+ph: the event type - 'X' for Complete events (see link to documentation
+below)
+

use // style comments, as mentioned here 
https://llvm.org/docs/CodingStandards.html#comment-formatting



Comment at: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp:450-457
+  /*
+  Since trace timestamps aren't yet supported in HTR, the ts (timestamp) 
and
+  dur (duration) are based on the block's offset in the trace and number of
+  instructions in the block, respectively. Using the block offset and the
+  number of instructions oversimplifies the true timing information of the
+  trace, nonetheless, these approximate timestamps/durations provide an
+  understandable visualization of the trace.

same here


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

https://reviews.llvm.org/D107674

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

[Lldb-commits] [PATCH] D107674: [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command

2021-08-09 Thread Jakob Johnson via Phabricator via lldb-commits

jj10306 updated this revision to Diff 365346.
jj10306 added a comment.

- fix comment style


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

https://reviews.llvm.org/D107674

Files:
  lldb/docs/htr.rst
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.h
  lldb/source/Plugins/TraceExporter/docs/htr.rst
  lldb/test/API/commands/trace/TestTraceExport.py

Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- lldb/test/API/commands/trace/TestTraceExport.py
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -34,22 +34,96 @@
 substrs=["error: Process is not being traced"],
 error=True)
 
-def testExportCreatesFile(self):
+
+def testHtrBasicSuperBlockPassFullCheck(self):
+'''
+Test the BasicSuperBlock pass of HTR.
+
+This test uses a very small trace so that the expected output is digestible and
+it's possible to manually verify the behavior of the algorithm.
+
+This test exhaustively checks that each entry
+in the output JSON is equal to the expected value.
+
+'''
+
 self.expect("trace load -v " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
 substrs=["intel-pt"])
 
 ctf_test_file = self.getBuildArtifact("ctf-test.json")
 
-if os.path.exists(ctf_test_file):
-remove_file(ctf_test_file)
 self.expect(f"thread trace export ctf --file {ctf_test_file}")
 self.assertTrue(os.path.exists(ctf_test_file))
 
+with open(ctf_test_file) as f:
+data = json.load(f)
 
-def testHtrBasicSuperBlockPass(self):
 '''
-Test the BasicSuperBlock pass of HTR
+The expected JSON contained by "ctf-test.json"
+
+dur: number of instructions in the block
+
+name: load address of the first instruction of the block and the
+name of the most frequently called function from the block (if applicable)
+
+ph: 'X' for Complete events (see link to documentation below)
+
+pid: the ID of the HTR layer the blocks belong to
+
+ts: offset from the beginning of the trace for the first instruction in the block
+
+See https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.j75x71ritcoy
+for documentation on the Trace Event Format
+'''
+# Comments on the right indicate if a block is a "head" and/or "tail"
+# See BasicSuperBlockMerge in TraceHTR.h for a description of the algorithm
+expected = [
+{"dur":1,"name":"0x400511","ph":"X","pid":0,"ts":0},
+{"dur":1,"name":"0x400518","ph":"X","pid":0,"ts":1},
+{"dur":1,"name":"0x40051f","ph":"X","pid":0,"ts":2},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":3}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":4}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":5},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":6},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":7}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":8}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":9},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":10},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":11}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":12}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":13},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":14},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":15}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":16}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":17},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":18},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":19}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":20}, # tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":3}},"dur":3,"name":"0x400511","ph":"X","pid":1,"ts":0},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":3}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":5},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":7}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":9},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":11}, # head, tail
+

[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization

2021-08-09 Thread Jakob Johnson via Phabricator via lldb-commits

jj10306 added a comment.

A more comprehensive test for this logic was added in 
https://reviews.llvm.org/D107674


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105741

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

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-09 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:19
+#include "lldb/lldb-types.h"
+#include "llvm/Analysis/ModuleSummaryAnalysis.h"
+#include "llvm/Support/Error.h"

this doesn't seem relevant



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:22
+#include "llvm/Support/JSON.h"
+#include 
+#include 

leave a black line before this



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:31
+
+llvm::Error TraceIntelPTSessionFileSaver::SaveToDisk(
+lldb::ProcessSP &process_sp, TraceIntelPT &trace_ipt, FileSpec directory) {

Better rename this class to TraceIntelPTSessionSaver, because this is not just 
working with a file, it's instead dealing with a directory of many files



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:56
+
+void TraceIntelPTSessionFileSaver::WriteIntelPTSchemaToFile(
+const JSONTraceIntelPTSchema &json_trace_intel_pt_schema,

To make these useful functions more generic so that they can be reused by other 
Trace technologies, you can create a file TraceSessionSaver.h/cpp in 
Plugins/Trace/common.
Then you can move this function to that file an call it 
WriteTraceSession(llvm::Value session_json, FileSpec directory)



Comment at: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:87
+llvm::Expected
+TraceIntelPTSessionFileSaver::BuildProcessesSection(lldb::ProcessSP 
&process_sp,
+TraceIntelPT &trace_ipt,

you could move BuildProcessesSection, BuildThreadsSection and 
BuildModulesSection to TraceSessionSaver.h/cpp if you provide a callback that 
receives a thread_id and returns a raw trace. Then almost all the code would be 
reusable by other Trace plug-ins


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

https://reviews.llvm.org/D107669

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

71 matches

Mail list logo