[Lldb-commits] [lldb] c62ef12 - [lldb] [test] Mark more lldb-server tests xfail on Windows

2021-04-12 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-03-30T18:49:04+02:00
New Revision: c62ef12079bcc7ce72040dddaae13408b120d995

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

LOG: [lldb] [test] Mark more lldb-server tests xfail on Windows

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 6677cfff84837..0764e3d6c75a6 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -426,16 +426,19 @@ def Hg_fails_on_pid(self, pass_pid):
 
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(oslist=["windows"])
 def test_Hg_fails_on_another_pid(self):
 self.build()
 self.set_inferior_startup_launch()
 self.Hg_fails_on_pid(1)
 
+@expectedFailureAll(oslist=["windows"])
 def test_Hg_fails_on_zero_pid(self):
 self.build()
 self.set_inferior_startup_launch()
 self.Hg_fails_on_pid(0)
 
+@expectedFailureAll(oslist=["windows"])
 def test_Hg_fails_on_minus_one_pid(self):
 self.build()
 self.set_inferior_startup_launch()



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


[Lldb-commits] [lldb] e3d3327 - [lldb] Remove reproducer from previous test run

2021-04-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-03-30T10:11:20-07:00
New Revision: e3d3327edbf133da6ed50767eed4560a541a751d

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

LOG: [lldb] Remove reproducer from previous test run

Added: 


Modified: 
lldb/test/Shell/Reproducer/Functionalities/TestImageList.test

Removed: 




diff  --git a/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test 
b/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
index ec8b36ea9576c..e630e4666d087 100644
--- a/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
+++ b/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
@@ -6,6 +6,7 @@
 # RUN: %clang_host %S/Inputs/stepping.c -g -o %t.out
 
 # RUN: rm -rf %t.txt
+# RUN: rm -rf %t.repro
 
 # RUN: echo "CAPTURE" >> %t.txt
 # RUN: %lldb -x -b  --capture --capture-path %t.repro \



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


[Lldb-commits] [lldb] 428b17c - [LLDB] Fix buildbots breakage due to TestGuessLanguage.py

2021-04-12 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2021-04-12T15:10:46+05:00
New Revision: 428b17ce70523b1c54cb89095fed39e84f5ca8fc

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

LOG: [LLDB] Fix buildbots breakage due to TestGuessLanguage.py

Fix LLDB buidbot breakage due to D99250.

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

Added: 


Modified: 
lldb/test/API/commands/frame/language/TestGuessLanguage.py

Removed: 




diff  --git a/lldb/test/API/commands/frame/language/TestGuessLanguage.py 
b/lldb/test/API/commands/frame/language/TestGuessLanguage.py
index 7b3515715e3bc..0974a0e1f858b 100644
--- a/lldb/test/API/commands/frame/language/TestGuessLanguage.py
+++ b/lldb/test/API/commands/frame/language/TestGuessLanguage.py
@@ -70,7 +70,7 @@ def do_test(self):
 thread = threads[0]
 
 c_frame_language = lldb.eLanguageTypeC99
-cxx_frame_language = lldb.eLanguageTypeC_plus_plus_11
+cxx_frame_language = lldb.eLanguageTypeC_plus_plus
 # gcc emits DW_LANG_C89 even if -std=c99 was specified
 if "gcc" in self.getCompiler():
 c_frame_language = lldb.eLanguageTypeC89



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


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

2021-04-12 Thread Diana Picus via Phabricator via lldb-commits
rovka added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:843
-  response.PutCString(";QThreadSuffixSupported+");
-  response.PutCString(";QListThreadsInStopReply+");
-  response.PutCString(";qEcho+");

This seems to have disappeared, shouldn't it be in 
GDBRemoteCommunicationServerCommon::HandleFeatures?


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

https://reviews.llvm.org/D100140

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


[Lldb-commits] [PATCH] D100146: [lldb] [gdb-remote client] Refactor handling qSupported

2021-04-12 Thread Diana Picus via Phabricator via lldb-commits
rovka accepted this revision.
rovka added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D100146

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


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

2021-04-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 336808.
mgorny marked an inline comment as done.
mgorny added a comment.

Fix missing `QListThreadsInStopReply+`.


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

https://reviews.llvm.org/D100140

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -219,6 +219,9 @@
 
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features) override;
+
 private:
   llvm::Expected> BuildTargetXml();
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3534,3 +3534,16 @@
 
   return tid;
 }
+
+std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures(
+const llvm::ArrayRef client_features) {
+  auto ret =
+  GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
+  ret.insert(ret.end(), {
+"qXfer:features:read+", "multiprocess+",
+#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
+"QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
+#endif
+  });
+  return ret;
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -145,6 +145,11 @@
   virtual FileSpec FindModuleFile(const std::string &module_path,
   const ArchSpec &arch);
 
+  // Process client_features (qSupported) and return an array of server features
+  // to be returned in response.
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features);
+
 private:
   ModuleSpec GetModuleInfo(llvm::StringRef module_path, llvm::StringRef triple);
 };
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
@@ -831,26 +831,11 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_qSupported(
 StringExtractorGDBRemote &packet) {
-  StreamGDBRemote response;
-
-  // Features common to lldb-platform and llgs.
-  uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet
- // size--debugger can always use less
-  response.Printf("PacketSize=%x", max_packet_size);
-
-  response.PutCString(";QStartNoAckMode+");
-  response.PutCString(";QThreadSuffixSupported+");
-  response.PutCString(";QListThreadsInStopReply+");
-  response.PutCString(";qEcho+");
-  response.PutCString(";qXfer:features:read+");
-#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
-  response.PutCString(";QPassSignals+");
-  response.PutCString(";qXfer:auxv:read+");
-  response.PutCString(";qXfer:libraries-svr4:read+");
-#endif
-  response.PutCString(";multiprocess+");
-
-  return SendPacketNoLock(response.GetString());
+  // Parse client-indicated features.
+  llvm::StringRef view = packet.GetStringRef();
+  llvm::SmallVector client_features;
+  view.split(client_features, ';');
+  return SendPacketNoLock(llvm::join(HandleFeatures(client_features), ";"));
 }
 
 GDBRemoteCommunication::PacketResult
@@ -1312,3 +1297,18 @@
 
   return matched_module_spec;
 }
