[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

2021-01-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls, krytarowski, mgorny.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This enables memory tag reading for lldb-server when
on AArch64 Linux.

This is done with a new packet "qMemTags". Which is following
the GDB memory tagging format. (currently in review)

The ability to parse this packet is indicated with the
"memory-tagging+" feature in a "qSupported" response.

The feature only indicates the ability to parse the
packets. So it is enabled for any AArch64 Linux target just
because it's the only one that will have a realistic chance
of the host having memory tagging.

lldb (client) will do further checks for specific processes
and memory ranges later.

To generalise the handling of memory tags a new interface
MemoryTagHandler has been added. This can be specialised per
tagging architecture/tagging scheme and handles things like
removing tags, changing them, diffing pointers, etc.

In lldb-server this will be accessible from the native register
context on Linux. Along with the ptrace operations for the
particular tagging scheme.

While the register context is about registers primarily,
it already holds some non register data like mmap details.
So I've chosen to add this handler there instead of inventing
another per architecture class.

This patch adds a tag handler for MTE on Arch64 Linux, so any other
platforms will simply error to show they do not support tagging.
(note that the handler isn't tied to the OS, AArch64 Linux is just
the only one supported at the moment)

The response to the qMemTags packet contains the "raw" tag data
we get back from ptrace. Clients will then use the
same memory tag handler to unpack that data in whatever form
is expected. (this part will be added in a follow up patch)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95601

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/include/lldb/Target/MemoryTagHandler.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/MemoryTagHandlerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagHandlerAArch64MTE.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/memory-tagging/Makefile
  lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
  lldb/test/API/tools/lldb-server/memory-tagging/main.c
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/MemoryTagHandlerAArch64MTETest.cpp

Index: lldb/unittests/Process/Utility/MemoryTagHandlerAArch64MTETest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/MemoryTagHandlerAArch64MTETest.cpp
@@ -0,0 +1,65 @@
+//===-- MemoryTagHandlerAArch64MTETest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Process/Utility/MemoryTagHandlerAArch64MTE.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(MemoryTagHandlerAArch64MTETest, RemoveLogicalTag) {
+  MemoryTagHandlerAArch64MTE handler;
+
+  // Should not remove surrounding bits
+  ASSERT_EQ((lldb::addr_t)0xf0f0,
+handler.RemoveLogicalTag(0xfef0));
+  // Untagged pointers are unchanged
+  ASSERT_EQ((lldb::addr_t)0x1034567812345678,
+handler.RemoveLogicalTag(0x1034567812345678));
+}
+
+TEST(MemoryTagHandlerAArch64MTETest, AlignToGranules) {
+  MemoryTagHandlerAArch64MTE handler;
+  // Reading nothing, no alignment needed
+  ASSERT_EQ(
+  MemoryTagHandlerAArch64MTE::TagRange(0, 0),
+  handler.AlignToGranules(MemoryTagHandlerAArch64MTE::TagRange(0, 0)));
+
+  // Ranges with 0 size are unchanged even if address is non 0
+  // (normally 0x1234 would be aligned to 0x1230)
+  ASSERT_EQ(
+  MemoryTagHa

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading for lldb

2021-01-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls, mgorny.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This commit adds a new command "memory tag read".
It looks much like "memory read" and mirrors its basic
behaviour.

(lldb) memory tag read new_buf_ptr new_buf_ptr+32
Logical tag: 0x9
Allocation tags:
[0x900f7ffa000, 0x900f7ffa010): 0x9
[0x900f7ffa010, 0x900f7ffa020): 0x0

(end address is optional and we default to reading 1 tag if it is omitted)

This makes use of the MemoryTagHandler previously added.
In lldb the tag handler can be accessed as a sub plugin of
the architecture plugin.

Meaning, to use this command you must have a specific architecture plugin
and that plugin must give you a valid tag handler. Otherwise memory
tagging is not supported.

During transport tags are packed in target specific ways so
lldb uses the handler to unpack those into individual tags.
For MTE ptrace already gives us one tag per byte so this is trivial.
(but you could imagine a situation where you get 2 4 bit tags
per byte and had to split them up in lldb)

In addition to getting a tag handler lldb will check that
the memory range asked for is tagged. It does this by looking
for the "mt" flag on the containing memory region.
(this lookup is done with untagged addresses and the
range is aligned to the effective range we need to read)

Target specific tests have been added to test/API/linux/aarch64/
and a test for targets that do not support memory tagging in
test/API/functionalities/memory/tag/.

The reason to split these is that the target specific test program
needs a specific toolchain, compiler arguments and includes.
It is not possible to write it in such a way that it can be used for
all the tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95602

Files:
  lldb/include/lldb/Core/Architecture.h
  lldb/include/lldb/Target/MemoryTagHandler.h
  lldb/include/lldb/Target/Process.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectMemoryTag.h
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
  lldb/source/Plugins/Architecture/AArch64/CMakeLists.txt
  lldb/source/Plugins/Architecture/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/MemoryTagHandlerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagHandlerAArch64MTE.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/memory/tag/Makefile
  lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
  lldb/test/API/functionalities/memory/tag/main.cpp
  lldb/test/API/linux/aarch64/mte_tag_read/Makefile
  lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
  lldb/test/API/linux/aarch64/mte_tag_read/main.c
  lldb/unittests/Process/Utility/MemoryTagHandlerAArch64MTETest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -657,3 +657,68 @@
   EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=0x1234"));
   EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=12345678123456789"));
 }
+
+static void
+check_qmemtags(TestClient &client, MockServer &server, size_t read_len,
+   const char *packet, llvm::StringRef response,
+   llvm::Optional> expected_tag_data) {
+  const auto &ReadMemoryTags = [&](size_t len, const char *packet,
+   llvm::StringRef response) {
+std::future result = std::async(std::launch::async, [&] {
+  return client.ReadMemoryTags(0xDEF0, read_len, 1);
+});
+
+HandlePacket(server, packet, response);
+return result.get();
+  };
+
+  auto result = ReadMemoryTags(0, packet, response);
+  if (expected_tag_data) {
+ASSERT_TRUE(result);
+llvm::ArrayRef expected(*expected_tag_data);
+llvm::ArrayRef got = result->GetData();
+ASSERT_THAT(expected, testing::ContainerEq(got));
+  } else {
+ASSERT_FALSE(result);
+  }
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) {
+  // Zero length reads are valid
+  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+  // The client layer does not check the length of the r

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

2021-01-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added reviewers: labath, omjavaid, emaste.
DavidSpickett added a subscriber: emaste.
DavidSpickett added a comment.

This goes together with https://reviews.llvm.org/D95602, I've split them to 
make review more manageable.

@emaste I saw you were doing something in BSD land around MTE, if you see any 
blockers from that side of things please say. Otherwise just FYI. Obviously 
this first version is only doing Linux so out of the box it wouldn't work but I 
think the logic is generic enough to be moved around later to support BSD(s).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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


[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading for lldb

2021-01-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added reviewers: omjavaid, palebedev, emaste.
DavidSpickett added a comment.

Couple of things I wanted to highlight for review.

I've put the tag handler on the architecture plugin, but it could also go on 
process directly like trace does. I figured tagging extensions are a per arch 
thing so it made logical sense, but code wise it does add some complexity.

The command's output is very simple and verbose, it could definitely do some 
things like not showing repeated tags:

  (lldb) memory tag read new_buf_ptr new_buf_ptr+
  Logical tag: 0x9
  Allocation tags:
  [0x900f7ffa000, 0x900f7ffa010): 0x9
  <... 9 repeats...>
  [<>, <>): 0x0

Or combine runs into the same row,

  (lldb) memory tag read new_buf_ptr new_buf_ptr+
  Logical tag: 0x9
  Allocation tags:
  [0x900f7ffa000, 0x900f7ffa000+): 0x9

So each row is not always 1 granule.

That could be added later as a flag --compact or make that the default and have 
a flag to be verbose. The command as is does the job well enough for small 
reads.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

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


[Lldb-commits] [PATCH] D77043: Use remote regnums in expedited list, value regs and invalidate regs

2021-01-28 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D77043#2515233 , @labath wrote:

> There's still one "opportunistic" snippet in `GDBRemoteRegisterContext.cpp`, 
> around line 404. Did you forget about that, or is there something special 
> about that case? (if so, then what?)

Answer inline.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:404
reg = reg_info->invalidate_regs[++idx]) {
+uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+eRegisterKindProcessPlugin, reg);

Using ConvertRegisterKindToRegisterNumber because SetRegisterIsValid requires 
register number as input unlike other instances where we needed to get register 
info.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1768
+eRegisterKindProcessPlugin, pair.first);
+gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData());
   }

