[Lldb-commits] [PATCH] D76476: Add formatter for libc++ std::unique_ptr

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: vsk, jingham.

This adds a formatter for libc++ `std::unique_ptr`. We currently already have 
formatters for `std::shared_ptr` and `std::weak_ptr`.

I also refactored `GetValueOfCompressedPair(...)` out of `LibCxxList.cpp` since 
I need the same functionality and it made sense to share it.


https://reviews.llvm.org/D76476

Files:
  lldb/include/lldb/DataFormatters/FormattersHelpers.h
  lldb/source/DataFormatters/FormattersHelpers.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+#include 
+
+int main()
+{
+  std::unique_ptr up_empty;
+  std::unique_ptr up_int = std::make_unique(10);
+  std::unique_ptr up_str = std::make_unique("hello");
+
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
@@ -0,0 +1,37 @@
+"""
+Test lldb data formatter for libc++ std::unique_ptr.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxUniquePtrDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test that that file and class static variables display correctly."""
+self.build()
+
+(self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+self.expect("frame variable up_empty",
+substrs=['(std::unique_ptr >) up_empty = nullptr {',
+   '__value_ = ',
+   '}'])
+
+self.expect("frame variable up_int",
+substrs=['(std::unique_ptr >) up_int = 10 {',
+   '__value_ = ',
+   '}'])
+
+self.expect("frame variable up_str",
+substrs=['up_str = "hello" {',
+   '__value_ = ',
+   '}'])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -std=c++14
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -290,15 +290,6 @@
m_element_type);
 }
 
-static ValueObjectSP GetValueOfCompressedPair(ValueObject ) {
-  ValueObjectSP value = pair.GetChildMemberWithName(ConstString("__value_"), true);
-  if (! value) {
-// pre-r300140 member name
-value = pair.GetChildMemberWithName(ConstString("__first_"), true);
-  }
-  return value;
-}
-
 bool ForwardListFrontEnd::Update() {
   AbstractListFrontEnd::Update();
 
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -43,6 +43,10 @@
 const TypeSummaryOptions
 ); // libc++ std::shared_ptr<> and std::weak_ptr<>
 
+// libc++ std::unique_ptr<>
+bool LibcxxUniquePointerSummaryProvider(ValueObject , Stream ,
+const TypeSummaryOptions );
+
 bool LibcxxFunctionSummaryProvider(
 ValueObject , Stream ,
 const TypeSummaryOptions ); // libc++ std::function<>
@@ -107,6 +111,26 @@
   lldb::ByteOrder m_byte_order;
 };
 
+class LibcxxUniquePtrSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 251526.

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