+
+std::vector GDBRemoteCommunicationServerCommon::HandleFeatures(
+const llvm::ArrayRef client_features) {
+  // 128KBytes is a reasonable max packet size--debugger can always use less.
+  constexpr uint32_t max_packet_size = 128 * 1024;
+
+  // Features common to platform server and llgs.
+  return {
+  llvm::formatv("PacketSize={0}", max_packet_size),
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",
+  };
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

2021-04-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:843
-  response.PutCString(";QThreadSuffixSupported+");
-  response.PutCString(";QListThreadsInStopReply+");
-  response.PutCString(";qEcho+");

rovka wrote:
> This seems to have disappeared, shouldn't it be in 
> GDBRemoteCommunicationServerCommon::HandleFeatures?
My mistake, sorry. Fixed now.


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

https://reviews.llvm.org/D100140

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


[Lldb-commits] [lldb] 34c697c - [lldb] Don't recursively load types of static member variables in the DWARF AST parser

2021-04-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-04-12T14:37:07+02:00
New Revision: 34c697c85e9d0af11a72ac4df5578aac94a627b3

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

LOG: [lldb] Don't recursively load types of static member variables in the 
DWARF AST parser

When LLDB's DWARF parser is parsing the member DIEs of a struct/class it
currently fully resolves the types of static member variables in a class before
adding the respective `VarDecl` to the record.

For record types fully resolving the type will also parse the member DIEs of the
respective class. The other way of resolving is just 'forward' resolving the 
type
which will try to load only the minimum amount of information about the type
(for records that would only be the name/kind of the type). Usually we always
resolve types on-demand so it's rarely useful to speculatively fully resolve
them on the first use.

This patch changes makes that we only 'forward' resolve the types of static
members. This solves the fact that LLDB unnecessarily loads debug information
to parse the type if it's maybe not needed later and it also avoids a crash 
where
the parsed type might in turn reference the surrounding class that is currently
being parsed.

The new test case demonstrates the crash that might happen. The crash happens
with the following steps:

1. We parse class `ToLayout` and it's members.

2. We parse the static class member and fully resolve its type
(`DependsOnParam2`).

3. That type has a non-static class member `DependsOnParam1` for which
LLDB will try to calculate the size.

4. The layout (and size)`DependsOnParam1` turns depends on the
`ToLayout` size/layout.

5. Clang will calculate the record layout/size for `ToLayout` even though we are
currently parsing it and it's missing it's non-static member.

The created is missing the offset for the yet unparsed non-static member. If we
later try to get the offset we end up hitting different asserts. Most common is
the one in `TypeSystemClang::DumpValue` where it checks that the record layout
has offsets for the current FieldDecl.

```
assert(field_idx < record_layout.getFieldCount());
```

Fixed rdar://67910011

Reviewed By: shafik

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

Added: 
lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/Makefile

lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/TestStaticMemberTypeDependingOnParentSize.py
lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/main.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
lldb/test/API/functionalities/lazy-loading/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index af01a8f535184..b9e10f94bf6cc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2534,7 +2534,7 @@ void DWARFASTParserClang::ParseSingleMember(
   if (accessibility == eAccessNone)
 accessibility = eAccessPublic;
   TypeSystemClang::AddVariableToRecordType(
-  class_clang_type, name, var_type->GetLayoutCompilerType(),
+  class_clang_type, name, var_type->GetForwardCompilerType(),
   accessibility);
 }
 return;

diff  --git a/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py 
b/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
index 4dceb3718..326315c838e5c 100644
--- a/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ b/lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -40,6 +40,7 @@ def setUp(self):
 class_in_namespace_decl = [class_decl_kind, "ClassInNamespace"]
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
+class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -56,6 +57,7 @@ def assert_no_decls_loaded(self):
 self.assert_decl_not_loaded(self.other_struct_decl)
 self.assert_decl_not_loaded(self.class_in_namespace_decl)
 self.assert_decl_not_loaded(self.class_member_decl)
+self.assert_decl_not_loaded(self.class_static_member_decl)
 self.assert_decl_not_loaded(self.unused_class_member_decl)
 
 def get_ast_dump(self):
@@ -228,6 +230,8 @@ def test_class_function_access_member(self):
 self.assert_decl_not_completed(self.unused_class_member_ptr_decl)
 # We loaded th

[Lldb-commits] [PATCH] D100180: [lldb] Don't recursively load types of static member variables in the DWARF AST parser

2021-04-12 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 rG34c697c85e9d: [lldb] Don't recursively load types of 
static member variables in the DWARF AST… (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100180

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
  lldb/test/API/functionalities/lazy-loading/main.cpp
  lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/Makefile
  
lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/TestStaticMemberTypeDependingOnParentSize.py
  lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/main.cpp

Index: lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/main.cpp
@@ -0,0 +1,28 @@
+// This class just serves as an indirection between LLDB and Clang. LLDB might
+// be tempted to check the member type of DependsOnParam2 for whether it's
+// in some 'currently-loading' state before trying to produce the record layout.
+// By inheriting from ToLayout this will make LLDB just check if
+// DependsOnParam1 is currently being loaded (which it's not) but it will
+template  struct DependsOnParam1 : ToLayoutParam {};
+// This class forces the memory layout of it's type parameter to be created.
+template  struct DependsOnParam2 {
+  DependsOnParam1 m;
+};
+
+// This is the class that LLDB has to generate the record layout for.
+struct ToLayout {
+  // A static member variable which memory layout depends on the surrounding
+  // class. This comes first so that if we accidentially generate the layout
+  // for static member types we end up recursively going back to 'ToLayout'
+  // before 'some_member' has been loaded.
+  static DependsOnParam2 a_static_member;
+  // Some dummy member variable. This is only there so that Clang can detect
+  // that the record layout is inconsistent (i.e., the number of fields in the
+  // layout doesn't fit to the fields in the declaration).
+  int some_member;
+};
+DependsOnParam2 ToLayout::a_static_member;
+
+ToLayout test_var;
+
+int main() { return test_var.some_member; }
Index: lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/TestStaticMemberTypeDependingOnParentSize.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/TestStaticMemberTypeDependingOnParentSize.py
@@ -0,0 +1,22 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+"""
+This tests a static member with a type which size depends on the
+surrounding class. LLDB should *not* try to generate the record layout
+for those types while parsing the members from debug info.
+"""
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+# Force the record layout for 'ToLayout' to be generated by printing
+# a value of it's type.
+self.expect("target variable test_var")
Index: lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/static_member_type_depending_on_parent_size/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -23,6 +23,7 @@
 // Class loading declarations.
 
 struct ClassMember { int i; };
+struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
 
@@ -34,12 +35,14 @@
 public:
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
+  static StaticClassMember static_member;
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
 return member.i; // Location: class function
   }
 };
+StaticClassMember ClassWeEnter::static_member;
 };
 
 ////
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
===
--- lldb/test/API/functionalities/lazy-loa

[Lldb-commits] [PATCH] D100212: [lldb] Delete dead StackFrameList::Merge

2021-04-12 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a5a94ed34b0: [lldb] Delete dead StackFrameList::Merge 
(authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100212

Files:
  lldb/include/lldb/Target/StackFrameList.h
  lldb/source/Target/StackFrameList.cpp

Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -823,105 +823,6 @@
   m_concrete_frames_fetched = 0;
 }
 