Here also we used ConvertRegisterKindToRegisterNumber because 
PrivateSetRegisterValue requires register number as input unlike other 
instances where we needed to get register info.


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

https://reviews.llvm.org/D77043

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


[Lldb-commits] [lldb] 0bca9a7 - Fix lldb-vscode builds on Windows targeting POSIX

2021-01-28 Thread Walter Erquinigo via lldb-commits

Author: Walter Erquinigo
Date: 2021-01-28T09:36:13-08:00
New Revision: 0bca9a7ce2eeaa9f1d732ffbc17769560a2b236e

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

LOG: Fix lldb-vscode builds on Windows targeting POSIX

@stella.stamenova found out that lldb-vscode's Win32 macros were failing
when building on windows targetings POSIX platforms.

I'm changing these macros for LLVM_ON_UNIX, which should be more
accurate.

Added: 


Modified: 
lldb/tools/lldb-vscode/IOStream.cpp
lldb/tools/lldb-vscode/IOStream.h
lldb/tools/lldb-vscode/RunInTerminal.cpp
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/IOStream.cpp 
b/lldb/tools/lldb-vscode/IOStream.cpp
index 4b11b90b4c2e..fdbfb554aedb 100644
--- a/lldb/tools/lldb-vscode/IOStream.cpp
+++ b/lldb/tools/lldb-vscode/IOStream.cpp
@@ -8,7 +8,7 @@
 
 #include "IOStream.h"
 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 #include 
 #else
 #include 