https://reviews.llvm.org/D76471

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/SDK.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -648,6 +648,7 @@
 const DWARFBaseDIE cu_die =
 dwarf_cu.GetNonSkeletonUnit().GetUnitDIEOnly();
 if (cu_die) {
+  module_sp->RegisterSDK(dwarf_cu.GetSDK(), ConstString(dwarf_cu.GetSysroot()));
   FileSpec cu_file_spec(cu_die.GetName(), dwarf_cu.GetPathStyle());
   if (cu_file_spec) {
 // If we have a full path to the compile unit, we don't need to
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -12,6 +12,7 @@
 #include "DWARFDIE.h"
 #include "DWARFDebugInfoEntry.h"
 #include "lldb/lldb-enumerations.h"
+#include "lldb/Utility/SDK.h"
 #include "llvm/Support/RWMutex.h"
 #include 
 
@@ -197,6 +198,10 @@
 
   uint32_t GetProducerVersionUpdate();
 
+  lldb_private::SDK GetSDK();
+
+  llvm::StringRef GetSysroot();
+
   uint64_t GetDWARFLanguageType();
 
   bool GetIsOptimized();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -667,6 +667,18 @@
   return *m_language_type;
 }
 
+SDK DWARFUnit::GetSDK() {
+  if (const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly())
+return SDK(die->GetAttributeValueAsString(this, DW_AT_APPLE_sdk, ""));
+  return {};
+}
+
+llvm::StringRef DWARFUnit::GetSysroot() {
+  if (const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly())
+return die->GetAttributeValueAsString(this, DW_AT_LLVM_sysroot, "");
+  return {};
+}
+
 bool DWARFUnit::GetIsOptimized() {
   if (m_is_optimized == eLazyBoolCalculate) {
 const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -11,6 +11,7 @@
 
 #include "Plugins/Platform/POSIX/PlatformPOSIX.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StructuredData.h"
 #include "llvm/ADT/StringRef.h"
@@ -96,6 +97,8 @@
   llvm::Expected
   FetchExtendedCrashInformation(lldb_private::Process ) override;
 
+  lldb_private::ConstString GetSDKPath(int type) override;
+  
   static llvm::StringRef GetSDKNameForType(SDKType type);
   static lldb_private::FileSpec GetXcodeSDK(SDKType type);
   static lldb_private::FileSpec GetXcodeContentsDirectory();
@@ -184,6 +187,7 @@
   static std::string FindXcodeContentsDirectoryInPath(llvm::StringRef path);
 
   std::string m_developer_directory;
+  std::array m_sdk_path;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformDarwin);
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1282,6 +1282,81 @@
   return g_command_line_tools_filespec;
 }
 
+SDK ::operator=(SDK other) {
+  m_name = other.m_name;
+  return *this;
+}
+
+bool SDK::operator==(SDK other) {
+  return m_name == other.m_name;
+}
+
+static PlatformDarwin::SDKType ParseSDKName(llvm::StringRef ) {
+  if (name.consume_front("MacOSX"))
+return PlatformDarwin::MacOSX;
+  if (name.consume_front("iPhoneSimulator"))
+return PlatformDarwin::iPhoneSimulator;
+  if (name.consume_front("iPhoneOS"))
+return PlatformDarwin::iPhoneOS;
+  if (name.consume_front("AppleTVSimulator"))
+return PlatformDarwin::AppleTVSimulator;
+  if (name.consume_front("AppleTVOS"))
+return PlatformDarwin::AppleTVOS;
+  if (name.consume_front("WatchSimulator"))
+return PlatformDarwin::WatchSimulator;
+  if (name.consume_front("WatchOS"))
+return PlatformDarwin::watchOS;
+  if (name.consume_front("bridgeOS"))
+return PlatformDarwin::bridgeOS;
+  if (name.consume_front("Linux"))
+return PlatformDarwin::Linux;
+  

[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, jingham, friss.
aprantl updated this revision to Diff 251526.

This is mostly useful for Swift support; it allows LLDB to substitute a 
matching SDK it shipped with instead of the sysroot path that was used at 
compile time.

rdar://problem/60640017


https://reviews.llvm.org/D76471

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/SDK.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -648,6 +648,7 @@
 const DWARFBaseDIE cu_die =
 dwarf_cu.GetNonSkeletonUnit().GetUnitDIEOnly();
 if (cu_die) {
+  module_sp->RegisterSDK(dwarf_cu.GetSDK(), ConstString(dwarf_cu.GetSysroot()));
   FileSpec cu_file_spec(cu_die.GetName(), dwarf_cu.GetPathStyle());
   if (cu_file_spec) {
 // If we have a full path to the compile unit, we don't need to
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -12,6 +12,7 @@
 #include "DWARFDIE.h"
 #include "DWARFDebugInfoEntry.h"
 #include "lldb/lldb-enumerations.h"
+#include "lldb/Utility/SDK.h"
 #include "llvm/Support/RWMutex.h"
 #include 
 
@@ -197,6 +198,10 @@
 
   uint32_t GetProducerVersionUpdate();
 
+  lldb_private::SDK GetSDK();
+
+  llvm::StringRef GetSysroot();
+
   uint64_t GetDWARFLanguageType();
 
   bool GetIsOptimized();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -667,6 +667,18 @@
   return *m_language_type;
 }
 
+SDK DWARFUnit::GetSDK() {
+  if (const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly())
+return SDK(die->GetAttributeValueAsString(this, DW_AT_APPLE_sdk, ""));
+  return {};
+}
+
+llvm::StringRef DWARFUnit::GetSysroot() {
+  if (const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly())
+return die->GetAttributeValueAsString(this, DW_AT_LLVM_sysroot, "");
+  return {};
+}
+
 bool DWARFUnit::GetIsOptimized() {
   if (m_is_optimized == eLazyBoolCalculate) {
 const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -11,6 +11,7 @@
 
 #include "Plugins/Platform/POSIX/PlatformPOSIX.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StructuredData.h"
 #include "llvm/ADT/StringRef.h"
@@ -96,6 +97,8 @@
   llvm::Expected
   FetchExtendedCrashInformation(lldb_private::Process ) override;
 
+  lldb_private::ConstString GetSDKPath(int type) override;
+  
   static llvm::StringRef GetSDKNameForType(SDKType type);
   static lldb_private::FileSpec GetXcodeSDK(SDKType type);
   static lldb_private::FileSpec GetXcodeContentsDirectory();
@@ -184,6 +187,7 @@
   static std::string FindXcodeContentsDirectoryInPath(llvm::StringRef path);
 
   std::string m_developer_directory;
+  std::array m_sdk_path;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformDarwin);
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1282,6 +1282,81 @@
   return g_command_line_tools_filespec;
 }
 
+SDK ::operator=(SDK other) {
+  m_name = other.m_name;
+  return *this;
+}
+
+bool SDK::operator==(SDK other) {
+  return m_name == other.m_name;
+}
+
+static PlatformDarwin::SDKType ParseSDKName(llvm::StringRef ) {
+  if (name.consume_front("MacOSX"))
+return PlatformDarwin::MacOSX;
+  if (name.consume_front("iPhoneSimulator"))
+return PlatformDarwin::iPhoneSimulator;
+  if (name.consume_front("iPhoneOS"))
+return PlatformDarwin::iPhoneOS;
+  if (name.consume_front("AppleTVSimulator"))
+return PlatformDarwin::AppleTVSimulator;
+  if (name.consume_front("AppleTVOS"))
+return PlatformDarwin::AppleTVOS;
+  if (name.consume_front("WatchSimulator"))
+return PlatformDarwin::WatchSimulator;
+  if 

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 251527.
wallace added a comment.

Now using the latest SBEnvironment API


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

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/environmentVariables/Makefile
  
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
  lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
  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
@@ -84,6 +84,11 @@
 "description": "Additional environment variables.",
 "default": []
 			},
+			"inheritEnvironment": {
+"type": "boolean",
+"description": "Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.",
+"default": false
+			},
 			"stopOnEntry": {
 "type": "boolean",
 "description": "Automatically stop after launch.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -40,6 +40,7 @@
 #include 
 #include 
 
+#include "lldb/API/SBEnvironment.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -1358,6 +1359,8 @@
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  bool launchWithDebuggerEnvironment =
+  GetBoolean(arguments, "inheritEnvironment", false);
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1374,6 +1377,12 @@
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  // Configure the inherit environment setting
+  std::ostringstream oss;
+  oss << "settings set target.inherit-env "
+  << (launchWithDebuggerEnvironment ? "true" : "false");
+  g_vsc.RunLLDBCommands(llvm::StringRef(), {oss.str()});
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1398,10 +1407,12 @@
   if (!args.empty())
 g_vsc.launch_info.SetArguments(MakeArgv(args).data(), true);
 
-  // Pass any environment variables along that the user specified.
-  auto envs = GetStrings(arguments, "env");
-  if (!envs.empty())
-g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
+  // This mimics what CommandObjectProcess does when launching a process
+  lldb::SBEnvironment env = g_vsc.target.GetEnvironment();
+  for (const auto _and_value : GetStrings(arguments, "env"))
+env.PutEntry(name_and_value.c_str());
+
+  g_vsc.launch_info.SetEnvironment(env, true);
 
   auto flags = g_vsc.launch_info.GetLaunchFlags();
 
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
@@ -0,0 +1,15 @@
+#include 
+#include 
+#include 
+#include 
+
+extern char **environ;
+
+int main(int argc, char const *argv[], char const *envp[]) {
+  char **env_var_pointer = environ;
+  for (char *env_variable = *env_var_pointer; env_variable;
+   env_variable = *++env_var_pointer) {
+printf("%s\n", env_variable);
+  }
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
@@ -0,0 +1,70 @@
+"""
+Test lldb-vscode environment variables
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+import os
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def getEnvOutputByProgram(self):
+env = {}
+for line in self.get_stdout().encode('utf-8').splitlines():
+(name, value) = line.split("=")
+env[name] = value
+return env
+
+@skipIfWindows
+@skipIfRemote
+def test_empty_environment(self):
+"""
+Tests running a process with an empty environment
+"""
+program = 

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1414
+auto kv = llvm::StringRef(name_and_value).split("=");
+env.Set(kv.first.str().c_str(), kv.second.str().c_str(), true);
+  }

We want to have a function in SBEnvironment that takes the "name=value" format:
```
env.SetEntry("FOO=bar");
```

We don't want people to have to split this themselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



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

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

[Frederic Riss via Phabricator via lldb-commits]

friss created this revision.
friss added reviewers: labath, jingham.
Herald added a project: LLDB.
friss added a comment.

This needs some tests, but I wanted to put it out there for comments.


The interactions between the environment settings (`target.env-vars`,
`target.inherit-env`) and the inferior life-cycle are non-obvious
today. For example, if `target.inherit-env` is set, the `target.env-vars`
setting will be augmented with the contents of the host environment
the first time the launch environment is queried (usually at
launch). After that point, toggling `target.inherit-env` will have no
effect as there's no tracking of what comes from the host and what is
a user setting.

This patch computes the environment every time it is queried rather
than updating the contents of the `target.env-vars` property. This
means that toggling the `target.inherit-env` property later will now
have the intended effect.

This patch also adds a `target.unset-env-vars` settings that one can
use to remove variables from the launch environment. Using this, you
can inherit all but a few of the host environment.

The way the launch environment is constructed is:

  1/ if `target.inherit-env` is set, then read the host environment
  into the launch environment.
  2/ Augment the launch environment with the contents of
  `target.env-vars`. This overrides any common values with the host
  environment.
  3/ Remove for the environment the variables listed in
  `target.unset-env`.

The one functional difference here that could be seen as a regression
is that `target.env-vars` will not contain the inferior environment
after launch. It's unclear that this is what any user would have
expected anyway, and we can make it clear(er) in the help that this
setting only contains the values specified by the user.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76470

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
@@ -81,6 +81,9 @@
   def EnvVars: Property<"env-vars", "Dictionary">,
 DefaultUnsignedValue<16>,
 Desc<"A list of all the environment variables to be passed to the executable's environment, and their values.">;
+  def UnsetEnvVars: Property<"unset-env-vars", "Array">,
+DefaultUnsignedValue<16>,
+Desc<"A list of environment variable names to be unset in the inferior's environment.">;
   def InheritEnv: Property<"inherit-env", "Boolean">,
 DefaultTrue,
 Desc<"Inherit the environment from the process that is running LLDB.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3357,16 +3357,13 @@
 
 class TargetOptionValueProperties : public OptionValueProperties {
 public:
-  TargetOptionValueProperties(ConstString name)
-  : OptionValueProperties(name), m_target(nullptr), m_got_host_env(false) {}
+  TargetOptionValueProperties(ConstString name) : OptionValueProperties(name) {}
 
   // This constructor is used when creating TargetOptionValueProperties when it
   // is part of a new lldb_private::Target instance. It will copy all current
   // global property values as needed
-  TargetOptionValueProperties(Target *target,
-  const TargetPropertiesSP _properties_sp)
-  : OptionValueProperties(*target_properties_sp->GetValueProperties()),
-m_target(target), m_got_host_env(false) {}
+  TargetOptionValueProperties(const TargetPropertiesSP _properties_sp)
+  : OptionValueProperties(*target_properties_sp->GetValueProperties()) {}
 
   const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx,
  bool will_modify,
@@ -3374,9 +3371,6 @@
 // When getting the value for a key from the target options, we will always
 // try and grab the setting from the current target if there is one. Else
 // we just use the one from this instance.
-if (idx == ePropertyEnvVars)
-  GetHostEnvironmentIfNeeded();
-
 if (exe_ctx) {
   Target *target = exe_ctx->GetTargetPtr();
   if (target) {
@@ -3389,41 +3383,6 @@
 }
 return ProtectedGetPropertyAtIndex(idx);
   }
-
-  lldb::TargetSP GetTargetSP() { return m_target->shared_from_this(); }
-
-protected:
-  void GetHostEnvironmentIfNeeded() const {
-if (!m_got_host_env) {
-  if (m_target) {
-m_got_host_env = true;
-const uint32_t idx = ePropertyInheritEnv;
-if (GetPropertyAtIndexAsBoolean(
-nullptr, idx, g_target_properties[idx].default_uint_value != 0)) {
-  PlatformSP platform_sp(m_target->GetPlatform());
-  if (platform_sp) {
-Environment env = platform_sp->GetEnvironment();
- 

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

This needs some tests, but I wanted to put it out there for comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76470



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

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin updated this revision to Diff 251512.
jarin added a comment.

Adding a tighter x64 test as suggested by labath@.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/threads-info/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/threads-info/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

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
@@ -31,6 +31,7 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -469,6 +470,119 @@
   return register_object;
 }
 
+static llvm::Optional
+GetRegisterValue(NativeRegisterContext _ctx, uint32_t generic_regnum) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
+  uint32_t reg_num = reg_ctx.ConvertRegisterKindToRegisterNumber(
+  eRegisterKindGeneric, generic_regnum);
+  const RegisterInfo *const reg_info_p =
+  reg_ctx.GetRegisterInfoAtIndex(reg_num);
+
+  if (reg_info_p == nullptr || reg_info_p->value_regs != nullptr) {
+LLDB_LOGF(log, "%s failed to get register info for register index %" PRIu32,
+  __FUNCTION__, reg_num);
+return {};
+  }
+
+  RegisterValue reg_value;
+  Status error = reg_ctx.ReadRegister(reg_info_p, reg_value);
+  if (error.Fail()) {
+LLDB_LOGF(log, "%s failed to read register '%s' index %" PRIu32 ": %s",
+  __FUNCTION__,
+  reg_info_p->name ? reg_info_p->name : "",
+  reg_num, error.AsCString());
+return {};
+  }
+  return reg_value;
+}
+
+static json::Object CreateMemoryChunk(json::Array _memory_chunks,
+  addr_t address,
+  std::vector ) {
+  json::Object chunk;
+  chunk.try_emplace("address", static_cast(address));
+  StreamString stream;
+  for (uint8_t b : bytes)
+stream.PutHex8(b);
+  chunk.try_emplace("bytes", stream.GetString().str());
+  return chunk;
+}
+
+static json::Array GetStackMemoryAsJSON(NativeProcessProtocol ,
+NativeThreadProtocol ) {
+  uint32_t address_size = process.GetArchitecture().GetAddressByteSize();
+  const size_t kStackTopMemoryInfoWordSize = 12;
+  size_t stack_top_memory_info_byte_size =
+  kStackTopMemoryInfoWordSize * address_size;
+  const size_t kMaxStackSize = 128 * 1024;
+  const size_t kMaxFrameSize = 4 * 1024;
+  size_t fp_and_ra_size = 2 * address_size;
+  const size_t kMaxFrameCount = 128;
+
+  NativeRegisterContext _ctx = thread.GetRegisterContext();
+
+  json::Array stack_memory_chunks;
+
+  lldb::addr_t sp_value;
+  if (llvm::Optional optional_sp_value =
+  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_SP)) {
+sp_value = optional_sp_value->GetAsUInt64();
+  } else {
+return stack_memory_chunks;
+  }
+  lldb::addr_t fp_value;
+  if (llvm::Optional optional_fp_value =
+  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_FP)) {
+fp_value = optional_fp_value->GetAsUInt64();
+  } else {
+return stack_memory_chunks;
+  }
+
+  // First, make sure we copy the top stack_top_memory_info_byte_size bytes
+  // from the stack.
+  size_t byte_count = std::min(stack_top_memory_info_byte_size,
+   static_cast(fp_value - sp_value));
+  std::vector buf(byte_count, 0);
+
+  size_t bytes_read = 0;
+  Status error = process.ReadMemoryWithoutTrap(sp_value, buf.data(), byte_count,
+   bytes_read);
+  if (error.Success() && bytes_read > 0) {
+buf.resize(bytes_read);
+stack_memory_chunks.push_back(
+std::move(CreateMemoryChunk(stack_memory_chunks, sp_value, buf)));
+  }
+
+  // Additionally, try to walk the frame pointer link chain. If the frame
+  // is too big or if the frame pointer points too far, stop the walk.
+  addr_t max_frame_pointer = sp_value + kMaxStackSize;
+  for (size_t i = 0; i < kMaxFrameCount; i++) {
+if (fp_value < sp_value || fp_value > sp_value + kMaxFrameSize ||
+fp_value > max_frame_pointer)
+  break;
+
+std::vector fp_ra_buf(fp_and_ra_size, 0);
+bytes_read = 0;
+error = process.ReadMemoryWithoutTrap(fp_value, fp_ra_buf.data(),
+  fp_and_ra_size, bytes_read);
+

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Djordje Todorovic via Phabricator via lldb-commits]

djtodoro added a comment.

Re-enabled with the d9b962100942 
.

If we face a failure again, since this enables the whole feature, I recommend 
reverting smaller pieces of the feature that was causing the problem, rather 
than reverting this patch.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 251375.
HsiangKai added a comment.

Replace std::find_if with llvm::find_if.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/test/tools/llvm-objdump/RISCV/lit.local.cfg
  llvm/test/tools/llvm-objdump/RISCV/unknown-arch-attr.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,69 @@
+//===- unittests/RISCVAttributeParserTest.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 "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct RISCVAttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  RISCVAttributeSection(unsigned tag, unsigned value) : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+static bool testAttribute(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+   unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  RISCVAttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+static bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testAttribute) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testAttribute(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testAttribute(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testAttribute) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testAttribute(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testAttribute(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,63 @@
+//===- unittests/ELFAttributeParserTest.cpp 

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48
+// For pre-standard ones, which correspond to sections being deprecated in
+// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_"
+// is added to the names.
+//

ikudrin wrote:
> dblaikie wrote:
> > ikudrin wrote:
> > > dblaikie wrote:
> > > > Probably not arbitrarily - in the sense that this is an extension that 
> > > > consumers/producers will need to agree to - so maybe saying that 
> > > > ("non-standard extension"/"proposed for standardization" or something 
> > > > to that effect) and/or linking to the dwarf-discuss thread to support 
> > > > why these values were chosen & they can't be changed arbitrarily.
> > > As far as the enum is internal, no one should really worry about the 
> > > actual values; they are not important and do not need any kind of proof. 
> > > They may be really arbitrary, that will change nothing. That is what I 
> > > meant when said "more or less".
> > > 
> > > The plan is that this patch supports DWARFv5 unit indexes, but not the 
> > > proposed combined indexes. When the combined indexes are approved, there 
> > > will be another patch, which moves the enum with all extensions in the 
> > > public space. At that moment the factual values will become important, 
> > > and the references to the descriptive document will be added. Do you 
> > > think it will be possible to add such a document to the [[ 
> > > http://wiki.dwarfstd.org | DWARF Wiki ]]?
> > Hmm, I'm confused then - ah, OK - so you've added the enum to support 
> > encoding the version 2 and version 5 tables into one internal data 
> > structure, but haven't extended it to actually dump or use (for parsing: eg 
> > to find the debug_loc.dwo contribution for a v4 unit described by a v5 
> > index) them when parsing/rendering a v5 index.
> > 
> > OK, sorry I hadn't realized that. Then, yes, the comment makes sense for 
> > now. Perhaps "the values are only used internally/not parsed from input 
> > files (if these values appear in input files they will be considered 
> > "unknown")" would be more suitable?
> > 
> > > The plan is that this patch supports DWARFv5 unit indexes, but not the 
> > > proposed combined indexes. When the combined indexes are approved, there 
> > > will be another patch, which moves the enum with all extensions in the 
> > > public space. At that moment the factual values will become important, 
> > > and the references to the descriptive document will be added. Do you 
> > > think it will be possible to add such a document to the DWARF Wiki?
> > 
> > Given the DWARF committee is not in session at the moment (I think) & it'll 
> > be a while before another spec is published - I think it'll be necessary 
> > and appropriate to implement support for the extension columns in 
> > llvm-dwarfdump at least before/when they're implemented in llvm-dwp (which 
> > will be soon) to support testing that functionality & working with such 
> > files.
> > 
> > Might be able to put something on the DWARF wiki, but I don't know much 
> > about it/how things go up there.
> > Perhaps "the values are only used internally/not parsed from input files 
> > (if these values appear in input files they will be considered "unknown")" 
> > would be more suitable?
> 
> The comment says something similar in lines 52-53. Do you think it should be 
> extended?
> 
> > I think it'll be necessary and appropriate to implement support for the 
> > extension columns in llvm-dwarfdump at least before/when they're 
> > implemented in llvm-dwp (which will be soon) to support testing that 
> > functionality & working with such files.
> 
> I think the same. The only concern in my side is that the proposal should be 
> formulated as an RFC or similar document before implementing it in the code 
> so that all the implementations can reference the same source. For my taste, 
> a link to the middle of a forum conversation cannot be considered as a 
> reliable source.
I think it might be simpler to remove the "more or less arbitrary" part - it 
invites questions - just how much less?

And maybe rephrase the "is intended to be" and make it "The enum is for 
internal use only & doesn't correspond to any input/output constants".

But otherwise I guess this is fairly temporary so not worth haggling over.



Comment at: llvm/test/DebugInfo/X86/dwp-v5-loclists.s:1-3
+## The test checks that v5 compile units in package files read their
+## location tables from .debug_loclists.dwo sections.
+## See dwp-v2-loc.s for pre-v5 units.

ikudrin wrote:
> dblaikie wrote:
> > Might be possible/better to test this without debug_abbrev and debug_info - 
> > make the test shorter & dump only the loclists section itself? Yeah, not 
> > exactly valid, but no reason the dumper shouldn't support it and it'd be a 
> > more targeted test (apply this suggestion to other tests 

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked an inline comment as done.
mib added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2164
+  unsigned int curr_scalar = curr_piece.GetScalar().UInt();
+  curr_scalar = curr_scalar << curr_width;
+  bit_pieces.resize(curr_width + piece_byte_size * 8);

aprantl wrote:
> how do you know the `unsigned int` is large enough for this operation?
TBH, I just chose the same return type as GetScalar::UInt.
I’ll change it to GetScalar::UInt128 with an llvm::APInt 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76449



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

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Note that this diff is more a quick draft than a “finished” patch, I’m not 
expecting to submit this until the DW_OP_bit_piece support is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76449



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

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked 3 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:936
   /// Insertion point for evaluating multi-piece expression.
-  uint64_t op_piece_offset = 0;
-  Value pieces; // Used for DW_OP_piece
+  llvm::BitVector bit_pieces;
+  llvm::BitVector bit_mask;

aprantl wrote:
> Is this only going to be used for DW_OP_bit_piece? Otherwise I would have 
> expected a name like `top(_of_stack)` or `result`.
Yes, the same BitVector is used for both but I  separated the DW_OP_piece 
changes for this diff, I forgot to change the name —‘



Comment at: lldb/source/Expression/DWARFExpression.cpp:2505
+  Value pieces;
+  pieces.AppendBytes(bit_pieces.getData().data(), bit_pieces.size() / 8);
   result = pieces;

vsk wrote:
> Can we assert that this doesn't drop any bits (bit_pieces.size() % 8 == 0)?
Actually, I think this should be changed to `std::ceil(bit_pieces.size() / 
8.0)` instead of putting an assert



Comment at: lldb/source/Utility/Scalar.cpp:200
   case e_uint512:
-return (m_integer.getBitWidth() / 8);
+return ceil(m_integer.getBitWidth() / 8.0);
   case e_float:

vsk wrote:
> aprantl wrote:
> > ?
> This is worth a comment, I'm not sure what this is for.
When the bitWidth is < 8, the division result is 0, but it should be 1 byte 
instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76449



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

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a reviewer: labath.
vsk added a comment.

Excited to see this take shape :)




Comment at: lldb/source/Expression/DWARFExpression.cpp:935
 
   /// Insertion point for evaluating multi-piece expression.
+  llvm::BitVector bit_pieces;

Stale comment here, would be nice to describe the purpose of the two bitvectors.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1261
 case DW_OP_const1u:
-  stack.push_back(Scalar((uint8_t)opcodes.GetU8()));
+  stack.push_back(Scalar((unsigned int)opcodes.GetU8()));
   break;

I don't really understand a bunch of these changes. Is the idea to prevent 
implicit conversion to a type with the wrong size/signedness? I think that 
should really be tackled separately. We can make `Scalar`'s constructors safe 
like this: https://godbolt.org/z/APUMA6

(basically stick `template::value, int> = 0>` in front of each overload)



Comment at: lldb/source/Expression/DWARFExpression.cpp:2505
+  Value pieces;
+  pieces.AppendBytes(bit_pieces.getData().data(), bit_pieces.size() / 8);
   result = pieces;

Can we assert that this doesn't drop any bits (bit_pieces.size() % 8 == 0)?



Comment at: lldb/source/Utility/Scalar.cpp:200
   case e_uint512:
-return (m_integer.getBitWidth() / 8);
+return ceil(m_integer.getBitWidth() / 8.0);
   case e_float:

aprantl wrote:
> ?
This is worth a comment, I'm not sure what this is for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76449



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

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 251479.
wallace added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

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/environmentVariables/Makefile
  
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
  lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
  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
@@ -84,6 +84,11 @@
 "description": "Additional environment variables.",
 "default": []
 			},
+			"inheritEnvironment": {
+"type": "boolean",
+"description": "Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.",
+"default": false
+			},
 			"stopOnEntry": {
 "type": "boolean",
 "description": "Automatically stop after launch.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -40,6 +40,7 @@
 #include 
 #include 
 
+#include "lldb/API/SBEnvironment.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -1358,6 +1359,8 @@
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  bool launchWithDebuggerEnvironment =
+  GetBoolean(arguments, "inheritEnvironment", false);
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1374,6 +1377,12 @@
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  // Configure the inherit environment setting
+  std::ostringstream oss;
+  oss << "settings set target.inherit-env "
+  << (launchWithDebuggerEnvironment ? "true" : "false");
+  g_vsc.RunLLDBCommands(llvm::StringRef(), {oss.str()});
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1398,10 +1407,13 @@
   if (!args.empty())
 g_vsc.launch_info.SetArguments(MakeArgv(args).data(), true);
 
-  // Pass any environment variables along that the user specified.
-  auto envs = GetStrings(arguments, "env");
-  if (!envs.empty())
-g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
+  // This mimics what CommandObjectProcess does when launching a process
+  lldb::SBEnvironment env = g_vsc.target.GetEnvironment();
+  for (const auto _and_value : GetStrings(arguments, "env")) {
+auto kv = llvm::StringRef(name_and_value).split("=");
+env.Set(kv.first.str().c_str(), kv.second.str().c_str(), true);
+  }
+  g_vsc.launch_info.SetEnvironment(env, true);
 
   auto flags = g_vsc.launch_info.GetLaunchFlags();
 
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
@@ -0,0 +1,15 @@
+#include 
+#include 
+#include 
+#include 
+
+extern char **environ;
+
+int main(int argc, char const *argv[], char const *envp[]) {
+  char **env_var_pointer = environ;
+  for (char *env_variable = *env_var_pointer; env_variable;
+   env_variable = *++env_var_pointer) {
+printf("%s\n", env_variable);
+  }
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
@@ -0,0 +1,70 @@
+"""
+Test lldb-vscode environment variables
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+import os
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def getEnvOutputByProgram(self):
+env = {}
+for line in self.get_stdout().encode('utf-8').splitlines():
+(name, value) = line.split("=")
+env[name] = value
+return env
+
+@skipIfWindows
+@skipIfRemote
+def test_empty_environment(self):
+"""
+Tests running a process with an empty 

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:936
   /// Insertion point for evaluating multi-piece expression.
-  uint64_t op_piece_offset = 0;
-  Value pieces; // Used for DW_OP_piece
+  llvm::BitVector bit_pieces;
+  llvm::BitVector bit_mask;

Is this only going to be used for DW_OP_bit_piece? Otherwise I would have 
expected a name like `top(_of_stack)` or `result`.



Comment at: lldb/source/Expression/DWARFExpression.cpp:937
+  llvm::BitVector bit_pieces;
+  llvm::BitVector bit_mask;
 

This should perhaps be called undef_mask? And should have a comment what it is 
used for.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1267
 case DW_OP_const2u:
-  stack.push_back(Scalar((uint16_t)opcodes.GetU16()));
+  stack.push_back(Scalar((unsigned int)opcodes.GetU16()));
   break;

This is not portable. The size of int could be anything depending on the host. 
Why not always use APIInt?



Comment at: lldb/source/Expression/DWARFExpression.cpp:2164
+  unsigned int curr_scalar = curr_piece.GetScalar().UInt();
+  curr_scalar = curr_scalar << curr_width;
+  bit_pieces.resize(curr_width + piece_byte_size * 8);

how do you know the `unsigned int` is large enough for this operation?



Comment at: lldb/source/Utility/Scalar.cpp:200
   case e_uint512:
-return (m_integer.getBitWidth() / 8);
+return ceil(m_integer.getBitWidth() / 8.0);
   case e_float:

?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76449



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 251472.
wallace added a comment.

fix grammar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBLaunchInfo.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Utility/Environment.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py

Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- /dev/null
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -0,0 +1,125 @@
+"""Test the SBEnvironment APIs."""
+
+
+
+from math import fabs
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class SBEnvironmentAPICase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# We use this function to test both kind of accessors:
+# .  Get*AtIndex and GetEntries
+def assertEqualEntries(self, env, entries):
+self.assertEqual(env.GetNumValues(), len(entries))
+for i in range(env.GetNumValues()):
+name = env.GetNameAtIndex(i)
+value = env.GetValueAtIndex(i)
+self.assertIn(name + "=" + value, entries)
+
+entries = env.GetEntries()
+self.assertEqual(entries.GetSize(), len(entries))
+for i in range(entries.GetSize()):
+(name, value) = entries.GetStringAtIndex(i).split("=")
+self.assertIn(name + "=" + value, entries)
+
+
+
+@add_test_categories(['pyapi'])
+def test_platform_environment(self):
+env = self.dbg.GetSelectedPlatform().GetEnvironment()
+# We assume at least PATH is set
+self.assertNotEqual(env.Get("PATH"), None)
+
+
+@add_test_categories(['pyapi'])
+def test_launch_info(self):
+target = self.dbg.CreateTarget("")
+launch_info = target.GetLaunchInfo()
+env = launch_info.GetEnvironment()
+self.assertEqual(env.GetNumValues(), 0)
+
+env.Set("FOO", "bar", overwrite=True)
+self.assertEqual(env.GetNumValues(), 1)
+
+# Make sure we only modify the copy of the launchInfo's environment
+self.assertEqual(launch_info.GetEnvironment().GetNumValues(), 0)
+
+launch_info.SetEnvironment(env, append=True)
+self.assertEqual(launch_info.GetEnvironment().GetNumValues(), 1)
+
+# Make sure we can replace the launchInfo's environment
+env.Clear()
+env.Set("BAR", "foo", overwrite=True)
+env.PutEntry("X=y")
+launch_info.SetEnvironment(env, append=False)
+self.assertEqualEntries(launch_info.GetEnvironment(), ["BAR=foo", "X=y"])
+
+
+@add_test_categories(['pyapi'])
+def test_target_environment(self):
+env = self.dbg.GetSelectedTarget().GetEnvironment()
+# There is no target, so env should be empty
+self.assertEqual(env.GetNumValues(), 0)
+self.assertEqual(env.Get("PATH"), None)
+
+target = self.dbg.CreateTarget("")
+env = target.GetEnvironment()
+path = env.Get("PATH")
+# Now there's a target, so at least PATH should exist
+self.assertNotEqual(path, None)
+
+# Make sure we are getting a copy by modifying the env we just got
+env.PutEntry("PATH=#" + path)
+self.assertEqual(target.GetEnvironment().Get("PATH"), path)
+
+@add_test_categories(['pyapi'])
+def test_creating_and_modifying_environment(self):
+env = lldb.SBEnvironment()
+
+self.assertEqual(env.Get("FOO"), None)
+self.assertEqual(env.Get("BAR"), None)
+
+# We also test empty values
+self.assertTrue(env.Set("FOO", "", overwrite=False))
+env.Set("BAR", "foo", overwrite=False)
+
+self.assertEqual(env.Get("FOO"), "")
+self.assertEqual(env.Get("BAR"), "foo")
+
+self.assertEqual(env.GetNumValues(), 2)
+
+self.assertEqualEntries(env, ["FOO=", "BAR=foo"])
+
+# Make sure modifications work
+self.assertFalse(env.Set("FOO", "bar", overwrite=False))
+self.assertEqual(env.Get("FOO"), "")
+
+env.PutEntry("FOO=bar")
+self.assertEqual(env.Get("FOO"), "bar")
+
+self.assertEqualEntries(env, ["FOO=bar", "BAR=foo"])
+
+# Make sure we can unset
+   

[Lldb-commits] [PATCH] D76450: [lldb/PlatformDarwin] Always delete destination file first in PutFile

[Jason Molenda via Phabricator via lldb-commits]

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

nice catch, lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76450



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 251470.
wallace added a comment.

- Added both kinds of APIs we were discussing. It will come handy for all 
different kind of usages
- Added an SBEnvironment API for SBLaunchInfo, which will be used in 
https://reviews.llvm.org/D74636
- Address all sorts of comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBLaunchInfo.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Utility/Environment.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py

Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- /dev/null
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -0,0 +1,125 @@
+"""Test the SBEnvironment APIs."""
+
+
+
+from math import fabs
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class SBEnvironmentAPICase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# We use this function to test both kind of accessors:
+# .  Get*AtIndex and GetEntries
+def assertEqualEntries(self, env, entries):
+self.assertEqual(env.GetNumValues(), len(entries))
+for i in range(env.GetNumValues()):
+name = env.GetNameAtIndex(i)
+value = env.GetValueAtIndex(i)
+self.assertIn(name + "=" + value, entries)
+
+entries = env.GetEntries()
+self.assertEqual(entries.GetSize(), len(entries))
+for i in range(entries.GetSize()):
+(name, value) = entries.GetStringAtIndex(i).split("=")
+self.assertIn(name + "=" + value, entries)
+
+
+
+@add_test_categories(['pyapi'])
+def test_platform_environment(self):
+env = self.dbg.GetSelectedPlatform().GetEnvironment()
+# We assume at least PATH is set
+self.assertNotEqual(env.Get("PATH"), None)
+
+
+@add_test_categories(['pyapi'])
+def test_launch_info(self):
+target = self.dbg.CreateTarget("")
+launch_info = target.GetLaunchInfo()
+env = launch_info.GetEnvironment()
+self.assertEqual(env.GetNumValues(), 0)
+
+env.Set("FOO", "bar", overwrite=True)
+self.assertEqual(env.GetNumValues(), 1)
+
+# Make sure we only modify the copy of the launchInfo's environment
+self.assertEqual(launch_info.GetEnvironment().GetNumValues(), 0)
+
+launch_info.SetEnvironment(env, append=True)
+self.assertEqual(launch_info.GetEnvironment().GetNumValues(), 1)
+
+# Make sure we can replace the launchInfo's environment
+env.Clear()
+env.Set("BAR", "foo", overwrite=True)
+env.PutEntry("X=y")
+launch_info.SetEnvironment(env, append=False)
+self.assertEqualEntries(launch_info.GetEnvironment(), ["BAR=foo", "X=y"])
+
+
+@add_test_categories(['pyapi'])
+def test_target_environment(self):
+env = self.dbg.GetSelectedTarget().GetEnvironment()
+# There is no target, so env should be empty
+self.assertEqual(env.GetNumValues(), 0)
+self.assertEqual(env.Get("PATH"), None)
+
+target = self.dbg.CreateTarget("")
+env = target.GetEnvironment()
+path = env.Get("PATH")
+# Now there's a target, so at least PATH should exist
+self.assertNotEqual(path, None)
+
+# Make sure we are getting a copy by modifying the env we just got
+env.PutEntry("PATH=#" + path)
+self.assertEqual(target.GetEnvironment().Get("PATH"), path)
+
+@add_test_categories(['pyapi'])
+def test_creating_and_modifying_environment(self):
+env = lldb.SBEnvironment()
+
+self.assertEqual(env.Get("FOO"), None)
+self.assertEqual(env.Get("BAR"), None)
+
+# We also test empty values
+self.assertTrue(env.Set("FOO", "", overwrite=False))
+env.Set("BAR", "foo", overwrite=False)
+
+self.assertEqual(env.Get("FOO"), "")
+self.assertEqual(env.Get("BAR"), "foo")
+
+self.assertEqual(env.GetNumValues(), 2)
+
+self.assertEqualEntries(env, ["FOO=", "BAR=foo"])
+
+# Make sure modifications work
+self.assertFalse(env.Set("FOO", "bar", overwrite=False))
+

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Frank Ch. Eigler via Phabricator via lldb-commits]

fche2 added inline comments.



Comment at: lldb/source/Host/common/DebugInfoD.cpp:59
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),

jankratochvil wrote:
> Here it will contact the server even if the binary does not contain any 
> build-id - LLDB then generates UUID as 4 bytes long one:
> ```
>   // Use 4 bytes of crc from the .gnu_debuglink section.
>   u32le data(gnu_debuglink_crc);
>   uuid = UUID::fromData(, sizeof(data));
> ```
> That is a needless performance regression.
> I sure do not like making such decision on the LLDB side. Maybe libdebuginfod 
> could rather make such optimization - IMO as Frank Eigler.
Could kkleine reject uuid of length 4 in the above test, i.e. something like:

if (!uuid.IsValid() || uuid.GetBytes().size() == sizeof(u32le)) // 
.gnu_debuglink crc32
   continue;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 2 inline comments as done.
wallace added inline comments.



Comment at: lldb/source/API/SBEnvironment.cpp:62-73
+  ConstString(std::next(m_opaque_up->begin(), index)->first())
+  .AsCString(""));
+}
+
+const char *SBEnvironment::GetValueAtIndex(size_t index) {
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetValueAtIndex, (size_t),
+ index);

clayborg wrote:
> labath wrote:
> > I don't think these need to const-stringified now, given that they are 
> > backed by the underlying map. We already have functions which return 
> > pointers to internal objects (e.g. SBStream::GetData).
> > 
> > @clayborg? 
> That's a tough one. I would like to think that any "const char *" that comes 
> back from an API that returns strings could just be pointer compared if 
> needed. So I like the idea that for strings that comes out of the API that 
> they are all const-ed and could easily be compared. I am fine with 
> SBStream::GetData() returning its own string because _it_ owns the data. So I 
> would vote to ConstString() any returned results
I'm adding a Clear method. With that, the backing char* might be wiped out and 
the python reference to this string might be invalid unless we use ConstString



Comment at: lldb/source/API/SBEnvironment.cpp:80-81
+ overwrite);
+  if (overwrite || m_opaque_up->find(name) == m_opaque_up->end()) {
+m_opaque_up->insert_or_assign(name, std::string(value));
+return LLDB_RECORD_RESULT(true);

labath wrote:
> how about `if(overwrite) insert_or_assign(name, value) else try_emplace(name, 
> value)`? (avoiding two map lookups)
teach me more


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D76450: [lldb/PlatformDarwin] Always delete destination file first in PutFile

[Frederic Riss via Phabricator via lldb-commits]

friss created this revision.
friss added a reviewer: jasonmolenda.
Herald added a project: LLDB.

The default behavior of Platform::PutFile is to open the file and
truncate it if it already exists. This works fine and is a sensible
default, but it interacts badly with code-signing on iOS, as doing so
invalidates the signature of the file (even if the new content has a
valid code signature).

We have a couple tests which on purpose reload a different binary with
the same name. Those tests are currently broken because of the above
interaction.

This patch simply makes the Darwin platform unconditionally delete the
destination file before sending the new one to work around this issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76450

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -25,6 +25,11 @@
 
   ~PlatformDarwin() override;
 
+  lldb_private::Status PutFile(const lldb_private::FileSpec ,
+   const lldb_private::FileSpec ,
+   uint32_t uid = UINT32_MAX,
+   uint32_t gid = UINT32_MAX) override;
+
   // lldb_private::Platform functions
   lldb_private::Status
   ResolveSymbolFile(lldb_private::Target ,
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -58,6 +58,18 @@
 /// inherited from by the plug-in instance.
 PlatformDarwin::~PlatformDarwin() {}
 
+lldb_private::Status
+PlatformDarwin::PutFile(const lldb_private::FileSpec ,
+const lldb_private::FileSpec ,
+uint32_t uid, uint32_t gid) {
+  // Unconditionally unlink the destination. If it is an executable,
+  // simply opening it and truncating its contents would invalidate
+  // its cached code signature.
+  Unlink(destination);
+  return PlatformPOSIX::PutFile(source, destination, uid, gid);
+}
+
+
 FileSpecList PlatformDarwin::LocateExecutableScriptingResources(
 Target *target, Module , Stream *feedback_stream) {
   FileSpecList file_list;


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -25,6 +25,11 @@
 
   ~PlatformDarwin() override;
 
+  lldb_private::Status PutFile(const lldb_private::FileSpec ,
+   const lldb_private::FileSpec ,
+   uint32_t uid = UINT32_MAX,
+   uint32_t gid = UINT32_MAX) override;
+
   // lldb_private::Platform functions
   lldb_private::Status
   ResolveSymbolFile(lldb_private::Target ,
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -58,6 +58,18 @@
 /// inherited from by the plug-in instance.
 PlatformDarwin::~PlatformDarwin() {}
 
+lldb_private::Status
+PlatformDarwin::PutFile(const lldb_private::FileSpec ,
+const lldb_private::FileSpec ,
+uint32_t uid, uint32_t gid) {
+  // Unconditionally unlink the destination. If it is an executable,
+  // simply opening it and truncating its contents would invalidate
+  // its cached code signature.
+  Unlink(destination);
+  return PlatformPOSIX::PutFile(source, destination, uid, gid);
+}
+
+
 FileSpecList PlatformDarwin::LocateExecutableScriptingResources(
 Target *target, Module , Stream *feedback_stream) {
   FileSpecList file_list;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76449: [lldb/Dwarf] Change DW_OP_piece to use an llvm::BitVector

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: aprantl, vsk, friss.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch changes the implementation of DW_OP_piece to use
llvm::BitVector instead of the lldb_private::Value.

This allow to have more granularity which would be useful to implement
DW_OP_bit_piece but also when combined with a second BitVector for
masking the unknown bits/bytes, improves the DWARF Expression
evaluation Since it will show a future patch, the unknown bits/bytes as
optimised rather than showing - misleading - zeroes.

Additionally, this patch fixes a overloading resolution on the Scalar
constructor when using fixed-size interger types instead of fundamentale types.
It also fixes a bug when requesting a Scalar size when the value is less
than a byte.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76449

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Utility/Scalar.cpp

Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -197,7 +197,7 @@
   case e_uint256:
   case e_sint512:
   case e_uint512:
-return (m_integer.getBitWidth() / 8);
+return ceil(m_integer.getBitWidth() / 8.0);
   case e_float:
 return sizeof(float_t);
   case e_double:
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -933,8 +933,8 @@
   uint32_t reg_num;
 
   /// Insertion point for evaluating multi-piece expression.
-  uint64_t op_piece_offset = 0;
-  Value pieces; // Used for DW_OP_piece
+  llvm::BitVector bit_pieces;
+  llvm::BitVector bit_mask;
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
@@ -1258,34 +1258,34 @@
 // 8-byte signed integer constant DW_OP_constu unsigned LEB128 integer
 // constant DW_OP_consts signed LEB128 integer constant
 case DW_OP_const1u:
-  stack.push_back(Scalar((uint8_t)opcodes.GetU8()));
+  stack.push_back(Scalar((unsigned int)opcodes.GetU8()));
   break;
 case DW_OP_const1s:
-  stack.push_back(Scalar((int8_t)opcodes.GetU8()));
+  stack.push_back(Scalar((int)opcodes.GetU8()));
   break;
 case DW_OP_const2u:
-  stack.push_back(Scalar((uint16_t)opcodes.GetU16()));
+  stack.push_back(Scalar((unsigned int)opcodes.GetU16()));
   break;
 case DW_OP_const2s:
-  stack.push_back(Scalar((int16_t)opcodes.GetU16()));
+  stack.push_back(Scalar((int)opcodes.GetU16()));
   break;
 case DW_OP_const4u:
-  stack.push_back(Scalar((uint32_t)opcodes.GetU32()));
+  stack.push_back(Scalar((unsigned int)opcodes.GetU32()));
   break;
 case DW_OP_const4s:
-  stack.push_back(Scalar((int32_t)opcodes.GetU32()));
+  stack.push_back(Scalar((int)opcodes.GetU32()));
   break;
 case DW_OP_const8u:
-  stack.push_back(Scalar((uint64_t)opcodes.GetU64()));
+  stack.push_back(Scalar((unsigned long)opcodes.GetU64()));
   break;
 case DW_OP_const8s:
-  stack.push_back(Scalar((int64_t)opcodes.GetU64()));
+  stack.push_back(Scalar((long)opcodes.GetU64()));
   break;
 case DW_OP_constu:
-  stack.push_back(Scalar(opcodes.GetULEB128()));
+  stack.push_back(Scalar((unsigned long)opcodes.GetULEB128()));
   break;
 case DW_OP_consts:
-  stack.push_back(Scalar(opcodes.GetSLEB128()));
+  stack.push_back(Scalar((long)opcodes.GetSLEB128()));
   break;
 
 // OPCODE: DW_OP_dup
@@ -2064,27 +2064,19 @@
 
   if (piece_byte_size > 0) {
 Value curr_piece;
+
+const uint64_t curr_width = std::max(bit_mask.size(), bit_pieces.size());
 
 if (stack.empty()) {
-  // In a multi-piece expression, this means that the current piece is
-  // not available. Fill with zeros for now by resizing the data and
-  // appending it
-  curr_piece.ResizeData(piece_byte_size);
-  // Note that "0" is not a correct value for the unknown bits.
-  // It would be better to also return a mask of valid bits together
-  // with the expression result, so the debugger can print missing
-  // members as "" or something.
-  ::memset(curr_piece.GetBuffer().GetBytes(), 0, piece_byte_size);
-  pieces.AppendDataToHostBuffer(curr_piece);
+  bit_mask.resize(curr_width + piece_byte_size * 8);
+  bit_mask.set(curr_width, curr_width + piece_byte_size * 8);
 } else {
   Status error;
   // Extract the current piece into "curr_piece"
   Value curr_piece_source_value(stack.back());
   stack.pop_back();
 
-  const Value::ValueType curr_piece_source_value_type =
-  

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D76111#1930783 , @labath wrote:

> The new interface is ok for me, but there are two things I want to mention:
>
> - the iteration via `Get{Name,Value}AtIndex` is going to be slow because the 
> underlying map (like most maps) does not have random access iterators. This 
> is the problem I was trying to solve with the `GetEntries`-like API, but I 
> agree that has its issues too.


I am guessing people won't use these much. I didn't know he underlying class 
used a map. No great solution here unless we modify the underlying class to 
have a vector with a map of name to index?

> - I agree that forcing a user to manually construct `name=value` strings if 
> he has then as separate entities is not good, but I also don't think we're 
> doing anyone a favor by not providing an api which would accept such strings. 
> The `name=value` format is very universal, and I think users will often have 
> the value in this format already (for example, D74636 
>  does). This means that those users would 
> have to implement `=`-splitting themselves before using this class. This is 
> why the internal Environment class provides both kinds of api, and maybe also 
> the reason why the c library provides both `putenv(3)` and `setenv(3)`.

Fine to add functions for this. The user might grab the environment dump in 
text and want to set all of those env vars, so having the API is fine with me.




Comment at: lldb/source/API/SBEnvironment.cpp:59
+ index);
+  if (index < 0 || index >= GetNumValues())
+return LLDB_RECORD_RESULT(nullptr);

labath wrote:
> `index < 0` is not possible now.
yeah, size_t is unsigned on all platforms.



Comment at: lldb/source/API/SBEnvironment.cpp:62-73
+  ConstString(std::next(m_opaque_up->begin(), index)->first())
+  .AsCString(""));
+}
+
+const char *SBEnvironment::GetValueAtIndex(size_t index) {
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetValueAtIndex, (size_t),
+ index);

labath wrote:
> I don't think these need to const-stringified now, given that they are backed 
> by the underlying map. We already have functions which return pointers to 
> internal objects (e.g. SBStream::GetData).
> 
> @clayborg? 
That's a tough one. I would like to think that any "const char *" that comes 
back from an API that returns strings could just be pointer compared if needed. 
So I like the idea that for strings that comes out of the API that they are all 
const-ed and could easily be compared. I am fine with SBStream::GetData() 
returning its own string because _it_ owns the data. So I would vote to 
ConstString() any returned results



Comment at: lldb/source/API/SBEnvironment.cpp:81
+  if (overwrite || m_opaque_up->find(name) == m_opaque_up->end()) {
+m_opaque_up->insert_or_assign(name, std::string(value));
+return LLDB_RECORD_RESULT(true);

Or do we change the underlying API in the m_opaque_up class to accept an 
overwrite arg?



Comment at: lldb/source/API/SBPlatform.cpp:658-660
+  if (platform_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(platform_sp->GetEnvironment()));
+  }

remove {} (clang code guidelines for single statement if)



Comment at: lldb/source/API/SBTarget.cpp:2396-2398
+  if (target_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(target_sp->GetEnvironment()));
+  }

remove {} (clang code guidelines for single statement if)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D76411: [debugserver] Implement hardware breakpoints for ARM64

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG90308a4da167: [debugserver] Implement hardware breakpoints 
for ARM64 (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76411

Files:
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
@@ -26,10 +26,12 @@
 
   DNBArchMachARM64(MachThread *thread)
   : m_thread(thread), m_state(), m_disabled_watchpoints(),
-m_watchpoint_hw_index(-1), m_watchpoint_did_occur(false),
+m_disabled_breakpoints(), m_watchpoint_hw_index(-1),
+m_watchpoint_did_occur(false),
 m_watchpoint_resume_single_step_enabled(false),
 m_saved_register_states() {
 m_disabled_watchpoints.resize(16);
+m_disabled_breakpoints.resize(16);
 memset(_dbg_save, 0, sizeof(m_dbg_save));
   }
 
@@ -62,7 +64,13 @@
   static const uint8_t *SoftwareBreakpointOpcode(nub_size_t byte_size);
   static uint32_t GetCPUType();
 
+  virtual uint32_t NumSupportedHardwareBreakpoints();
   virtual uint32_t NumSupportedHardwareWatchpoints();
+
+  virtual uint32_t EnableHardwareBreakpoint(nub_addr_t addr, nub_size_t size,
+bool also_set_on_task);
+  virtual bool DisableHardwareBreakpoint(uint32_t hw_break_index,
+ bool also_set_on_task);
   virtual uint32_t EnableHardwareWatchpoint(nub_addr_t addr, nub_size_t size,
 bool read, bool write,
 bool also_set_on_task);
@@ -229,10 +237,11 @@
   State m_state;
   arm_debug_state64_t m_dbg_save;
 
-  // arm64 doesn't keep the disabled watchpoint values in the debug register
-  // context like armv7;
+  // arm64 doesn't keep the disabled watchpoint and breakpoint values in the
+  // debug register context like armv7;
   // we need to save them aside when we disable them temporarily.
   std::vector m_disabled_watchpoints;
+  std::vector m_disabled_breakpoints;
 
   // The following member variables should be updated atomically.
   int32_t m_watchpoint_hw_index;
Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -666,6 +666,112 @@
   return g_num_supported_hw_watchpoints;
 }
 
+uint32_t DNBArchMachARM64::NumSupportedHardwareBreakpoints() {
+  // Set the init value to something that will let us know that we need to
+  // autodetect how many breakpoints are supported dynamically...
+  static uint32_t g_num_supported_hw_breakpoints = UINT_MAX;
+  if (g_num_supported_hw_breakpoints == UINT_MAX) {
+// Set this to zero in case we can't tell if there are any HW breakpoints
+g_num_supported_hw_breakpoints = 0;
+
+size_t len;
+uint32_t n = 0;
+len = sizeof(n);
+if (::sysctlbyname("hw.optional.breakpoint", , , NULL, 0) == 0) {
+  g_num_supported_hw_breakpoints = n;
+  DNBLogThreadedIf(LOG_THREAD, "hw.optional.breakpoint=%u", n);
+} else {
+// For AArch64 we would need to look at ID_AA64DFR0_EL1 but debugserver runs in
+// EL0 so it can't access that reg.  The kernel should have filled in the
+// sysctls based on it though.
+#if defined(__arm__)
+  uint32_t register_DBGDIDR;
+
+  asm("mrc p14, 0, %0, c0, c0, 0" : "=r"(register_DBGDIDR));
+  uint32_t numWRPs = bits(register_DBGDIDR, 31, 28);
+  // Zero is reserved for the WRP count, so don't increment it if it is zero
+  if (numWRPs > 0)
+numWRPs++;
+  g_num_supported_hw_breakpoints = numWRPs;
+  DNBLogThreadedIf(LOG_THREAD,
+   "Number of supported hw breakpoint via asm():  %d",
+   g_num_supported_hw_breakpoints);
+#endif
+}
+  }
+  return g_num_supported_hw_breakpoints;
+}
+
+uint32_t DNBArchMachARM64::EnableHardwareBreakpoint(nub_addr_t addr,
+nub_size_t size,
+bool also_set_on_task) {
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::EnableHardwareBreakpoint(addr = "
+   "0x%8.8llx, size = %zu)",
+   (uint64_t)addr, size);
+
+  const uint32_t num_hw_breakpoints = NumSupportedHardwareBreakpoints();
+
+  nub_addr_t aligned_bp_address = addr;
+  uint32_t control_value = 0;
+
+  switch (size) {
+  case 2:

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 251428.
wallace added a comment.

support python2.7


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2833,7 +2833,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
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
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,31 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+self.create_debug_adaptor()
+# The underlying lldb-vscode process must be alive
+self.assertEqual(self.vscode.process.poll(), None)
+
+# The lldb-vscode process should finish even though
+# we didn't close the communication socket explicitly
+self.vscode.request_disconnect()
+
+# Wait until the underlying lldb-vscode process dies.
+# We need to do this because the popen.wait function in python2.7
+# doesn't have a timeout argument.
+for _ in range(10):
+time.sleep(1)
+if self.vscode.process.poll() is not None:
+break
+# Check the return code
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2833,7 +2833,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
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
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,31 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+self.create_debug_adaptor()
+# The underlying lldb-vscode process must be alive
+self.assertEqual(self.vscode.process.poll(), None)
+
+# The lldb-vscode process should finish even though
+# we didn't close the communication socket explicitly
+self.vscode.request_disconnect()
+
+# Wait until the underlying lldb-vscode process dies.
+# We need to do this because the popen.wait function in python2.7
+# doesn't have a timeout argument.
+for _ in range(10):
+time.sleep(1)
+if self.vscode.process.poll() is not None:
+break
+# Check the return code
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Well, I think I'll implement both kind of accessors in the API to account for 
all possible cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D76411: [debugserver] Implement hardware breakpoints for ARM64

[Jason Molenda via Phabricator via lldb-commits]

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

Nice patch, thanks for doing this.  I wouldn't have added the fallback code in 
`DNBArchMachARM64::NumSupportedHardwareBreakpoints` to handle the case where 
the sysctl fails - it doesn't fail -  but there's no harm in it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D76411



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

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 251425.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2833,7 +2833,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
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
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,21 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+self.create_debug_adaptor()
+# The underlying lldb-vscode process must be alive
+self.assertEqual(self.vscode.process.poll(), None)
+self.vscode.request_disconnect()
+# The lldb-vscode process should finish even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.wait(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2833,7 +2833,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
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
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,21 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+self.create_debug_adaptor()
+# The underlying lldb-vscode process must be alive
+self.assertEqual(self.vscode.process.poll(), None)
+self.vscode.request_disconnect()
+# The lldb-vscode process should finish even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.wait(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 90308a4 - [debugserver] Implement hardware breakpoints for ARM64

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-19T11:55:48-07:00
New Revision: 90308a4da167c531a16fcecc3927596fffd5d5d5

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

LOG: [debugserver] Implement hardware breakpoints for ARM64

Add support for hardware breakpoints on ARM64.

Differential revision: https://reviews.llvm.org/D76411

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index f99dbc48b128..e5d4b05d987c 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -666,6 +666,112 @@ uint32_t 
DNBArchMachARM64::NumSupportedHardwareWatchpoints() {
   return g_num_supported_hw_watchpoints;
 }
 
+uint32_t DNBArchMachARM64::NumSupportedHardwareBreakpoints() {
+  // Set the init value to something that will let us know that we need to
+  // autodetect how many breakpoints are supported dynamically...
+  static uint32_t g_num_supported_hw_breakpoints = UINT_MAX;
+  if (g_num_supported_hw_breakpoints == UINT_MAX) {
+// Set this to zero in case we can't tell if there are any HW breakpoints
+g_num_supported_hw_breakpoints = 0;
+
+size_t len;
+uint32_t n = 0;
+len = sizeof(n);
+if (::sysctlbyname("hw.optional.breakpoint", , , NULL, 0) == 0) {
+  g_num_supported_hw_breakpoints = n;
+  DNBLogThreadedIf(LOG_THREAD, "hw.optional.breakpoint=%u", n);
+} else {
+// For AArch64 we would need to look at ID_AA64DFR0_EL1 but debugserver runs in
+// EL0 so it can't access that reg.  The kernel should have filled in the
+// sysctls based on it though.
+#if defined(__arm__)
+  uint32_t register_DBGDIDR;
+
+  asm("mrc p14, 0, %0, c0, c0, 0" : "=r"(register_DBGDIDR));
+  uint32_t numWRPs = bits(register_DBGDIDR, 31, 28);
+  // Zero is reserved for the WRP count, so don't increment it if it is 
zero
+  if (numWRPs > 0)
+numWRPs++;
+  g_num_supported_hw_breakpoints = numWRPs;
+  DNBLogThreadedIf(LOG_THREAD,
+   "Number of supported hw breakpoint via asm():  %d",
+   g_num_supported_hw_breakpoints);
+#endif
+}
+  }
+  return g_num_supported_hw_breakpoints;
+}
+
+uint32_t DNBArchMachARM64::EnableHardwareBreakpoint(nub_addr_t addr,
+nub_size_t size,
+bool also_set_on_task) {
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::EnableHardwareBreakpoint(addr = "
+   "0x%8.8llx, size = %zu)",
+   (uint64_t)addr, size);
+
+  const uint32_t num_hw_breakpoints = NumSupportedHardwareBreakpoints();
+
+  nub_addr_t aligned_bp_address = addr;
+  uint32_t control_value = 0;
+
+  switch (size) {
+  case 2:
+control_value = (0x3 << 5) | 7;
+aligned_bp_address &= ~1;
+break;
+  case 4:
+control_value = (0xfu << 5) | 7;
+aligned_bp_address &= ~3;
+break;
+  };
+
+  // Read the debug state
+  kern_return_t kret = GetDBGState(false);
+  if (kret == KERN_SUCCESS) {
+// Check to make sure we have the needed hardware support
+uint32_t i = 0;
+
+for (i = 0; i < num_hw_breakpoints; ++i) {
+  if ((m_state.dbg.__bcr[i] & BCR_ENABLE) == 0)
+break; // We found an available hw breakpoint slot (in i)
+}
+
+// See if we found an available hw breakpoint slot above
+if (i < num_hw_breakpoints) {
+  m_state.dbg.__bvr[i] = aligned_bp_address;
+  m_state.dbg.__bcr[i] = control_value;
+
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::EnableHardwareBreakpoint() "
+   "adding breakpoint on address 0x%llx with control "
+   "register value 0x%x",
+   (uint64_t)m_state.dbg.__bvr[i],
+   (uint32_t)m_state.dbg.__bcr[i]);
+
+  // The kernel will set the MDE_ENABLE bit in the MDSCR_EL1 for us
+  // automatically, don't need to do it here.
+  kret = SetDBGState(also_set_on_task);
+
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::"
+   "EnableHardwareBreakpoint() "
+   "SetDBGState() => 0x%8.8x.",
+   kret);
+
+  if (kret == KERN_SUCCESS)
+return i;
+} else {
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::"
+   "EnableHardwareBreakpoint(): All "
+   "hardware 

[Lldb-commits] [PATCH] D76216: Improve step over performance

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Pavel or Jim, could you possibly land this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216



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

[Lldb-commits] [lldb] 7b24425 - Reland [lldb] Fix string summary of an empty NSPathStore2

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-19T18:50:26+01:00
New Revision: 7b2442584e40f97693c38c0d79b83f770d557039

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

LOG: Reland [lldb] Fix string summary of an empty NSPathStore2

(This is D68010 but I also set the new parameter in LibStdcpp.cpp to fix
the Debian tests).

Summary:
Printing a summary for an empty NSPathStore2 string currently prints random 
bytes behind the empty string pointer from memory (rdar://55575888).

It seems the reason for this is that the SourceSize parameter in the 
`ReadStringAndDumpToStreamOptions` - which is supposed to contain the string
length - actually uses the length 0 as a magic value for saying "read as much 
as possible from the buffer" which is clearly wrong for empty strings.

This patch adds another flag that indicates if we have know the string length 
or not and makes this behaviour dependent on that (which seemingly
was the original purpose of this magic value).

Reviewers: aprantl, JDevlieghere, shafik

Reviewed By: aprantl

Subscribers: christof, abidh, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/StringPrinter.h
lldb/source/DataFormatters/StringPrinter.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/ObjC/NSString.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/StringPrinter.h 
b/lldb/include/lldb/DataFormatters/StringPrinter.h
index 6f8869cc2a1e..5842cde893d8 100644
--- a/lldb/include/lldb/DataFormatters/StringPrinter.h
+++ b/lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -115,9 +115,15 @@ class StringPrinter {
 
 lldb::ProcessSP GetProcessSP() const { return m_process_sp; }
 
+void SetHasSourceSize(bool e) { m_has_source_size = e; }
+
+bool HasSourceSize() const { return m_has_source_size; }
+
   private:
 uint64_t m_location = 0;
 lldb::ProcessSP m_process_sp;
+/// True iff we know the source size of the string.
+bool m_has_source_size = false;
   };
 
   class ReadBufferAndDumpToStreamOptions : public DumpToStreamOptions {

diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 92dd71d17b8c..4515b67b2adf 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -525,27 +525,33 @@ static bool ReadUTFBufferAndDumpToStream(
   if (!options.GetStream())
 return false;
 
-  uint32_t sourceSize = options.GetSourceSize();
+  uint32_t sourceSize;
   bool needs_zero_terminator = options.GetNeedsZeroTermination();
 
   bool is_truncated = false;
   const auto max_size = 
process_sp->GetTarget().GetMaximumSizeOfStringSummary();
 
-  if (!sourceSize) {
+  if (options.HasSourceSize()) {
+sourceSize = options.GetSourceSize();
+if (!options.GetIgnoreMaxLength()) {
+  if (sourceSize > max_size) {
+sourceSize = max_size;
+is_truncated = true;
+  }
+}
+  } else {
 sourceSize = max_size;
 needs_zero_terminator = true;
-  } else if (!options.GetIgnoreMaxLength()) {
-if (sourceSize > max_size) {
-  sourceSize = max_size;
-  is_truncated = true;
-}
   }
 
   const int bufferSPSize = sourceSize * type_width;
 
   lldb::DataBufferSP buffer_sp(new DataBufferHeap(bufferSPSize, 0));
 
-  if (!buffer_sp->GetBytes())
+  // Check if we got bytes. We never get any bytes if we have an empty
+  // string, but we still continue so that we end up actually printing
+  // an empty string ("").
+  if (sourceSize != 0 && !buffer_sp->GetBytes())
 return false;
 
   Status error;

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 78f58754cc31..b4af67ecee0d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -259,6 +259,7 @@ bool 
lldb_private::formatters::LibStdcppStringSummaryProvider(
   if (error.Fail())
 return false;
   options.SetSourceSize(size_of_data);
+  options.SetHasSourceSize(true);
 
   if (!StringPrinter::ReadStringAndDumpToStream<
   StringPrinter::StringElementType::UTF8>(options)) {
@@ -319,6 +320,7 @@ bool 
lldb_private::formatters::LibStdcppWStringSummaryProvider(
   if (error.Fail())
 return false;
   options.SetSourceSize(size_of_data);
+  options.SetHasSourceSize(true);
   options.SetPrefixToken("L");
 
   switch (wchar_size) {

[Lldb-commits] [lldb] 50f1985 - [lldb][NFC] Delete the original UserExpression before trying to reparse it with FixIts.

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-19T18:10:57+01:00
New Revision: 50f198535363c6882b02c3dde7bec31e723c4762

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

LOG: [lldb][NFC] Delete the original UserExpression before trying to reparse it 
with FixIts.

Currently when an expression fails to parse and we have a FixIt, we keep
the failed UserExpression around while trying to parse the expression with
applied fixits. This means that we have this rather confusing control flow:

1. Original expression created and parsing attempted.
2. Expression with applied FixIts is created and parsing attempted.
3. Original expression is destroyed and parser deconstructed.
4. Expression with applied FixIts is destroyed and parser deconstructed.

This patch just deletes the original expression so that step 2 and 3 are
swapped and the whole process looks more like just sequentially parsing two
expressions (which is what we actually do here).

Doesn't fix anything just makes the code less fragile.

Added: 


Modified: 
lldb/source/Expression/UserExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index 0243cc0c3750..5bd2321e48dd 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -259,6 +259,10 @@ UserExpression::Evaluate(ExecutionContext _ctx,
 
   // If there is a fixed expression, try to parse it:
   if (!parse_success) {
+// Delete the expression that failed to parse before attempting to parse
+// the next expression.
+user_expression_sp.reset();
+
 execution_results = lldb::eExpressionParseError;
 if (fixed_expression && !fixed_expression->empty() &&
 options.GetAutoApplyFixIts()) {



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

[Lldb-commits] [PATCH] D76216: Improve step over performance

[Jim Ingham via Phabricator via lldb-commits]

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

Sorry, forgot to select the action...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216



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

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This test just seems wrong to me.  We can pretend that processes that haven't 
crashed don't have crash_info, and manually suppress the output in lldb if the 
stop state isn't a crash.  But it seems useful to ask "what are the crash_info 
bits in flight right now" even if you haven't crashed.  So that doesn't seem 
right to me.  Or you have to acknowledge that you have no way of knowing 
whether there is crash info data hanging around, in which case any test that 
depends on there being none is an invalid test.  I'd delete this whole test, 
since it seems to me like it will only ever succeed by accident.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76407



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

[Lldb-commits] [lldb] 76a5451 - [lldb/testsuite] un-XFail TestInlineStepping.py on linux and windows

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-03-19T09:24:11-07:00
New Revision: 76a5451a524c0cecdf21a03844ba628bfe857369

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

LOG: [lldb/testsuite] un-XFail TestInlineStepping.py on linux and windows

It looks like my tweak in ecc6c426977f5 made the test pass on windows
and the linux aarch64 bot.

Added: 


Modified: 
lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py 
b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 40e29e614ad6..8e84566d9f69 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -16,9 +16,6 @@ class TestInlineStepping(TestBase):
 @expectedFailureAll(
 compiler="icc",
 bugnumber="# Not really a bug.  ICC combines two inlined functions.")
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr32343")
-@expectedFailureAll(archs=["aarch64"], oslist=["linux"],
-bugnumber="llvm.org/pr44057")
 def test_with_python_api(self):
 """Test stepping over and into inlined functions."""
 self.build()



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



Comment at: lldb/source/Host/common/DebugInfoD.cpp:39
+
+llvm::Expected findSource(UUID buildID, const std::string ) {
+  if (!buildID.IsValid())

`const UUID ` as it is even bigger (40 bytes) than `std::string` (32 
bytes).



Comment at: lldb/source/Host/common/DebugInfoD.cpp:56
+  //   return 0; // continue
+  // });
+

Excessive leftover comment.



Comment at: lldb/source/Host/common/DebugInfoD.cpp:59
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),

Here it will contact the server even if the binary does not contain any 
build-id - LLDB then generates UUID as 4 bytes long one:
```
  // Use 4 bytes of crc from the .gnu_debuglink section.
  u32le data(gnu_debuglink_crc);
  uuid = UUID::fromData(, sizeof(data));
```
That is a needless performance regression.
I sure do not like making such decision on the LLDB side. Maybe libdebuginfod 
could rather make such optimization - IMO as Frank Eigler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D76408: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

[Frederic Riss via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe154cbb124a6: [lldb/testsuite] XFail TestBuiltinTrap.py not 
only on linux (authored by friss).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76408

Files:
  lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py


Index: lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
===
--- lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
+++ lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
@@ -23,7 +23,7 @@
 
 # gcc generates incorrect linetable
 @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android")
-@expectedFailureAll(oslist=['linux'], archs=['arm', 'aarch64'])
+@expectedFailureAll(archs=['arm', 'aarch64'])
 @skipIfWindows
 def test_with_run_command(self):
 """Test that LLDB handles a function with __builtin_trap correctly."""


Index: lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
===
--- lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
+++ lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
@@ -23,7 +23,7 @@
 
 # gcc generates incorrect linetable
 @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android")
-@expectedFailureAll(oslist=['linux'], archs=['arm', 'aarch64'])
+@expectedFailureAll(archs=['arm', 'aarch64'])
 @skipIfWindows
 def test_with_run_command(self):
 """Test that LLDB handles a function with __builtin_trap correctly."""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76406: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

[Frederic Riss via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGecc6c426977f: [lldb/testsuite] Fix TestInlineStepping on 
arm64 with newer compilers (authored by friss).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76406

Files:
  lldb/test/API/functionalities/inline-stepping/calling.cpp


Index: lldb/test/API/functionalities/inline-stepping/calling.cpp
===
--- lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -75,7 +75,7 @@
 void
 caller_trivial_2 ()
 {
-inline_trivial_1 (); // In caller_trivial_2.
+asm volatile ("nop"); inline_trivial_1 (); // In caller_trivial_2.
 inline_value += 1;  // At increment in caller_trivial_2.
 }
 
@@ -88,7 +88,7 @@
 void
 inline_trivial_1 ()
 {
-inline_trivial_2(); // In inline_trivial_1.
+asm volatile ("nop"); inline_trivial_2(); // In inline_trivial_1.
 inline_value += 1;  // At increment in inline_trivial_1.
 }
 


Index: lldb/test/API/functionalities/inline-stepping/calling.cpp
===
--- lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -75,7 +75,7 @@
 void
 caller_trivial_2 ()
 {
-inline_trivial_1 (); // In caller_trivial_2.
+asm volatile ("nop"); inline_trivial_1 (); // In caller_trivial_2.
 inline_value += 1;  // At increment in caller_trivial_2.
 }
 
@@ -88,7 +88,7 @@
 void
 inline_trivial_1 ()
 {
-inline_trivial_2(); // In inline_trivial_1.
+asm volatile ("nop"); inline_trivial_2(); // In inline_trivial_1.
 inline_value += 1;  // At increment in inline_trivial_1.
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



Comment at: lldb/source/Core/SourceManager.cpp:422
   if (num_matches > 1) {
-SymbolContext sc;
+// SymbolContext sc;
 CompileUnit *test_cu = nullptr;

This comment should not stay there during check-in.



Comment at: lldb/source/Core/SourceManager.cpp:438
   if (!got_multiple) {
-SymbolContext sc;
+// SymbolContext sc;
 sc_list.GetContextAtIndex(0, sc);

This comment should not stay there during check-in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

[Frederic Riss via Phabricator via lldb-commits]

friss abandoned this revision.
friss added a comment.

I skipped the test for embedded in 8758d02074be7b80b804fad19e8b7de6ebd43c31 
. I'll 
abandon this for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76407



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

[Lldb-commits] [lldb] 99a0cbb - [lldb/Test] Remove debug print in supports_hw_breakpoints.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-19T08:35:22-07:00
New Revision: 99a0cbb42321ef5b967ba4aac51cea5f5804bb81

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

LOG: [lldb/Test] Remove debug print in supports_hw_breakpoints.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
 
b/lldb/test/API/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
index 74f2fbb0c1a0..61e417113101 100644
--- 
a/lldb/test/API/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ 
b/lldb/test/API/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -19,7 +19,6 @@ def supports_hw_breakpoints(self):
 CURRENT_EXECUTABLE_SET)
 self.runCmd("breakpoint set -b main --hardware")
 self.runCmd("run")
-print(self.res.GetOutput())
 if 'stopped' in self.res.GetOutput():
 return 'Hardware breakpoints are supported'
 return None



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

[Lldb-commits] [lldb] 8758d02 - [lldb/testsuite] Skip part of TestProcessCrashInfo.py on Darwin embedded

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-03-19T08:26:00-07:00
New Revision: 8758d02074be7b80b804fad19e8b7de6ebd43c31

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

LOG: [lldb/testsuite] Skip part of TestProcessCrashInfo.py on Darwin embedded

See https://reviews.llvm.org/D76407 for discussion.

Added: 


Modified: 
lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py 
b/lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
index d0f47de83eea..6ef5018204fd 100644
--- a/lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
+++ b/lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
@@ -69,6 +69,8 @@ def test_api(self):
 
 self.assertIn("pointer being freed was not allocated", 
stream.GetData())
 
+# dyld leaves permanent crash_info records when testing on device.
+@skipIfDarwinEmbedded
 def test_on_sane_process(self):
 """Test that lldb doesn't fetch the extended crash information
 dictionnary from a 'sane' stopped process."""



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

[Lldb-commits] [lldb] e154cbb - [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-03-19T08:25:59-07:00
New Revision: e154cbb124a63057356dede8bca5bdbd2f60d44c

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

LOG: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

Summary:
TestBuiltinTrap fail on darwin embedded because the `__builin_trap`
builtin doesn't get any line info attached to it by clang when
building for arm64.

The test was already XFailed for linux arm(64), I presume for the same
reasons. This patch just XFails it independently of the platform.

Reviewers: labath

Subscribers: kristof.beyls, danielkiss, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py

Removed: 




diff  --git a/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py 
b/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
index 22de873e29fa..added4ef508a 100644
--- a/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
+++ b/lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
@@ -23,7 +23,7 @@ def setUp(self):
 
 # gcc generates incorrect linetable
 @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android")
-@expectedFailureAll(oslist=['linux'], archs=['arm', 'aarch64'])
+@expectedFailureAll(archs=['arm', 'aarch64'])
 @skipIfWindows
 def test_with_run_command(self):
 """Test that LLDB handles a function with __builtin_trap correctly."""



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

[Lldb-commits] [lldb] ecc6c42 - [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-03-19T08:25:59-07:00
New Revision: ecc6c426977f59f69b683d67ceb3157fb694ce09

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

LOG: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

Summary:
TestInlineStepping tests LLDB's ability to step in the presence of
inline frames. The testcase source has a number of functions and some
of them are marked `always_inline`.

The test is built around the assumption that the inline function will
be fully represented once inlined, but this is not true with the
current arm64 code generation. For example:

void caller() {
 always_inline_function(); // Step here
}

When stppeing into `caller()` above, you might immediatly end up in
the inlines frame for `always_inline_function()`, because there might
literally be no code associated with `caller()` itself.

This patch hacks around the issue by adding an `asm volatile("nop")`
on some lines with inlined calls where we expect to be able to
step. Like so:

void caller() {
 asm volatile("nop"); always_inline_function(); // Step here
}

This guarantees there is always going to be one instruction for this
line in the caller.

Reviewers: labath, jingham

Subscribers: kristof.beyls, danielkiss, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/test/API/functionalities/inline-stepping/calling.cpp

Removed: 




diff  --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp 
b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index 9982fbf42734..49179ce7c978 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -75,7 +75,7 @@ caller_trivial_1 ()
 void
 caller_trivial_2 ()
 {
-inline_trivial_1 (); // In caller_trivial_2.
+asm volatile ("nop"); inline_trivial_1 (); // In caller_trivial_2.
 inline_value += 1;  // At increment in caller_trivial_2.
 }
 
@@ -88,7 +88,7 @@ called_by_inline_trivial ()
 void
 inline_trivial_1 ()
 {
-inline_trivial_2(); // In inline_trivial_1.
+asm volatile ("nop"); inline_trivial_2(); // In inline_trivial_1.
 inline_value += 1;  // At increment in inline_trivial_1.
 }
 



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251383.
kwk added a comment.

- Validate that the server received the request from debuginfod client


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,134 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+
+//We cd into the directory before compiling to get DW_AT_comp_dir pickup
+//%t.buildroot as well so it will be replaced by /my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod; \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck 

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

[Frederic Riss via Phabricator via lldb-commits]

friss marked an inline comment as done.
friss added a comment.

In D76407#1930693 , @labath wrote:

> Could the inferior manually wipe the crash annotation?


This is not something you're supposed to wipe. It's additional information that 
is going to b dumped in the crashlog if the process crashes. If dyld runs in 
some kind of degraded mode, it will put this annotation there at program 
startup time and never remove it.




Comment at: 
lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py:72
 
-def test_on_sane_process(self):
-"""Test that lldb doesn't fetch the extended crash information

mib wrote:
> What about adding a `@skipIfDarwinEmbedded` decorator for this test ? IIUC, 
> it would skip the test of Darwin armv7/arm64 targets (iOS) but still run on 
> other platforms.
I could do this, and it would fix the issue I'm seeing. I chose to remove it 
completely, because there's no guarantee that what the test exercises is true. 
But I guess I can just `@skipIfDarwinEmbedded` now and we'll see if the test 
causes any more trouble in other scenarios


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76407



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251376.
kwk marked 6 inline comments as done.
kwk added a comment.

- Removed not needed forward decl
- Format comments for better readability in my test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,128 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+
+//We cd into the directory before compiling to get DW_AT_comp_dir pickup
+//%t.buildroot as well so it will be replaced by /my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'echo "SERVER LOG:"; cat %t.server.log; kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: 

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath I've improved my test file for readability.




Comment at: lldb/include/lldb/Host/DebugInfoD.h:26
+
+llvm::Error findSource(UUID buildID, const std::string ,
+   std::string _path);

labath wrote:
> Expected ?
Removed.



Comment at: lldb/include/lldb/Host/DebugInfoD.h:16-18
+namespace llvm {
+class Error;
+} // End of namespace llvm

labath wrote:
> I guess this is not needed now.
Right.



Comment at: lldb/packages/Python/lldbsuite/test/httpserver.py:75
+# Block only for 0.5 seconds max
+httpd.timeout = 0.5
+# Allow for reusing the address

labath wrote:
> What exactly is this timeout for? It seems rather small...
uff, I guess I had an idea when I wrote it but its lost now.  



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:4
 
-The concrete subclass can override lldbtest.TesBase in order to inherit the
+The concrete subclass can override lldbtest.TestBase in order to inherit the
 common behavior for unitest.TestCase.setUp/tearDown implemented in this file.

labath wrote:
> just commit this separately.  no review needed.
Done in 44361782e2c252c8886cd77f6b7d4ebe64fb6e8d.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [lldb] 4436178 - [lldb] fix typo in comment for lldbtest.py

[Konrad Kleine via lldb-commits]

Author: Konrad Kleine
Date: 2020-03-19T10:08:11-04:00
New Revision: 44361782e2c252c8886cd77f6b7d4ebe64fb6e8d

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

LOG: [lldb] fix typo in comment for lldbtest.py

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index f8f916036f9a..966d460ea13d 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1,7 +1,7 @@
 """
 LLDB module which provides the abstract base class of lldb test case.
 
-The concrete subclass can override lldbtest.TesBase in order to inherit the
+The concrete subclass can override lldbtest.TestBase in order to inherit the
 common behavior for unitest.TestCase.setUp/tearDown implemented in this file.
 
 The subclass should override the attribute mydir in order for the python 
runtime



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

[Lldb-commits] [lldb] d9b9621 - Reland D73534: [DebugInfo] Enable the debug entry values feature by default

[Djordje Todorovic via lldb-commits]

Author: Djordje Todorovic
Date: 2020-03-19T13:57:30+01:00
New Revision: d9b962100942c71a4c26debaa716f7ab0c4ea8a1

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

LOG: Reland D73534: [DebugInfo] Enable the debug entry values feature by default

The issue that was causing the build failures was fixed with the D76164.

Added: 
llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/CC1Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
lldb/packages/Python/lldbsuite/test/decorators.py

lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/CodeGen/LiveDebugValues.cpp
llvm/lib/CodeGen/TargetOptionsImpl.cpp
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp
llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
llvm/test/CodeGen/X86/call-site-info-output.ll
llvm/test/DebugInfo/AArch64/dbgcall-site-float-entry-value.ll
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
llvm/test/DebugInfo/MIR/ARM/call-site-info-vmovd.mir
llvm/test/DebugInfo/MIR/ARM/call-site-info-vmovs.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
llvm/test/DebugInfo/X86/dbg-value-range.ll
llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
llvm/test/DebugInfo/X86/loclists-dwp.ll
llvm/test/tools/llvm-locstats/locstats.ll

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 3c8b0eeb47a5..e047054447f3 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -63,7 +63,6 @@ CODEGENOPT(ExperimentalNewPassManager, 1, 0) ///< Enables the 
new, experimental
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
-CODEGENOPT(EnableDebugEntryValues, 1, 0) ///< Emit call site parameter dbg info
 CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
///< '-g' + 'O>0' level.
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs

diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index b7a2826d8fcb..cc30893703df 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -388,8 +388,6 @@ def flto_visibility_public_std:
 def flto_unit: Flag<["-"], "flto-unit">,
 HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable 
opt)">;
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
-def femit_debug_entry_values : Flag<["-"], "femit-debug-entry-values">,
-HelpText<"Enables debug info about call site 

[Lldb-commits] [lldb] 718d941 - Revert "[lldb] Fix string summary of an empty NSPathStore2"

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-19T13:08:39+01:00
New Revision: 718d94187dbb2388dbc84deb5d49004fd552817c

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

LOG: Revert "[lldb] Fix string summary of an empty NSPathStore2"

This reverts commit 939ca455e72e822450013eff37c9ea7746850350.

This failed on the debian bot for some reason:
  File 
"/home/worker/lldb-x86_64-debian/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py",
 line 67, in test_with_run_command
"s summary wrong")
AssertionError: 'L"hello world! מזל 
טוב!\\0!\\0\\0\\0A\\0\\Ufffd\\Ufffd\\Ufffd\\ [truncated]... != 
'L"hello world! מזל טוב!"'
Diff is 2156 characters long. Set self.maxDiff to None to see it. : s summary 
wrong

Added: 


Modified: 
lldb/include/lldb/DataFormatters/StringPrinter.h
lldb/source/DataFormatters/StringPrinter.cpp
lldb/source/Plugins/Language/ObjC/NSString.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/StringPrinter.h 
b/lldb/include/lldb/DataFormatters/StringPrinter.h
index 5842cde893d8..6f8869cc2a1e 100644
--- a/lldb/include/lldb/DataFormatters/StringPrinter.h
+++ b/lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -115,15 +115,9 @@ class StringPrinter {
 
 lldb::ProcessSP GetProcessSP() const { return m_process_sp; }
 
-void SetHasSourceSize(bool e) { m_has_source_size = e; }
-
-bool HasSourceSize() const { return m_has_source_size; }
-
   private:
 uint64_t m_location = 0;
 lldb::ProcessSP m_process_sp;
-/// True iff we know the source size of the string.
-bool m_has_source_size = false;
   };
 
   class ReadBufferAndDumpToStreamOptions : public DumpToStreamOptions {

diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 4515b67b2adf..92dd71d17b8c 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -525,33 +525,27 @@ static bool ReadUTFBufferAndDumpToStream(
   if (!options.GetStream())
 return false;
 
-  uint32_t sourceSize;
+  uint32_t sourceSize = options.GetSourceSize();
   bool needs_zero_terminator = options.GetNeedsZeroTermination();
 
   bool is_truncated = false;
   const auto max_size = 
process_sp->GetTarget().GetMaximumSizeOfStringSummary();
 
-  if (options.HasSourceSize()) {
-sourceSize = options.GetSourceSize();
-if (!options.GetIgnoreMaxLength()) {
-  if (sourceSize > max_size) {
-sourceSize = max_size;
-is_truncated = true;
-  }
-}
-  } else {
+  if (!sourceSize) {
 sourceSize = max_size;
 needs_zero_terminator = true;
+  } else if (!options.GetIgnoreMaxLength()) {
+if (sourceSize > max_size) {
+  sourceSize = max_size;
+  is_truncated = true;
+}
   }
 
   const int bufferSPSize = sourceSize * type_width;
 
   lldb::DataBufferSP buffer_sp(new DataBufferHeap(bufferSPSize, 0));
 
-  // Check if we got bytes. We never get any bytes if we have an empty
-  // string, but we still continue so that we end up actually printing
-  // an empty string ("").
-  if (sourceSize != 0 && !buffer_sp->GetBytes())
+  if (!buffer_sp->GetBytes())
 return false;
 
   Status error;

diff  --git a/lldb/source/Plugins/Language/ObjC/NSString.cpp 
b/lldb/source/Plugins/Language/ObjC/NSString.cpp
index 7c4afb36b588..65256dc7acbd 100644
--- a/lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -170,7 +170,6 @@ bool lldb_private::formatters::NSStringSummaryProvider(
   options.SetStream();
   options.SetQuote('"');
   options.SetSourceSize(explicit_length);
-  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -183,7 +182,6 @@ bool lldb_private::formatters::NSStringSummaryProvider(
   options.SetProcessSP(process_sp);
   options.SetStream();
   options.SetSourceSize(explicit_length);
-  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -201,7 +199,6 @@ bool lldb_private::formatters::NSStringSummaryProvider(
 options.SetStream();
 options.SetQuote('"');
 options.SetSourceSize(explicit_length);

[Lldb-commits] [PATCH] D68010: [lldb] Fix string summary of an empty NSPathStore2

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG939ca455e72e: [lldb] Fix string summary of an empty 
NSPathStore2 (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68010

Files:
  lldb/include/lldb/DataFormatters/StringPrinter.h
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
@@ -17,6 +17,7 @@
 
 NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
+  NSString *empty = @"";
 	NSString *str0 = [[NSNumber numberWithUnsignedLongLong:0xFF] stringValue];
 	NSString *str1 = [NSString stringWithCString:"A rather short ASCII NSString object is here" encoding:NSASCIIStringEncoding];
 	NSString *str2 = [NSString stringWithUTF8String:"A rather short UTF8 NSString object is here"];
@@ -69,6 +70,7 @@
 
 	NSArray *components = @[@"usr", @"blah", @"stuff"];
 	NSString *path = [NSString pathWithComponents: components];
+  NSString *empty_path = [empty stringByDeletingPathExtension];
 
   const unichar someOfTheseAreNUL[] = {'a',' ', 'v','e','r','y',' ',
   'm','u','c','h',' ','b','o','r','i','n','g',' ','t','a','s','k',
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
@@ -76,8 +76,8 @@
 self.expect('frame variable hebrew', substrs=['לילה טוב'])
 
 def nsstring_data_formatter_commands(self):
-self.expect('frame variable str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
-substrs=[
+self.expect('frame variable empty str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
+substrs=['(NSString *) empty = ', ' @""',
  # '(NSString *) str0 = ',' @"255"',
  '(NSString *) str1 = ', ' @"A rather short ASCII NSString object is here"',
  '(NSString *) str2 = ', ' @"A rather short UTF8 NSString object is here"',
@@ -104,6 +104,8 @@
 
 self.expect('expr -d run-target -- path', substrs=['usr/blah/stuff'])
 self.expect('frame variable path', substrs=['usr/blah/stuff'])
+self.expect('expr -d run-target -- empty_path', substrs=['@""'])
+self.expect('frame variable empty_path', substrs=['@""'])
 
 def nsstring_withNULs_commands(self):
 """Check that the NSString formatter supports embedded NULs in the text"""
Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -170,6 +170,7 @@
   options.SetStream();
   options.SetQuote('"');
   options.SetSourceSize(explicit_length);
+  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -182,6 +183,7 @@
   options.SetProcessSP(process_sp);
   options.SetStream();
   options.SetSourceSize(explicit_length);
+  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -199,6 +201,7 @@
 options.SetStream();
 options.SetQuote('"');
 options.SetSourceSize(explicit_length);
+options.SetHasSourceSize(has_explicit_length);
 options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
 options.SetLanguage(summary_options.GetLanguage());
@@ -221,6 +224,7 @@
 options.SetStream();
 options.SetQuote('"');
 options.SetSourceSize(explicit_length);
+options.SetHasSourceSize(has_explicit_length);
 options.SetNeedsZeroTermination(!has_explicit_length);
 

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Djordje Todorovic via Phabricator via lldb-commits]

djtodoro added a comment.

Since we landed the fix for the issue related to the D75036 
, I'll reland this again.


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin marked 2 inline comments as done.
ikudrin added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48
+// For pre-standard ones, which correspond to sections being deprecated in
+// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_"
+// is added to the names.
+//

dblaikie wrote:
> ikudrin wrote:
> > dblaikie wrote:
> > > Probably not arbitrarily - in the sense that this is an extension that 
> > > consumers/producers will need to agree to - so maybe saying that 
> > > ("non-standard extension"/"proposed for standardization" or something to 
> > > that effect) and/or linking to the dwarf-discuss thread to support why 
> > > these values were chosen & they can't be changed arbitrarily.
> > As far as the enum is internal, no one should really worry about the actual 
> > values; they are not important and do not need any kind of proof. They may 
> > be really arbitrary, that will change nothing. That is what I meant when 
> > said "more or less".
> > 
> > The plan is that this patch supports DWARFv5 unit indexes, but not the 
> > proposed combined indexes. When the combined indexes are approved, there 
> > will be another patch, which moves the enum with all extensions in the 
> > public space. At that moment the factual values will become important, and 
> > the references to the descriptive document will be added. Do you think it 
> > will be possible to add such a document to the [[ http://wiki.dwarfstd.org 
> > | DWARF Wiki ]]?
> Hmm, I'm confused then - ah, OK - so you've added the enum to support 
> encoding the version 2 and version 5 tables into one internal data structure, 
> but haven't extended it to actually dump or use (for parsing: eg to find the 
> debug_loc.dwo contribution for a v4 unit described by a v5 index) them when 
> parsing/rendering a v5 index.
> 
> OK, sorry I hadn't realized that. Then, yes, the comment makes sense for now. 
> Perhaps "the values are only used internally/not parsed from input files (if 
> these values appear in input files they will be considered "unknown")" would 
> be more suitable?
> 
> > The plan is that this patch supports DWARFv5 unit indexes, but not the 
> > proposed combined indexes. When the combined indexes are approved, there 
> > will be another patch, which moves the enum with all extensions in the 
> > public space. At that moment the factual values will become important, and 
> > the references to the descriptive document will be added. Do you think it 
> > will be possible to add such a document to the DWARF Wiki?
> 
> Given the DWARF committee is not in session at the moment (I think) & it'll 
> be a while before another spec is published - I think it'll be necessary and 
> appropriate to implement support for the extension columns in llvm-dwarfdump 
> at least before/when they're implemented in llvm-dwp (which will be soon) to 
> support testing that functionality & working with such files.
> 
> Might be able to put something on the DWARF wiki, but I don't know much about 
> it/how things go up there.
> Perhaps "the values are only used internally/not parsed from input files (if 
> these values appear in input files they will be considered "unknown")" would 
> be more suitable?

The comment says something similar in lines 52-53. Do you think it should be 
extended?

> I think it'll be necessary and appropriate to implement support for the 
> extension columns in llvm-dwarfdump at least before/when they're implemented 
> in llvm-dwp (which will be soon) to support testing that functionality & 
> working with such files.

I think the same. The only concern in my side is that the proposal should be 
formulated as an RFC or similar document before implementing it in the code so 
that all the implementations can reference the same source. For my taste, a 
link to the middle of a forum conversation cannot be considered as a reliable 
source.



Comment at: llvm/test/DebugInfo/X86/dwp-v5-loclists.s:1-3
+## The test checks that v5 compile units in package files read their
+## location tables from .debug_loclists.dwo sections.
+## See dwp-v2-loc.s for pre-v5 units.

dblaikie wrote:
> Might be possible/better to test this without debug_abbrev and debug_info - 
> make the test shorter & dump only the loclists section itself? Yeah, not 
> exactly valid, but no reason the dumper shouldn't support it and it'd be a 
> more targeted test (apply this suggestion to other tests if 
> possible/acceptable too)
This test is for changes in the constructor of `DWARFUnit`. It checks that 
`DWARFUnit` takes locations from the right place, so all four sections are 
necessary.


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

https://reviews.llvm.org/D75929



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai marked 2 inline comments as done.
HsiangKai added inline comments.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1727
+
+  if (Tag % 2)
+IsIntegerValue = false;

jhenderson wrote:
> I don't understand why this line changed, but more importantly, the `2` looks 
> like a magic number and it is unclear why that value is the correct one. Is 
> there another way of writing this that would be more correct (e.g. bitwise 
> `&` against a known flag value)?
I have added a comment for it.



Comment at: llvm/unittests/Support/ELFAttributeParserTest.cpp:17
+static void testParseError(ArrayRef bytes, const char *msg) {
+  ARMAttributeParser parser;
+  Error e = parser.parse(bytes, support::little);

jhenderson wrote:
> If these tests are for the generic parts of the attribute parser, you should 
> probably define a concrete parser type that is neither ARM nor RISC-V, and 
> use that in the tests, e.g. `TestAttributeParser`.
> 
> If that's not practical for whatever reason, you need to put a comment 
> somewhere in this file indicating that the `ARMAttributeParser` is used here 
> for generality. Alternatively, consider changing this function to take a 
> template argument for the parser type, and call the test for all instantiated 
> parser types, or simply duplicating the contents of this function, but for 
> the other parser type.
Thanks for your suggestions. It makes sense. I have added a concrete parser for 
testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023



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

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 251244.
HsiangKai added a comment.

Address @jhenderson's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/test/tools/llvm-objdump/RISCV/lit.local.cfg
  llvm/test/tools/llvm-objdump/RISCV/unknown-arch-attr.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,69 @@
+//===- unittests/RISCVAttributeParserTest.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 "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct RISCVAttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  RISCVAttributeSection(unsigned tag, unsigned value) : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+static bool testAttribute(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+   unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  RISCVAttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+static bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testAttribute) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testAttribute(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testAttribute(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testAttribute) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testAttribute(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testAttribute(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,63 @@
+//===- unittests/ELFAttributeParserTest.cpp ---===//

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

[Hsiangkai Wang via Phabricator via lldb-commits]

HsiangKai updated this revision to Diff 251287.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023

Files:
  lld/ELF/InputFiles.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  llvm/include/llvm/BinaryFormat/ELF.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/include/llvm/Support/ARMAttributeParser.h
  llvm/include/llvm/Support/ARMBuildAttributes.h
  llvm/include/llvm/Support/ELFAttributeParser.h
  llvm/include/llvm/Support/ELFAttributes.h
  llvm/include/llvm/Support/RISCVAttributeParser.h
  llvm/include/llvm/Support/RISCVAttributes.h
  llvm/lib/Object/ELF.cpp
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/ARMAttributeParser.cpp
  llvm/lib/Support/ARMBuildAttrs.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/ELFAttributeParser.cpp
  llvm/lib/Support/ELFAttributes.cpp
  llvm/lib/Support/RISCVAttributeParser.cpp
  llvm/lib/Support/RISCVAttributes.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/attribute.s
  llvm/test/MC/RISCV/invalid-attribute.s
  llvm/test/tools/llvm-objdump/RISCV/lit.local.cfg
  llvm/test/tools/llvm-objdump/RISCV/unknown-arch-attr.test
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/unittests/Support/ARMAttributeParser.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/ELFAttributeParserTest.cpp
  llvm/unittests/Support/RISCVAttributeParserTest.cpp

Index: llvm/unittests/Support/RISCVAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/RISCVAttributeParserTest.cpp
@@ -0,0 +1,69 @@
+//===- unittests/RISCVAttributeParserTest.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 "llvm/Support/RISCVAttributeParser.h"
+#include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/ELFAttributes.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace llvm;
+
+struct RISCVAttributeSection {
+  unsigned Tag;
+  unsigned Value;
+
+  RISCVAttributeSection(unsigned tag, unsigned value) : Tag(tag), Value(value) {}
+
+  void write(raw_ostream ) {
+OS.flush();
+// length = length + "riscv\0" + TagFile + ByteSize + Tag + Value;
+// length = 17 bytes
+
+OS << 'A' << (uint8_t)17 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << "riscv" << '\0';
+OS << (uint8_t)1 << (uint8_t)7 << (uint8_t)0 << (uint8_t)0 << (uint8_t)0;
+OS << (uint8_t)Tag << (uint8_t)Value;
+  }
+};
+
+static bool testAttribute(unsigned Tag, unsigned Value, unsigned ExpectedTag,
+   unsigned ExpectedValue) {
+  std::string buffer;
+  raw_string_ostream OS(buffer);
+  RISCVAttributeSection Section(Tag, Value);
+  Section.write(OS);
+  ArrayRef Bytes(reinterpret_cast(OS.str().c_str()),
+  OS.str().size());
+
+  RISCVAttributeParser Parser;
+  cantFail(Parser.parse(Bytes, support::little));
+
+  Optional Attr = Parser.getAttributeValue(ExpectedTag);
+  return Attr.hasValue() && Attr.getValue() == ExpectedValue;
+}
+
+static bool testTagString(unsigned Tag, const char *name) {
+  return ELFAttrs::attrTypeAsString(Tag, RISCVAttrs::RISCVAttributeTags)
+ .str() == name;
+}
+
+TEST(StackAlign, testAttribute) {
+  EXPECT_TRUE(testTagString(4, "Tag_stack_align"));
+  EXPECT_TRUE(
+  testAttribute(4, 4, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4));
+  EXPECT_TRUE(
+  testAttribute(4, 16, RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16));
+}
+
+TEST(UnalignedAccess, testAttribute) {
+  EXPECT_TRUE(testTagString(6, "Tag_unaligned_access"));
+  EXPECT_TRUE(testAttribute(6, 0, RISCVAttrs::UNALIGNED_ACCESS,
+RISCVAttrs::NOT_ALLOWED));
+  EXPECT_TRUE(
+  testAttribute(6, 1, RISCVAttrs::UNALIGNED_ACCESS, RISCVAttrs::ALLOWED));
+}
Index: llvm/unittests/Support/ELFAttributeParserTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/ELFAttributeParserTest.cpp
@@ -0,0 +1,63 @@
+//===- unittests/ELFAttributeParserTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48
+// For pre-standard ones, which correspond to sections being deprecated in
+// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_"
+// is added to the names.
+//

ikudrin wrote:
> dblaikie wrote:
> > Probably not arbitrarily - in the sense that this is an extension that 
> > consumers/producers will need to agree to - so maybe saying that 
> > ("non-standard extension"/"proposed for standardization" or something to 
> > that effect) and/or linking to the dwarf-discuss thread to support why 
> > these values were chosen & they can't be changed arbitrarily.
> As far as the enum is internal, no one should really worry about the actual 
> values; they are not important and do not need any kind of proof. They may be 
> really arbitrary, that will change nothing. That is what I meant when said 
> "more or less".
> 
> The plan is that this patch supports DWARFv5 unit indexes, but not the 
> proposed combined indexes. When the combined indexes are approved, there will 
> be another patch, which moves the enum with all extensions in the public 
> space. At that moment the factual values will become important, and the 
> references to the descriptive document will be added. Do you think it will be 
> possible to add such a document to the [[ http://wiki.dwarfstd.org | DWARF 
> Wiki ]]?
Hmm, I'm confused then - ah, OK - so you've added the enum to support encoding 
the version 2 and version 5 tables into one internal data structure, but 
haven't extended it to actually dump or use (for parsing: eg to find the 
debug_loc.dwo contribution for a v4 unit described by a v5 index) them when 
parsing/rendering a v5 index.

OK, sorry I hadn't realized that. Then, yes, the comment makes sense for now. 
Perhaps "the values are only used internally/not parsed from input files (if 
these values appear in input files they will be considered "unknown")" would be 
more suitable?

> The plan is that this patch supports DWARFv5 unit indexes, but not the 
> proposed combined indexes. When the combined indexes are approved, there will 
> be another patch, which moves the enum with all extensions in the public 
> space. At that moment the factual values will become important, and the 
> references to the descriptive document will be added. Do you think it will be 
> possible to add such a document to the DWARF Wiki?

Given the DWARF committee is not in session at the moment (I think) & it'll be 
a while before another spec is published - I think it'll be necessary and 
appropriate to implement support for the extension columns in llvm-dwarfdump at 
least before/when they're implemented in llvm-dwp (which will be soon) to 
support testing that functionality & working with such files.

Might be able to put something on the DWARF wiki, but I don't know much about 
it/how things go up there.



Comment at: llvm/test/DebugInfo/X86/dwp-v5-loclists.s:1-3
+## The test checks that v5 compile units in package files read their
+## location tables from .debug_loclists.dwo sections.
+## See dwp-v2-loc.s for pre-v5 units.

Might be possible/better to test this without debug_abbrev and debug_info - 
make the test shorter & dump only the loclists section itself? Yeah, not 
exactly valid, but no reason the dumper shouldn't support it and it'd be a more 
targeted test (apply this suggestion to other tests if possible/acceptable too)


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

https://reviews.llvm.org/D75929



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

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin marked 3 inline comments as done.
ikudrin added a comment.

In D75929#1926834 , @labath wrote:

> (btw, is there a test case for the "unknown column" code path?)


Yes, it is checked in 
`llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s`, which was added in 
D75609  and then extended in D75668 
.

As for unknown columns in general, I believe they are not that important to 
complicate the code too much. Before D75609 , 
`llvm-dwarfdump` just crashed when saw them. `dwarfdump` prints some useless 
(for a user) error message. An unknown column cannot be used by clients of the 
library because they do not know what to do with it. Dumping is the only reason 
to support unknown identifiers, and that should be done as simple as possible. 
If the current implementation seems too complicated, we can consider, for 
example, dropping printing raw IDs for unknown sections.




Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48
+// For pre-standard ones, which correspond to sections being deprecated in
+// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_"
+// is added to the names.
+//

dblaikie wrote:
> Probably not arbitrarily - in the sense that this is an extension that 
> consumers/producers will need to agree to - so maybe saying that 
> ("non-standard extension"/"proposed for standardization" or something to that 
> effect) and/or linking to the dwarf-discuss thread to support why these 
> values were chosen & they can't be changed arbitrarily.
As far as the enum is internal, no one should really worry about the actual 
values; they are not important and do not need any kind of proof. They may be 
really arbitrary, that will change nothing. That is what I meant when said 
"more or less".

The plan is that this patch supports DWARFv5 unit indexes, but not the proposed 
combined indexes. When the combined indexes are approved, there will be another 
patch, which moves the enum with all extensions in the public space. At that 
moment the factual values will become important, and the references to the 
descriptive document will be added. Do you think it will be possible to add 
such a document to the [[ http://wiki.dwarfstd.org | DWARF Wiki ]]?



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp:96-100
   Version = IndexData.getU32(OffsetPtr);
+  if (Version != 2) {
+*OffsetPtr = BeginOffset;
+Version = IndexData.getU16(OffsetPtr);
+if (Version != 5)

dblaikie wrote:
> What endianness is this encoded with? If it's little endian, then a 2 byte 
> field with 2 bytes of zero padding should be the same as reading a 4 bytes, 
> or the other way around, right? So perhaps we could just always read it as 2 
> bytes then 2 bytes of padding rather than having to the version/reset/reread 
> dance here?
The endianness comes from the input file. I cannot find anything in the 
standard or the pre-standard proposal that would restrict it to be only 
little-endian. Thus, we should handle both cases.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp:131
 
+  // Fix InfoColumnKind: in DWARFv5, type units also lay in .debug_info.dwo.
+  if (Header.Version == 5)

dblaikie wrote:
> jhenderson wrote:
> > also lay -> are
> Should we be fixing it up here - or should we avoid setting it incorrectly in 
> the first place?
As it is written at the moment, we have to pass something to distinct TU and CU 
indexes, for v2 cases. We may pass, say, a bool, but `true` and `false` are not 
that descriptive as `DW_SECT_INFO` and `DW_SECT_TYPES`. I believe that for the 
purpose of this patch, this fix is the simplest thing to do.

BTW, to support the combined TU index, the code should be adjusted anyway 
because where probably will be two "main" columns, one for v2 type units in 
`.debug_types.dwo` and another for v5 type units in `.debug_info.dwo`.


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

https://reviews.llvm.org/D75929



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

[Lldb-commits] [lldb] 939ca45 - [lldb] Fix string summary of an empty NSPathStore2

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-03-19T12:20:35+01:00
New Revision: 939ca455e72e822450013eff37c9ea7746850350

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

LOG: [lldb] Fix string summary of an empty NSPathStore2

Summary:
Printing a summary for an empty NSPathStore2 string currently prints random 
bytes behind the empty string pointer from memory (rdar://55575888).

It seems the reason for this is that the SourceSize parameter in the 
`ReadStringAndDumpToStreamOptions` - which is supposed to contain the string
length - actually uses the length 0 as a magic value for saying "read as much 
as possible from the buffer" which is clearly wrong for empty strings.

This patch adds another flag that indicates if we have know the string length 
or not and makes this behaviour dependent on that (which seemingly
was the original purpose of this magic value).

Reviewers: aprantl, JDevlieghere, shafik

Reviewed By: aprantl

Subscribers: christof, abidh, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/StringPrinter.h
lldb/source/DataFormatters/StringPrinter.cpp
lldb/source/Plugins/Language/ObjC/NSString.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/StringPrinter.h 
b/lldb/include/lldb/DataFormatters/StringPrinter.h
index 6f8869cc2a1e..5842cde893d8 100644
--- a/lldb/include/lldb/DataFormatters/StringPrinter.h
+++ b/lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -115,9 +115,15 @@ class StringPrinter {
 
 lldb::ProcessSP GetProcessSP() const { return m_process_sp; }
 
+void SetHasSourceSize(bool e) { m_has_source_size = e; }
+
+bool HasSourceSize() const { return m_has_source_size; }
+
   private:
 uint64_t m_location = 0;
 lldb::ProcessSP m_process_sp;
+/// True iff we know the source size of the string.
+bool m_has_source_size = false;
   };
 
   class ReadBufferAndDumpToStreamOptions : public DumpToStreamOptions {

diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 92dd71d17b8c..4515b67b2adf 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -525,27 +525,33 @@ static bool ReadUTFBufferAndDumpToStream(
   if (!options.GetStream())
 return false;
 
-  uint32_t sourceSize = options.GetSourceSize();
+  uint32_t sourceSize;
   bool needs_zero_terminator = options.GetNeedsZeroTermination();
 
   bool is_truncated = false;
   const auto max_size = 
process_sp->GetTarget().GetMaximumSizeOfStringSummary();
 
-  if (!sourceSize) {
+  if (options.HasSourceSize()) {
+sourceSize = options.GetSourceSize();
+if (!options.GetIgnoreMaxLength()) {
+  if (sourceSize > max_size) {
+sourceSize = max_size;
+is_truncated = true;
+  }
+}
+  } else {
 sourceSize = max_size;
 needs_zero_terminator = true;
-  } else if (!options.GetIgnoreMaxLength()) {
-if (sourceSize > max_size) {
-  sourceSize = max_size;
-  is_truncated = true;
-}
   }
 
   const int bufferSPSize = sourceSize * type_width;
 
   lldb::DataBufferSP buffer_sp(new DataBufferHeap(bufferSPSize, 0));
 
-  if (!buffer_sp->GetBytes())
+  // Check if we got bytes. We never get any bytes if we have an empty
+  // string, but we still continue so that we end up actually printing
+  // an empty string ("").
+  if (sourceSize != 0 && !buffer_sp->GetBytes())
 return false;
 
   Status error;

diff  --git a/lldb/source/Plugins/Language/ObjC/NSString.cpp 
b/lldb/source/Plugins/Language/ObjC/NSString.cpp
index 65256dc7acbd..7c4afb36b588 100644
--- a/lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -170,6 +170,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
   options.SetStream();
   options.SetQuote('"');
   options.SetSourceSize(explicit_length);
+  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -182,6 +183,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
   options.SetProcessSP(process_sp);
   options.SetStream();
   options.SetSourceSize(explicit_length);
+  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py:72
 
-def test_on_sane_process(self):
-"""Test that lldb doesn't fetch the extended crash information

What about adding a `@skipIfDarwinEmbedded` decorator for this test ? IIUC, it 
would skip the test of Darwin armv7/arm64 targets (iOS) but still run on other 
platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76407



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D75750#1929124 , @kwk wrote:

> @labath I've updated my patch and would love to hear your opinion on it. So 
> far I've only written the python `ServeDirectoryWithHTTP()` function with 
> proper doctest and documentation but since you mentioned the `0` port thingy 
> I've tried that on the command line when using `python -m http.server 0` and 
> it works smoothly. That's why I've included the `llvm-lit` test I was working 
> on.


Being able to use 0 to auto-assign a port is definitely a big improvement, but 
there still the question of retrieving that port and sychronization that goes 
with it, which you've now done with a while loop + sniffing through the server 
log.

And I still haven't gotten used to how the comments in your lit tests are way 
longer than the test itself. If I think about that harder, I guess the thing 
that really bothers me about that (even though I normally like comments) is 
that there is no visual distinction between "comments" and "code" this way -- 
it all shows up as grey in the review tool. Can't say that's really your fault, 
but it does make it hard to see what that test is doing nonetheless. (Some 
areas of llvm have a convention to use `##` for "real" comments, which I guess 
can make things slightly better, but I still haven't seen comments this big 
there...

So overall, I think this version is better than what you had before, but it 
still doesn't convince me that this is better than python.




Comment at: lldb/include/lldb/Host/DebugInfoD.h:16-18
+namespace llvm {
+class Error;
+} // End of namespace llvm

I guess this is not needed now.



Comment at: lldb/packages/Python/lldbsuite/test/httpserver.py:75
+# Block only for 0.5 seconds max
+httpd.timeout = 0.5
+# Allow for reusing the address

What exactly is this timeout for? It seems rather small...



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:4
 
-The concrete subclass can override lldbtest.TesBase in order to inherit the
+The concrete subclass can override lldbtest.TestBase in order to inherit the
 common behavior for unitest.TestCase.setUp/tearDown implemented in this file.

just commit this separately.  no review needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The new interface is ok for me, but there are two things I want to mention:

- the iteration via `Get{Name,Value}AtIndex` is going to be slow because the 
underlying map (like most maps) does not have random access iterators. This is 
the problem I was trying to solve with the `GetEntries`-like API, but I agree 
that has its issues too.
- I agree that forcing a user to manually construct `name=value` strings if he 
has then as separate entities is not good, but I also don't think we're doing 
anyone a favor by not providing an api which would accept such strings. The 
`name=value` format is very universal, and I think users will often have the 
value in this format already (for example, D74636 
 does). This means that those users would have 
to implement `=`-splitting themselves before using this class. This is why the 
internal Environment class provides both kinds of api, and maybe also the 
reason why the c library provides both `putenv(3)` and `setenv(3)`.




Comment at: lldb/source/API/SBEnvironment.cpp:53
+  return LLDB_RECORD_RESULT(
+  ConstString(m_opaque_up->lookup(name)).AsCString(""));
+}

Please reuse the result of `m_opaque_up->find(name)` here to avoid double 
lookup.



Comment at: lldb/source/API/SBEnvironment.cpp:59
+ index);
+  if (index < 0 || index >= GetNumValues())
+return LLDB_RECORD_RESULT(nullptr);

`index < 0` is not possible now.



Comment at: lldb/source/API/SBEnvironment.cpp:62-73
+  ConstString(std::next(m_opaque_up->begin(), index)->first())
+  .AsCString(""));
+}
+
+const char *SBEnvironment::GetValueAtIndex(size_t index) {
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetValueAtIndex, (size_t),
+ index);

I don't think these need to const-stringified now, given that they are backed 
by the underlying map. We already have functions which return pointers to 
internal objects (e.g. SBStream::GetData).

@clayborg? 



Comment at: lldb/source/API/SBEnvironment.cpp:80-81
+ overwrite);
+  if (overwrite || m_opaque_up->find(name) == m_opaque_up->end()) {
+m_opaque_up->insert_or_assign(name, std::string(value));
+return LLDB_RECORD_RESULT(true);

how about `if(overwrite) insert_or_assign(name, value) else try_emplace(name, 
value)`? (avoiding two map lookups)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for updating this patch. This seems ok to me (modulo comments here and 
the other patch), but I think it would be better to move all of the SB changes 
into that other patch. (the reason I requested this update was to see whether 
the other patch has all that's needed to implement the given functionality -- 
and I suspected there would be some bits missing :))




Comment at: lldb/bindings/interface/SBLaunchInfo.i:65
 void
-SetEnvironmentEntries (const char **envp, bool append);
+SetEnvironmentEntries (const char * const*envp, bool append);
+

This is going to change the mangled name of the function. Either add a new 
overload or `const_cast` at the call site. (But if you implement the other 
comment, neither of those will be needed).



Comment at: lldb/include/lldb/API/SBEnvironment.h:62
 
+  lldb_private::Environment () const;
+

Usually, these functions are called `get()` (when they return a pointer) or 
`ref()` (for a reference).



Comment at: lldb/source/API/SBLaunchInfo.cpp:198
+ (const lldb::SBEnvironment &, bool), env, append)
+  SetEnvironmentEntries(env.GetInnerEnvironment().getEnvp().get(), append);
+}