-void StackFrameList::Merge(std::unique_ptr &curr_up,
-   lldb::StackFrameListSP &prev_sp) {
-  std::unique_lock current_lock, previous_lock;
-  if (curr_up)
-current_lock = std::unique_lock(curr_up->m_mutex);
-  if (prev_sp)
-previous_lock = std::unique_lock(prev_sp->m_mutex);
-
-#if defined(DEBUG_STACK_FRAMES)
-  StreamFile s(stdout, false);
-  s.PutCString("\n\nStackFrameList::Merge():\nPrev:\n");
-  if (prev_sp)
-prev_sp->Dump(&s);
-  else
-s.PutCString("NULL");
-  s.PutCString("\nCurr:\n");
-  if (curr_up)
-curr_up->Dump(&s);
-  else
-s.PutCString("NULL");
-  s.EOL();
-#endif
-
-  if (!curr_up || curr_up->GetNumFrames(false) == 0) {
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString("No current frames, leave previous frames alone...\n");
-#endif
-curr_up.release();
-return;
-  }
-
-  if (!prev_sp || prev_sp->GetNumFrames(false) == 0) {
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString("No previous frames, so use current frames...\n");
-#endif
-// We either don't have any previous frames, or since we have more than one
-// current frames it means we have all the frames and can safely replace
-// our previous frames.
-prev_sp.reset(curr_up.release());
-return;
-  }
-
-  const uint32_t num_curr_frames = curr_up->GetNumFrames(false);
-
-  if (num_curr_frames > 1) {
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString(
-"We have more than one current frame, so use current frames...\n");
-#endif
-// We have more than one current frames it means we have all the frames and
-// can safely replace our previous frames.
-prev_sp.reset(curr_up.release());
-
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString("\nMerged:\n");
-prev_sp->Dump(&s);
-#endif
-return;
-  }
-
-  StackFrameSP prev_frame_zero_sp(prev_sp->GetFrameAtIndex(0));
-  StackFrameSP curr_frame_zero_sp(curr_up->GetFrameAtIndex(0));
-  StackID curr_stack_id(curr_frame_zero_sp->GetStackID());
-  StackID prev_stack_id(prev_frame_zero_sp->GetStackID());
-
-#if defined(DEBUG_STACK_FRAMES)
-  const uint32_t num_prev_frames = prev_sp->GetNumFrames(false);
-  s.Printf("\n%u previous frames with one current frame\n", num_prev_frames);
-#endif
-
-  // We have only a single current frame
-  // Our previous stack frames only had a single frame as well...
-  if (curr_stack_id == prev_stack_id) {
-#if defined(DEBUG_STACK_FRAMES)
-s.Printf("\nPrevious frame #0 is same as current frame #0, merge the "
- "cached data\n");
-#endif
-
-curr_frame_zero_sp->UpdateCurrentFrameFromPreviousFrame(
-*prev_frame_zero_sp);
-//prev_frame_zero_sp->UpdatePreviousFrameFromCurrentFrame
-//(*curr_frame_zero_sp);
-//prev_sp->SetFrameAtIndex (0, prev_frame_zero_sp);
-  } else if (curr_stack_id < prev_stack_id) {
-#if defined(DEBUG_STACK_FRAMES)
-s.Printf("\nCurrent frame #0 has a stack ID that is less than the previous "
- "frame #0, insert current frame zero in front of previous\n");
-#endif
-prev_sp->m_frames.insert(prev_sp->m_frames.begin(), curr_frame_zero_sp);
-  }
-
-  curr_up.release();
-
-#if defined(DEBUG_STACK_FRAMES)
-  s.PutCString("\nMerged:\n");
-  prev_sp->Dump(&s);
-#endif
-}
-
 lldb::StackFrameSP
 StackFrameList::GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr) {
   const_iterator pos;
Index: lldb/include/lldb/Target/StackFrameList.h
===
--- lldb/include/lldb/Target/StackFrameList.h
+++ lldb/include/lldb/Target/StackFrameList.h
@@ -89,9 +89,6 @@
 
   bool SetFrameAtIndex(uint32_t idx, lldb::StackFrameSP &frame_sp);
 
-  static void Merge(std::unique_ptr &curr_up,
-lldb::StackFrameListSP &prev_sp);
-
   void GetFramesUpTo(uint32_t end_idx);
 
   void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind &unwinder);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 5a5a94e - [lldb] Delete dead StackFrameList::Merge

2021-04-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-04-12T14:49:20+02:00
New Revision: 5a5a94ed34b07079046ac81e7e97d980ce2c834f

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

LOG: [lldb] Delete dead StackFrameList::Merge

That code is unused since it's check-in in 2010 (and I believe it would leak
memory when called as it releases the passed unique_ptr), so let's delete it.

Reviewed By: vsk

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

Added: 


Modified: 
lldb/include/lldb/Target/StackFrameList.h
lldb/source/Target/StackFrameList.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/StackFrameList.h 
b/lldb/include/lldb/Target/StackFrameList.h
index 1b0b986d7059d..c98995cad36fd 100644
--- a/lldb/include/lldb/Target/StackFrameList.h
+++ b/lldb/include/lldb/Target/StackFrameList.h
@@ -89,9 +89,6 @@ class StackFrameList {
 
   bool SetFrameAtIndex(uint32_t idx, lldb::StackFrameSP &frame_sp);
 
-  static void Merge(std::unique_ptr &curr_up,
-lldb::StackFrameListSP &prev_sp);
-
   void GetFramesUpTo(uint32_t end_idx);
 
   void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind &unwinder);

diff  --git a/lldb/source/Target/StackFrameList.cpp 
b/lldb/source/Target/StackFrameList.cpp
index 95ebfb58a7ec0..ed40356bef604 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -823,105 +823,6 @@ void StackFrameList::Clear() {
   m_concrete_frames_fetched = 0;
 }
 