@@ -33,7 +33,7 @@ StreamDescriptor::~StreamDescriptor() {
 return;
 
   if (m_is_socket)
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 ::closesocket(m_socket);
 #else
 ::close(m_socket);
@@ -108,7 +108,7 @@ bool InputStream::read_full(std::ofstream *log, size_t 
length,
 }
 if (bytes_read < 0) {
   int reason = 0;
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
   if (descriptor.m_is_socket)
 reason = WSAGetLastError();
   else

diff  --git a/lldb/tools/lldb-vscode/IOStream.h 
b/lldb/tools/lldb-vscode/IOStream.h
index 603ae9adcc2a..1ec7ac3ed0f9 100644
--- a/lldb/tools/lldb-vscode/IOStream.h
+++ b/lldb/tools/lldb-vscode/IOStream.h
@@ -9,7 +9,9 @@
 #ifndef LLDB_TOOLS_LLDB_VSCODE_IOSTREAM_H
 #define LLDB_TOOLS_LLDB_VSCODE_IOSTREAM_H
 
-#if defined(_WIN32)
+#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
+
+#if !LLVM_ON_UNIX
 // We need to #define NOMINMAX in order to skip `min()` and `max()` macro
 // definitions that conflict with other system headers.
 // We also need to #undef GetObject (which is defined to GetObjectW) because

diff  --git a/lldb/tools/lldb-vscode/RunInTerminal.cpp 
b/lldb/tools/lldb-vscode/RunInTerminal.cpp
index 4db2806924ca..29edf5ca381d 100644
--- a/lldb/tools/lldb-vscode/RunInTerminal.cpp
+++ b/lldb/tools/lldb-vscode/RunInTerminal.cpp
@@ -6,7 +6,9 @@
 //
 
//===--===//
 
-#if !defined(WIN32)
+#include "RunInTerminal.h"
+
+#if LLVM_ON_UNIX
 #include 
 #include 
 #include 
@@ -21,8 +23,6 @@
 
 #include "lldb/lldb-defines.h"
 
-#include "RunInTerminal.h"
-
 using namespace llvm;
 
 namespace lldb_vscode {

diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp 
b/lldb/tools/lldb-vscode/VSCode.cpp
index e9fdc17f4147..4d0e281c1b8d 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -14,7 +14,7 @@
 #include "VSCode.h"
 #include "llvm/Support/FormatVariadic.h"
 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 #define NOMINMAX
 #include 
 #include 
@@ -41,7 +41,7 @@ VSCode::VSCode()
   stop_at_entry(false), is_attach(false),
   reverse_request_seq(0), waiting_for_run_in_terminal(false) {
   const char *log_file_path = getenv("LLDBVSCODE_LOG");
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
   // while the value is just 10 on Darwin/Linux. Setting the file mode to 
binary
   // fixes this.

diff  --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h
index 8e7dfc078934..a2e1cac8ecf9 100644
--- a/lldb/tools/lldb-vscode/VSCode.h
+++ b/lldb/tools/lldb-vscode/VSCode.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_TOOLS_LLDB_VSCODE_VSCODE_H
 #define LLDB_TOOLS_LLDB_VSCODE_VSCODE_H
 
+#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
+
 #include 
 #include 
 #include 

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 69eb2e70aa6d..b7f39cbb1cb5 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -6,6 +6,8 @@
 //
 
//===--===//
 
+#include "VSCode.h"
+
 #include 
 #include 
 #include 
@@ -14,7 +16,7 @@
 #include 
 #include 
 #include 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 // We need to #define NOMINMAX in order to skip `min()` and `max()` macro
 // definitions that conflict with other system headers.
 // We also need to #undef GetObject (which is defined to GetObjectW) because
@@ -52,9 +54,8 @@
 
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
-#include "VSCode.h"
 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
 #endif
@@ -131,7 +132,7 @@ SOCKET AcceptConne

[Lldb-commits] [lldb] 0bca9a7 - Fix lldb-vscode builds on Windows targeting POSIX

2021-01-28 Thread Walter Erquinigo via lldb-commits

Author: Walter Erquinigo
Date: 2021-01-28T09:36:13-08:00
New Revision: 0bca9a7ce2eeaa9f1d732ffbc17769560a2b236e

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

LOG: Fix lldb-vscode builds on Windows targeting POSIX

@stella.stamenova found out that lldb-vscode's Win32 macros were failing
when building on windows targetings POSIX platforms.

I'm changing these macros for LLVM_ON_UNIX, which should be more
accurate.

Added: 


Modified: 
lldb/tools/lldb-vscode/IOStream.cpp
lldb/tools/lldb-vscode/IOStream.h
lldb/tools/lldb-vscode/RunInTerminal.cpp
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/IOStream.cpp 
b/lldb/tools/lldb-vscode/IOStream.cpp
index 4b11b90b4c2e..fdbfb554aedb 100644
--- a/lldb/tools/lldb-vscode/IOStream.cpp
+++ b/lldb/tools/lldb-vscode/IOStream.cpp
@@ -8,7 +8,7 @@
 
 #include "IOStream.h"
 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 #include 
 #else
 #include 
@@ -33,7 +33,7 @@ StreamDescriptor::~StreamDescriptor() {
 return;
 
   if (m_is_socket)
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 ::closesocket(m_socket);
 #else
 ::close(m_socket);
@@ -108,7 +108,7 @@ bool InputStream::read_full(std::ofstream *log, size_t 
length,
 }
 if (bytes_read < 0) {
   int reason = 0;
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
   if (descriptor.m_is_socket)
 reason = WSAGetLastError();
   else

diff  --git a/lldb/tools/lldb-vscode/IOStream.h 
b/lldb/tools/lldb-vscode/IOStream.h
index 603ae9adcc2a..1ec7ac3ed0f9 100644
--- a/lldb/tools/lldb-vscode/IOStream.h
+++ b/lldb/tools/lldb-vscode/IOStream.h
@@ -9,7 +9,9 @@
 #ifndef LLDB_TOOLS_LLDB_VSCODE_IOSTREAM_H
 #define LLDB_TOOLS_LLDB_VSCODE_IOSTREAM_H
 
-#if defined(_WIN32)
+#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
+
+#if !LLVM_ON_UNIX
 // We need to #define NOMINMAX in order to skip `min()` and `max()` macro
 // definitions that conflict with other system headers.
 // We also need to #undef GetObject (which is defined to GetObjectW) because

diff  --git a/lldb/tools/lldb-vscode/RunInTerminal.cpp 
b/lldb/tools/lldb-vscode/RunInTerminal.cpp
index 4db2806924ca..29edf5ca381d 100644
--- a/lldb/tools/lldb-vscode/RunInTerminal.cpp
+++ b/lldb/tools/lldb-vscode/RunInTerminal.cpp
@@ -6,7 +6,9 @@
 //
 
//===--===//
 
-#if !defined(WIN32)
+#include "RunInTerminal.h"
+
+#if LLVM_ON_UNIX
 #include 
 #include 
 #include 
@@ -21,8 +23,6 @@
 
 #include "lldb/lldb-defines.h"
 
-#include "RunInTerminal.h"
-
 using namespace llvm;
 
 namespace lldb_vscode {

diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp 
b/lldb/tools/lldb-vscode/VSCode.cpp
index e9fdc17f4147..4d0e281c1b8d 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -14,7 +14,7 @@
 #include "VSCode.h"
 #include "llvm/Support/FormatVariadic.h"
 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 #define NOMINMAX
 #include 
 #include 
@@ -41,7 +41,7 @@ VSCode::VSCode()
   stop_at_entry(false), is_attach(false),
   reverse_request_seq(0), waiting_for_run_in_terminal(false) {
   const char *log_file_path = getenv("LLDBVSCODE_LOG");
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
   // while the value is just 10 on Darwin/Linux. Setting the file mode to 
binary
   // fixes this.

diff  --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h
index 8e7dfc078934..a2e1cac8ecf9 100644
--- a/lldb/tools/lldb-vscode/VSCode.h
+++ b/lldb/tools/lldb-vscode/VSCode.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_TOOLS_LLDB_VSCODE_VSCODE_H
 #define LLDB_TOOLS_LLDB_VSCODE_VSCODE_H
 
+#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
+
 #include 
 #include 
 #include 

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 69eb2e70aa6d..b7f39cbb1cb5 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -6,6 +6,8 @@
 //
 
//===--===//
 
+#include "VSCode.h"
+
 #include 
 #include 
 #include 
@@ -14,7 +16,7 @@
 #include 
 #include 
 #include 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 // We need to #define NOMINMAX in order to skip `min()` and `max()` macro
 // definitions that conflict with other system headers.
 // We also need to #undef GetObject (which is defined to GetObjectW) because
@@ -52,9 +54,8 @@
 
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
-#include "VSCode.h"
 
-#if defined(_WIN32)
+#if !LLVM_ON_UNIX
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
 #endif
@@ -131,7 +132,7 @@ SOCKET AcceptConne

[Lldb-commits] [PATCH] D94857: [VFS] Combine VFSFromYamlDirIterImpl and OverlayFSDirIterImpl into a single implementation (NFC)

2021-01-28 Thread Nathan Hawes via Phabricator via lldb-commits
nathawes updated this revision to Diff 320031.
nathawes added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Rename `error_code` to `EC` in `shouldFallBackToExternalFS()` to match 
existing conventions in the file.
- Update a missed `RedirectingFileEntry` reference in lldb to its new name 
(`FileEntry`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94857

Files:
  lldb/source/Host/common/FileSystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -452,19 +452,27 @@
 
 namespace {
 
-class OverlayFSDirIterImpl : public llvm::vfs::detail::DirIterImpl {
-  OverlayFileSystem &Overlays;
-  std::string Path;
-  OverlayFileSystem::iterator CurrentFS;
+/// Combines and deduplicates directory entries across multiple file systems.
+class CombiningDirIterImpl : public llvm::vfs::detail::DirIterImpl {
+  using FileSystemPtr = llvm::IntrusiveRefCntPtr;
+
+  /// File systems to check for entries in. Processed in reverse order.
+  SmallVector FSList;
+  /// The directory iterator for the current filesystem.
   directory_iterator CurrentDirIter;
+  /// The path of the directory to iterate the entries of.
+  std::string DirPath;
+  /// The set of names already returned as entries.
   llvm::StringSet<> SeenNames;
 
+  /// Sets \c CurrentDirIter to an iterator of \c DirPath in the next file
+  /// system in the list, or leaves it as is (at its end position) if we've
+  /// already gone through them all.
   std::error_code incrementFS() {
-assert(CurrentFS != Overlays.overlays_end() && "incrementing past end");
-++CurrentFS;
-for (auto E = Overlays.overlays_end(); CurrentFS != E; ++CurrentFS) {
+while (!FSList.empty()) {
   std::error_code EC;
-  CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
+  CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+  FSList.pop_back();
   if (EC && EC != errc::no_such_file_or_directory)
 return EC;
   if (CurrentDirIter != directory_iterator())
@@ -500,11 +508,24 @@
   }
 
 public:
-  OverlayFSDirIterImpl(const Twine &Path, OverlayFileSystem &FS,
+  CombiningDirIterImpl(ArrayRef FileSystems, std::string Dir,
std::error_code &EC)
-  : Overlays(FS), Path(Path.str()), CurrentFS(Overlays.overlays_begin()) {
-CurrentDirIter = (*CurrentFS)->dir_begin(Path, EC);
-EC = incrementImpl(true);
+  : FSList(FileSystems.begin(), FileSystems.end()),
+DirPath(std::move(Dir)) {
+if (!FSList.empty()) {
+  CurrentDirIter = FSList.back()->dir_begin(DirPath, EC);
+  FSList.pop_back();
+  if (!EC || EC == errc::no_such_file_or_directory)
+EC = incrementImpl(true);
+}
+  }
+
+  CombiningDirIterImpl(directory_iterator FirstIter, FileSystemPtr Fallback,
+   std::string FallbackDir, std::error_code &EC)
+  : FSList({Fallback}), CurrentDirIter(FirstIter),
+DirPath(std::move(FallbackDir)) {
+if (!EC || EC == errc::no_such_file_or_directory)
+  EC = incrementImpl(true);
   }
 
   std::error_code increment() override { return incrementImpl(false); }
@@ -515,7 +536,7 @@
 directory_iterator OverlayFileSystem::dir_begin(const Twine &Dir,
 std::error_code &EC) {
   return directory_iterator(
-  std::make_shared(Dir, *this, EC));
+  std::make_shared(FSList, Dir.str(), EC));
 }
 
 void ProxyFileSystem::anchor() {}
@@ -1019,48 +1040,47 @@
 }
 }
 
-// FIXME: reuse implementation common with OverlayFSDirIterImpl as these
-// iterators are conceptually similar.
-class llvm::vfs::VFSFromYamlDirIterImpl
+/// Directory iterator implementation for \c RedirectingFileSystem's
+/// directory entries.
+class llvm::vfs::RedirectingFSDirIterImpl
 : public llvm::vfs::detail::DirIterImpl {
   std::string Dir;
-  RedirectingFileSystem::RedirectingDirectoryEntry::iterator Current, End;
-
-  // To handle 'fallthrough' mode we need to iterate at first through
-  // RedirectingDirectoryEntry and then through ExternalFS. These operations are
-  // done sequentially, we just need to keep a track of what kind of iteration
-  // we are currently performing.
-
-  /// Flag telling if we should iterate through ExternalFS or stop at the last
-  /// RedirectingDirectoryEntry::iterator.
-  bool IterateExternalFS;
-  /// Flag telling if we have switched to iterating through ExternalFS.
-  bool IsExternalFSCurrent = false;
-  FileSystem &ExternalFS;
-  directory_iterator ExternalDirIter;
-  llvm::StringSet<> SeenNames;
-
-  /// To combine multiple iterations, different methods are responsible for
-  /// different iteration steps.
-  /// @{
+  RedirectingFileSystem

[Lldb-commits] [lldb] b2545b7 - [lldb] Use `foo is None` instead of `not foo` in darwin.py

2021-01-28 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-01-28T20:03:44-08:00
New Revision: b2545b71d121ac913e56faff3b704f3957f941b7

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

LOG: [lldb] Use `foo is None` instead of `not foo` in darwin.py

Explicitly compare to None when checking the triple's components so we
don't bail out when one of them is the empty string.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/darwin.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/darwin.py 
b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
index fd25c0c2f115..c4a057e7158a 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -43,12 +43,12 @@ def get_triple():
 
 # Get the SDK from the os and env.
 sdk = lldbutil.get_xcode_sdk(os, env)
-if not sdk:
+if sdk is None:
 return None, None, None, None
 
 # Get the version from the SDK.
 version = lldbutil.get_xcode_sdk_version(sdk)
-if not version:
+if version is None:
 return None, None, None, None
 
 return vendor, os, version, env
@@ -86,7 +86,7 @@ def getArchCFlags(self, arch):
 """Returns the ARCH_CFLAGS for the make system."""
 # Get the triple components.
 vendor, os, version, env = get_triple()
-if not vendor or not os or not version or not env:
+if vendor is None or os is None or version is None or env is None:
 return ""
 
 # Construct the triple from its components.



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