[Lldb-commits] [PATCH] D114746: [lldb] Generalize ParsedDWARFTypeAttributes

2021-12-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I can't help the feeling that the boilerplate introduced by the enum 
compression is just not worth the benefit it brings, but otherwise this looks 
ok apart from the two inline comments.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h:90
+
+  // clang-format off
+  enum DWARFAttributeFlags : unsigned {

What's up with this?

I would hope that clang-format is able to reasonably format a simple enum 
declaration like this.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:12-16
+#include "DWARFDIE.h"
 #include "DWARFDefines.h"
 #include "DWARFFormValue.h"
+#include "lldb/Core/Declaration.h"
+#include "lldb/Utility/ConstString.h"

revert this.


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

https://reviews.llvm.org/D114746

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


[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

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

This is what I hand in mind. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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


[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

2021-12-07 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.

I am still not sure this is the right solution, but a) I don't know what *is* 
the right solution; and (b) I don't think this makes the situation worse; so I 
am hitting accept.

In D113930#3175022 , @zequanwu wrote:

> In D113930#3174406 , @labath wrote:
>
>> So I have a feeling that the real solution here is to avoid creating an ast 
>> entry for the function in the first phase. Then you should be able to create 
>> the full class declaration in the second phase, as you will have complete 
>> information there.
>
> If we don't create ast entries at `ParseFunctions`, non-class-member 
> functions will not have ast decls. If we just create ast decls for 
> non-class-member functions in `ParseFunctions`, we are expecting that 
> `ParseTypes` is always called after `ParseFunctions` to complete the decl 
> creation for member functions, which is not right.

I don't think this is as bad as it may sound at first. For lldb-test and 
ParseAllDebugSymbols purposes, I would even say its perfectly fine.

Overall I am not too worried about what happens in the `ParseAllDebugSymbols` 
scenario. I am more interested in what happens during "normal" operation when 
all of this is parsed lazily, in response to FindFunctions/FindTypes/... 
queries. (*) If everything works fine there then I think this would even be 
preferable, since you generally want to parse only the minimal amount of 
information. But I don't really even know how all of this works in DWARF, so I 
don't want to force you to redesign everything.

(*) A corollary of this is that I don't really trust that SymbolFileDWARF is 
doing "the right thing" in response to `ParseAllDebugSymbols`, since the main 
use case is lazy parsing, and `ParseAllDebugSymbols` only started being used 
relatively recently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

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


[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

2021-12-07 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 392305.
zrthxn added a comment.

Removed old/dead code and commented out sections


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115178

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/Generic.h
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h

Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
@@ -45,6 +45,10 @@
 LibStdcppBitsetSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibStdcppOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP);
+
 SyntheticChildrenFrontEnd *
 LibStdcppVectorIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
+++ /dev/null
@@ -1,84 +0,0 @@
-//===-- LibCxxOptional.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 "LibCxx.h"
-#include "lldb/DataFormatters/FormattersHelpers.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-namespace {
-
-class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
-public:
-  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
-Update();
-  }
-
-  size_t GetIndexOfChildWithName(ConstString name) override {
-return formatters::ExtractIndexFromString(name.GetCString());
-  }
-
-  bool MightHaveChildren() override { return true; }
-  bool Update() override;
-  size_t CalculateNumChildren() override { return m_has_value ? 1U : 0U; }
-  ValueObjectSP GetChildAtIndex(size_t idx) override;
-
-private:
-  /// True iff the option contains a value.
-  bool m_has_value = false;
-};
-} // namespace
-
-bool OptionalFrontEnd::Update() {
-  ValueObjectSP engaged_sp(
-  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
-
-  if (!engaged_sp)
-return false;
-
-  // __engaged_ is a bool flag and is true if the optional contains a value.
-  // Converting it to unsigned gives us a size of 1 if it contains a value
-  // and 0 if not.
-  m_has_value = engaged_sp->GetValueAsUnsigned(0) != 0;
-
-  return false;
-}
-
-ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
-  if (!m_has_value)
-return ValueObjectSP();
-
-  // __val_ contains the underlying value of an optional if it has one.
-  // Currently because it is part of an anonymous union GetChildMemberWithName()
-  // does not peer through and find it unless we are at the parent itself.
-  // We can obtain the parent through __engaged_.
-  ValueObjectSP val_sp(
-  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
-  ->GetParent()
-  ->GetChildAtIndex(0, true)
-  ->GetChildMemberWithName(ConstString("__val_"), true));
-
-  if (!val_sp)
-return ValueObjectSP();
-
-  CompilerType holder_type = val_sp->GetCompilerType();
-
-  if (!holder_type)
-return ValueObjectSP();
-
-  return val_sp->Clone(ConstString("Value"));
-}
-
-SyntheticChildrenFrontEnd *
-formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-  lldb::ValueObjectSP valobj_sp) {
-  if (valobj_sp)
-return new OptionalFrontEnd(*valobj_sp);
-  return nullptr;
-}
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -170,8 +170,8 @@
   lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
-LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-  lldb::ValueObjectSP valobj_sp);
+LibcxxOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+   lldb::ValueObjectSP valobj_sp);
 
 SyntheticChildrenFrontEnd *
 LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
Index: lldb

[Lldb-commits] [PATCH] D115178: Unify libstdcpp and libcxx formatters for `std::optional`

2021-12-07 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 392308.
zrthxn added a comment.

Cosmetic changes and incorporate requested changes from review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115178

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/Generic.h
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h

Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
@@ -45,6 +45,10 @@
 LibStdcppBitsetSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibStdcppOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP);
+
 SyntheticChildrenFrontEnd *
 LibStdcppVectorIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
 lldb::ValueObjectSP);
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
+++ /dev/null
@@ -1,84 +0,0 @@
-//===-- LibCxxOptional.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 "LibCxx.h"
-#include "lldb/DataFormatters/FormattersHelpers.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-namespace {
-
-class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
-public:
-  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
-Update();
-  }
-
-  size_t GetIndexOfChildWithName(ConstString name) override {
-return formatters::ExtractIndexFromString(name.GetCString());
-  }
-
-  bool MightHaveChildren() override { return true; }
-  bool Update() override;
-  size_t CalculateNumChildren() override { return m_has_value ? 1U : 0U; }
-  ValueObjectSP GetChildAtIndex(size_t idx) override;
-
-private:
-  /// True iff the option contains a value.
-  bool m_has_value = false;
-};
-} // namespace
-
-bool OptionalFrontEnd::Update() {
-  ValueObjectSP engaged_sp(
-  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
-
-  if (!engaged_sp)
-return false;
-
-  // __engaged_ is a bool flag and is true if the optional contains a value.
-  // Converting it to unsigned gives us a size of 1 if it contains a value
-  // and 0 if not.
-  m_has_value = engaged_sp->GetValueAsUnsigned(0) != 0;
-
-  return false;
-}
-
-ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
-  if (!m_has_value)
-return ValueObjectSP();
-
-  // __val_ contains the underlying value of an optional if it has one.
-  // Currently because it is part of an anonymous union GetChildMemberWithName()
-  // does not peer through and find it unless we are at the parent itself.
-  // We can obtain the parent through __engaged_.
-  ValueObjectSP val_sp(
-  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
-  ->GetParent()
-  ->GetChildAtIndex(0, true)
-  ->GetChildMemberWithName(ConstString("__val_"), true));
-
-  if (!val_sp)
-return ValueObjectSP();
-
-  CompilerType holder_type = val_sp->GetCompilerType();
-
-  if (!holder_type)
-return ValueObjectSP();
-
-  return val_sp->Clone(ConstString("Value"));
-}
-
-SyntheticChildrenFrontEnd *
-formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-  lldb::ValueObjectSP valobj_sp) {
-  if (valobj_sp)
-return new OptionalFrontEnd(*valobj_sp);
-  return nullptr;
-}
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -170,8 +170,8 @@
   lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
-LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-  lldb::ValueObjectSP valobj_sp);
+LibcxxOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+   lldb::ValueObjectSP valobj_sp);
 
 SyntheticChildrenFrontEnd *
 LibcxxVariantFrontEndCreator(CXXSyntheticChildren 

[Lldb-commits] [PATCH] D115151: [lldb/qemu] Add emulator-args setting

2021-12-07 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

LGTM.

Not clear what qemu options will be most useful yet and a catch all is always 
useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115151

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


[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

2021-12-07 Thread Thorsten via Phabricator via lldb-commits
tschuett added a comment.

I believe the `Basic` library in Clang does not depend on other Clang 
libraries. It is a tail library. Do you envision a Basic, Utility, ... library 
that does not depend on Clang and provides basic utilities.


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

https://reviews.llvm.org/D115211

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


[Lldb-commits] [lldb] f72ae5c - [lldb] Fix windows path guessing for root paths

2021-12-07 Thread Jaroslav Sevcik via lldb-commits

Author: Jaroslav Sevcik
Date: 2021-12-07T11:16:04+01:00
New Revision: f72ae5cba1d625301aa8a668a3d54ec3ac5b7806

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

LOG: [lldb] Fix windows path guessing for root paths

Fix recognizing ":\" as a windows path.

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

Added: 


Modified: 
lldb/source/Utility/FileSpec.cpp
lldb/unittests/Utility/FileSpecTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/FileSpec.cpp 
b/lldb/source/Utility/FileSpec.cpp
index 601edb86c1b0c..24f8c2b1c23fc 100644
--- a/lldb/source/Utility/FileSpec.cpp
+++ b/lldb/source/Utility/FileSpec.cpp
@@ -310,7 +310,7 @@ llvm::Optional 
FileSpec::GuessPathStyle(llvm::StringRef absolut
 return Style::posix;
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
-  if (absolute_path.size() > 3 && llvm::isAlpha(absolute_path[0]) &&
+  if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
   absolute_path.substr(1, 2) == R"(:\)")
 return Style::windows;
   return llvm::None;

diff  --git a/lldb/unittests/Utility/FileSpecTest.cpp 
b/lldb/unittests/Utility/FileSpecTest.cpp
index 3dd355284ce0f..64b72bec483e5 100644
--- a/lldb/unittests/Utility/FileSpecTest.cpp
+++ b/lldb/unittests/Utility/FileSpecTest.cpp
@@ -198,8 +198,10 @@ TEST(FileSpecTest, GuessPathStyle) {
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
 }
 
 TEST(FileSpecTest, GetPath) {



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


[Lldb-commits] [PATCH] D115104: [lldb] Fix guessing of windows path style

2021-12-07 Thread Jaroslav Sevcik via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf72ae5cba1d6: [lldb] Fix windows path guessing for root 
paths (authored by jarin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115104

Files:
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -198,8 +198,10 @@
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
 }
 
 TEST(FileSpecTest, GetPath) {
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -310,7 +310,7 @@
 return Style::posix;
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
-  if (absolute_path.size() > 3 && llvm::isAlpha(absolute_path[0]) &&
+  if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
   absolute_path.substr(1, 2) == R"(:\)")
 return Style::windows;
   return llvm::None;


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -198,8 +198,10 @@
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
 }
 
 TEST(FileSpecTest, GetPath) {
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -310,7 +310,7 @@
 return Style::posix;
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
-  if (absolute_path.size() > 3 && llvm::isAlpha(absolute_path[0]) &&
+  if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
   absolute_path.substr(1, 2) == R"(:\)")
 return Style::windows;
   return llvm::None;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D114911: [lldb] Introduce a FreeBSDKernel plugin for vmcores

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

Fix segv when there's no executable module. Move dependency check to LLDBConfig 
and use `find_package()`-style check. Add initial test files (still WIP).


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

https://reviews.llvm.org/D114911

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
@@ -0,0 +1,38 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_386
+  Entry:   0x8F9000
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x1AB7B00
+AddressAlign:0x80
+Offset:  0x12B7AB0
+Size:0x2D48D8
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x80
+  - Name:dumppcb
+Type:STT_OBJECT
+Section: .bss
+Value:   0x1D2D9A0
+Size:0xC0
+  - Name:hz
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x1D4053C
+Size:0x4
+  - Name:IdlePDPT
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x1D8B044
+Size:0x4
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
@@ -0,0 +1,30 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AARCH64
+  Entry:   0x0800
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x00C35000
+AddressAlign:0x1000
+Size:0x37F000
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x
+  - Name:dumppcb
+Type:STT_OBJECT
+Section: .bss
+Value:   0x00DF3790
+Size:0x560
+  - Name:hz
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x00E2651C
+Size:0x4
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
@@ -0,0 +1,38 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x8037C000
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x819BA380
+AddressAlign:0x80
+Offset:  0x17BA348
+Size:0x445C80
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x8000
+  - N

[Lldb-commits] [PATCH] D114911: [lldb] Introduce a FreeBSDKernel plugin for vmcores

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

Remove a debugging hack, make the test actually test something.


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

https://reviews.llvm.org/D114911

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
@@ -0,0 +1,38 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_386
+  Entry:   0x8F9000
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x1AB7B00
+AddressAlign:0x80
+Offset:  0x12B7AB0
+Size:0x2D48D8
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x80
+  - Name:dumppcb
+Type:STT_OBJECT
+Section: .bss
+Value:   0x1D2D9A0
+Size:0xC0
+  - Name:hz
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x1D4053C
+Size:0x4
+  - Name:IdlePDPT
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x1D8B044
+Size:0x4
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
@@ -0,0 +1,30 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AARCH64
+  Entry:   0x0800
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x00C35000
+AddressAlign:0x1000
+Size:0x37F000
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x
+  - Name:dumppcb
+Type:STT_OBJECT
+Section: .bss
+Value:   0x00DF3790
+Size:0x560
+  - Name:hz
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x00E2651C
+Size:0x4
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
@@ -0,0 +1,38 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x8037C000
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x819BA380
+AddressAlign:0x80
+Offset:  0x17BA348
+Size:0x445C80
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x8000
+  - Name:KPML4phys
+Type: 

[Lldb-commits] [PATCH] D114911: [lldb] Introduce a FreeBSDKernel plugin for vmcores

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

Finish that one tet.


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

https://reviews.llvm.org/D114911

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_arm64.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_i386.h
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDKernel/RegisterContextFreeBSDKernel_x86_64.h
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.cpp
  lldb/source/Plugins/Process/FreeBSDKernel/ThreadFreeBSDKernel.h
  
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
  lldb/test/API/functionalities/postmortem/FreeBSDKernel/vmcore-amd64-full.bz2

Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-i386.yaml
@@ -0,0 +1,38 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_386
+  Entry:   0x8F9000
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x1AB7B00
+AddressAlign:0x80
+Offset:  0x12B7AB0
+Size:0x2D48D8
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x80
+  - Name:dumppcb
+Type:STT_OBJECT
+Section: .bss
+Value:   0x1D2D9A0
+Size:0xC0
+  - Name:hz
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x1D4053C
+Size:0x4
+  - Name:IdlePDPT
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x1D8B044
+Size:0x4
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-arm64.yaml
@@ -0,0 +1,30 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AARCH64
+  Entry:   0x0800
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x00C35000
+AddressAlign:0x1000
+Size:0x37F000
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x
+  - Name:dumppcb
+Type:STT_OBJECT
+Section: .bss
+Value:   0x00DF3790
+Size:0x560
+  - Name:hz
+Type:STT_OBJECT
+Section: .bss
+Binding: STB_GLOBAL
+Value:   0x00E2651C
+Size:0x4
Index: lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/FreeBSDKernel/kernel-amd64.yaml
@@ -0,0 +1,38 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_FREEBSD
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x8037C000
+Sections:
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x819BA380
+AddressAlign:0x80
+Offset:  0x17BA348
+Size:0x445C80
+Symbols:
+  - Name:kernbase
+Index:   SHN_ABS
+Binding: STB_GLOBAL
+Value:   0x8000
+  - Name:KPML4phys
+Type:STT_OBJECT
+Section: .bs

[Lldb-commits] [PATCH] D115151: [lldb/qemu] Add emulator-args setting

2021-12-07 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG611fdde4c765: [lldb/qemu] Add emulator-args setting 
(authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115151

Files:
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- lldb/test/API/qemu/qemu.py
+++ lldb/test/API/qemu/qemu.py
@@ -53,6 +53,7 @@
 parser = argparse.ArgumentParser(description=_description,
 formatter_class=argparse.RawDescriptionHelpFormatter)
 parser.add_argument('-g', metavar="unix-socket", required=True)
+parser.add_argument('-fake-arg', dest="fake-arg")
 parser.add_argument('program', help="The program to 'emulate'.")
 parser.add_argument("args", nargs=argparse.REMAINDER)
 args = parser.parse_args()
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- lldb/test/API/qemu/TestQemuLaunch.py
+++ lldb/test/API/qemu/TestQemuLaunch.py
@@ -20,7 +20,7 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 def set_emulator_setting(self, name, value):
-self.runCmd("settings set platform.plugin.qemu-user.%s %s" %
+self.runCmd("settings set -- platform.plugin.qemu-user.%s %s" %
 (name, value))
 
 def setUp(self):
@@ -43,7 +43,7 @@
 self.set_emulator_setting("architecture", self.getArchitecture())
 self.set_emulator_setting("emulator-path", emulator)
 
-def test_basic_launch(self):
+def _run_and_get_state(self):
 self.build()
 exe = self.getBuildArtifact()
 
@@ -63,7 +63,11 @@
 
 # Verify the qemu invocation parameters.
 with open(self.getBuildArtifact("state.log")) as s:
-state = json.load(s)
+return json.load(s)
+
+def test_basic_launch(self):
+state = self._run_and_get_state()
+
 self.assertEqual(state["program"], self.getBuildArtifact())
 self.assertEqual(state["args"],
 ["dump:" + self.getBuildArtifact("state.log")])
@@ -159,3 +163,9 @@
 target.Launch(info, error)
 self.assertTrue(error.Fail())
 self.assertIn("doesn't exist", error.GetCString())
+
+def test_extra_args(self):
+self.set_emulator_setting("emulator-args", "-fake-arg fake-value")
+state = self._run_and_get_state()
+
+self.assertEqual(state["fake-arg"], "fake-value")
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
@@ -9,4 +9,8 @@
 Global,
 DefaultStringValue<"">,
 Desc<"Path to the emulator binary.">;
+  def EmulatorArgs: Property<"emulator-args", "Args">,
+Global,
+DefaultStringValue<"">,
+Desc<"Extra arguments to pass to the emulator.">;
 }
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -47,6 +47,13 @@
 return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr,
  ePropertyEmulatorPath);
   }
+
+  Args GetEmulatorArgs() {
+Args result;
+m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEmulatorArgs,
+  result);
+return result;
+  }
 };
 
 static PluginProperties &GetGlobalProperties() {
@@ -112,8 +119,10 @@
 llvm::sys::fs::createUniquePath(socket_model, socket_path, false);
   } while (FileSystem::Instance().Exists(socket_path));
 
-  Args args(
-  {qemu, "-g", socket_path, launch_info.GetExecutableFile().GetPath()});
+  Args args({qemu, "-g", socket_path});
+  args.AppendArguments(GetGlobalProperties().GetEmulatorArgs());
+  args.AppendArgument("--");
+  args.AppendArgument(launch_info.GetExecutableFile().GetPath());
   for (size_t i = 1; i < launch_info.GetArguments().size(); ++i)
 args.AppendArgument(launch_info.GetArguments()[i].ref());
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 611fdde - [lldb/qemu] Add emulator-args setting

2021-12-07 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-12-07T14:19:43+01:00
New Revision: 611fdde4c765d1ed213744263cb2bbc04bd5060c

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

LOG: [lldb/qemu] Add emulator-args setting

This setting allows the user to pass additional arguments to the qemu instance.
While we may want to introduce dedicated settings for the most common qemu
arguments (-cpu, for one), having this setting allows us to avoid creating a
setting for every possible argument.

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

Added: 


Modified: 
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
lldb/test/API/qemu/TestQemuLaunch.py
lldb/test/API/qemu/qemu.py

Removed: 




diff  --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp 
b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
index 36cf0f010b4cc..82b9bc078ce6c 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -47,6 +47,13 @@ class PluginProperties : public Properties {
 return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr,
  
ePropertyEmulatorPath);
   }
+
+  Args GetEmulatorArgs() {
+Args result;
+m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEmulatorArgs,
+  result);
+return result;
+  }
 };
 
 static PluginProperties &GetGlobalProperties() {
@@ -112,8 +119,10 @@ lldb::ProcessSP 
PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
 llvm::sys::fs::createUniquePath(socket_model, socket_path, false);
   } while (FileSystem::Instance().Exists(socket_path));
 
-  Args args(
-  {qemu, "-g", socket_path, launch_info.GetExecutableFile().GetPath()});
+  Args args({qemu, "-g", socket_path});
+  args.AppendArguments(GetGlobalProperties().GetEmulatorArgs());
+  args.AppendArgument("--");
+  args.AppendArgument(launch_info.GetExecutableFile().GetPath());
   for (size_t i = 1; i < launch_info.GetArguments().size(); ++i)
 args.AppendArgument(launch_info.GetArguments()[i].ref());
 

diff  --git 
a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td 
b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
index abfab7f59de40..19de9c810841e 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
@@ -9,4 +9,8 @@ let Definition = "platformqemuuser" in {
 Global,
 DefaultStringValue<"">,
 Desc<"Path to the emulator binary.">;
+  def EmulatorArgs: Property<"emulator-args", "Args">,
+Global,
+DefaultStringValue<"">,
+Desc<"Extra arguments to pass to the emulator.">;
 }