-void StackFrameList::Merge(std::unique_ptr &curr_up,
-   lldb::StackFrameListSP &prev_sp) {
-  std::unique_lock current_lock, previous_lock;
-  if (curr_up)
-current_lock = std::unique_lock(curr_up->m_mutex);
-  if (prev_sp)
-previous_lock = std::unique_lock(prev_sp->m_mutex);
-
-#if defined(DEBUG_STACK_FRAMES)
-  StreamFile s(stdout, false);
-  s.PutCString("\n\nStackFrameList::Merge():\nPrev:\n");
-  if (prev_sp)
-prev_sp->Dump(&s);
-  else
-s.PutCString("NULL");
-  s.PutCString("\nCurr:\n");
-  if (curr_up)
-curr_up->Dump(&s);
-  else
-s.PutCString("NULL");
-  s.EOL();
-#endif
-
-  if (!curr_up || curr_up->GetNumFrames(false) == 0) {
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString("No current frames, leave previous frames alone...\n");
-#endif
-curr_up.release();
-return;
-  }
-
-  if (!prev_sp || prev_sp->GetNumFrames(false) == 0) {
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString("No previous frames, so use current frames...\n");
-#endif
-// We either don't have any previous frames, or since we have more than one
-// current frames it means we have all the frames and can safely replace
-// our previous frames.
-prev_sp.reset(curr_up.release());
-return;
-  }
-
-  const uint32_t num_curr_frames = curr_up->GetNumFrames(false);
-
-  if (num_curr_frames > 1) {
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString(
-"We have more than one current frame, so use current frames...\n");
-#endif
-// We have more than one current frames it means we have all the frames and
-// can safely replace our previous frames.
-prev_sp.reset(curr_up.release());
-
-#if defined(DEBUG_STACK_FRAMES)
-s.PutCString("\nMerged:\n");
-prev_sp->Dump(&s);
-#endif
-return;
-  }
-
-  StackFrameSP prev_frame_zero_sp(prev_sp->GetFrameAtIndex(0));
-  StackFrameSP curr_frame_zero_sp(curr_up->GetFrameAtIndex(0));
-  StackID curr_stack_id(curr_frame_zero_sp->GetStackID());
-  StackID prev_stack_id(prev_frame_zero_sp->GetStackID());
-
-#if defined(DEBUG_STACK_FRAMES)
-  const uint32_t num_prev_frames = prev_sp->GetNumFrames(false);
-  s.Printf("\n%u previous frames with one current frame\n", num_prev_frames);
-#endif
-
-  // We have only a single current frame
-  // Our previous stack frames only had a single frame as well...
-  if (curr_stack_id == prev_stack_id) {
-#if defined(DEBUG_STACK_FRAMES)
-s.Printf("\nPrevious frame #0 is same as current frame #0, merge the "
- "cached data\n");
-#endif
-
-curr_frame_zero_sp->UpdateCurrentFrameFromPreviousFrame(
-*prev_frame_zero_sp);
-//prev_frame_zero_sp->UpdatePreviousFrameFromCurrentFrame
-//(*curr_frame_zero_sp);
-//prev_sp->SetFrameAtIndex (0, prev_frame_zero_sp);
-  } else if (curr_stack_id < prev_stack_id) {
-#if defined(DEBUG_STACK_FRAMES)
-s.Printf("\nCurrent frame #0 has a stack ID that is less than the previous 
"
- "frame #0, insert current frame zero in front of previous\n");
-#endif
-prev_sp->m_frames.insert(prev_sp->m_frames.begin(), curr_frame_zero_sp);
-  }
-
-  curr_up.release();
-
-#if defined(DEBUG_STACK_FRAMES)
-  s.PutCString("\nMerged:\n");
-  prev_sp->Dump(&s);
-#endif
-}
-
 lldb::StackFrameSP
 StackFrameList::GetStackFrameSPForStackFrame

[Lldb-commits] [PATCH] D100193: [lldb] Require x86 backend for a bunch of DWARF tests

2021-04-12 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.

Thanks, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100193

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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 created this revision.
augusto2112 added reviewers: aprantl, jasonmolenda.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100338

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -175,6 +175,9 @@
   def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
 DefaultFalse,
 Desc<"Enable debugging of LLDB-internal utility expressions.">;
+  def FetchReadonlySectionsFromFileCache: 
Property<"fetch-readonly-sections-from-file-cache", "Boolean">,
+DefaultTrue,
+Desc<"Enables reading bytes from the file cache instead of process when 
the address LLDB is reading from falls in a readable but not writable section">;
 }
 
 let Definition = "process_experimental" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1753,6 +1753,15 @@
   if (!resolved_addr.IsValid())
 resolved_addr = addr;
 
+  if (GetFetchReadonlySectionsFromFileCache()) {
+SectionSP section_sp(addr.GetSection());
+if (section_sp) {
+  auto permissions = section_sp->GetPermissions();
+  prefer_file_cache |= (permissions & ePermissionsWritable) == 0 &&
+  (permissions & ePermissionsReadable) == 1;
+}
+  }
+
   if (prefer_file_cache) {
 bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
 if (bytes_read > 0)
@@ -4355,6 +4364,17 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, debug);
 }
 
+bool TargetProperties::GetFetchReadonlySectionsFromFileCache() const {
+  const uint32_t idx = ePropertyFetchReadonlySectionsFromFileCache;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
+void TargetProperties::SetFetchReadonlySectionsFromFileCache(bool b) {
+  const uint32_t idx = ePropertyFetchReadonlySectionsFromFileCache;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+}
+
 // Target::TargetEventData
 
 Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp)
Index: lldb/include/lldb/Target/Target.h
===
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -231,6 +231,11 @@
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetFetchReadonlySectionsFromFileCache() const;
+
+  void SetFetchReadonlySectionsFromFileCache(bool b);
+
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -175,6 +175,9 @@
   def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
 DefaultFalse,
 Desc<"Enable debugging of LLDB-internal utility expressions.">;
+  def FetchReadonlySectionsFromFileCache: Property<"fetch-readonly-sections-from-file-cache", "Boolean">,
+DefaultTrue,
+Desc<"Enables reading bytes from the file cache instead of process when the address LLDB is reading from falls in a readable but not writable section">;
 }
 
 let Definition = "process_experimental" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1753,6 +1753,15 @@
   if (!resolved_addr.IsValid())
 resolved_addr = addr;
 
+  if (GetFetchReadonlySectionsFromFileCache()) {
+SectionSP section_sp(addr.GetSection());
+if (section_sp) {
+  auto permissions = section_sp->GetPermissions();
+  prefer_file_cache |= (permissions & ePermissionsWritable) == 0 &&
+  (permissions & ePermissionsReadable) == 1;
+}
+  }
+
   if (prefer_file_cache) {
 bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
 if (bytes_read > 0)
@@ -4355,6 +4364,17 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, debug);
 }
 