This is a pretty long-winded way of implementing this functionality, as the 
first thing the other function will do is recreate an Environment object. It 
would be better to have the other function redirect to this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



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

[Lldb-commits] [PATCH] D76406: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

[Pavel Labath via Phabricator via lldb-commits]

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

This doesn't sound too bad. I am not sure if "nop" is a valid opcode on all 
targets, but if it isn't we could try hiding it in a macro, or replace it with 
an increment of a volatile/atomic variable or a call to a noinline function 
(the latter would probably require changes in test expectations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76406



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

[Lldb-commits] [PATCH] D76216: Improve step over performance

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76216#1930689 , @jarin wrote:

> I also see that build-bot is not happy about my patch. Clang-tidy somewhat 
> mysteriously fails on missing lldb/Target/ThreadPlanStepOverRange.h, which I 
> did not touch at all (neither the fail nor the #include). Any idea what that 
> is about?


These pre-merge tests are a fairly new thing, and they don't fully work for 
lldb currently. Just ignore that for now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216



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

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py:50-53
+time.sleep(1)
+# lldb-vscode process must have already finished even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)

1 second is pretty short, and I wouldn't be surprised if it causes spurious 
failures.

Using `popen.wait()` would be _much_ better -- you can use a much larger 
timeout, while still having the test terminate quickly in the common case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314



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

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Could the inferior manually wipe the crash annotation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76407



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

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D74398#1930021 , @jarin wrote:

> In D74398#1929019 , @labath wrote:
>
> > Thanks. Could you also add the other kind of test (the one inline asm) I 
> > mentioned. In an ideal world we'd have a test case for every boundary 
> > condition, but we're pretty far from that right now. Even so, one test case 
> > like that would be nice.
>
>
> Pavel, thank you for the careful review! I still do not quite understand how 
> the asm test should look like and where it should live. Are you asking for 
> creating a new directory with a compiled program below and building the 
> machinery to stop at the right place and check the stack? If yes, that sounds 
> like a fairly time-consuming undertaking, and I am afraid I cannot invest 
> much more time into this.


Yes, that is kind of what I am proposing, but I don't think it should be that 
hard. The creation of a new directory is necessary because of how our test 
suite expects there to be one inferior per directory, and we want to build a 
new inferior here. The "inferior would basically consist of a single `main` 
function containing something like the inline asm I mentioned previously (it 
probably doesn't compile right now, but I hope it shows the general idea). 
"Stopping at the right place" is implemented via the `int3` opcode. Then the 
test would just run the program, wait for it to stop, and examine the 
jThreadsInfo response. It should expect to see one thread containing two 
(three?) memory records. The only tricky part would be validating the contents 
of those records, as they will not have fully static data due to the 
unpredictability of the %rsp value. But the test could compare that to the 
value of %rsp it gets through the `p` packet. Or maybe the inferior could align 
the stack at say 1MB boundary, and then the test could verify that rsp/rbp have 
the expected values modulo 1MB.

> Perhaps it is better if we stick this patch only into our lldb branch, this 
> one should be pretty easy for us to rebase.

That's something you'll have to decide. I'm still very much interested in 
having this patch, but I am also trying to maintain a higher bar for test 
coverage. I see this as sort of similar to the register reading tests we added 
recently (`lldb/test/Shell/Register`. Before that, we only had tests which try 
to read all registers and expect to get /a/ value. That's sort of similar to 
what your first test does -- it is somewhat useful, and it is cross-platform, 
but it doesn't really check all that much. Also, if we ever find a bug in this 
code, it will be impossible to write a regression test for it using that method.

This other test would be more similar to the "new" register tests. They are a 
bit trickier to write, but they enable you to write stronger assertions about 
what the code is actually supposed to do -- not just in the "normal" case, but 
also in various boundary conditions.

> As for the timings with local lldb on Linux, I see the time for jThreadsInfo 
> of 100 threads with fairly shallow stacks (10 entries or so) taking 20ms 
> locally, as opposed to 4ms before this change. The jThreadsInfo reply size is 
> 80kB, up from 11kB. While this seems excessive, I do not see any extra memory 
> traffic for getting a thread list with the patch,  whereas without this patch 
> we get over 100 roundtrips (each is at least 512 bytes of payload) to display 
> the top of the stack of each thread.

Thanks, for checking this out. 20ms for 100 threads doesn't sound too bad. I am 
guessing the statistics would look better if you also showed how many packets 
this saved. I would expect a net saving from all the memory read packets we can 
avoid sending now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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

[Lldb-commits] [PATCH] D76408: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a reviewer: omjavaid.
labath added a comment.
This revision is now accepted and ready to land.

I don't currently have arm hardware to try this on, but it sounds like a safe 
assumption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76408



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

[Lldb-commits] [PATCH] D76216: Improve step over performance

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Thanks Greg, I will wait for Jim's comment.

I also see that build-bot is not happy about my patch. Clang-tidy somewhat 
mysteriously fails on missing lldb/Target/ThreadPlanStepOverRange.h, which I 
did not touch at all (neither the fail nor the #include). Any idea what that is 
about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251297.
kwk added a comment.

- Fix NameError: name 'TRUE' is not defined


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,121 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//--
+// Test Purpose:
+// =
+// Test that we can display the source of functions using debuginfod when the
+// original source file is no longer present.
+// 
+// The debuginfod client requires a buildid in the binary, so we compile one in.
+// We can create the directory structure on disc that the client expects on a
+// webserver that serves source files. Then we fire up a python based http
+// server in the root of that mock directory, set the DEBUGINFOD_URLS
+// environment variable and let LLDB do the rest. 
+//
+// Go here to find more about debuginfod:
+// https://sourceware.org/elfutils/Debuginfod.html
+//--
+
+// We copy this file because we want to compile and later move it away
+// ===
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+// We cd into the directory before compiling to get DW_AT_comp_dir pickup
+// %t.buildroot as well so it will be replaced by /my/new/path.
+// ===
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+// We move the original source file to a directory that looks like a debuginfod
+// URL part.
+// 
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+// Adjust where debuginfod stores cache files:
+// ===
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+// Start HTTP file server on port picked by OS and wait until it is ready
+// The server will be closed on exit of the test.
+// ==
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'echo "SERVER LOG:"; cat %t.server.log; kill $PID;' EXIT INT
+
+// Extract HTTP address from the first line of the server log
+// (e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+// ==
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: 

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Just watch the build bots carefully when you check this in!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314



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

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk planned changes to this revision.
kwk added a comment.

In D75750#1929967 , @jankratochvil 
wrote:

> On Fedora 31 x86_64 with LLDB using python3 I got:
>
>   llvm-lit: .../llvm-monorepo2/llvm/utils/lit/lit/TestingConfig.py:102: 
> fatal: unable to parse config file 
> '.../llvm-monorepo2-clangassert/tools/lldb/test/Shell/lit.site.cfg.py', 
> traceback: Traceback (most recent call last):
> File ".../llvm-monorepo2/llvm/utils/lit/lit/TestingConfig.py", line 89, 
> in load_from_path 
>   exec(compile(data, path, 'exec'), cfg_globals, None)
> File 
> ".../llvm-monorepo2-clangassert/tools/lldb/test/Shell/lit.site.cfg.py", line 
> 20, in 
>   config.lldb_enable_debuginfod = TRUE
>   NameError: name 'TRUE' is not defined
>   make[3]: *** [tools/lldb/test/CMakeFiles/check-lldb-lit.dir/build.make:58: 
> tools/lldb/test/CMakeFiles/check-lldb-lit] Error 2
>


Right, I manually fixed it locally and have forgotton to fix it. Thank you 
@jankratochvil for bringing it up.

> It helped to change:
> 
>   -  set(Debuginfod_FOUND TRUE)
>   +  set(Debuginfod_FOUND 1)

I'm sure this helps but we have a better way by using 
`llvm_canonicalize_cmake_booleans` in CMake. I did use this before for 
minidebuginfo and LZMA integration but the place in which I've put it was moved 
around which is why I needed some time. Expect a fix soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[Greg Clayton via Phabricator via lldb-commits]

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

Just some header doc cleanup before we submit. Thanks for sticking with these 
changes!




Comment at: lldb/include/lldb/API/SBEnvironment.h:24
+
+  const lldb::SBEnvironment =(const lldb::SBEnvironment );
+

const means nothing in these classes since the ivar will never get modified, so 
you can remote the leading "const" for the return value. 



Comment at: lldb/include/lldb/API/SBEnvironment.h:27
+  /// Return the value of a given environment variable.
+  ///
+  /// \return

need \param entries in header doc



Comment at: lldb/include/lldb/API/SBEnvironment.h:29
+  /// \return
+  /// The value of the enviroment variable or null if not present.
+  const char *Get(const char *name);

Rephrase the return value docs a bit. Maybe:

```
/// The value of the environment variable or null if not present. If the 
environment variable has no value but is present a valid pointer to an empty 
string will be returned.
```

Is that what we are doing? Returning "" when variable is there but has no value?



Comment at: lldb/include/lldb/API/SBEnvironment.h:32
+
+  /// \return
+  /// The name at the given index or null if the index is invalid.

need brief description and \param entries in header doc



Comment at: lldb/include/lldb/API/SBEnvironment.h:36
+
+  /// \return
+  /// The value at the given index or null if the index is invalid.

need brief description and \param entries in header doc



Comment at: lldb/include/lldb/API/SBEnvironment.h:37
+  /// \return
+  /// The value at the given index or null if the index is invalid.
+  const char *GetValueAtIndex(size_t index);

Need to clarify what happens when the env far is there but has no value. Should 
match the Get(const char*) function return value with what ever we say is the 
solution for that.



Comment at: lldb/include/lldb/API/SBEnvironment.h:40
+
+  size_t GetNumValues();
+

Move above the *AtIndex(size_t) calls so users see this first in API.



Comment at: lldb/include/lldb/API/SBEnvironment.h:44
+  /// If the variable exists, its value is updated only if overwrite is true.
+  ///
+  /// \return

need \param entries in header doc



Comment at: lldb/include/lldb/API/SBEnvironment.h:50
+  /// Unset an environment variable if exists.
+  ///
+  /// \return

need \param entries in header doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314



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

[Lldb-commits] [PATCH] D76216: Improve step over performance

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Jim is the one that really needs to mark this as accepted as the thread plans 
are one of his many areas of expertise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216



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

[Lldb-commits] [PATCH] D76411: [debugserver] Implement hardware breakpoints for ARM64

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added subscribers: danielkiss, kristof.beyls.

Add support for hardware breakpoints on ARM64


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D76411

Files:
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
@@ -26,10 +26,12 @@
 
   DNBArchMachARM64(MachThread *thread)
   : m_thread(thread), m_state(), m_disabled_watchpoints(),
-m_watchpoint_hw_index(-1), m_watchpoint_did_occur(false),
+m_disabled_breakpoints(), m_watchpoint_hw_index(-1),
+m_watchpoint_did_occur(false),
 m_watchpoint_resume_single_step_enabled(false),
 m_saved_register_states() {
 m_disabled_watchpoints.resize(16);
+m_disabled_breakpoints.resize(16);
 memset(_dbg_save, 0, sizeof(m_dbg_save));
   }
 
@@ -62,7 +64,13 @@
   static const uint8_t *SoftwareBreakpointOpcode(nub_size_t byte_size);
   static uint32_t GetCPUType();
 
+  virtual uint32_t NumSupportedHardwareBreakpoints();
   virtual uint32_t NumSupportedHardwareWatchpoints();
+
+  virtual uint32_t EnableHardwareBreakpoint(nub_addr_t addr, nub_size_t size,
+bool also_set_on_task);
+  virtual bool DisableHardwareBreakpoint(uint32_t hw_break_index,
+ bool also_set_on_task);
   virtual uint32_t EnableHardwareWatchpoint(nub_addr_t addr, nub_size_t size,
 bool read, bool write,
 bool also_set_on_task);
@@ -229,10 +237,11 @@
   State m_state;
   arm_debug_state64_t m_dbg_save;
 
-  // arm64 doesn't keep the disabled watchpoint values in the debug register
-  // context like armv7;
+  // arm64 doesn't keep the disabled watchpoint and breakpoint values in the
+  // debug register context like armv7;
   // we need to save them aside when we disable them temporarily.
   std::vector m_disabled_watchpoints;
+  std::vector m_disabled_breakpoints;
 
   // The following member variables should be updated atomically.
   int32_t m_watchpoint_hw_index;
Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -666,6 +666,112 @@
   return g_num_supported_hw_watchpoints;
 }
 
+uint32_t DNBArchMachARM64::NumSupportedHardwareBreakpoints() {
+  // Set the init value to something that will let us know that we need to
+  // autodetect how many breakpoints are supported dynamically...
+  static uint32_t g_num_supported_hw_breakpoints = UINT_MAX;
+  if (g_num_supported_hw_breakpoints == UINT_MAX) {
+// Set this to zero in case we can't tell if there are any HW breakpoints
+g_num_supported_hw_breakpoints = 0;
+
+size_t len;
+uint32_t n = 0;
+len = sizeof(n);
+if (::sysctlbyname("hw.optional.breakpoint", , , NULL, 0) == 0) {
+  g_num_supported_hw_breakpoints = n;
+  DNBLogThreadedIf(LOG_THREAD, "hw.optional.breakpoint=%u", n);
+} else {
+// For AArch64 we would need to look at ID_AA64DFR0_EL1 but debugserver runs in
+// EL0 so it can't access that reg.  The kernel should have filled in the
+// sysctls based on it though.
+#if defined(__arm__)
+  uint32_t register_DBGDIDR;
+
+  asm("mrc p14, 0, %0, c0, c0, 0" : "=r"(register_DBGDIDR));
+  uint32_t numWRPs = bits(register_DBGDIDR, 31, 28);
+  // Zero is reserved for the WRP count, so don't increment it if it is zero
+  if (numWRPs > 0)
+numWRPs++;
+  g_num_supported_hw_breakpoints = numWRPs;
+  DNBLogThreadedIf(LOG_THREAD,
+   "Number of supported hw breakpoint via asm():  %d",
+   g_num_supported_hw_breakpoints);
+#endif
+}
+  }
+  return g_num_supported_hw_breakpoints;
+}
+
+uint32_t DNBArchMachARM64::EnableHardwareBreakpoint(nub_addr_t addr,
+nub_size_t size,
+bool also_set_on_task) {
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::EnableHardwareBreakpoint(addr = "
+   "0x%8.8llx, size = %zu)",
+   (uint64_t)addr, size);
+
+  const uint32_t num_hw_breakpoints = NumSupportedHardwareBreakpoints();
+
+  nub_addr_t aligned_bp_address = addr;
+  uint32_t control_value = 0;
+
+  switch (size) {
+  case 2:
+control_value = (0x3 << 5) | 7;
+aligned_bp_address &= ~1;
+break;
+  case 4:
+control_value = (0xfu