diff  --git a/lldb/test/API/qemu/TestQemuLaunch.py 
b/lldb/test/API/qemu/TestQemuLaunch.py
index 36a032190a608..259b381d26721 100644
--- a/lldb/test/API/qemu/TestQemuLaunch.py
+++ b/lldb/test/API/qemu/TestQemuLaunch.py
@@ -20,7 +20,7 @@ class TestQemuLaunch(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 def set_emulator_setting(self, name, value):
-self.runCmd("settings set platform.plugin.qemu-user.%s %s" %
+self.runCmd("settings set -- platform.plugin.qemu-user.%s %s" %
 (name, value))
 
 def setUp(self):
@@ -43,7 +43,7 @@ def setUp(self):
 self.set_emulator_setting("architecture", self.getArchitecture())
 self.set_emulator_setting("emulator-path", emulator)
 
-def test_basic_launch(self):
+def _run_and_get_state(self):
 self.build()
 exe = self.getBuildArtifact()
 
@@ -63,7 +63,11 @@ def test_basic_launch(self):
 
 # Verify the qemu invocation parameters.
 with open(self.getBuildArtifact("state.log")) as s:
-state = json.load(s)
+return json.load(s)
+
+def test_basic_launch(self):
+state = self._run_and_get_state()
+
 self.assertEqual(state["program"], self.getBuildArtifact())
 self.assertEqual(state["args"],
 ["dump:" + self.getBuildArtifact("state.log")])
@@ -159,3 +163,9 @@ def test_bad_emulator_path(self):
 target.Launch(info, error)
 self.assertTrue(error.Fail())
 self.assertIn("doesn't exist", error.GetCString())
+
+def test_extra_args(self):
+self.set_emulator_setting("emulator-args", "-fake-arg fake-value")
+state = self._run_and_get_state()
+
+self.assertEqual(state["fake-arg"], "fake-value")

diff  --git a/lldb/test/API/qemu/qemu.py b/lldb/test/API/qemu/qemu.py
index 97a9efba81a9b..eca85f8ac99b9 100755
--- a/lldb/test/API/qemu/qemu.py
+++ b/lldb/

[Lldb-commits] [lldb] d4083a2 - [lldb] Fix flakyness in TestQemuLaunch.test_stdio_redirect

2021-12-07 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-12-07T14:19:44+01:00
New Revision: d4083a296ac828440604574e7a9fac70ede96d66

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

LOG: [lldb] Fix flakyness in TestQemuLaunch.test_stdio_redirect

The test was flaky because it was trying to read from the (redirected)
stdout file before the data was been flushed to it. This would not be a
problem for a "normal" debug session, but since here the emulator and
the target binary coexist in the same process (and this is true both for
real qemu and our fake implementation), there
is a window of time between the stub returning an exit packet (which is
the event that the test is waiting for) and the process really exiting
(which is when the normal flushing happens).

This patch adds an explicit flush to work around this. Theoretically,
it's possible that real code could run into this issue as well, but such
a use case is not very likely. If we wanted to fix this for real, we
could add some code which waits for the host process to terminate (in
addition to receiving the termination packet), but this is somewhat
complicated by the fact that this code lives in the gdb-remote process
plugin.

Added: 


Modified: 
lldb/test/API/qemu/qemu.py

Removed: 




diff  --git a/lldb/test/API/qemu/qemu.py b/lldb/test/API/qemu/qemu.py
index eca85f8ac99b9..4b94c7689657e 100755
--- a/lldb/test/API/qemu/qemu.py
+++ b/lldb/test/API/qemu/qemu.py
@@ -35,8 +35,10 @@ def cont(self):
 json.dump(self._state, f)
 elif action == "stdout":
 sys.stdout.write(data)
+sys.stdout.flush()
 elif action == "stderr":
 sys.stderr.write(data)
+sys.stderr.flush()
 elif action == "stdin":
 self._state[data] = sys.stdin.readline()
 else:



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


[Lldb-commits] [PATCH] D115246: [lldb/qemu] Separate host and target environments

2021-12-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: DavidSpickett, mgorny, JDevlieghere.
labath requested review of this revision.
Herald added a project: LLDB.

Qemu normally forwards its (host) environment variables to the emulated
process. While this works fine for most variables, there are some (few, but
fairly important) variables where this is not possible. LD_LIBRARY_PATH
is the probably the most important of those -- we don't want the library
search path for the emulated libraries to interfere with the libraries
that the emulator itself needs.

For this reason, qemu provides a mechanism (QEMU_SET_ENV,
QEMU_UNSET_ENV) to set variables only for the emulated process. This
patch makes use of that functionality to pass any user-provided
variables to the emulated process. Since we're piggy-backing on the
normal lldb environment-handling mechanism, all the usual mechanism to
provide environment (target.env-vars setting, SBLaunchInfo, etc.) work
out-of-the-box, and the only thing we need to do is to properly
construct the qemu environment variables.

This patch also adds a new setting -- target-env-vars, which represents
environment variables which are added (on top of the host environment)
to the default launch environments of all (qemu) targets. The reason for
its existence is to enable the configuration (e.g., from a startup
script) of the default launch environment, before any target is created.
The idea is that this would contain the variables (like the
aforementioned LD_LIBRARY_PATH) common to all targets being debugged on
the given system. The user is, of course, free to customize the
environment for a particular target in the usual manner.

The reason I do not want to use/recommend the "global" version of the
target.env-vars setting for this purpose is that the setting would apply
to all targets, whereas the settings (their values) I have mentioned
would be specific to the given platform.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115246

Files:
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td
  lldb/test/API/qemu/TestQemuLaunch.py
  lldb/test/API/qemu/qemu.py

Index: lldb/test/API/qemu/qemu.py
===
--- lldb/test/API/qemu/qemu.py
+++ lldb/test/API/qemu/qemu.py
@@ -1,6 +1,7 @@
 import argparse
 import socket
 import json
+import os
 import sys
 
 import use_lldb_suite
@@ -60,7 +61,9 @@
 parser.add_argument("args", nargs=argparse.REMAINDER)
 args = parser.parse_args()
 
-emulator = FakeEmulator(args.g, vars(args))
+state = vars(args)
+state["environ"] = dict(os.environ)
+emulator = FakeEmulator(args.g, state)
 emulator.run()
 
 if __name__ == "__main__":
Index: lldb/test/API/qemu/TestQemuLaunch.py
===
--- lldb/test/API/qemu/TestQemuLaunch.py
+++ lldb/test/API/qemu/TestQemuLaunch.py
@@ -43,7 +43,7 @@
 self.set_emulator_setting("architecture", self.getArchitecture())
 self.set_emulator_setting("emulator-path", emulator)
 
-def _run_and_get_state(self):
+def _create_target(self):
 self.build()
 exe = self.getBuildArtifact()
 
@@ -52,11 +52,21 @@
 target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
 self.assertSuccess(error)
 self.assertEqual(target.GetPlatform().GetName(), "qemu-user")
+return target
+
+def _run_and_get_state(self, target=None, info=None):
+if target is None:
+target = self._create_target()
+
+if info is None:
+info = target.GetLaunchInfo()
 
 # "Launch" the process. Our fake qemu implementation will pretend it
 # immediately exited.
-process = target.LaunchSimple(
-["dump:" + self.getBuildArtifact("state.log")], None, None)
+info.SetArguments(["dump:" + self.getBuildArtifact("state.log")], True)
+error = lldb.SBError()
+process = target.Launch(info, error)
+self.assertSuccess(error)
 self.assertIsNotNone(process)
 self.assertEqual(process.GetState(), lldb.eStateExited)
 self.assertEqual(process.GetExitStatus(), 0x47)
@@ -73,25 +83,21 @@
 ["dump:" + self.getBuildArtifact("state.log")])
 
 def test_stdio_pty(self):
-self.build()
-exe = self.getBuildArtifact()
-
-# Create a target using our platform
-error = lldb.SBError()
-target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
-self.assertSuccess(error)
+target = self._create_target()
 
-info = lldb.SBLaunchInfo([
+info = target.GetLaunchInfo()
+info.SetArguments([
 "stdin:stdin",
 "stdout:STDOUT CONTENT\n",
 "stderr:STDERR CONTENT\n",
 "dump

[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

2021-12-07 Thread Lasse Folger via Phabricator via lldb-commits
lassefolger added inline comments.



Comment at: lldb/source/Symbol/SymbolFile.cpp:148
+  std::string(scope.GetCString()), type_class,
+  true);
+}

I need to change this. This was correct in the first iteration but is no longer 
correct in the current iteration of this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627

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


[Lldb-commits] [PATCH] D115246: [lldb/qemu] Separate host and target environments

2021-12-07 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp:121
+auto host_it = host.find(KV.first());
+if (host_it == host.end() || host_it->second != KV.second)
+  set_env.push_back(Environment::compose(KV));

Please comment with the meaning here. Which I think is if this var is not set 
for the host, or it is set but to a different value.



Comment at: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp:127
+  for (const auto &KV : host) {
+if (target.count(KV.first()) == 0)
+  unset_env.push_back(KV.first());

And here you're saying if the setting is set for host but not target, unset it?

And then I thought doesn't this just leave you with the content of `target` but 
that's not the goal here. You start with:
target environment
host environment

You want:
host environment plus `QEMU_SET/UNSET_ENV` based on the target environment's 
content

Since you might still need some of the vars that get unset, before the point 
where qemu's emulation begins, right? So maybe a bad example idk, but you might 
want LD_LIBRARY_PATH set to one thing to run qemu properly, but then you want 
it to be changed when qemu does its emulation.

Which is why you can't just use the target environment and discard the host 
completely.

Could you add a motivating example as a comment to the function? I think it's 
quite clearly written but easy to come to that incorrect conclusion on first 
read.



Comment at: 
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td:18
+ElementType<"String">,
+Desc<"Extra variables to add to target environment.">;
 }

"to emulated target environment", just to be super clear?



Comment at: lldb/test/API/qemu/qemu.py:65
+state = vars(args)
+state["environ"] = dict(os.environ)
+emulator = FakeEmulator(args.g, state)

Do you do this so that you know you have a copy, or because os.environ is not 
quite acting like a dictionary enough?

A quick check shows it does set/get key but I'm sure it's not 100% like a plain 
dict somehow.

If it's to ensure a copy, not a reference to the object please note that in a 
comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115246

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


[Lldb-commits] [PATCH] D115246: [lldb/qemu] Separate host and target environments

2021-12-07 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Oh and:

  This patch also adds a new setting -- target-env-vars

Might want to clarify that it adds this setting **to the qemu user platform**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115246

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


[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

2021-12-07 Thread Andy Yankovsky via Phabricator via lldb-commits
werat added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2461
+  bool has_scope = !scope.IsEmpty();
+  llvm::StringRef sc(scope.GetStringRef());
+  std::string storage;

nit: maybe call it `scope_ref`?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2470
+  const char *qn = die.GetQualifiedName(storage);
+  if (qn && !llvm::StringRef(qn).startswith(sc))
+return true;

`llvm::StringRef(storage)` is slightly more efficient here than 
`llvm::StringRef(qn)`. The latter uses strlen in the constructor to calculate 
the length of the source string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627

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


[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

2021-12-07 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D115211#3175581 , @labath wrote:

> I am definitely not sad to see lldbBase go away, but I am wondering if we 
> really need a separate library for this functionality (function).
>
> The only reason I can think of is that this function forces a dependency on 
> clang. If we put it in say Utility, then all of lldb would depend on a (very 
> tiny) piece of clang. I don't think that most of lldb would care about that, 
> but this isn't technically correct for lldb-server. It doesn't use clang, and 
> only depends on it because of the general tangledness of lldb dependencies. 
> In an ideal world one would even be able to build lldb-server without having 
> clang checked out. However, given that:
>
> - we are pretty far from an ideal world, and if we ever reach it, we could 
> make this small dependency conditional on the existence of clang
> - it's really not clear (to me) what else should belong in the Basic library
>
> I am thinking that we could just put this stuff into Utility. It's true that 
> the clang version stuff lives in a library called "Basic", but that's because 
> this is the name clang chose for its lowest level library. In lldb the lowest 
> level library is Utility.
>
> WDYT?

Yep, the clang dependency is the reason I kept it separate, and the problem 
gets worse downstream where you might have other compilers (i.e. Swift, but 
maybe Rust one day :-) that want to be part of the version number string too. 
Generally speaking we are pretty far from an ideal world dependency-wise, but I 
think the clang dependencies is one place where Alex has made quite a bit of 
progress over the past years. If we don't need to have the version be available 
in the "lowest level library", we can do it relatively nicely by having each 
TypeSystem provide a compilers-specific version string and incorporating that 
in the final version. That stuff lives in Target, which is a bit of an awkward 
place for an LLDB version number. That approach wouldn't work today anyway, 
because we need the version in Utility for the reproducers, and while I have 
different plants for those, a version number is going to remain pretty 
critical. I can imagine other things like Greg's statistics wanting to include 
a version number too.

What's your main concern about making it a separate library? I'm asking because 
I think the tradeoff between that and linking clang/swift into Utility leans in 
favor of a separate library, but I'd like to better understand your perspective.

In D115211#3175792 , @tschuett wrote:

> I believe the `Basic` library in Clang does not depend on other Clang 
> libraries. It is a tail library. Do you envision a Basic, Utility, ... 
> library that does not depend on Clang and provides basic utilities.
>
> Maybe 2 tail libraries with utilities. One with dependencies on Clang and one 
> without.

No, other than the version I can't imagine another utility that would have to 
rely on clang. I picked Basic just for consistency in the header path, I don't 
envision it being anything more than the version (which is why I floated 
Version as an alternative).


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

https://reviews.llvm.org/D115211

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


[Lldb-commits] [lldb] 244258e - Modify DataEncoder to be able to encode data in an object owned buffer.

2021-12-07 Thread Greg Clayton via lldb-commits

Author: Greg Clayton
Date: 2021-12-07T09:44:57-08:00
New Revision: 244258e35acc92b3293e91ddecee6a8c8613ec20

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

LOG: Modify DataEncoder to be able to encode data in an object owned buffer.

DataEncoder was previously made to modify data within an existing buffer. As 
the code progressed, new clients started using DataEncoder to create binary 
data. In these cases the use of this class was possibly, but only if you knew 
exactly how large your buffer would be ahead of time. This patchs adds the 
ability for DataEncoder to own a buffer that can be dynamically resized as data 
is appended to the buffer.

Change in this patch:
- Allow a DataEncoder object to be created that owns a DataBufferHeap object 
that can dynamically grow as data is appended
- Add new methods that start with "Append" to append data to the buffer and 
grow it as needed
- Adds full testing of the API to assure modifications don't regress any 
functionality
- Has two constructors: one that uses caller owned data and one that creates an 
object with object owned data
- "Append" methods only work if the object owns it own data
- Removes the ability to specify a shared memory buffer as no one was using 
this functionality. This allows us to switch to a case where the object owns 
its own data in a DataBufferHeap that can be resized as data is added

"Put" methods work on both caller and object owned data.
"Append" methods work on only object owned data where we can grow the buffer. 
These methods will return false if called on a DataEncoder object that has 
caller owned data.

The main reason for these modifications is to be able to use the DateEncoder 
objects instead of llvm::gsym::FileWriter in https://reviews.llvm.org/D113789. 
This patch wants to add the ability to create symbol table caching to LLDB and 
the code needs to build binary caches and save them to disk.

Reviewed By: labath

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

Added: 
lldb/unittests/Utility/DataEncoderTest.cpp

Modified: 
lldb/include/lldb/Utility/DataEncoder.h
lldb/include/lldb/lldb-forward.h
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/Platform/Android/AdbClient.cpp
lldb/source/Utility/DataEncoder.cpp
lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
lldb/unittests/Utility/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Utility/DataEncoder.h 
b/lldb/include/lldb/Utility/DataEncoder.h
index b944c09d5c47f..ed3e448b23049 100644
--- a/lldb/include/lldb/Utility/DataEncoder.h
+++ b/lldb/include/lldb/Utility/DataEncoder.h
@@ -16,6 +16,9 @@
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+
 #include 
 #include 
 
@@ -26,21 +29,34 @@ namespace lldb_private {
 /// An binary data encoding class.
 ///
 /// DataEncoder is a class that can encode binary data (swapping if needed) to
-/// a data buffer. The data buffer can be caller owned, or can be shared data
-/// that can be shared between multiple DataEncoder or DataEncoder instances.
+/// a data buffer. The DataEncoder can be constructed with data that will be
+/// copied into the internally owned buffer. This allows data to be modified
+/// in the internal buffer. The DataEncoder object can also be constructed with
+/// just a byte order and address size and data can be appended to the
+/// internally owned buffer.
+///
+/// Clients can get a shared pointer to the data buffer when done modifying or
+/// creating the data to keep the data around after the lifetime of a
+/// DataEncoder object. \see GetDataBuffer
 ///
-/// \see DataBuffer
+/// Client can get a reference to the object owned data as an array by calling
+/// the GetData method. \see GetData
 class DataEncoder {
 public:
   /// Default constructor.
   ///
-  /// Initialize all members to a default empty state.
+  /// Initialize all members to a default empty state and create a empty memory
+  /// buffer that can be appended to. The ByteOrder and address size will be 
set
+  /// to match the current host system.
   DataEncoder();
 
-  /// Construct with a buffer that is owned by the caller.
+  /// Construct an encoder that copies the specified data into the object owned
+  /// data buffer.
   ///
-  /// This constructor allows us to use data that is owned by the caller. The
-  /// data must stay around as long as this object is valid.
+  /// This constructor is designed to be used when you have a data buffer and
+  /// want to modify values within the buffer. A copy of the data will be made
+  /// in the internally owned buffer and that data can be fixed up and appended
+  /// to.
   ///
   /// \param[in] data
   /// A pointer to caller 

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

2021-12-07 Thread Greg Clayton via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG244258e35acc: Modify DataEncoder to be able to encode data 
in an object owned buffer. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

Files:
  lldb/include/lldb/Utility/DataEncoder.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Utility/DataEncoder.cpp
  lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/DataEncoderTest.cpp

Index: lldb/unittests/Utility/DataEncoderTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/DataEncoderTest.cpp
@@ -0,0 +1,534 @@
+//===-- DataEncoderTest.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 "gtest/gtest.h"
+
+#include "lldb/Utility/DataEncoder.h"
+#include "llvm/ADT/ArrayRef.h"
+#include 
+using namespace lldb_private;
+using namespace llvm;
+
+TEST(DataEncoderTest, PutU8) {
+  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const uint32_t addr_size = 4;
+
+  uint32_t offset = 0;
+  DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
+  addr_size);
+  offset = encoder.PutU8(offset, 11);
+  ASSERT_EQ(offset, 1U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 2, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 12);
+  ASSERT_EQ(offset, 2U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 3, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 13);
+  ASSERT_EQ(offset, 3U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 4, 5, 6, 7, 8}));
+  offset = encoder.PutU8(offset, 14);
+  ASSERT_EQ(offset, 4U);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+  // Check that putting a number to an invalid offset doesn't work and returns
+  // an error offset and doesn't modify the buffer.
+  ASSERT_EQ(encoder.PutU8(init.size(), 15), UINT32_MAX);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({11, 12, 13, 14, 5, 6, 7, 8}));
+}
+
+TEST(DataEncoderTest, AppendUnsignedLittle) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x33, 0x22}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x33, 0x22, 0x77, 0x66, 0x55, 0x44,
+   0xFF, 0xEE, 0xDD, 0xCC, 0xBB, 0xAA, 0x99, 0x88}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+}
+
+TEST(DataEncoderTest, AppendUnsignedBig) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendU8(0x11);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11}));
+  encoder.AppendU16(0x2233);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33}));
+  encoder.AppendU32(0x44556677);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77}));
+  encoder.AppendU64(0x8899AABBCCDDEEFF);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+   0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Little) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x44, 0x33, 0x22, 0x11}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x44, 0x33, 0x22, 0x11, 0x55, 0x00, 0x00, 0x00}));
+}
+
+TEST(DataEncoderTest, AppendAddress4Big) {
+  const uint32_t addr_size = 4;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderBig, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(), ArrayRef({0x11, 0x22, 0x33, 0x44}));
+  encoder.AppendAddress(0x55);
+  ASSERT_EQ(encoder.GetData(),
+ArrayRef({0x11, 0x22, 0x33, 0x44, 0x00, 0x00, 0x00, 0x55}));
+}
+
+TEST(DataEncoderTest, AppendAddress8Little) {
+  const uint32_t addr_size = 8;
+  std::vector expected;
+  DataEncoder encoder(lldb::eByteOrderLittle, addr_size);
+  encoder.AppendAddress(0x11223344);
+  ASSERT_EQ(encoder.GetData(),
+ 

[Lldb-commits] [PATCH] D115201: [lldb] Move UpdateSymbolContextScopeForType

2021-12-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

Can you add a motivation for this to your summary, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115201

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


[Lldb-commits] [lldb] a3a8ed3 - [LLDB][NativePDB] Fix function decl creation for class methods

2021-12-07 Thread Zequan Wu via lldb-commits

Author: Zequan Wu
Date: 2021-12-07T10:41:28-08:00
New Revision: a3a8ed33a1d6a5047aae29790e02f3c7955297af

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

LOG: [LLDB][NativePDB] Fix function decl creation for class methods

This is a split of D113724. Calling `TypeSystemClang::AddMethodToCXXRecordType`
to create function decls for class methods.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index c29fc2230a674..7ce2f1451580f 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -30,6 +30,70 @@ using namespace lldb_private::npdb;
 using namespace llvm::codeview;
 using namespace llvm::pdb;
 
+namespace {
+struct CreateMethodDecl : public TypeVisitorCallbacks {
+  CreateMethodDecl(PdbIndex &m_index, TypeSystemClang &m_clang,
+   TypeIndex func_type_index,
+   clang::FunctionDecl *&function_decl,
+   lldb::opaque_compiler_type_t parent_ty,
+   llvm::StringRef proc_name, CompilerType func_ct)
+  : m_index(m_index), m_clang(m_clang), func_type_index(func_type_index),
+function_decl(function_decl), parent_ty(parent_ty),
+proc_name(proc_name), func_ct(func_ct) {}
+  PdbIndex &m_index;
+  TypeSystemClang &m_clang;
+  TypeIndex func_type_index;
+  clang::FunctionDecl *&function_decl;
+  lldb::opaque_compiler_type_t parent_ty;
+  llvm::StringRef proc_name;
+  CompilerType func_ct;
+
+  llvm::Error visitKnownMember(CVMemberRecord &cvr,
+   OverloadedMethodRecord &overloaded) override {
+TypeIndex method_list_idx = overloaded.MethodList;
+
+CVType method_list_type = m_index.tpi().getType(method_list_idx);
+assert(method_list_type.kind() == LF_METHODLIST);
+
+MethodOverloadListRecord method_list;
+llvm::cantFail(TypeDeserializer::deserializeAs(
+method_list_type, method_list));
+
+for (const OneMethodRecord &method : method_list.Methods) {
+  if (method.getType().getIndex() == func_type_index.getIndex())
+AddMethod(overloaded.Name, method.getAccess(), method.getOptions(),
+  method.Attrs);
+}
+
+return llvm::Error::success();
+  }
+
+  llvm::Error visitKnownMember(CVMemberRecord &cvr,
+   OneMethodRecord &record) override {
+AddMethod(record.getName(), record.getAccess(), record.getOptions(),
+  record.Attrs);
+return llvm::Error::success();
+  }
+
+  void AddMethod(llvm::StringRef name, MemberAccess access,
+ MethodOptions options, MemberAttributes attrs) {
+if (name != proc_name || function_decl)
+  return;
+lldb::AccessType access_type = TranslateMemberAccess(access);
+bool is_virtual = attrs.isVirtual();
+bool is_static = attrs.isStatic();
+bool is_artificial = (options & MethodOptions::CompilerGenerated) ==
+ MethodOptions::CompilerGenerated;
+function_decl = m_clang.AddMethodToCXXRecordType(
+parent_ty, proc_name,
+/*mangled_name=*/nullptr, func_ct, /*access=*/access_type,
+/*is_virtual=*/is_virtual, /*is_static=*/is_static,
+/*is_inline=*/false, /*is_explicit=*/false,
+/*is_attr_used=*/false, /*is_artificial=*/is_artificial);
+  }
+};
+} // namespace
+
 static llvm::Optional FindSymbolScope(PdbIndex &index,
  PdbCompilandSymId id) 
{
   CVSymbol sym = index.ReadSymbolRecord(id);
@@ -681,7 +745,8 @@ bool PdbAstBuilder::CompleteTagDecl(clang::TagDecl &tag) {
   // Visit all members of this class, then perform any finalization necessary
   // to complete the class.
   CompilerType ct = ToCompilerType(tag_qt);
-  UdtRecordCompleter completer(best_ti, ct, tag, *this, m_index);
+  UdtRecordCompleter completer(best_ti, ct, tag, *this, m_index,
+   m_cxx_record_map);
   auto error =
   llvm::codeview::visitMemberRecordStream(field_list_cvt.data(), 
completer);
   completer.complete();
@@ -1014,8 +1079,62 @@ PdbAstBuilder::GetOrCreateFunctionDecl(PdbCompilandSymId 
func_id) {
   proc_name.consume_front(context_name);
   proc_name.consume_front("::")

[Lldb-commits] [PATCH] D113930: [LLDB][NativePDB] Fix function decl creation for class methods

2021-12-07 Thread Zequan Wu via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3a8ed33a1d6: [LLDB][NativePDB] Fix function decl creation 
for class methods (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D113930?vs=391777&id=392465#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113930

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
  lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/find-functions.cpp
@@ -1,7 +1,7 @@
 // clang-format off
 // REQUIRES: lld, x86
 
-// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /GR- /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 
 // RUN: lldb-test symbols --find=function --name=main --function-flags=full %t.exe \
@@ -13,6 +13,46 @@
 // RUN: lldb-test symbols --find=function --name=varargs_fn --function-flags=full %t.exe \
 // RUN: | FileCheck %s --check-prefix=FIND-VAR
 
+// RUN: lldb-test symbols --find=function --name=Struct::simple_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-SIMPLE
+
+// RUN: lldb-test symbols --find=function --name=Struct::virtual_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-VIRTUAL
+
+// RUN: lldb-test symbols --find=function --name=Struct::static_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-STATIC-METHOD
+
+// RUN: lldb-test symbols --find=function --name=Struct::overloaded_method --function-flags=full %t.exe \
+// RUN: | FileCheck %s --check-prefix=FIND-OVERLOAD
+
+struct Struct {
+  int simple_method() {
+return 1;
+  }
+
+  virtual int virtual_method() {
+return 2;
+  }
+
+  static int static_method() {
+return 3;
+  }
+
+  int overloaded_method() {
+return 4 + overloaded_method('a') + overloaded_method('a', 1);
+  }
+protected:
+  virtual int overloaded_method(char c) {
+return 5;
+  }
+private:
+  static int overloaded_method(char c, int i, ...) {
+return 6;
+  }
+};
+
+Struct s;
+
 static int static_fn() {
   return 42;
 }
@@ -22,7 +62,8 @@
 }
 
 int main(int argc, char **argv) {
-  return static_fn() + varargs_fn(argc, argc);
+  return static_fn() + varargs_fn(argc, argc) + s.simple_method() +
+  Struct::static_method() + s.virtual_method() + s.overloaded_method();
 }
 
 // FIND-MAIN:  Function: id = {{.*}}, name = "main"
@@ -33,3 +74,17 @@
 
 // FIND-VAR:  Function: id = {{.*}}, name = "{{.*}}varargs_fn{{.*}}"
 // FIND-VAR-NEXT: FuncType: id = {{.*}}, compiler_type = "int (int, int, ...)"
+
+// FIND-SIMPLE:  Function: id = {{.*}}, name = "{{.*}}Struct::simple_method{{.*}}"
+// FIND-SIMPLE-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-VIRTUAL:  Function: id = {{.*}}, name = "{{.*}}Struct::virtual_method{{.*}}"
+// FIND-VIRTUAL-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-STATIC-METHOD:  Function: id = {{.*}}, name = "{{.*}}Struct::static_method{{.*}}"
+// FIND-STATIC-METHOD-NEXT: FuncType: id = {{.*}}, compiler_type = "int (void)"
+
+// FIND-OVERLOAD: Function: id = {{.*}}, name = "{{.*}}Struct::overloaded_method{{.*}}"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (void)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char)"
+// FIND-OVERLOAD: FuncType: id = {{.*}}, compiler_type = "int (char, int, ...)"
Index: lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/ast-methods.cpp
@@ -4,7 +4,9 @@
 // RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -GR- -c /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
-// RUN: %p/Inputs/ast-methods.lldbinit 2>&1 | FileCheck %s
+// RUN: %p/Inputs/ast-methods.lldbinit 2>&1 | FileCheck %s --check-prefix=AST
+
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols --dump-ast %t.exe | FileCheck %s --check-prefix=SYMBOL
 
 struct Struct {
   void simple_method() {}
@@ -21,17 +23,33 @@
 Struct s;
 
 int main(int argc, char **argv) {
+  s.simple_method();
+  s.static_method();
+  s.virtual_method();
+  s.o

[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

2021-12-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

The main reason I wanted to avoid a separate library is because I could not 
imagine how would it fit into the general library landscape in lldb. It's also 
moderately strange to have a library for just a single function, but that is 
something I can live with.

If the idea is that this library would consist of just the single function 
~forever, then I think that `Version` is a better name, and I don't really have 
a problem with that -- that is pretty much the status quo anyway.

One disadvantage of that is that the version number becomes hard to get from 
lower level parts of lldb. So far the only use we had for that were the 
reproducers, so if you say you can handle that, then I guess it should be fine.

One advantage of that is that it opens up the possibility for different 
components of lldb to report different version string. For example, lldb-server 
could write its own GetVersion function (which wouldn't include clang), and 
that would look a lot more natural if the "main" version function lives in a 
library that it does not even use.


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

https://reviews.llvm.org/D115211

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


[Lldb-commits] [lldb] cfe5d76 - Fix buildbot after https://reviews.llvm.org/D115073.

2021-12-07 Thread Greg Clayton via lldb-commits

Author: Greg Clayton
Date: 2021-12-07T12:03:45-08:00
New Revision: cfe5d768be95ae0a62cf430e56e7762cebf81a65

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

LOG: Fix buildbot after https://reviews.llvm.org/D115073.

Added: 


Modified: 
lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp

Removed: 




diff  --git a/lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp 
b/lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
index 36f91e1163787..ff7b4220f5991 100644
--- a/lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
+++ b/lldb/unittests/Process/POSIX/NativeProcessELFTest.cpp
@@ -39,7 +39,7 @@ std::unique_ptr CreateAuxvData(
 encoder.AppendAddress(pair.second);
   }
   return llvm::MemoryBuffer::getMemBufferCopy(
-  llvm::toStringRef(buffer_sp->GetData()), "");
+  llvm::toStringRef(encoder.GetData()), "");
 }
 
 } // namespace



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


[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

2021-12-07 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Fixed a buildbot issue for linux with:

commit cfe5d768be95ae0a62cf430e56e7762cebf81a65 
 (HEAD -> 
main, origin/main, origin/HEAD)
Author: Greg Clayton 
Date:   Tue Dec 7 12:03:45 2021 -0800

  Fix buildbot after https://reviews.llvm.org/D115073.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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


[Lldb-commits] [PATCH] D115126: [LLDB] Add unit tests for Editline keyboard shortcuts

2021-12-07 Thread Neal via Phabricator via lldb-commits
nealsid updated this revision to Diff 392497.
nealsid edited the summary of this revision.
nealsid added a comment.

Testing ARC, no action necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115126

Files:
  lldb/include/lldb/Host/Editline.h
  lldb/source/Host/common/Editline.cpp
  lldb/unittests/Editline/EditlineTest.cpp

Index: lldb/unittests/Editline/EditlineTest.cpp
===
--- lldb/unittests/Editline/EditlineTest.cpp
+++ lldb/unittests/Editline/EditlineTest.cpp
@@ -17,6 +17,7 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -28,7 +29,12 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringList.h"
 
+using namespace std::chrono_literals;
+
+#define ESCAPE "\x1b"
+
 using namespace lldb_private;
+using namespace testing;
 
 namespace {
 const size_t TIMEOUT_MILLIS = 5000;
@@ -80,6 +86,12 @@
 
   void ConsumeAllOutput();
 
+  const std::string GetEditlineOutput() {
+std::unique_lock lock(outputReadMutex);
+outputReadCv.wait(lock);
+return testOutputBuffer.str();
+  }
+
 private:
   bool IsInputComplete(lldb_private::Editline *editline,
lldb_private::StringList &lines);
@@ -91,6 +103,9 @@
   int _pty_secondary_fd;
 
   std::unique_ptr _el_secondary_file;
+  std::stringstream testOutputBuffer;
+  std::mutex outputReadMutex;
+  std::condition_variable outputReadCv;
 };
 
 EditlineAdapter::EditlineAdapter()
@@ -110,11 +125,21 @@
   EXPECT_THAT_ERROR(_pty.OpenSecondary(O_RDWR), llvm::Succeeded());
   _pty_secondary_fd = _pty.GetSecondaryFileDescriptor();
 
-  _el_secondary_file.reset(new FilePointer(fdopen(_pty_secondary_fd, "rw")));
+  _el_secondary_file.reset(new FilePointer(fdopen(_pty_secondary_fd, "w+")));
   EXPECT_FALSE(nullptr == *_el_secondary_file);
   if (*_el_secondary_file == nullptr)
 return;
 
+  // We have to set the output stream we pass to Editline as not using
+  // buffered I/O.  Otherwise we are missing editline's output when we
+  // close the stream in the keybinding test (i.e. the EOF comes
+  // before data previously written to the stream by editline).  This
+  // behavior isn't as I understand the spec becuse fclose should
+  // flush the stream, but my best guess is that it's some unexpected
+  // interaction with stream I/O and ptys.
+  EXPECT_EQ(setvbuf(*_el_secondary_file, nullptr, _IONBF, 0), 0)
+  << "Could not set editline output stream to use unbuffered I/O.";
+
   // Create an Editline instance.
   _editline_sp.reset(new lldb_private::Editline(
   "gtest editor", *_el_secondary_file, *_el_secondary_file,
@@ -207,7 +232,7 @@
 void EditlineAdapter::ConsumeAllOutput() {
   FilePointer output_file(fdopen(_pty_primary_fd, "r"));
 
-  int ch;
+  char ch;
   while ((ch = fgetc(output_file)) != EOF) {
 #if EDITLINE_TEST_DUMP_OUTPUT
 char display_str[] = {0, 0, 0};
@@ -229,15 +254,18 @@
   break;
 }
 printf(" 0x%02x (%03d) (%s)\n", ch, ch, display_str);
-// putc(ch, stdout);
 #endif
+if (ch == '\n') {
+  outputReadCv.notify_one();
+}
+testOutputBuffer << ch;
   }
 }
 
 class EditlineTestFixture : public ::testing::Test {
   SubsystemRAII subsystems;
   EditlineAdapter _el_adapter;
-  std::shared_ptr _sp_output_thread;
+  std::thread _sp_output_thread;
 
 public:
   static void SetUpTestCase() {
@@ -252,14 +280,13 @@
   return;
 
 // Dump output.
-_sp_output_thread =
-std::make_shared([&] { _el_adapter.ConsumeAllOutput(); });
+_sp_output_thread = std::thread([&] { _el_adapter.ConsumeAllOutput(); });
   }
 
   void TearDown() override {
 _el_adapter.CloseInput();
-if (_sp_output_thread)
-  _sp_output_thread->join();
+if (_sp_output_thread.joinable())
+  _sp_output_thread.join();
   }
 
   EditlineAdapter &GetEditlineAdapter() { return _el_adapter; }
@@ -309,4 +336,131 @@
   EXPECT_THAT(reported_lines, testing::ContainerEq(input_lines));
 }
 
+namespace lldb_private {
+
+// Parameter structure for parameterized tests.
+struct KeybindingTestValue {
+  // A number that is used to name the test, so test output can be
+  // mapped back to a specific input.
+  const std::string testNumber;
+  // Whether this keyboard shortcut is only bound in multi-line mode.
+  bool multilineOnly;
+  // The actual key sequence.
+  const std::string keySequence;
+  // The command the key sequence is supposed to execute.
+  const std::string commandName;
+  // This is initialized to the keySequence by default, but gtest has
+  // some errors when the test name as created by the overloaded
+  // operator<< has embedded newlines.  This is even true when we
+  // specify a custom test name function, as we do below when we
+  // instantiate the test case.  In cases where the keyboard shortcut
+  // has a newline or carriage return, this field in the struct can be
+  // set to some

[Lldb-commits] [lldb] 220854a - Fix buildbots after https://reviews.llvm.org/D115073.

2021-12-07 Thread Greg Clayton via lldb-commits

Author: Greg Clayton
Date: 2021-12-07T12:19:00-08:00
New Revision: 220854a47bdc0c281dbaafb54411ec65e76212b3

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

LOG: Fix buildbots after https://reviews.llvm.org/D115073.

Added: 


Modified: 
lldb/unittests/Utility/DataEncoderTest.cpp

Removed: 




diff  --git a/lldb/unittests/Utility/DataEncoderTest.cpp 
b/lldb/unittests/Utility/DataEncoderTest.cpp
index 4c3797e2371e0..5f5ff37bced1a 100644
--- a/lldb/unittests/Utility/DataEncoderTest.cpp
+++ b/lldb/unittests/Utility/DataEncoderTest.cpp
@@ -15,7 +15,7 @@ using namespace lldb_private;
 using namespace llvm;
 
 TEST(DataEncoderTest, PutU8) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
 
   uint32_t offset = 0;
@@ -166,7 +166,7 @@ TEST(DataEncoderTest, AppendCString) {
 }
 
 TEST(DataEncoderTest, PutU16Little) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   uint32_t offset = 0;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
@@ -190,7 +190,7 @@ TEST(DataEncoderTest, PutU16Little) {
 }
 
 TEST(DataEncoderTest, PutU16Big) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   uint32_t offset = 0;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderBig,
@@ -214,7 +214,7 @@ TEST(DataEncoderTest, PutU16Big) {
 }
 
 TEST(DataEncoderTest, PutU32Little) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
 
   uint32_t offset = 0;
@@ -233,7 +233,7 @@ TEST(DataEncoderTest, PutU32Little) {
 }
 
 TEST(DataEncoderTest, PutU32Big) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
 
   uint32_t offset = 0;
@@ -252,7 +252,7 @@ TEST(DataEncoderTest, PutU32Big) {
 }
 
 TEST(DataEncoderTest, PutU64Little) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   uint32_t offset = 0;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
@@ -267,7 +267,7 @@ TEST(DataEncoderTest, PutU64Little) {
 }
 
 TEST(DataEncoderTest, PutU64Big) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   uint32_t offset = 0;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderBig,
@@ -282,7 +282,7 @@ TEST(DataEncoderTest, PutU64Big) {
 }
 
 TEST(DataEncoderTest, PutUnsignedLittle) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   uint32_t offset = 0;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
@@ -312,7 +312,7 @@ TEST(DataEncoderTest, PutUnsignedLittle) {
 }
 
 TEST(DataEncoderTest, PutUnsignedBig) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   uint32_t offset = 0;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderBig,
@@ -343,7 +343,7 @@ TEST(DataEncoderTest, PutUnsignedBig) {
 }
 
 TEST(DataEncoderTest, PutData) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   char one_byte[] = {11};
   char two_bytes[] = {12, 13};
@@ -354,11 +354,11 @@ TEST(DataEncoderTest, PutData) {
   // Test putting zero bytes from a invalid array (NULL)
   offset = encoder.PutData(offset, nullptr, 0);
   ASSERT_EQ(offset, 0U);
-  ASSERT_EQ(encoder.GetData(), init);
+  ASSERT_EQ(encoder.GetData(), ArrayRef(init));
   // Test putting zero bytes from a valid array
   offset = encoder.PutData(offset, one_byte, 0);
   ASSERT_EQ(offset, 0U);
-  ASSERT_EQ(encoder.GetData(), init);
+  ASSERT_EQ(encoder.GetData(), ArrayRef(init));
   // Test putting one byte from a valid array
   offset = encoder.PutData(offset, one_byte, sizeof(one_byte));
   ASSERT_EQ(offset, 1U);
@@ -372,13 +372,13 @@ TEST(DataEncoderTest, PutData) {
 }
 
 TEST(DataEncoderTest, PutCString) {
-  const ArrayRef init = {1, 2, 3, 4, 5, 6, 7, 8};
+  const std::vector init = {1, 2, 3, 4, 5, 6, 7, 8};
   const uint32_t addr_size = 4;
   DataEncoder encoder(init.data(), init.size(), lldb::eByteOrderLittle,
   addr_size);
   // Test putting invalid string pointer
   ASSERT_EQ(encoder.PutCString(0, nullptr), UINT32_MAX);
-  ASSERT_EQ(encoder.GetData(), init);
+  ASSERT_EQ

[Lldb-commits] [PATCH] D115073: Modify DataEncoder to be able to encode data in an object owned buffer.

2021-12-07 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Another fix for buildbots. Don't use llvm::ArrayRef to refer to data, store in 
std::vector<>. In release builds this will fail.

commit 220854a47bdc0c281dbaafb54411ec65e76212b3 
 (HEAD -> 
main, origin/main, origin/HEAD)
Author: Greg Clayton 
Date:   Tue Dec 7 12:19:00 2021 -0800

  Fix buildbots after https://reviews.llvm.org/D115073.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115073

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


[Lldb-commits] [PATCH] D115277: [lldb] Workaround for type-like entities defined in a lexical block

2021-12-07 Thread Kristina Bessonova via Phabricator via lldb-commits
krisb created this revision.
krisb added reviewers: jingham, aprantl, JDevlieghere.
Herald added a subscriber: pengfei.
krisb requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

D113741  and D113743 
 makes types defined in a lexical (bracketed) 
block
correctly scoped within this block, which reveals a lack of support of
such a case in lldb.

Consider the following example:

* thread #1, name = 'a.out', stop reason = step over
frame #0: 0x0040111d a.out`foo(a=1) at test_lldb.cpp:5:12
   1  int foo(int a) {
   2  {
   3typedef int Int;
   4Int local = a;
-> 5return local;
   6  }
   7}
   8
(lldb) p local
error: expression failed to parse:
error: :45:31: no member named 'local' in namespace 
'$__lldb_local_vars'
using $__lldb_local_vars::local;
  ^
error: :1:1: use of undeclared identifier 'local'
local
  ^

lldb failed to find 'local' variable typed with a lexical block defined type 
(typedef Int).
The immediate cause of this issue is that clang failed to import this typedef:

  lldb   (x86_64) a.out: DWARFASTParserClang::ParseTypeFromDWARF (die = 
0x0070, decl_ctx = 0x2509cb0 (die 0x0055)) DW_TAG_typedef name = 'Int')
  lldb   (x86_64) a.out: SymbolFileDWARF::ResolveTypeUID (die = 0x0070) 
DW_TAG_typedef 'Int'
  lldb   (x86_64) a.out: SymbolFileDWARF::ResolveTypeUID (die = 0x0096) 
DW_TAG_base_type 'int'
  lldb   Compiler diagnostic:
  
  lldb   Couldn't import type: UnsupportedConstruct
  lldb   Couldn't copy a variable's type into the parser's AST context

The importing wasn't success because clang wasn't able to import
typedef's context, which is actually a BlockDecl (import for BlockDecl isn't 
implemented,
but this is still a question to me why DW_TAG_lexical_block is associated with 
a BlockDecl).

This workaround makes everything behaves as it was before D113741 
 / D113743 .
While looking for a context for a type-like entity scoped within a lexical 
block,
GetDeclContextDIEContainingDIE() will return a DeclContext of a parent 
subprogram,
instead of a BlockDecl.

Despite the patch doesn't fix the issue properly, looks like a step
forward to me as lldb at least may now properly display variables of
such types, which may appear in the DWARF generated by other compilers
(gcc as an example).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115277

Files:
  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
@@ -2616,9 +2616,27 @@
 case DW_TAG_structure_type:
 case DW_TAG_union_type:
 case DW_TAG_class_type:
-case DW_TAG_lexical_block:
 case DW_TAG_subprogram:
   return die;
+case DW_TAG_lexical_block: {
+  if (orig_die.Tag() != DW_TAG_structure_type &&
+  orig_die.Tag() != DW_TAG_union_type &&
+  orig_die.Tag() != DW_TAG_class_type &&
+  orig_die.Tag() != DW_TAG_enumeration_type &&
+  orig_die.Tag() != DW_TAG_typedef)
+return die;
+  // Make local type's context to be a subprogram.
+  // FIXME: DWARFASTParserClang doesn't properly support type-like
+  // entities scoped within a lexical block. This workaround makes
+  // it possible to correctly display variables that have such a type,
+  // but lldb still isn't able to handle properly handle more than one
+  // local type with the same name.
+  while (true) {
+die = die.GetParent();
+if (die.Tag() == DW_TAG_subprogram)
+  return die;
+  }
+}
 case DW_TAG_inlined_subroutine: {
   DWARFDIE abs_die = die.GetReferencedDIE(DW_AT_abstract_origin);
   if (abs_die) {


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2616,9 +2616,27 @@
 case DW_TAG_structure_type:
 case DW_TAG_union_type:
 case DW_TAG_class_type:
-case DW_TAG_lexical_block:
 case DW_TAG_subprogram:
   return die;
+case DW_TAG_lexical_block: {
+  if (orig_die.Tag() != DW_TAG_structure_type &&
+  orig_die.Tag() != DW_TAG_union_type &&
+  orig_die.Tag() != DW_TAG_class_type &&
+  orig_d

[Lldb-commits] [lldb] f758859 - Fix error reporting for "process load" and add a test for it.

2021-12-07 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2021-12-07T15:08:05-08:00
New Revision: f75885977cefe4d5ebbe51f85b353bb9989dd777

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

LOG: Fix error reporting for "process load" and add a test for it.

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

Added: 


Modified: 
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/test/API/functionalities/load_unload/TestLoadUnload.py
lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py

Removed: 




diff  --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp 
b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 719109c863e7a..1d989b268b74d 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -589,6 +589,8 @@ 
PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
   result_ptr->image_ptr = dlopen(name, RTLD_LAZY);
   if (result_ptr->image_ptr)
 result_ptr->error_str = nullptr;
+  else
+result_ptr->error_str = dlerror();
   return nullptr;
 }
 

diff  --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py 
b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index 9f4a99dc69de5..f3b715c4c35ed 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -240,6 +240,15 @@ def run_lldb_process_load_and_unload_commands(self):
 else:
 remoteDylibPath = localDylibPath
 
+# First make sure that we get some kind of error if process load fails.
+# We print some error even if the load fails, which isn't formalized.
+# The only plugin at present (Posix) that supports this says "unknown 
reasons".
+# If another plugin shows up, let's require it uses "unknown error" as 
well.
+non_existant_shlib = "/NoSuchDir/NoSuchSubdir/ReallyNo/NotAFile"
+self.expect("process load %s"%(non_existant_shlib), error=True, 
matching=False,
+patterns=["unknown reasons"])
+
+
 # Make sure that a_function does not exist at this point.
 self.expect(
 "image lookup -n a_function",

diff  --git 
a/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py 
b/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
index cfa55c4055a56..ca7808c212795 100644
--- a/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
+++ b/lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
@@ -65,7 +65,12 @@ def test_load_using_paths(self):
 # First try with no correct directories on the path, and make sure 
that doesn't blow up:
 token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
 self.assertEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Only looked on 
the provided path.")
-
+# Make sure we got some error back in this case.  Since we don't 
actually know what
+# the error will look like, let's look for the absence of "unknown 
reasons".
+error_str = error.description
+self.assertNotEqual(len(error_str), 0, "Got an empty error string")
+self.assertNotIn("unknown reasons", error_str, "Error string had 
unknown reasons")
+
 # Now add the correct dir to the paths list and try again:
 paths.AppendString(self.hidden_dir)
 token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
@@ -121,8 +126,6 @@ def test_load_using_paths(self):
 
 process.UnloadImage(token)
 
-
-
 # Finally, passing in an absolute path should work like the basename:
 # This should NOT work because we've taken hidden_dir off the paths:
 abs_spec = lldb.SBFileSpec(os.path.join(self.hidden_dir, 
self.lib_name))
@@ -137,5 +140,3 @@ def test_load_using_paths(self):
 
 self.assertNotEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Got a valid 
token")
 self.assertEqual(out_spec, lldb.SBFileSpec(self.hidden_lib), "Found 
the expected library")
-
-



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


[Lldb-commits] [PATCH] D115017: Fix error reporting for "process load" and add a test for it.

2021-12-07 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf75885977cef: Fix error reporting for "process 
load" and add a test for it. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115017

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py


Index: lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
===
--- lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
+++ lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
@@ -65,7 +65,12 @@
 # First try with no correct directories on the path, and make sure 
that doesn't blow up:
 token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
 self.assertEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Only looked on 
the provided path.")
-
+# Make sure we got some error back in this case.  Since we don't 
actually know what
+# the error will look like, let's look for the absence of "unknown 
reasons".
+error_str = error.description
+self.assertNotEqual(len(error_str), 0, "Got an empty error string")
+self.assertNotIn("unknown reasons", error_str, "Error string had 
unknown reasons")
+
 # Now add the correct dir to the paths list and try again:
 paths.AppendString(self.hidden_dir)
 token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
@@ -121,8 +126,6 @@
 
 process.UnloadImage(token)
 
-
-
 # Finally, passing in an absolute path should work like the basename:
 # This should NOT work because we've taken hidden_dir off the paths:
 abs_spec = lldb.SBFileSpec(os.path.join(self.hidden_dir, 
self.lib_name))
@@ -137,5 +140,3 @@
 
 self.assertNotEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Got a valid 
token")
 self.assertEqual(out_spec, lldb.SBFileSpec(self.hidden_lib), "Found 
the expected library")
-
-
Index: lldb/test/API/functionalities/load_unload/TestLoadUnload.py
===
--- lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -240,6 +240,15 @@
 else:
 remoteDylibPath = localDylibPath
 
+# First make sure that we get some kind of error if process load fails.
+# We print some error even if the load fails, which isn't formalized.
+# The only plugin at present (Posix) that supports this says "unknown 
reasons".
+# If another plugin shows up, let's require it uses "unknown error" as 
well.
+non_existant_shlib = "/NoSuchDir/NoSuchSubdir/ReallyNo/NotAFile"
+self.expect("process load %s"%(non_existant_shlib), error=True, 
matching=False,
+patterns=["unknown reasons"])
+
+
 # Make sure that a_function does not exist at this point.
 self.expect(
 "image lookup -n a_function",
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -589,6 +589,8 @@
   result_ptr->image_ptr = dlopen(name, RTLD_LAZY);
   if (result_ptr->image_ptr)
 result_ptr->error_str = nullptr;
+  else
+result_ptr->error_str = dlerror();
   return nullptr;
 }
 


Index: lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
===
--- lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
+++ lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
@@ -65,7 +65,12 @@
 # First try with no correct directories on the path, and make sure that doesn't blow up:
 token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
 self.assertEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Only looked on the provided path.")
-
+# Make sure we got some error back in this case.  Since we don't actually know what
+# the error will look like, let's look for the absence of "unknown reasons".
+error_str = error.description
+self.assertNotEqual(len(error_str), 0, "Got an empty error string")
+self.assertNotIn("unknown reasons", error_str, "Error string had unknown reasons")
+
 # Now add the correct dir to the paths list and try again:
 paths.AppendString(self.hidden_dir)
 token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
@@ -121,8 +126,6 @@
 
 process.UnloadImage(token

[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

2021-12-07 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 392590.
JDevlieghere added a comment.

Rename Basic -> Version


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

https://reviews.llvm.org/D115211

Files:
  lldb/include/lldb/Version/Version.h
  lldb/include/lldb/Version/Version.inc.in
  lldb/include/lldb/lldb-private.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/CMakeLists.txt
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectVersion.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Version/CMakeLists.txt
  lldb/source/Version/Version.cpp
  lldb/tools/lldb-server/CMakeLists.txt
  lldb/tools/lldb-server/lldb-server.cpp
  lldb/tools/lldb-test/CMakeLists.txt

Index: lldb/tools/lldb-test/CMakeLists.txt
===
--- lldb/tools/lldb-test/CMakeLists.txt
+++ lldb/tools/lldb-test/CMakeLists.txt
@@ -6,7 +6,6 @@
   SystemInitializerTest.cpp
 
   LINK_LIBS
-lldbBase
 lldbBreakpoint
 lldbCore
 lldbDataFormatters
@@ -17,6 +16,7 @@
 lldbSymbol
 lldbTarget
 lldbUtility
+lldbVersion
 ${LLDB_ALL_PLUGINS}
 
   LINK_COMPONENTS
Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -8,7 +8,7 @@
 
 #include "SystemInitializerLLGS.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
-#include "lldb/lldb-private.h"
+#include "lldb/Version/Version.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
Index: lldb/tools/lldb-server/CMakeLists.txt
===
--- lldb/tools/lldb-server/CMakeLists.txt
+++ lldb/tools/lldb-server/CMakeLists.txt
@@ -46,9 +46,9 @@
 SystemInitializerLLGS.cpp
 
 LINK_LIBS
-  lldbBase
   lldbHost
   lldbInitialization
+  lldbVersion
   ${LLDB_PLUGINS}
   lldbPluginInstructionARM
   lldbPluginInstructionMIPS
Index: lldb/source/Version/Version.cpp
===
--- /dev/null
+++ lldb/source/Version/Version.cpp
@@ -0,0 +1,73 @@
+//===-- Version.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 "lldb/Version/Version.h"
+#include "VCSVersion.inc"
+#include "lldb/Version/Version.inc"
+#include "clang/Basic/Version.h"
+
+static const char *GetLLDBVersion() {
+#ifdef LLDB_FULL_VERSION_STRING
+  return LLDB_FULL_VERSION_STRING;
+#else
+  return "lldb version " CLANG_VERSION_STRING;
+#endif
+}
+
+static const char *GetLLDBRevision() {
+#ifdef LLDB_REVISION
+  return LLDB_REVISION;
+#else
+  return nullptr;
+#endif
+}
+
+static const char *GetLLDBRepository() {
+#ifdef LLDB_REPOSITORY
+  return LLDB_REPOSITORY;
+#else
+  return nullptr;
+#endif
+}
+
+const char *lldb_private::GetVersion() {
+  static std::string g_version_str;
+
+  if (g_version_str.empty()) {
+const char *lldb_version = GetLLDBVersion();
+const char *lldb_repo = GetLLDBRepository();
+const char *lldb_rev = GetLLDBRevision();
+g_version_str += lldb_version;
+if (lldb_repo || lldb_rev) {
+  g_version_str += " (";
+  if (lldb_repo)
+g_version_str += lldb_repo;
+  if (lldb_repo && lldb_rev)
+g_version_str += " ";
+  if (lldb_rev) {
+g_version_str += "revision ";
+g_version_str += lldb_rev;
+  }
+  g_version_str += ")";
+}
+
+std::string clang_rev(clang::getClangRevision());
+if (clang_rev.length() > 0) {
+  g_version_str += "\n  clang revision ";
+  g_version_str += clang_rev;
+}
+
+std::string llvm_rev(clang::getLLVMRevision());
+if (llvm_rev.length() > 0) {
+  g_version_str += "\n  llvm revision ";
+  g_version_str += llvm_rev;
+}
+  }
+
+  return g_version_str.c_str();
+}
Index: lldb/source/Version/CMakeLists.txt
===
--- /dev/null
+++ lldb/source/Version/CMakeLists.txt
@@ -0,0 +1,42 @@
+if(LLDB_VERSION_STRING)
+  set(LLDB_FULL_VERSION_STRING LLDB_VERSION_STRING)
+endif()
+
+# Configure the VCSVersion.inc file.
+set(vcs_version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
+
+find_first_existing_vc_file("${LLDB_SOURCE_DIR}" lldb_vc)
+
+if(lldb_vc AND LLVM_APPEND_VC_REV)
+  set(lldb_source_dir ${LLDB_SOURCE_DIR})
+endif()
+
+add_custom_command(OUTPUT "${vcs_version_inc}"
+  DEPENDS "${lldb_vc}" "${generate_vcs_version_script}"
+  COMMAND ${C

[Lldb-commits] [PATCH] D115308: [LLDB][DWARF] Fix duplicate TypeSP in type list

2021-12-07 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu created this revision.
zequanwu added reviewers: labath, shafik, teemperor.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115308

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp


Index: lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp
@@ -8,6 +8,7 @@
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=type %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+// RUN: lldb-test symbols --find=none %t | FileCheck --check-prefix=TYPES %s
 //
 // RUN: %clang %s -g -c -o %t --target=x86_64-apple-macosx
 // RUN: lldb-test symbols --name=foo --find=type %t | \
@@ -16,6 +17,7 @@
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=type %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+// RUN: lldb-test symbols --find=none %t | FileCheck --check-prefix=TYPES %s
 
 // RUN: %clang %s -c -o %t.o --target=x86_64-pc-linux -gdwarf-5 -gpubnames
 // RUN: ld.lld %t.o -o %t
@@ -26,6 +28,7 @@
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=type %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+// RUN: lldb-test symbols --find=none %t | FileCheck --check-prefix=TYPES %s
 
 // NAMES: Name: .debug_names
 
@@ -34,25 +37,32 @@
 // CONTEXT: Found 1 types:
 struct foo { };
 // NAME-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-1]]
+// TYPES: name = "foo", size = 1, decl = find-basic-type.cpp:[[@LINE-2]]
 
 namespace bar {
 int context;
 struct foo {};
 // NAME-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-1]]
 // CONTEXT-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-2]]
+// TYPES: name = "foo", size = 1, decl = find-basic-type.cpp:[[@LINE-3]]
 namespace baz {
 struct foo {};
 // NAME-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-1]]
+// TYPES: name = "foo", size = 1, decl = find-basic-type.cpp:[[@LINE-2]]
 }
 }
 
 struct sbar {
+  // TYPES: name = "sbar", size = 1, decl = find-basic-type.cpp:[[@LINE-1]]
   struct foo {};
-// NAME-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-1]]
+  // NAME-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-1]]
+  // TYPES: name = "foo", size = 1, decl = find-basic-type.cpp:[[@LINE-2]]
 };
 
 struct foobar {};
+// TYPES: name = "foobar", size = 1, decl = find-basic-type.cpp:[[@LINE-1]]
 
 struct Foo {};
+// TYPES: name = "Foo", size = 1, decl = find-basic-type.cpp:[[@LINE-1]]
 
 extern "C" void _start(foo, bar::foo, bar::baz::foo, sbar::foo, foobar, Foo) {}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1530,7 +1530,6 @@
 return type_sp;
 
   SymbolFileDWARF *dwarf = die.GetDWARF();
-  TypeList &type_list = dwarf->GetTypeList();
   DWARFDIE sc_parent_die = SymbolFileDWARF::GetParentSymbolContextDIE(die);
   dw_tag_t sc_parent_tag = sc_parent_die.Tag();
 
@@ -1550,10 +1549,6 @@
   if (symbol_context_scope != nullptr)
 type_sp->SetSymbolContextScope(symbol_context_scope);
 
-  // We are ready to put this type into the uniqued list up at the module
-  // level.
-  type_list.Insert(type_sp);
-
   dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
   return type_sp;
 }


Index: lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp
@@ -8,6 +8,7 @@
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=type %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+// RUN: lldb-test symbols --find=none %t | FileCheck --check-prefix=TYPES %s
 //
 // RUN: %clang %s -g -c -o %t --target=x86_64-apple-macosx
 // RUN: lldb-test symbols --name=foo --find=type %t | \
@@ -16,6 +17,7 @@
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=type %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+// RUN: lldb-test symbols --find=none %t | FileCheck --check-prefix=TYPES %s
 
 // RUN: %clang %s -c -o %t.o --target=x86_64-pc-linux -gdwarf-5 -gpubnames
 // RUN: ld.lld %t.o -o %t
@@ -26,6 +28,7 @@
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=type %t | \
 // RUN:   FileCheck --check-prefix=EMPTY %s
+// RUN: lldb-test symbols --find

[Lldb-commits] [PATCH] D115277: [lldb] Workaround for type-like entities defined in a lexical block

2021-12-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

I worry this makes the case where a function has two definitions of the same 
type in one function a more confusing error than the "can't find type" error 
you would get without this patch.  Is this of sufficient urgency that we can't 
take the time to fix it correctly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115277

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


[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial location at function start

2021-12-07 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It can happen that a line entry reports that some source code is located
at line 0. In DWARF, line 0 is a special location which indicates that
code has no 1-1 mapping with source.

When stopping in one of those artificial locations, lldb doesn't know which
line to display and shows the beginning of the file instead.

This patch mitigates this behaviour by checking if the current symbol context
of the line entry has a matching function, in which case, it slides the
source listing to the start of that function.

This patch also shows the user a warning explaining why lldb couldn't
show sources at that location.

rdar://83118425

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115313

Files:
  lldb/source/Target/StackFrame.cpp
  lldb/test/API/source-manager/TestSourceManager.py
  lldb/test/API/source-manager/artificial_location.c


Index: lldb/test/API/source-manager/artificial_location.c
===
--- /dev/null
+++ lldb/test/API/source-manager/artificial_location.c
@@ -0,0 +1,8 @@
+int foo() {
+  return 42;
+}
+
+int main() {
+#line 0
+  return foo();
+}
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -199,7 +199,6 @@
 SOURCE_DISPLAYED_CORRECTLY,
 substrs=['Hello world'])
 
-
 # The '-b' option shows the line table locations from the debug 
information
 # that indicates valid places to set source level breakpoints.
 
@@ -265,3 +264,22 @@
 substrs=['stopped',
  'main-copy.c:%d' % self.line,
  'stop reason = breakpoint'])
+
+def test_artificial_source_location(self):
+src_file = 'artificial_location.c'
+d = {'C_SOURCES': src_file }
+self.build(dictionary=d)
+
+lldbutil.run_to_source_breakpoint(
+self, 'main',
+lldb.SBFileSpec(src_file, False))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("run", RUN_SUCCEEDED,
+substrs=['stop reason = breakpoint', '%s:%d' % 
(src_file,0),
+ 'Warning: the current PC is an artificial',
+ 'location unassociated with a particular line in 
',
+ src_file])
+
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -1883,14 +1883,26 @@
   if (m_sc.comp_unit && m_sc.line_entry.IsValid()) {
 have_debuginfo = true;
 if (source_lines_before > 0 || source_lines_after > 0) {
+  uint32_t start_line = m_sc.line_entry.line;
+  if (!start_line && m_sc.function) {
+FileSpec source_file;
+m_sc.function->GetStartLineSourceInfo(source_file, start_line);
+  }
+  
   size_t num_lines =
   target->GetSourceManager().DisplaySourceLinesWithLineNumbers(
-  m_sc.line_entry.file, m_sc.line_entry.line,
+  m_sc.line_entry.file, start_line,
   m_sc.line_entry.column, source_lines_before,
   source_lines_after, "->", &strm);
   if (num_lines != 0)
 have_source = true;
-  // TODO: Give here a one time warning if source file is missing.
+  if (!num_lines || !m_sc.line_entry.line ||
+  m_sc.line_entry.line == LLDB_INVALID_LINE_NUMBER) {
+strm.Printf("Warning: the current PC is an artificial location "
+"unassociated with a particular line in %s.",
+m_sc.line_entry.file.GetFilename().GetCString());
+strm.EOL();
+  }
 }
   }
   switch (disasm_display) {


Index: lldb/test/API/source-manager/artificial_location.c
===
--- /dev/null
+++ lldb/test/API/source-manager/artificial_location.c
@@ -0,0 +1,8 @@
+int foo() {
+  return 42;
+}
+
+int main() {
+#line 0
+  return foo();
+}
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -199,7 +199,6 @@
 SOURCE_DISPLAYED_CORRECTLY,
 substrs=['Hello world'])
 
-
 # The '-b' option shows the line table locations from the debug information
 # that indicates valid pla

[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial location at function start

2021-12-07 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 392622.
mib added reviewers: aprantl, jingham.
Herald added a subscriber: JDevlieghere.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115313

Files:
  lldb/source/Target/StackFrame.cpp
  lldb/test/API/source-manager/TestSourceManager.py
  lldb/test/API/source-manager/artificial_location.c


Index: lldb/test/API/source-manager/artificial_location.c
===
--- /dev/null
+++ lldb/test/API/source-manager/artificial_location.c
@@ -0,0 +1,6 @@
+int foo() { return 42; }
+
+int main() {
+#line 0
+  return foo();
+}
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -199,7 +199,6 @@
 SOURCE_DISPLAYED_CORRECTLY,
 substrs=['Hello world'])
 
-
 # The '-b' option shows the line table locations from the debug 
information
 # that indicates valid places to set source level breakpoints.
 
@@ -265,3 +264,19 @@
 substrs=['stopped',
  'main-copy.c:%d' % self.line,
  'stop reason = breakpoint'])
+
+def test_artificial_source_location(self):
+src_file = 'artificial_location.c'
+d = {'C_SOURCES': src_file }
+self.build(dictionary=d)
+
+lldbutil.run_to_source_breakpoint(
+self, 'main',
+lldb.SBFileSpec(src_file, False))
+
+self.expect("run", RUN_SUCCEEDED,
+substrs=['stop reason = breakpoint', '%s:%d' % 
(src_file,0),
+ 'Warning: the current PC is an artificial',
+ 'location unassociated with a particular line in 
',
+ src_file])
+
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -1883,14 +1883,25 @@
   if (m_sc.comp_unit && m_sc.line_entry.IsValid()) {
 have_debuginfo = true;
 if (source_lines_before > 0 || source_lines_after > 0) {
+  uint32_t start_line = m_sc.line_entry.line;
+  if (!start_line && m_sc.function) {
+FileSpec source_file;
+m_sc.function->GetStartLineSourceInfo(source_file, start_line);
+  }
+
   size_t num_lines =
   target->GetSourceManager().DisplaySourceLinesWithLineNumbers(
-  m_sc.line_entry.file, m_sc.line_entry.line,
-  m_sc.line_entry.column, source_lines_before,
-  source_lines_after, "->", &strm);
+  m_sc.line_entry.file, start_line, m_sc.line_entry.column,
+  source_lines_before, source_lines_after, "->", &strm);
   if (num_lines != 0)
 have_source = true;
-  // TODO: Give here a one time warning if source file is missing.
+  if (!num_lines || !m_sc.line_entry.line ||
+  m_sc.line_entry.line == LLDB_INVALID_LINE_NUMBER) {
+strm.Printf("Warning: the current PC is an artificial location "
+"unassociated with a particular line in %s.",
+m_sc.line_entry.file.GetFilename().GetCString());
+strm.EOL();
+  }
 }
   }
   switch (disasm_display) {


Index: lldb/test/API/source-manager/artificial_location.c
===
--- /dev/null
+++ lldb/test/API/source-manager/artificial_location.c
@@ -0,0 +1,6 @@
+int foo() { return 42; }
+
+int main() {
+#line 0
+  return foo();
+}
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -199,7 +199,6 @@
 SOURCE_DISPLAYED_CORRECTLY,
 substrs=['Hello world'])
 
-
 # The '-b' option shows the line table locations from the debug information
 # that indicates valid places to set source level breakpoints.
 
@@ -265,3 +264,19 @@
 substrs=['stopped',
  'main-copy.c:%d' % self.line,
  'stop reason = breakpoint'])
+
+def test_artificial_source_location(self):
+src_file = 'artificial_location.c'
+d = {'C_SOURCES': src_file }
+self.build(dictionary=d)
+
+lldbutil.run_to_source_breakpoint(
+self, 'main',
+lldb.SBFileSpec(src_file, False))
+
+self.expect("run", RUN_SUCCEEDED,
+substrs=['stop reason = breakpoint', '%s:%d' % (src_file,0),
+   

[Lldb-commits] [PATCH] D115313: [lldb/Target] Slide source display for artificial location at function start

2021-12-07 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

Side note: The src_file is not to be trusted/used either - once line 0 is 
specified, nothing else in that line entry is valid. LLVM lets the previous 
line entries file persist because this reduces encoding size (by not having to 
switch all the fields in the line table - only the line number - back and forth 
over a line number 0 area). eg: previous entry in the line table might be from 
a #include of some code (as in clang/llvm's use of .def files, for instance) 
into a function, or from some inlining above the line 0 region, etc.

So maybe "artificial location in function " might be suitable? (the actual 
code at line 0 might still be from some inlining (LLVM does try to scope it - 
so the instruction should have the nearest common scope (in terms of lexical 
scopes or inlined functions) so if A has B inlined, B has C and D inlined into 
it and some code is commoned between C and D, it should be attributed to the 
inlined region of B - but if it's hoisted out of a basic block, I don't think 
we can properly attribute it to any scope, and so we'd have to attribute it to 
A in the scope DIE information (in all these cases it'd still have line 0, 
though).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115313

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


[Lldb-commits] [PATCH] D115308: [LLDB][DWARF] Fix duplicate TypeSP in type list

2021-12-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for fixing this. I assume that the type is already being added to the 
type list somewhere else -- it'd be nice to say, for future archaeologists' 
sake, where that actually happens.

But my main question is really about the test.




Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-type.cpp:40
 // NAME-DAG: name = "foo", {{.*}} decl = find-basic-type.cpp:[[@LINE-1]]
+// TYPES: name = "foo", size = 1, decl = find-basic-type.cpp:[[@LINE-2]]
 

Does this actually check that the type is not emitted more than once?

This is the reason why the other checks have the "Found  types" assertion 
above. We currently don't have such output from --find=none, but feel free to 
add something.

It might also be better to make this a separate test, as the output also 
includes things like `int` -- it would start to get messy if you included all 
of that here, and this test was about testing something different anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115308

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