+bool TargetProperties::GetFetchReadonlySectionsFromFileCache() const {
+  const uint32_t idx = ePropertyFetchReadonlySectionsFromFileCache;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
+void TargetProperties::SetFetchReadonlySectionsFromFileCache(bool b) {
+  const uint32_t idx = ePropertyFetchReadonlySectionsFromFileCache;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, i

[Lldb-commits] [PATCH] D100340: [lldb-vscode] Add postRunCommands

2021-04-12 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added reviewers: clayborg, aadsm.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This diff ass postRunCommands, which are the counterpart of the preRunCommands. 
TThey will be executed right after the target is launched or attached 
correctly, which means that the targets can assume that the target is running.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100340

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -161,6 +161,11 @@
 "description": "Commands executed just before the program is launched.",
 "default": []
 			},
+			"postRunCommands": {
+"type": "array",
+"description": "Commands executed just as soon as the program is correctly launched. The program is in a stopped state when these commands run.",
+"default": []
+			},
 			"launchCommands": {
 "type": "array",
 "description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail.",
@@ -237,6 +242,11 @@
 "description": "Commands executed just before the program is attached to.",
 "default": []
 			},
+			"postRunCommands": {
+"type": "array",
+"description": "Commands executed as soon as the program is correctly attached to. The program is in a stopped state when these commands run.",
+"default": []
+			},
 			"stopCommands": {
 "type": "array",
 "description": "Commands executed each time the program stops.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -569,6 +569,8 @@
   llvm::StringRef core_file = GetString(arguments, "coreFile");
   g_vsc.stop_at_entry =
   core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
+  std::vector postRunCommands =
+  GetStrings(arguments, "postRunCommands");
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
@@ -637,12 +639,14 @@
   if (error.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", std::string(error.GetCString()));
+  } else {
+g_vsc.RunLLDBCommands("Running postRunCommands:", postRunCommands);
   }
+
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
   if (error.Success()) {
 SendProcessEvent(Attach);
 g_vsc.SendJSON(CreateEventObject("initialized"));
-// SendThreadStoppedEvent();
   }
 }
 
@@ -1607,6 +1611,8 @@
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   g_vsc.terminate_commands = GetStrings(arguments, "terminateCommands");
   auto launchCommands = GetStrings(arguments, "launchCommands");
+  std::vector postRunCommands =
+  GetStrings(arguments, "postRunCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
@@ -1688,7 +1694,10 @@
   if (error.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", std::string(error.GetCString()));
+  } else {
+g_vsc.RunLLDBCommands("Running postRunCommands:", postRunCommands);
   }
+
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 
   if (g_vsc.is_attach)
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -313,12 +313,14 @@
 program = self.getBuildArtifact("a.out")
 initCommands = ['target list', 'platform list']
 preRunCommands = ['image list a.out', 'image dump sections a.out']
+postRunCommands = ['help trace', 'help process trace']
 stopCommands = ['frame variable', 'bt']
 exitCommands = ['expr 2+3', 'expr 3+4']
 terminateCommands = ['expr 4+2']
 self.build_and_launch(program,
   initComma

[Lldb-commits] [lldb] ba62ebc - [lldb] Disable Shell/Subporcess with reproducers

2021-04-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-04-12T13:08:14-07:00
New Revision: ba62ebc48e8c424ce3a78ba01acda679d536dd47

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

LOG: [lldb] Disable Shell/Subporcess with reproducers

Added: 
lldb/test/Shell/Subprocess/lit.local.cfg

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/Subprocess/lit.local.cfg 
b/lldb/test/Shell/Subprocess/lit.local.cfg
new file mode 100644
index 0..c9b378b7a8a5a
--- /dev/null
+++ b/lldb/test/Shell/Subprocess/lit.local.cfg
@@ -0,0 +1,2 @@
+if 'lldb-repro' in config.available_features:
+  config.unsupported = True



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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/source/Target/TargetProperties.td:180
+DefaultTrue,
+Desc<"Enables reading bytes from the file cache instead of process when 
the address LLDB is reading from falls in a readable but not writable section">;
 }

What do you think about `FetchReadonlyDataFromFileCache`?

@jasonmolenda: Does this description sound better?

```
Desc<"Prefer reading read-only data from object files on disk. When 
debugging remotely this can be much faster than fetching memory from the 
target, but it might miss modifications performed by the operating system at 
load time.">;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lldb/source/Target/Target.cpp:1760
+  auto permissions = section_sp->GetPermissions();
+  prefer_file_cache |= (permissions & ePermissionsWritable) == 0 &&
+  (permissions & ePermissionsReadable) == 1;

With https://lldb.llvm.org/cpp_reference/classlldb__private_1_1Flags.html
you can write this as `Flags(permissions).Test(ePermissionsWritable)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-12 Thread John Ericson via Phabricator via lldb-commits
Ericson2314 added a comment.

I simplified the description at the top in light of feedback (some harder 
issues we are just punting on for now). @compnerd does this now look good to 
you then? Does anyone want to approve the libunwind said of things? Anything 
else this is waiting on?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 PAC elf-core stack unwinder support

2021-04-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Hi Omair, sorry for the delay in looking at this.  It seems like we have two 
overlapping patches here, @justincohen  patch in 
https://reviews.llvm.org/D98886 and this one.

I'm not convinced that we'll see a target where we have noncontiguous bits used 
for addressing (the best reason for using a mask), but Justin points out that 
Linux ARM's NT_ARM_PAC_MASK (gettable by ptrace PTRACE_GETREGSET) is expressed 
as a mask (and googling around, it looks like you can have a different mask for 
code pointers and data pointers, so Process would really need to store both, 
but it may be that for crashpad only the code mask needs to be tracked in 
Process).

lldb needs to allow for users to set the number of bits used for addressing in 
terms of a number -- 52 in your example -- but given that Linux wants to give 
us a mask via the ptrace call, I think we should preserve and use that mask.  
Darwin's API also provide the value in terms of the number of bits used for 
addressing, and as a user setting I think that's the most natural way of 
handling it and we should allow it.  But internal to Process, we should convert 
this to a CodeAddress mask and down in the ABI's where we support AArch v8.3 
ptrauth, we use bit 55 to clear/set the non-addressing high bits.

Omair, Justin, what do you think here?  I don't think it's especially hard to 
accept this in terms of # of bits OR a mask, and we should use the more general 
internal rep in lldb.  Another alternative would be "the mask should be 
converted to the # of bits in addressing and stored in Process in those terms".

(Honestly the idea of an address mask makes me nervous all over - if the rule 
is "take bit 55 and set all non-addressing bits to 0 or 1 depending on that", 
then we're implicitly saying "the mask only masks off high bits".  If it ever 
masked off bit 0, for instance, on a code address, then we're going to set that 
to 0 or 1 when we set/clear non-addressing bits?  Obviously not.  So it's not 
REALLY a mask, is it, it's just the # of bits used for addressing in another 
more flexible format that you can't actually flex or it all breaks. :)


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

https://reviews.llvm.org/D99944

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


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

2021-04-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 336977.
mgorny added a comment.

clang-format


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

https://reviews.llvm.org/D100140

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -219,6 +219,9 @@
 
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
+  virtual std::vector HandleFeatures(
+  const llvm::ArrayRef client_features) override;
+
 private:
   llvm::Expected> BuildTargetXml();
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3534,3 +3534,16 @@
 
   return tid;
 }
+
+std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures(
+const llvm::ArrayRef client_features) {
+  auto ret =
+  GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
+  ret.insert(ret.end(), {
+"qXfer:features:read+", "multiprocess+",
+#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
+"QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
+#endif
+  });
+  return ret;
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -145,6 +145,11 @@
   virtual FileSpec FindModuleFile(const std::string &module_path,
   const ArchSpec &arch);
 
+  // Process client_features (qSupported) and return an array of server features
+  // to be returned in response.
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features);
+
 private:
   ModuleSpec GetModuleInfo(llvm::StringRef module_path, llvm::StringRef triple);
 };
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
@@ -831,26 +831,11 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_qSupported(
 StringExtractorGDBRemote &packet) {
-  StreamGDBRemote response;
-
-  // Features common to lldb-platform and llgs.
-  uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet
- // size--debugger can always use less
-  response.Printf("PacketSize=%x", max_packet_size);
-
-  response.PutCString(";QStartNoAckMode+");
-  response.PutCString(";QThreadSuffixSupported+");
-  response.PutCString(";QListThreadsInStopReply+");
-  response.PutCString(";qEcho+");
-  response.PutCString(";qXfer:features:read+");
-#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
-  response.PutCString(";QPassSignals+");
-  response.PutCString(";qXfer:auxv:read+");
-  response.PutCString(";qXfer:libraries-svr4:read+");
-#endif
-  response.PutCString(";multiprocess+");
-
-  return SendPacketNoLock(response.GetString());
+  // Parse client-indicated features.
+  llvm::StringRef view = packet.GetStringRef();
+  llvm::SmallVector client_features;
+  view.split(client_features, ';');
+  return SendPacketNoLock(llvm::join(HandleFeatures(client_features), ";"));
 }
 
 GDBRemoteCommunication::PacketResult
@@ -1312,3 +1297,18 @@
 
   return matched_module_spec;
 }
+
+std::vector GDBRemoteCommunicationServerCommon::HandleFeatures(
+const llvm::ArrayRef client_features) {
+  // 128KBytes is a reasonable max packet size--debugger can always use less.
+  constexpr uint32_t max_packet_size = 128 * 1024;
+
+  // Features common to platform server and llgs.
+  return {
+  llvm::formatv("PacketSize={0}", max_packet_size),
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",
+  };
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D100146: [lldb] [gdb-remote client] Refactor handling qSupported

2021-04-12 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 rG3842de49f655: [lldb] [gdb-remote client] Refactor handling 
qSupported (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100146

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

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
@@ -601,7 +601,8 @@
 
   // Given the list of compression types that the remote debug stub can support,
   // possibly enable compression if we find an encoding we can handle.
-  void MaybeEnableCompression(std::vector supported_compressions);
+  void MaybeEnableCompression(
+  llvm::ArrayRef supported_compressions);
 
   bool DecodeProcessInfoResponse(StringExtractorGDBRemote &response,
  ProcessInstanceInfo &process_info);
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
@@ -345,6 +345,9 @@
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
   m_supports_multiprocess = eLazyBoolNo;
+  m_supports_qEcho = eLazyBoolNo;
+  m_supports_QPassSignals = eLazyBoolNo;
+
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
   // not, we assume no limit
 
@@ -362,97 +365,51 @@
   if (SendPacketAndWaitForResponse(packet.GetString(), response,
/*send_async=*/false) ==
   PacketResult::Success) {
-const char *response_cstr = response.GetStringRef().data();
-
 // Hang on to the qSupported packet, so that platforms can do custom
 // configuration of the transport before attaching/launching the process.
-m_qSupported_response = response_cstr;
-
-if (::strstr(response_cstr, "qXfer:auxv:read+"))
-  m_supports_qXfer_auxv_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:libraries-svr4:read+"))
-  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
-if (::strstr(response_cstr, "augmented-libraries-svr4-read")) {
-  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
-  m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
-}
-if (::strstr(response_cstr, "qXfer:libraries:read+"))
-  m_supports_qXfer_libraries_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:features:read+"))
-  m_supports_qXfer_features_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:memory-map:read+"))
-  m_supports_qXfer_memory_map_read = eLazyBoolYes;
-
-// Look for a list of compressions in the features list e.g.
-// qXfer:features:read+;PacketSize=2;qEcho+;SupportedCompressions=zlib-
-// deflate,lzma
-const char *features_list = ::strstr(response_cstr, "qXfer:features:");
-if (features_list) {
-  const char *compressions =
-  ::strstr(features_list, "SupportedCompressions=");
-  if (compressions) {
-std::vector supported_compressions;
-compressions += sizeof("SupportedCompressions=") - 1;
-const char *end_of_compressions = strchr(compressions, ';');
-if (end_of_compressions == nullptr) {
-  end_of_compressions = strchr(compressions, '\0');
-}
-const char *current_compression = compressions;
-while (current_compression < end_of_compressions) {
-  const char *next_compression_name = strchr(current_compression, ',');
-  const char *end_of_this_word = next_compression_name;
-  if (next_compression_name == nullptr ||
-  end_of_compressions < next_compression_name) {
-end_of_this_word = end_of_compressions;
-  }
-
-  if (end_of_this_word) {
-if (end_of_this_word == current_compression) {
-  current_compression++;
-} else {
-  std::string this_compression(
-  current_compression, end_of_this_word - current_compression);
-  supported_compressions.push_back(this_compression);
-  current_compression = end_of_this_word + 1;
-}
-  } else {
-supported_compressions.push_back(current_compression);
-current_compression = end_of_compressions;
-  }
+m_qSupported_response = response.GetStrin

[Lldb-commits] [lldb] 3842de4 - [lldb] [gdb-remote client] Refactor handling qSupported

2021-04-12 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-04-13T00:23:07+02:00
New Revision: 3842de49f6551f597b4c7c78caa8ba7003755cec

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

LOG: [lldb] [gdb-remote client] Refactor handling qSupported

Refactor the qSupported handler to split the reply into an array,
and identify features within the array rather than searching the string
for partial matches.  While at it, use StringRef.split() to process
the compression list instead of reinventing the wheel.

Switch the arguments to MaybeEnableCompression() to use an ArrayRef
of StringRefs to simplify parameter passing from GetRemoteQSupported().

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index c032fc20b5f3c..2db985c632db1 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -345,6 +345,9 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
   m_supports_multiprocess = eLazyBoolNo;
+  m_supports_qEcho = eLazyBoolNo;
+  m_supports_QPassSignals = eLazyBoolNo;
+
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
   // not, we assume no limit
 
@@ -362,97 +365,51 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   if (SendPacketAndWaitForResponse(packet.GetString(), response,
/*send_async=*/false) ==
   PacketResult::Success) {
-const char *response_cstr = response.GetStringRef().data();
-
 // Hang on to the qSupported packet, so that platforms can do custom
 // configuration of the transport before attaching/launching the process.
-m_qSupported_response = response_cstr;
-
-if (::strstr(response_cstr, "qXfer:auxv:read+"))
-  m_supports_qXfer_auxv_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:libraries-svr4:read+"))
-  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
-if (::strstr(response_cstr, "augmented-libraries-svr4-read")) {
-  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
-  m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
-}
-if (::strstr(response_cstr, "qXfer:libraries:read+"))
-  m_supports_qXfer_libraries_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:features:read+"))
-  m_supports_qXfer_features_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:memory-map:read+"))
-  m_supports_qXfer_memory_map_read = eLazyBoolYes;
-
-// Look for a list of compressions in the features list e.g.
-// qXfer:features:read+;PacketSize=2;qEcho+;SupportedCompressions=zlib-
-// deflate,lzma
-const char *features_list = ::strstr(response_cstr, "qXfer:features:");
-if (features_list) {
-  const char *compressions =
-  ::strstr(features_list, "SupportedCompressions=");
-  if (compressions) {
-std::vector supported_compressions;
-compressions += sizeof("SupportedCompressions=") - 1;
-const char *end_of_compressions = strchr(compressions, ';');
-if (end_of_compressions == nullptr) {
-  end_of_compressions = strchr(compressions, '\0');
-}
-const char *current_compression = compressions;
-while (current_compression < end_of_compressions) {
-  const char *next_compression_name = strchr(current_compression, ',');
-  const char *end_of_this_word = next_compression_name;
-  if (next_compression_name == nullptr ||
-  end_of_compressions < next_compression_name) {
-end_of_this_word = end_of_compressions;
-  }
-
-  if (end_of_this_word) {
-if (end_of_this_word == current_compression) {
-  current_compression++;
-} else {
-  std::string this_compression(
-  current_compression, end_of_this_word - current_compression);
-  supported_compressions.push_back(this_compression);
-  current_compression = end_of_this_word + 1;
-}
-  } else {
-supported_compressions.push_back(current_compression);
-current_compression = end_of_compressions;
-  }
+m_qSupported_response = response.GetStringRef().str();
+
+llvm::SmallVector server_featu

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Another question for @jasonmolenda: Do you happen to know if the existing 
`prefer_file_cache` mechanism applies relocations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [lldb] 6c4f250 - Revert "[lldb] [gdb-remote client] Refactor handling qSupported"

2021-04-12 Thread Ahmed Bougacha via lldb-commits

Author: Ahmed Bougacha
Date: 2021-04-12T18:06:09-07:00
New Revision: 6c4f2508e4278ac789230cb05f2bb56a8a7297dc

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

LOG: Revert "[lldb] [gdb-remote client] Refactor handling qSupported"

This reverts commit 3842de49f6551f597b4c7c78caa8ba7003755cec.

It fails to build, with errors such as:
  GDBRemoteCommunicationClient.cpp:1005:20:
  error: no viable overloaded '='
  avail_name = compression;

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 2db985c632db1..c032fc20b5f3c 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -345,9 +345,6 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
   m_supports_multiprocess = eLazyBoolNo;
-  m_supports_qEcho = eLazyBoolNo;
-  m_supports_QPassSignals = eLazyBoolNo;
-
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
   // not, we assume no limit
 
@@ -365,51 +362,97 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   if (SendPacketAndWaitForResponse(packet.GetString(), response,
/*send_async=*/false) ==
   PacketResult::Success) {
+const char *response_cstr = response.GetStringRef().data();
+
 // Hang on to the qSupported packet, so that platforms can do custom
 // configuration of the transport before attaching/launching the process.
-m_qSupported_response = response.GetStringRef().str();
-
-llvm::SmallVector server_features;
-response.GetStringRef().split(server_features, ';');
-
-for (auto x : server_features) {
-  if (x == "qXfer:auxv:read+")
-m_supports_qXfer_auxv_read = eLazyBoolYes;
-  else if (x == "qXfer:libraries-svr4:read+")
-m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
-  else if (x == "augmented-libraries-svr4-read") {
-m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
-m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
-  } else if (x == "qXfer:libraries:read+")
-m_supports_qXfer_libraries_read = eLazyBoolYes;
-  else if (x == "qXfer:features:read+")
-m_supports_qXfer_features_read = eLazyBoolYes;
-  else if (x == "qXfer:memory-map:read+")
-m_supports_qXfer_memory_map_read = eLazyBoolYes;
-  else if (x == "qEcho")
-m_supports_qEcho = eLazyBoolYes;
-  else if (x == "QPassSignals+")
-m_supports_QPassSignals = eLazyBoolYes;
-  else if (x == "multiprocess+")
-m_supports_multiprocess = eLazyBoolYes;
-  // Look for a list of compressions in the features list e.g.
-  // 
qXfer:features:read+;PacketSize=2;qEcho+;SupportedCompressions=zlib-
-  // deflate,lzma
-  else if (x.consume_front("SupportedCompressions=")) {
-llvm::SmallVector compressions;
-x.split(compressions, ',');
-if (!compressions.empty())
-  MaybeEnableCompression(compressions);
-  } else if (x.consume_front("PacketSize=")) {
-StringExtractorGDBRemote packet_response(x);
-m_max_packet_size =
-packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
-if (m_max_packet_size == 0) {
-  m_max_packet_size = UINT64_MAX; // Must have been a garbled response
-  Log *log(
-  ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
-  LLDB_LOGF(log, "Garbled PacketSize spec in qSupported response");
+m_qSupported_response = response_cstr;
+
+if (::strstr(response_cstr, "qXfer:auxv:read+"))
+  m_supports_qXfer_auxv_read = eLazyBoolYes;
+if (::strstr(response_cstr, "qXfer:libraries-svr4:read+"))
+  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
+if (::strstr(response_cstr, "augmented-libraries-svr4-read")) {
+  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
+  m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
+}
+if (::strstr(response_cstr, "qXfer:libraries:read+"))
+  m_supports_qXfer_libraries_read = eLazyBoolYes;
+if (::strstr(response_cstr, "qXfer:features:read+"))
+  m_supports_qXfer_features_read = eLazyBoolYes;
+if (::strstr(response_cstr, "qXfer:memory-map:read+"))
+  m_supports_qXfer_memory_map_r

[Lldb-commits] [lldb] 30f591c - [lldb] Disable TestLaunchProcessPosixSpawn.py with reproducers

2021-04-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-04-12T18:32:10-07:00
New Revision: 30f591c3869f3bbe6eca1249dcef1b8337312de6

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

LOG: [lldb] Disable TestLaunchProcessPosixSpawn.py with reproducers

Added: 


Modified: 
lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py

Removed: 




diff  --git a/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py 
b/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py
index 335f75e04bfbc..fc26de705b6fd 100644
--- a/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py
+++ b/lldb/test/API/macosx/posix_spawn/TestLaunchProcessPosixSpawn.py
@@ -53,6 +53,7 @@ def run_arch(self, exe, arch):
 @skipUnlessDarwin
 @skipIfDarwinEmbedded
 @skipTestIfFn(no_haswell)
+@skipIfReproducer # Test relies on inferior output
 def test_haswell(self):
 self.build()
 exe = self.getBuildArtifact("fat.out")
@@ -62,6 +63,7 @@ def test_haswell(self):
 @skipUnlessDarwin
 @skipIfDarwinEmbedded
 @skipTestIfFn(no_apple_silicon)
+@skipIfReproducer # Test relies on inferior output
 def test_apple_silicon(self):
 self.build()
 exe = self.getBuildArtifact("fat.out")



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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

In D100338#2684550 , @aprantl wrote:

> Another question for @jasonmolenda: Do you happen to know if the existing 
> `prefer_file_cache` mechanism applies relocations?

We can test it to make sure like

  % cat >a.c
  char *globstr = "hello";
  % clang -dynamiclib -o liba.dylib a.c
  % cat >b.c
  extern char *globstr;
  int main() { char *c = globstr; return c[0]; }
  % clang -g -o b.out b.c -L. -la
  % lldb b.out
  (lldb) disass -b -n main
  b.out`main:
  b.out[0x13f90] <+0>:  55pushq  %rbp
  b.out[0x13f91] <+1>:  48 89 e5  movq   %rsp, %rbp
  b.out[0x13f94] <+4>:  48 8b 05 65 00 00 00  movq   0x65(%rip), %rax   
   ; (void *)0x
  
  (lldb) br s -n main
  (lldb) r
  (lldb) disass -b -n main
  b.out`main:
  0x13f90 <+0>:  55pushq  %rbp
  0x13f91 <+1>:  48 89 e5  movq   %rsp, %rbp
  0x13f94 <+4>:  48 8b 05 65 00 00 00  movq   0x65(%rip), %rax  
; (void *)0x000100128000: globstr
  0x13f9b <+11>: c7 45 fc 00 00 00 00  movl   $0x0, -0x4(%rbp)
  
  (lldb) ima loo -va 0x13f9b+0x65
Address: b.out[0x00014000] (b.out.__DATA_CONST.__got + 0)
Summary: (void *)0x000100128000: globstr
 Module: file = "/tmp/b.out", arch = "x86_64"
  
  (lldb) x/gx 0x13f9b+0x65
  0x14000: 0x000100128000
  
  (lldb) tar mod dump sect b.out
  
0x0003 data-ptrs[0x00014000-0x00014008)  rw-  
0x4000 0x0008 0x0006 b.out.__DATA_CONST.__got

This __DATA,got section is marked as rw, so we read it from memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I'm not super convinced on the setting.  It seems like something you'd add some 
people could work around a mistake we've made here.

I think we have two cases:

Someone calls Target::ReadMemory prefer_file_cache=true on a writable section.  
This is an easy mistake for people to make, they assume prefer_file_cache will 
only prefer the file cache on read-only Sections.

Someone reads Target::ReadMemory with prefer_file_cache=false on a read-only 
section.  This is *often* a mistake, but there are times where we genuinely 
want to see raw memory.

Let's think about what the default behavior of Target::ReadMemory should be. 
The majority use case is that we prefer the file cache if it is a read-only 
section.  It's a performance bug in almost every case to behave differently, I 
maintain.  There are times when we want to show actual raw memory to people.  
Some naughty programs that manage to modify themselves, we need to see that 
real naughtiness, not show what the file had and pretend that is what is in 
memory.

I think Target::ReadMemory should have a force_live_memory argument, not a 
prefer_file_cache.  It could be the final argument with a default value of 
force_live_memory==false.  Almost everyone should let Target::ReadMemory check 
if this address is in a RO-section, and let it get the data from the file cache 
if so.  If it's not in a RO section, or any section, read from live memory.  
And have the force_live_memory arg to override this.

This is a larger set of changes though, and I don't mean to sign anyone up for 
more than they intended.  But, just thinking of this from a clean slate, I 
think that's the better design.  Adrian, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Hm, a little more thinking.  Target::ReadMemory prefers a file cache if it is 
available.  Process::ReadMemory only uses live memory.  But Process::ReadMemory 
doesn't work when we have run lldb on a file and not run yet.  e.g.

  % lldb
  (lldb) tar cr -d /tmp/b.out
  Current executable set to '/tmp/b.out' (x86_64).
  (lldb) disass -n main
  b.out`main:
  b.out[0x13f90] <+0>:  pushq  %rbp
  b.out[0x13f91] <+1>:  movq   %rsp, %rbp
  b.out[0x13f94] <+4>:  movq   0x65(%rip), %rax  ; (void 
*)0x
  b.out[0x13f9b] <+11>: movl   $0x0, -0x4(%rbp)
  
  (lldb) ima loo -va 0x13f9b+0x65
Address: b.out[0x00014000] (b.out.__DATA_CONST.__got + 0)
Summary: (void *)0x
 Module: file = "/tmp/b.out", arch = "x86_64"
  
  (lldb) x/gx 0x13f9b+0x65
  0x14000: 0x
  (lldb) 

I don't have a Process, so I must be using Target::ReadMemory here.  and even 
though this section is read-write, we don't have any live memory so we're 
fetching it from the file.  So that's an interesting extra wrinkle I didn't 
remember from Target::ReadMemory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 PAC elf-core stack unwinder support

2021-04-12 Thread Justin Cohen via Phabricator via lldb-commits
justincohen added a comment.

In D99944#2684280 , @jasonmolenda 
wrote:

> Omair, Justin, what do you think here?  I don't think it's especially hard to 
> accept this in terms of # of bits OR a mask, and we should use the more 
> general internal rep in lldb.  Another alternative would be "the mask should 
> be converted to the # of bits in addressing and stored in Process in those 
> terms".

From a minidump/crashpad perspective, we are fine with either approach.  We 
plan to a use a mask within the minidump format, but we can convert it to # of 
bits as necessary.


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

https://reviews.llvm.org/D99944

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


[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 PAC elf-core stack unwinder support

2021-04-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I think this will come down to, who wants to implement a patch that can set/get 
either form through Process API.  The mask might be a more general format, so 
if Justin is willing to update his patch to that we can add Omair's test case 
to his patch?


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

https://reviews.llvm.org/D99944

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


[Lldb-commits] [lldb] 7dbb427 - [lldb] Fix replaying TestMemoryRead.py from reproducer

2021-04-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-04-12T21:10:09-07:00
New Revision: 7dbb4274ef92c32b02c427d4844f3ddfdd05ef58

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

LOG: [lldb] Fix replaying TestMemoryRead.py from reproducer

Remap the external file to the one embedded in the reproducer.

Added: 


Modified: 
lldb/test/API/functionalities/memory/read/TestMemoryRead.py

Removed: 




diff  --git a/lldb/test/API/functionalities/memory/read/TestMemoryRead.py 
b/lldb/test/API/functionalities/memory/read/TestMemoryRead.py
index ceea4ab2f067a..3efda021584b5 100644
--- a/lldb/test/API/functionalities/memory/read/TestMemoryRead.py
+++ b/lldb/test/API/functionalities/memory/read/TestMemoryRead.py
@@ -150,6 +150,8 @@ def test_memory_read_file(self):
 golden_output = res.GetOutput()
 
 memory_read_file = self.getBuildArtifact("memory-read-output")
+if configuration.is_reproducer_replay():
+memory_read_file = self.getReproducerRemappedPath(memory_read_file)
 
 def check_file_content(expected):
 with open(memory_read_file) as f:



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


[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

2021-04-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added subscribers: jingham, clayborg.
jasonmolenda added a comment.

Trying to reword my thinking a little more clearly.

Process::ReadMemory - only works when you have a Process.  Always reads 
directly from memory.

Target::ReadMemory - can be used with or without a Process.

1. If force_live_memory == true, will read from memory if there is a Process.  
Else will read from file, if a Section covers this address.

2. If the address is in a Section that is read-only, read from the file.

3. If the address is in a section that is read-write, read from memory if there 
is a Process.  Else read from the file, if a Section covers this address.

And I would make Target::ReadMemory's force_live_memory argument have a default 
value of false, because few people intend for this.

I'm open to hearing different opinions on this rethink!  eg  @clayborg @jingham


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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


[Lldb-commits] [PATCH] D100146: [lldb] [gdb-remote client] Refactor handling qSupported

2021-04-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.
Herald added a subscriber: JDevlieghere.

looks great, just fix the build errors :)




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:375
+
+for (auto x : server_features) {
+  if (x == "qXfer:auxv:read+")

not a big deal, but this probably shouldn't be auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100146

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


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

2021-04-12 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

cool




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:835
+  // Parse client-indicated features.
+  llvm::StringRef view = packet.GetStringRef();
+  llvm::SmallVector client_features;

remove variable used only once?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1310-1311
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",

I think that these two should also be llgs-specific.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h:151
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features);
+

this `const` is useless


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

https://reviews.llvm.org/D100140

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


[Lldb-commits] [PATCH] D100195: [lldb] Require x86 for unwind no-return test

2021-04-12 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D100195#2679590 , @DavidSpickett 
wrote:

> (apologies for the review spam, you're the author of the test in this case)

np

> lldb in fact crashes when you try to load the core file. Would be nice to 
> give a good failure message if we can but skipping this test makes sense 
> anyway.

Agree completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100195

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