[Lldb-commits] [PATCH] D69371: [ARM64] Cleanup and speedup NativeRegisterContextLinux_arm64

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 232501.
omjavaid added a comment.

Hi

I have updated this patch after incorporating previous suggestions like moving 
InvalidateAllRegisters declaration to NativeRegisterContextLinux and also 
relocated calls to InvalidateAllRegisters from NativeProcessLinux to 
NativeThreadLinux. We now call InvalidateAllRegisters just before we resume or 
single step via ptrace. Other than that on any register write register will be 
be invalidated by WriteGPR and WriteFPR as was happening previously. We dont 
really need a invalidate on detach because thread wont exist after that so I 
have removed it InvalidateAllRegisters which was being called at detach.

Also testing for GPR or FPR validity now happens outside WriteGPR and WriteFPR.

Thoughts?


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

https://reviews.llvm.org/D69371

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp

Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -241,6 +241,9 @@
   if (signo != LLDB_INVALID_SIGNAL_NUMBER)
 data = signo;
 
+  // Before thread resumes, clear any cached register data structures
+  GetRegisterContext().InvalidateAllRegisters();
+
   return NativeProcessLinux::PtraceWrapper(PTRACE_CONT, GetID(), nullptr,
reinterpret_cast(data));
 }
@@ -262,6 +265,9 @@
   if (signo != LLDB_INVALID_SIGNAL_NUMBER)
 data = signo;
 
+  // Before thread resumes, clear any cached register data structures
+  GetRegisterContext().InvalidateAllRegisters();
+
   // If hardware single-stepping is not supported, we just do a continue. The
   // breakpoint on the next instruction has been setup in
   // NativeProcessLinux::Resume.
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -40,6 +40,8 @@
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP _sp) override;
 
+  void InvalidateAllRegisters() override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
@@ -77,11 +79,6 @@
   enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
 
 protected:
-  Status DoReadRegisterValue(uint32_t offset, const char *reg_name,
- uint32_t size, RegisterValue ) override;
-
-  Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
-  const RegisterValue ) override;
 
   Status ReadGPR() override;
 
@@ -125,8 +122,17 @@
 uint32_t fpcr;
   };
 
-  uint64_t m_gpr_arm64[k_num_gpr_registers_arm64]; // 64-bit general purpose
-   // registers.
+  struct GPR {
+uint64_t x[31];
+uint64_t sp;
+uint64_t pc;
+uint64_t pstate;
+  };
+
+  bool m_gpr_is_valid;
+  bool m_fpu_is_valid;
+
+  GPR m_gpr_arm64; // 64-bit general purpose registers.
   RegInfo m_reg_info;
   FPU m_fpr; // floating-point registers including extended register sets.
 
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -152,6 +152,9 @@
   m_max_hwp_supported = 16;
   m_max_hbp_supported = 16;
   m_refresh_hwdebug_info = true;
+
+  m_gpr_is_valid = false;
+  m_fpu_is_valid = false;
 }
 
 uint32_t NativeRegisterContextLinux_arm64::GetRegisterSetCount() const {
@@ -185,41 +188,39 @@
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 
-  if (IsFPR(reg)) {
-error = ReadFPR();
-if (error.Fail())
-  return error;
-  } else {
-uint32_t full_reg = reg;
-bool is_subreg = reg_info->invalidate_regs &&
- (reg_info->invalidate_regs[0] != LLDB_INVALID_REGNUM);
+  if (reg == LLDB_INVALID_REGNUM)
+return Status("no lldb regnum for %s", reg_info && reg_info->name
+   ? reg_info->name
+   : "");
+
+  uint8_t *src;
+  uint32_t offset;
 
-if (is_subreg) {
-  // Read the full aligned 64-bit register.
-  full_reg = reg_info->invalidate_regs[0];
+  if (IsGPR(reg)) {
+if (!m_gpr_is_valid) {
+  error = ReadGPR();
+  if (error.Fail())
+return error;

[Lldb-commits] [lldb] 51ce067 - [lldb] NFC: less nesting in SearchFilter.cpp

[Konrad Kleine via lldb-commits]

Author: Konrad Kleine
Date: 2019-12-06T08:38:33+01:00
New Revision: 51ce067a442ee6381527b12d113f51906b0245a8

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

LOG: [lldb] NFC: less nesting in SearchFilter.cpp

I was working on SearchFilter.cpp and felt it a bit too complex in some cases 
in terms of nesting and logic flow.

Reviewers: teemperor, JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Core/SearchFilter.cpp

Removed: 




diff  --git a/lldb/source/Core/SearchFilter.cpp 
b/lldb/source/Core/SearchFilter.cpp
index 077aa8967425..9902166be522 100644
--- a/lldb/source/Core/SearchFilter.cpp
+++ b/lldb/source/Core/SearchFilter.cpp
@@ -208,10 +208,12 @@ void SearchFilter::Search(Searcher ) {
 return;
   empty_sc.target_sp = m_target_sp;
 
-  if (searcher.GetDepth() == lldb::eSearchDepthTarget)
+  if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
 searcher.SearchCallback(*this, empty_sc, nullptr);
-  else
-DoModuleIteration(empty_sc, searcher);
+return;
+  }
+  
+  DoModuleIteration(empty_sc, searcher);
 }
 
 void SearchFilter::SearchInModuleList(Searcher , ModuleList ) 
{
@@ -221,20 +223,20 @@ void SearchFilter::SearchInModuleList(Searcher , 
ModuleList ) {
 return;
   empty_sc.target_sp = m_target_sp;
 
-  if (searcher.GetDepth() == lldb::eSearchDepthTarget)
+  if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
 searcher.SearchCallback(*this, empty_sc, nullptr);
-  else {
-std::lock_guard guard(modules.GetMutex());
-const size_t numModules = modules.GetSize();
-
-for (size_t i = 0; i < numModules; i++) {
-  ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
-  if (ModulePasses(module_sp)) {
-if (DoModuleIteration(module_sp, searcher) ==
-Searcher::eCallbackReturnStop)
-  return;
-  }
-}
+return;
+  }
+
+  std::lock_guard guard(modules.GetMutex());
+  const size_t numModules = modules.GetSize();
+
+  for (size_t i = 0; i < numModules; i++) {
+ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
+if (!ModulePasses(module_sp))
+  continue;
+if (DoModuleIteration(module_sp, searcher) == 
Searcher::eCallbackReturnStop)
+  return;
   }
 }
 
@@ -248,45 +250,47 @@ SearchFilter::DoModuleIteration(const lldb::ModuleSP 
_sp,
 Searcher::CallbackReturn
 SearchFilter::DoModuleIteration(const SymbolContext ,
 Searcher ) {
-  if (searcher.GetDepth() >= lldb::eSearchDepthModule) {
-if (context.module_sp) {
-  if (searcher.GetDepth() == lldb::eSearchDepthModule) {
-SymbolContext matchingContext(context.module_sp.get());
-searcher.SearchCallback(*this, matchingContext, nullptr);
-  } else {
-return DoCUIteration(context.module_sp, context, searcher);
-  }
+  if (searcher.GetDepth() < lldb::eSearchDepthModule)
+return Searcher::eCallbackReturnContinue;
+
+  if (context.module_sp) {
+if (searcher.GetDepth() != lldb::eSearchDepthModule)
+  return DoCUIteration(context.module_sp, context, searcher);
+
+SymbolContext matchingContext(context.module_sp.get());
+searcher.SearchCallback(*this, matchingContext, nullptr);
+return Searcher::eCallbackReturnContinue;
+  }
+
+  const ModuleList _images = m_target_sp->GetImages();
+  std::lock_guard guard(target_images.GetMutex());
+
+  size_t n_modules = target_images.GetSize();
+  for (size_t i = 0; i < n_modules; i++) {
+// If this is the last level supplied, then call the callback directly,
+// otherwise descend.
+ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked(i));
+if (!ModulePasses(module_sp))
+  continue;
+
+if (searcher.GetDepth() == lldb::eSearchDepthModule) {
+  SymbolContext matchingContext(m_target_sp, module_sp);
+
+  Searcher::CallbackReturn shouldContinue =
+  searcher.SearchCallback(*this, matchingContext, nullptr);
+  if (shouldContinue == Searcher::eCallbackReturnStop ||
+  shouldContinue == Searcher::eCallbackReturnPop)
+return shouldContinue;
 } else {
-  const ModuleList _images = m_target_sp->GetImages();
-  std::lock_guard guard(target_images.GetMutex());
-
-  size_t n_modules = target_images.GetSize();
-  for (size_t i = 0; i < n_modules; i++) {
-// If this is the last level supplied, then call the callback directly,
-// otherwise descend.
-ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked(i));
-if (!ModulePasses(module_sp))
-  continue;
-
-if (searcher.GetDepth() == lldb::eSearchDepthModule) {
-  SymbolContext matchingContext(m_target_sp, module_sp);
-
-  

[Lldb-commits] [PATCH] D71022: [lldb] NFC: less nesting in SearchFilter.cpp

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk requested review of this revision.
kwk marked 2 inline comments as done.
kwk added a comment.

Addressed all review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71022



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

[Lldb-commits] [PATCH] D71022: [lldb] NFC: less nesting in SearchFilter.cpp

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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

- Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71022

Files:
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -208,10 +208,12 @@
 return;
   empty_sc.target_sp = m_target_sp;
 
-  if (searcher.GetDepth() == lldb::eSearchDepthTarget)
+  if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
 searcher.SearchCallback(*this, empty_sc, nullptr);
-  else
-DoModuleIteration(empty_sc, searcher);
+return;
+  }
+  
+  DoModuleIteration(empty_sc, searcher);
 }
 
 void SearchFilter::SearchInModuleList(Searcher , ModuleList ) {
@@ -221,20 +223,20 @@
 return;
   empty_sc.target_sp = m_target_sp;
 
-  if (searcher.GetDepth() == lldb::eSearchDepthTarget)
+  if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
 searcher.SearchCallback(*this, empty_sc, nullptr);
-  else {
-std::lock_guard guard(modules.GetMutex());
-const size_t numModules = modules.GetSize();
-
-for (size_t i = 0; i < numModules; i++) {
-  ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
-  if (ModulePasses(module_sp)) {
-if (DoModuleIteration(module_sp, searcher) ==
-Searcher::eCallbackReturnStop)
-  return;
-  }
-}
+return;
+  }
+
+  std::lock_guard guard(modules.GetMutex());
+  const size_t numModules = modules.GetSize();
+
+  for (size_t i = 0; i < numModules; i++) {
+ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
+if (!ModulePasses(module_sp))
+  continue;
+if (DoModuleIteration(module_sp, searcher) == Searcher::eCallbackReturnStop)
+  return;
   }
 }
 
@@ -248,45 +250,47 @@
 Searcher::CallbackReturn
 SearchFilter::DoModuleIteration(const SymbolContext ,
 Searcher ) {
-  if (searcher.GetDepth() >= lldb::eSearchDepthModule) {
-if (context.module_sp) {
-  if (searcher.GetDepth() == lldb::eSearchDepthModule) {
-SymbolContext matchingContext(context.module_sp.get());
-searcher.SearchCallback(*this, matchingContext, nullptr);
-  } else {
-return DoCUIteration(context.module_sp, context, searcher);
-  }
+  if (searcher.GetDepth() < lldb::eSearchDepthModule)
+return Searcher::eCallbackReturnContinue;
+
+  if (context.module_sp) {
+if (searcher.GetDepth() != lldb::eSearchDepthModule)
+  return DoCUIteration(context.module_sp, context, searcher);
+
+SymbolContext matchingContext(context.module_sp.get());
+searcher.SearchCallback(*this, matchingContext, nullptr);
+return Searcher::eCallbackReturnContinue;
+  }
+
+  const ModuleList _images = m_target_sp->GetImages();
+  std::lock_guard guard(target_images.GetMutex());
+
+  size_t n_modules = target_images.GetSize();
+  for (size_t i = 0; i < n_modules; i++) {
+// If this is the last level supplied, then call the callback directly,
+// otherwise descend.
+ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked(i));
+if (!ModulePasses(module_sp))
+  continue;
+
+if (searcher.GetDepth() == lldb::eSearchDepthModule) {
+  SymbolContext matchingContext(m_target_sp, module_sp);
+
+  Searcher::CallbackReturn shouldContinue =
+  searcher.SearchCallback(*this, matchingContext, nullptr);
+  if (shouldContinue == Searcher::eCallbackReturnStop ||
+  shouldContinue == Searcher::eCallbackReturnPop)
+return shouldContinue;
 } else {
-  const ModuleList _images = m_target_sp->GetImages();
-  std::lock_guard guard(target_images.GetMutex());
-
-  size_t n_modules = target_images.GetSize();
-  for (size_t i = 0; i < n_modules; i++) {
-// If this is the last level supplied, then call the callback directly,
-// otherwise descend.
-ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked(i));
-if (!ModulePasses(module_sp))
-  continue;
-
-if (searcher.GetDepth() == lldb::eSearchDepthModule) {
-  SymbolContext matchingContext(m_target_sp, module_sp);
-
-  Searcher::CallbackReturn shouldContinue =
-  searcher.SearchCallback(*this, matchingContext, nullptr);
-  if (shouldContinue == Searcher::eCallbackReturnStop ||
-  shouldContinue == Searcher::eCallbackReturnPop)
-return shouldContinue;
-} else {
-  Searcher::CallbackReturn shouldContinue =
-  DoCUIteration(module_sp, context, searcher);
-  if (shouldContinue == Searcher::eCallbackReturnStop)
-return shouldContinue;
-  else if (shouldContinue == Searcher::eCallbackReturnPop)
-continue;
-}
-  }
+  Searcher::CallbackReturn shouldContinue =
+  

[Lldb-commits] [PATCH] D71105: [lldb/Reproducers] Support multiple GDB remotes

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

This patch is still missing a test and could use another pass over the 
`PacketRecorder` and `ProcessGDBRemoteProvider` to eliminate code duplication 
with `DataRecorder` and the `CommandProvider` on which they're based 
respectively. I wasn't able to get around to that today but figured I'd upload 
the patch anyway to give you a change to look at the approach.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71105



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

[Lldb-commits] [PATCH] D71105: [lldb/Reproducers] Support multiple GDB remotes

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

When running the test suite with always capture on, a handful of tests are 
failing because they have multiple targets and therefore multiple GDB remote 
connections. The current reproducer infrastructure is capable of dealing with 
that.

This patch reworks the GDB remote provider to support multiple GDB remote 
connections, similar to how the reproducers support shadowing multiple command 
interpreter inputs. The provider now keeps a list of packet recorders which 
deal with a single GDB remote connection. During replay we rely on the order of 
creation to match the number of packets to the GDB remote connection.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71105

Files:
  lldb/include/lldb/Utility/GDBRemote.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Utility/GDBRemote.cpp
  lldb/source/Utility/Reproducer.cpp

Index: lldb/source/Utility/Reproducer.cpp
===
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/GDBRemote.h"
 #include "lldb/Utility/LLDBAssert.h"
 
 #include "llvm/Support/FileSystem.h"
@@ -255,7 +256,7 @@
 DataRecorder *CommandProvider::GetNewDataRecorder() {
   std::size_t i = m_data_recorders.size() + 1;
   std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") +
-  llvm::Twine(i) + llvm::Twine(".txt"))
+  llvm::Twine(i) + llvm::Twine(".yaml"))
  .str();
   auto recorder_or_error =
   DataRecorder::Create(GetRoot().CopyByAppendingPathComponent(filename));
@@ -304,47 +305,63 @@
   os << m_cwd << "\n";
 }
 
-llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() {
-  FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file);
+void ProcessGDBRemoteProvider::Keep() {
+  std::vector files;
+  for (auto  : m_packet_recorders) {
+files.push_back(recorder->GetFilename().GetPath());
+  }
 
-  std::error_code EC;
-  m_stream_up = std::make_unique(history_file.GetPath(), EC,
- sys::fs::OpenFlags::OF_Text);
-  return m_stream_up.get();
+  FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file);
+  std::error_code ec;
+  llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+  if (ec)
+return;
+  yaml::Output yout(os);
+  yout << files;
 }
 
-std::unique_ptr CommandLoader::Create(Loader *loader) {
-  if (!loader)
-return {};
-
-  FileSpec file = loader->GetFile();
-  if (!file)
-return {};
+void ProcessGDBRemoteProvider::Discard() { m_packet_recorders.clear(); }
 
-  auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
-  if (auto err = error_or_file.getError())
-return {};
-
-  std::vector files;
-  llvm::yaml::Input yin((*error_or_file)->getBuffer());
-  yin >> files;
-
-  if (auto err = yin.error())
-return {};
+llvm::Expected>
+PacketRecorder::Create(const FileSpec ) {
+  std::error_code ec;
+  auto recorder = std::make_unique(std::move(filename), ec);
+  if (ec)
+return llvm::errorCodeToError(ec);
+  return std::move(recorder);
+}
 
-  for (auto  : files) {
-FileSpec absolute_path =
-loader->GetRoot().CopyByAppendingPathComponent(file);
-file = absolute_path.GetPath();
+PacketRecorder *ProcessGDBRemoteProvider::GetNewPacketRecorder() {
+  std::size_t i = m_packet_recorders.size() + 1;
+  std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") +
+  llvm::Twine(i) + llvm::Twine(".yaml"))
+ .str();
+  auto recorder_or_error =
+  PacketRecorder::Create(GetRoot().CopyByAppendingPathComponent(filename));
+  if (!recorder_or_error) {
+llvm::consumeError(recorder_or_error.takeError());
+return nullptr;
   }
 
-  return std::make_unique(std::move(files));
+  m_packet_recorders.push_back(std::move(*recorder_or_error));
+  return m_packet_recorders.back().get();
 }
 
-llvm::Optional CommandLoader::GetNextFile() {
-  if (m_index >= m_files.size())
-return {};
-  return m_files[m_index++];
+void PacketRecorder::Record(const GDBRemotePacket ) {
+  if (!m_record)
+return;
+  yaml::Output yout(m_os);
+  yout << const_cast(packet);
+  m_os.flush();
+}
+

[Lldb-commits] [PATCH] D71099: [lldb][test] Handle .categories lookup for inline tests.

[pre-merge checks [bot] via Phabricator via lldb-commits]

merge_guards_bot added a comment.

Build result: pass - 60539 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71099



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

[Lldb-commits] [PATCH] D71099: [lldb][test] Handle .categories lookup for inline tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
rupprecht added reviewers: labath, JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When creating a test with `lldbinline.MakeInlineTest()`, the reported 
`inspect.getfile(test.__class__)` is `lldbtest.pyc`, meaning any `.categories` 
file will be ineffective for those tests. Check for the test_filename first, 
which inline tests will set.

Additionally, raise an error with the starting dir if `.categories` is not 
found. This makes the problem more obvious when it occurs: when the test is 
separated from the test framework tree.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71099

Files:
  lldb/packages/Python/lldbsuite/test/test_result.py


Index: lldb/packages/Python/lldbsuite/test/test_result.py
===
--- lldb/packages/Python/lldbsuite/test/test_result.py
+++ lldb/packages/Python/lldbsuite/test/test_result.py
@@ -113,8 +113,14 @@
 """
 import inspect
 import os.path
-folder = inspect.getfile(test.__class__)
-folder = os.path.dirname(folder)
+# Use test.test_filename if the test was created with
+# lldbinline.MakeInlineTest().
+if hasattr(test, 'test_filename'):
+start_path = test.test_filename
+else:
+start_path = inspect.getfile(test.__class__)
+
+folder = os.path.dirname(start_path)
 while folder != '/':
 categories_file_name = os.path.join(folder, ".categories")
 if os.path.exists(categories_file_name):
@@ -127,6 +133,7 @@
 else:
 folder = os.path.dirname(folder)
 continue
+raise Exception("Did not find a .categories file, starting at: %s" % 
start_path)
 
 
 def getCategoriesForTest(self, test):


Index: lldb/packages/Python/lldbsuite/test/test_result.py
===
--- lldb/packages/Python/lldbsuite/test/test_result.py
+++ lldb/packages/Python/lldbsuite/test/test_result.py
@@ -113,8 +113,14 @@
 """
 import inspect
 import os.path
-folder = inspect.getfile(test.__class__)
-folder = os.path.dirname(folder)
+# Use test.test_filename if the test was created with
+# lldbinline.MakeInlineTest().
+if hasattr(test, 'test_filename'):
+start_path = test.test_filename
+else:
+start_path = inspect.getfile(test.__class__)
+
+folder = os.path.dirname(start_path)
 while folder != '/':
 categories_file_name = os.path.join(folder, ".categories")
 if os.path.exists(categories_file_name):
@@ -127,6 +133,7 @@
 else:
 folder = os.path.dirname(folder)
 continue
+raise Exception("Did not find a .categories file, starting at: %s" % start_path)
 
 
 def getCategoriesForTest(self, test):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0d236d8 - [lldb] Update hardcoded Makefile.rules inclusions.

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2019-12-05T15:50:03-08:00
New Revision: 0d236d8b4f8aecc258e26ad53755a39d9b76032e

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

LOG: [lldb] Update hardcoded Makefile.rules inclusions.

This replaces `include $(LEVEL)/Makefile.rules` with `include Makefile.rules`.
The lldb test driver already passes the include path when running make, and 
specifically looking for "../../Makefile.rules" forces the test to be in a 
specific location.
Removing this hardcoded relative path will make it possible to move this test 
as-is.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/functionalities/float-display/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
lldb/packages/Python/lldbsuite/test/lang/cpp/thread_local/Makefile
lldb/packages/Python/lldbsuite/test/macosx/macabi/Makefile

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/float-display/Makefile 
b/lldb/packages/Python/lldbsuite/test/functionalities/float-display/Makefile
index f5a47fcc46cc..c9319d6e6888 100644
--- a/lldb/packages/Python/lldbsuite/test/functionalities/float-display/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/float-display/Makefile
@@ -1,3 +1,2 @@
-LEVEL = ../../make
 C_SOURCES := main.c
-include $(LEVEL)/Makefile.rules
+include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
index a49ffa84c547..db8fa57abb91 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
@@ -1,5 +1,3 @@
-LEVEL = ../../../make
 CXX_SOURCES := main.cpp
-include $(LEVEL)/Makefile.rules
 CXXFLAGS_EXTRAS := -O2 -glldb -Xclang -femit-debug-entry-values
 include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/cpp/thread_local/Makefile 
b/lldb/packages/Python/lldbsuite/test/lang/cpp/thread_local/Makefile
index 99bfa7e03b47..3d0b98f13f3d 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/cpp/thread_local/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/thread_local/Makefile
@@ -1,3 +1,2 @@
-LEVEL = ../../../make
 CXX_SOURCES := main.cpp
-include $(LEVEL)/Makefile.rules
+include Makefile.rules

diff  --git a/lldb/packages/Python/lldbsuite/test/macosx/macabi/Makefile 
b/lldb/packages/Python/lldbsuite/test/macosx/macabi/Makefile
index 61e3337e1d9e..2123af1dd701 100644
--- a/lldb/packages/Python/lldbsuite/test/macosx/macabi/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/macosx/macabi/Makefile
@@ -1,5 +1,3 @@
-LEVEL = ../../make
-
 C_SOURCES := main.c
 LD_EXTRAS := -L. -lfoo
 
@@ -12,4 +10,4 @@ libfoo.dylib: foo.c
$(MAKE) -f $(MAKEFILE_RULES) \
DYLIB_ONLY=YES DYLIB_NAME=foo DYLIB_C_SOURCES=foo.c
 
-include $(LEVEL)/Makefile.rules
+include Makefile.rules



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

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D70622#1771601 , @rupprecht wrote:

> In D70622#1765674 , @JDevlieghere 
> wrote:
>
> > No objections to moving the test. Jordan, let me know if you're up for it, 
> > otherwise I'm happy to take care of it.
>
>
> I tried moving it today. The rabbit hole goes deep :D
>  The test framework seems pretty tied to the directory layout (because 
> python), e.g. those must be in a directory called "lldbsuite" to be able to 
> "import lldbsuite"
>  The test infrastructure also seems to assume tests + test framework are in 
> the same tree (e.g. the LLDB_TEST env var is used for both), so moving the 
> individual test directories and leaving just the framework classes in 
> lldbsuite breaks a few things.
>
> I agree it's probably cleaner to move the tests, so I'll keep poking. 
> Suggestions welcome though.


I have it down to ~5 failing tests (some may be preexisting failures), so I'll 
start pulling apart my change into submittable pieces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70622



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

[Lldb-commits] [PATCH] D70883: [vscode.py] Make read_packet only return None when EOF

[António Afonso via Phabricator via lldb-commits]

aadsm abandoned this revision.
aadsm added a comment.

Fair enough, I haven't seen evidence of this (haven't searched for it) but I 
imagine IDEs need to ignore this as well otherwise they just barf if they're 
expecting `Content-Length` and a wild print appears. The notion of stdout of 
SBDebugger is the SBCommandReturnObject which doesn't print right away (only 
when the command finishes executing) and from my previous tests .flush() 
doesn't help with this. I believe this is the reason why people opt to use 
print instead in their custom commands. But I don't think it's a reasonable 
assumption (or api contract) to tell people they can't use print or it breaks 
lldb-vscode.

I'm going to fix this in lldb-vscode itself to wrap all stdout in a proper DAP 
console response.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70883



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D70622#1765674 , @JDevlieghere 
wrote:

> No objections to moving the test. Jordan, let me know if you're up for it, 
> otherwise I'm happy to take care of it.


I tried moving it today. The rabbit hole goes deep :D
The test framework seems pretty tied to the directory layout (because python), 
e.g. those must be in a directory called "lldbsuite" to be able to "import 
lldbsuite"
The test infrastructure also seems to assume tests + test framework are in the 
same tree (e.g. the LLDB_TEST env var is used for both), so moving the 
individual test directories and leaving just the framework classes in lldbsuite 
breaks a few things.

I agree it's probably cleaner to move the tests, so I'll keep poking. 
Suggestions welcome though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70622



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

[Lldb-commits] [PATCH] D70992: Replacing to use of ul suffix in GetMaxU64Bitfield since it not guarenteed to be 64 bit

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfffd70291e12: [LLDB] Replacing use of ul suffix in 
GetMaxU64Bitfield since it not guarenteed… (authored by shafik).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D70992?vs=232218=232380#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70992

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp


Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -13,7 +13,7 @@
 using namespace lldb_private;
 
 TEST(DataExtractorTest, GetBitfield) {
-  uint8_t buffer[] = {0x01, 0x23, 0x45, 0x67};
+  uint8_t buffer[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF};
   DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));
   DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void 
*));
@@ -24,6 +24,15 @@
   ASSERT_EQ(buffer[1], LE.GetMaxU64Bitfield(, sizeof(buffer), 8, 8));
   offset = 0;
   ASSERT_EQ(buffer[1], BE.GetMaxU64Bitfield(, sizeof(buffer), 8, 8));
+  offset = 0;
+  ASSERT_EQ(static_cast(0x0123456789ABCDEF),
+BE.GetMaxU64Bitfield(, sizeof(buffer), 64, 0));
+  offset = 0;
+  ASSERT_EQ(static_cast(0x01234567),
+BE.GetMaxU64Bitfield(, sizeof(buffer), 32, 0));
+  offset = 0;
+  ASSERT_EQ(static_cast(0x012345678),
+BE.GetMaxU64Bitfield(, sizeof(buffer), 36, 0));
 
   offset = 0;
   ASSERT_EQ(int8_t(buffer[1]),
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -577,18 +577,28 @@
 uint64_t DataExtractor::GetMaxU64Bitfield(offset_t *offset_ptr, size_t size,
   uint32_t bitfield_bit_size,
   uint32_t bitfield_bit_offset) const {
+  assert(bitfield_bit_size <= 64);
   uint64_t uval64 = GetMaxU64(offset_ptr, size);
-  if (bitfield_bit_size > 0) {
-int32_t lsbcount = bitfield_bit_offset;
-if (m_byte_order == eByteOrderBig)
-  lsbcount = size * 8 - bitfield_bit_offset - bitfield_bit_size;
-if (lsbcount > 0)
-  uval64 >>= lsbcount;
-uint64_t bitfield_mask = ((1ul << bitfield_bit_size) - 1);
-if (!bitfield_mask && bitfield_bit_offset == 0 && bitfield_bit_size == 64)
-  return uval64;
-uval64 &= bitfield_mask;
-  }
+
+  if (bitfield_bit_size == 0)
+return uval64;
+
+  int32_t lsbcount = bitfield_bit_offset;
+  if (m_byte_order == eByteOrderBig)
+lsbcount = size * 8 - bitfield_bit_offset - bitfield_bit_size;
+
+  if (lsbcount > 0)
+uval64 >>= lsbcount;
+
+  uint64_t bitfield_mask =
+  (bitfield_bit_size == 64
+   ? std::numeric_limits::max()
+   : ((static_cast(1) << bitfield_bit_size) - 1));
+  if (!bitfield_mask && bitfield_bit_offset == 0 && bitfield_bit_size == 64)
+return uval64;
+
+  uval64 &= bitfield_mask;
+
   return uval64;
 }
 


Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -13,7 +13,7 @@
 using namespace lldb_private;
 
 TEST(DataExtractorTest, GetBitfield) {
-  uint8_t buffer[] = {0x01, 0x23, 0x45, 0x67};
+  uint8_t buffer[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF};
   DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));
   DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
@@ -24,6 +24,15 @@
   ASSERT_EQ(buffer[1], LE.GetMaxU64Bitfield(, sizeof(buffer), 8, 8));
   offset = 0;
   ASSERT_EQ(buffer[1], BE.GetMaxU64Bitfield(, sizeof(buffer), 8, 8));
+  offset = 0;
+  ASSERT_EQ(static_cast(0x0123456789ABCDEF),
+BE.GetMaxU64Bitfield(, sizeof(buffer), 64, 0));
+  offset = 0;
+  ASSERT_EQ(static_cast(0x01234567),
+BE.GetMaxU64Bitfield(, sizeof(buffer), 32, 0));
+  offset = 0;
+  ASSERT_EQ(static_cast(0x012345678),
+BE.GetMaxU64Bitfield(, sizeof(buffer), 36, 0));
 
   offset = 0;
   ASSERT_EQ(int8_t(buffer[1]),
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -577,18 +577,28 @@
 uint64_t DataExtractor::GetMaxU64Bitfield(offset_t *offset_ptr, size_t size,
   uint32_t bitfield_bit_size,
   uint32_t bitfield_bit_offset) const {
+  assert(bitfield_bit_size <= 64);
   uint64_t uval64 = 

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1462f5a4c138: [lldb][NFC] Move Address and AddressRange 
functions out of Stream and let them… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71052

Files:
  lldb/include/lldb/Utility/Stream.h
  lldb/source/Core/Address.cpp
  lldb/source/Core/AddressRange.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Target/ThreadPlanRunToAddress.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Target/ThreadPlanStepInstruction.cpp
  lldb/source/Target/ThreadPlanStepOverRange.cpp
  lldb/source/Target/ThreadPlanStepThrough.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/VMRange.cpp
  lldb/unittests/Utility/StreamTest.cpp

Index: lldb/unittests/Utility/StreamTest.cpp
===
--- lldb/unittests/Utility/StreamTest.cpp
+++ lldb/unittests/Utility/StreamTest.cpp
@@ -37,94 +37,90 @@
 }
 
 TEST_F(StreamTest, AddressPrefix) {
-  s.Address(0x1, 1, "foo");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "foo");
   EXPECT_EQ("foo0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressEmptyPrefix) {
-  s.Address(0x1, 1, nullptr);
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, "");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSuffix) {
-  s.Address(0x1, 1, nullptr, "foo");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, "foo");
   EXPECT_EQ("0x01foo", TakeValue());
 }
 
 TEST_F(StreamTest, AddressNoSuffix) {
-  s.Address(0x1, 1, nullptr, nullptr);
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, nullptr, "");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressPrefixAndSuffix) {
-  s.Address(0x1, 1, "foo", "bar");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "foo", "bar");
   EXPECT_EQ("foo0x01bar", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSize) {
-  s.Address(0x0, 0);
+  DumpAddress(s.AsRawOstream(), 0x0, 0);
   EXPECT_EQ("0x0", TakeValue());
-  s.Address(0x1, 0);
+  DumpAddress(s.AsRawOstream(), 0x1, 0);
   EXPECT_EQ("0x1", TakeValue());
 
-  s.Address(0x1, 1);
+  DumpAddress(s.AsRawOstream(), 0x1, 1);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0xf1, 1);
+  DumpAddress(s.AsRawOstream(), 0xf1, 1);
   EXPECT_EQ("0xf1", TakeValue());
-  s.Address(0xff, 1);
+  DumpAddress(s.AsRawOstream(), 0xff, 1);
   EXPECT_EQ("0xff", TakeValue());
-  s.Address(0x100, 1);
+  DumpAddress(s.AsRawOstream(), 0x100, 1);
   EXPECT_EQ("0x100", TakeValue());
 
-  s.Address(0xf00, 4);
+  DumpAddress(s.AsRawOstream(), 0xf00, 4);
   EXPECT_EQ("0x0f00", TakeValue());
-  s.Address(0x100, 8);
+  DumpAddress(s.AsRawOstream(), 0x100, 8);
   EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x100, 10);
-  EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x1234, 10);
-  EXPECT_EQ("0x1234", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRange) {
-  s.AddressRange(0x100, 0x101, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeEmptyRange) {
-  s.AddressRange(0x100, 0x100, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x100, 2);
   EXPECT_EQ("[0x0100-0x0100)", TakeValue());
-  s.AddressRange(0x0, 0x0, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x0, 0x0, 2);
   EXPECT_EQ("[0x-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeInvalidRange) {
-  s.AddressRange(0x100, 0x0FF, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x0FF, 2);
   EXPECT_EQ("[0x0100-0x00ff)", TakeValue());
-  s.AddressRange(0x100, 0x0, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x0, 2);
   EXPECT_EQ("[0x0100-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeSize) {
-  s.AddressRange(0x100, 0x101, 0);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 0);
   EXPECT_EQ("[0x100-0x101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 4);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 4);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
 
-  s.AddressRange(0x100, 0x101, 4);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 4);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
-  s.AddressRange(0x1, 0x101, 4);
+  DumpAddressRange(s.AsRawOstream(), 0x1, 0x101, 4);
   EXPECT_EQ("[0x0001-0x0101)", 

[Lldb-commits] [lldb] 1462f5a - [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-05T14:41:33+01:00
New Revision: 1462f5a4c138bb20f38a579a29d12ab4e5fb6191

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

LOG: [lldb][NFC] Move Address and AddressRange functions out of Stream and let 
them take raw_ostream

Summary:
Yet another step on the long road towards getting rid of lldb's Stream class.

We probably should just make this some kind of member of Address/AddressRange, 
but it seems quite often we just push
in random integers in there and this is just about getting rid of Stream and 
not improving arbitrary APIs.

I had to rename another `DumpAddress` function in FormatEntity that is dumping 
the content of an address to make Clang happy.

Reviewers: labath

Reviewed By: labath

Subscribers: JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Utility/Stream.h
lldb/source/Core/Address.cpp
lldb/source/Core/AddressRange.cpp
lldb/source/Core/DumpDataExtractor.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/source/Symbol/Block.cpp
lldb/source/Target/ThreadPlanRunToAddress.cpp
lldb/source/Target/ThreadPlanStepInRange.cpp
lldb/source/Target/ThreadPlanStepInstruction.cpp
lldb/source/Target/ThreadPlanStepOverRange.cpp
lldb/source/Target/ThreadPlanStepThrough.cpp
lldb/source/Utility/Stream.cpp
lldb/source/Utility/VMRange.cpp
lldb/unittests/Utility/StreamTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Stream.h 
b/lldb/include/lldb/Utility/Stream.h
index a3a33178086e..18a16a3461c1 100644
--- a/lldb/include/lldb/Utility/Stream.h
+++ b/lldb/include/lldb/Utility/Stream.h
@@ -222,47 +222,6 @@ class Stream {
   Stream <<(int32_t sval) = delete;
   Stream <<(int64_t sval) = delete;
 
-  /// Output an address value to this stream.
-  ///
-  /// Put an address \a addr out to the stream with optional \a prefix and \a
-  /// suffix strings.
-  ///
-  /// \param[in] addr
-  /// An address value.
-  ///
-  /// \param[in] addr_size
-  /// Size in bytes of the address, used for formatting.
-  ///
-  /// \param[in] prefix
-  /// A prefix C string. If nullptr, no prefix will be output.
-  ///
-  /// \param[in] suffix
-  /// A suffix C string. If nullptr, no suffix will be output.
-  void Address(uint64_t addr, uint32_t addr_size, const char *prefix = nullptr,
-   const char *suffix = nullptr);
-
-  /// Output an address range to this stream.
-  ///
-  /// Put an address range \a lo_addr - \a hi_addr out to the stream with
-  /// optional \a prefix and \a suffix strings.
-  ///
-  /// \param[in] lo_addr
-  /// The start address of the address range.
-  ///
-  /// \param[in] hi_addr
-  /// The end address of the address range.
-  ///
-  /// \param[in] addr_size
-  /// Size in bytes of the address, used for formatting.
-  ///
-  /// \param[in] prefix
-  /// A prefix C string. If nullptr, no prefix will be output.
-  ///
-  /// \param[in] suffix
-  /// A suffix C string. If nullptr, no suffix will be output.
-  void AddressRange(uint64_t lo_addr, uint64_t hi_addr, uint32_t addr_size,
-const char *prefix = nullptr, const char *suffix = 
nullptr);
-
   /// Output a C string to the stream.
   ///
   /// Print a C string \a cstr to the stream.
@@ -452,6 +411,54 @@ class Stream {
   RawOstreamForward m_forwarder;
 };
 
+/// Output an address value to this stream.
+///
+/// Put an address \a addr out to the stream with optional \a prefix and \a
+/// suffix strings.
+///
+/// \param[in] s
+/// The output stream.
+///
+/// \param[in] addr
+/// An address value.
+///
+/// \param[in] addr_size
+/// Size in bytes of the address, used for formatting.
+///
+/// \param[in] prefix
+/// A prefix C string. If nullptr, no prefix will be output.
+///
+/// \param[in] suffix
+/// A suffix C string. If nullptr, no suffix will be output.
+void DumpAddress(llvm::raw_ostream , uint64_t addr, uint32_t addr_size,
+ const char *prefix = nullptr, const char *suffix = nullptr);
+
+/// Output an address range to this stream.
+///
+/// Put an address range \a lo_addr - \a hi_addr out to the stream with
+/// optional \a prefix and \a suffix strings.
+///
+/// \param[in] s
+/// The output stream.
+///
+/// \param[in] lo_addr
+/// The start address of the address range.
+///
+/// \param[in] hi_addr
+/// The end address of the address range.
+///
+/// \param[in] addr_size
+/// Size in bytes of the address, used for formatting.
+///
+/// \param[in] prefix

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 232322.
teemperor marked an inline comment as done.
teemperor added a comment.

- Moved to format_hex.
- Removed a now failing part of StreamTest that tested printing an address on a 
160 bit system (which LLDB doesn't support yet).


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

https://reviews.llvm.org/D71052

Files:
  lldb/include/lldb/Utility/Stream.h
  lldb/source/Core/Address.cpp
  lldb/source/Core/AddressRange.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Target/ThreadPlanRunToAddress.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Target/ThreadPlanStepInstruction.cpp
  lldb/source/Target/ThreadPlanStepOverRange.cpp
  lldb/source/Target/ThreadPlanStepThrough.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/VMRange.cpp
  lldb/unittests/Utility/StreamTest.cpp

Index: lldb/unittests/Utility/StreamTest.cpp
===
--- lldb/unittests/Utility/StreamTest.cpp
+++ lldb/unittests/Utility/StreamTest.cpp
@@ -37,94 +37,90 @@
 }
 
 TEST_F(StreamTest, AddressPrefix) {
-  s.Address(0x1, 1, "foo");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "foo");
   EXPECT_EQ("foo0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressEmptyPrefix) {
-  s.Address(0x1, 1, nullptr);
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, "");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSuffix) {
-  s.Address(0x1, 1, nullptr, "foo");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, "foo");
   EXPECT_EQ("0x01foo", TakeValue());
 }
 
 TEST_F(StreamTest, AddressNoSuffix) {
-  s.Address(0x1, 1, nullptr, nullptr);
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, nullptr, "");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressPrefixAndSuffix) {
-  s.Address(0x1, 1, "foo", "bar");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "foo", "bar");
   EXPECT_EQ("foo0x01bar", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSize) {
-  s.Address(0x0, 0);
+  DumpAddress(s.AsRawOstream(), 0x0, 0);
   EXPECT_EQ("0x0", TakeValue());
-  s.Address(0x1, 0);
+  DumpAddress(s.AsRawOstream(), 0x1, 0);
   EXPECT_EQ("0x1", TakeValue());
 
-  s.Address(0x1, 1);
+  DumpAddress(s.AsRawOstream(), 0x1, 1);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0xf1, 1);
+  DumpAddress(s.AsRawOstream(), 0xf1, 1);
   EXPECT_EQ("0xf1", TakeValue());
-  s.Address(0xff, 1);
+  DumpAddress(s.AsRawOstream(), 0xff, 1);
   EXPECT_EQ("0xff", TakeValue());
-  s.Address(0x100, 1);
+  DumpAddress(s.AsRawOstream(), 0x100, 1);
   EXPECT_EQ("0x100", TakeValue());
 
-  s.Address(0xf00, 4);
+  DumpAddress(s.AsRawOstream(), 0xf00, 4);
   EXPECT_EQ("0x0f00", TakeValue());
-  s.Address(0x100, 8);
+  DumpAddress(s.AsRawOstream(), 0x100, 8);
   EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x100, 10);
-  EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x1234, 10);
-  EXPECT_EQ("0x1234", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRange) {
-  s.AddressRange(0x100, 0x101, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeEmptyRange) {
-  s.AddressRange(0x100, 0x100, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x100, 2);
   EXPECT_EQ("[0x0100-0x0100)", TakeValue());
-  s.AddressRange(0x0, 0x0, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x0, 0x0, 2);
   EXPECT_EQ("[0x-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeInvalidRange) {
-  s.AddressRange(0x100, 0x0FF, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x0FF, 2);
   EXPECT_EQ("[0x0100-0x00ff)", TakeValue());
-  s.AddressRange(0x100, 0x0, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x0, 2);
   EXPECT_EQ("[0x0100-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeSize) {
-  s.AddressRange(0x100, 0x101, 0);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 0);
   EXPECT_EQ("[0x100-0x101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 4);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 4);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
 
-  s.AddressRange(0x100, 0x101, 4);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 4);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
-  s.AddressRange(0x1, 0x101, 4);
+  DumpAddressRange(s.AsRawOstream(), 0x1, 0x101, 4);
   

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:260
 
-  bool GetLocation(lldb::addr_t func_load_addr, lldb::addr_t pc,
-   lldb::offset_t , lldb::offset_t );
+  void RelocateLowHighPC(lldb::addr_t load_function_start, lldb::addr_t 
_pc,
+ lldb::addr_t _pc) const;

JDevlieghere wrote:
> Should this be part of the DWARFExpression API? It feels like a static 
> function might be sufficient. Maybe it could take a range directly?
It's a private member function, so it's not really a part of any API. However, 
given that now it's called from only a single place, it's not actually useful. 
It served a purpose in the preceding patch (I'm going to need a stamp on that 
too BTW), since there it had like five callers, but now that this patch 
centralizes them, I can just inline it.



Comment at: lldb/source/Expression/DWARFExpression.cpp:53
+  if (data.ValidOffsetForDataOfSize(offset, index_size))
+return data.GetMaxU64_unchecked(, index_size);
+  return LLDB_INVALID_ADDRESS;

JDevlieghere wrote:
> Why the unchecked?
Because I've already checked that the offset is valid (that's the difference 
between the checked and unchecked versions, though I'm not really sure if we 
need them).



Comment at: lldb/source/Expression/DWARFExpression.cpp:57
+
+static std::unique_ptr
+GetLocationTable(DWARFExpression::LocationListFormat format, const 
DataExtractor ) {

aprantl wrote:
> Doxygen comment?
I've added /something/, though I think the purpose is mostly obvious here.

Btw, this part is going to get refactored further, since the current design 
doesn't really permit mixing DWARF 4&5 location lists within a single object 
file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 232319.
labath marked 10 inline comments as done.
labath added a comment.

- "inline" RelocateLowHighPC
- add some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug_loc.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
===
--- lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
+++ lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
@@ -1,24 +1,82 @@
-# Test debug_loc parsing, including the cases of invalid input. The exact
+# Test location list handling, including the cases of invalid input. The exact
 # behavior in the invalid cases is not particularly important, but it should be
 # "reasonable".
 
 # REQUIRES: x86
 
-# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" -o exit \
-# RUN:   | FileCheck %s
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOC=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOCLISTS=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s --check-prefix=CHECK --check-prefix=LOCLISTS
 
 # CHECK-LABEL: image lookup -v -a 0
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 RDI,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 RAX,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x3", type = "int", location = DW_OP_reg1 RDX,
 
+# CHECK-LABEL: image dump symfile
+# CHECK: CompileUnit{0x}
+# CHECK:   Function{
+# CHECK: Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x, 0x0001): DW_OP_reg5 RDI
+# CHECK-NEXT:  [0x0001, 0x0006): DW_OP_reg0 RAX
+# CHECK: Variable{{.*}}, name = "x1", {{.*}}, scope = parameter
+# CHECK: Variable{{.*}}, name = "x2", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  error: unexpected end of data
+# CHECK: Variable{{.*}}, name = "x3", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x0002, 0x0003): DW_OP_reg1 RDX
+# LOCLISTS:  Variable{{.*}}, name = "x4", {{.*}}, scope = parameter, location =
+# LOCLISTS-NEXT: DW_LLE_startx_length   (0xdead, 0x0001): DW_OP_reg2 RCX
+
+.ifdef LOC
+.macro OFFSET_PAIR lo hi
+.quad \lo
+.quad \hi
+.endm
+
+.macro BASE_ADDRESS base
+.quad -1
+.quad \base
+.endm
+
+.macro EXPR_SIZE sz
+.short \sz
+.endm
+
+.macro END_OF_LIST
+.quad 0
+.quad 0
+.endm
+.endif
+
+.ifdef LOCLISTS
+.macro OFFSET_PAIR lo hi
+.byte   4   # DW_LLE_offset_pair
+.uleb128 \lo
+.uleb128 \hi
+.endm
+
+.macro BASE_ADDRESS base
+.byte   6   # DW_LLE_base_address
+.quad \base
+.endm
+
+.macro EXPR_SIZE sz
+.uleb128 \sz
+.endm
+
+.macro END_OF_LIST
+.byte   0   # DW_LLE_end_of_list
+.endm
+.endif
+
 .type   f,@function
 f:  # @f
 .Lfunc_begin0:
@@ -44,33 +102,48 @@
 .Linfo_string4:
 .asciz  "int"
 
+.ifdef LOC
 .section.debug_loc,"",@progbits
+.endif
+.ifdef LOCLISTS
+.section.debug_loclists,"",@progbits
+.long   .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 # Length
+.Ldebug_loclist_table_start0:
+.short  5   # Version
+.byte   8   # Address size
+.byte   0   # Segment selector size
+.long   0   # Offset entry count
+.endif
 .Ldebug_loc0:
-.quad   .Lfunc_begin0-.Lfunc_begin0
-.quad   .Ltmp0-.Lfunc_begin0
-.short  1   # Loc expr size
+OFFSET_PAIR .Lfunc_begin0-.Lfunc_begin0, .Ltmp0-.Lfunc_begin0
+EXPR_SIZE 1
 .byte   85  # super-register DW_OP_reg5
-.quad   .Ltmp0-.Lfunc_begin0
-.quad   .Lfunc_end0-.Lfunc_begin0
-.short  1   # Loc expr size
+OFFSET_PAIR   .Ltmp0-.Lfunc_begin0, .Lfunc_end0-.Lfunc_begin0
+EXPR_SIZE 1
 .byte   80  # super-register 

[Lldb-commits] [PATCH] D71056: [lldb] Add a test for how we lazily create Clang AST nodes

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: shafik.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.

One of the ways we try to make LLDB faster is by only creating the Clang 
declarations (and loading the associated types)
when we actually need them for something. For example an evaluated expression 
might need to load types to
type check and codegen the expression.

Currently this mechanism isn't really tested, so we currently have no way to 
know how many Clang nodes we load and
when we load them. In general there seems to be some confusion when and why 
certain Clang nodes are created.
As we are about to make some changes to the code which is creating Clang AST 
nodes we probably should have
a test that at least checks that the current behaviour doesn't change. It also 
serves as some kind of documentation
on the current behaviour.

The test in this patch is just evaluating some expressions and checks which 
Clang nodes are created due to this in the
module AST. The check happens by looking at the AST dump of the current module 
and then scanning it for the
declarations we are looking for.

I'm aware that there are things missing in this test (inheritance, template 
parameters, non-expression evaluation commands)
but I'll expand it in follow up patches.

Also this test found two potential bugs in LLDB which are documented near the 
respective asserts in the test:

1. LLDB seems to always load all types of local variables even when we don't 
reference them in the expression. We had patches

that tried to prevent this but it seems that didn't work as well as it should 
have (even though we don't complete these
types).

2. We always seem to complete the first field of any record we run into. This 
has the funny side effect that LLDB is faster when

all classes in a project have an arbitrary `char unused;` as their first 
member. We probably want to fix this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71056

Files:
  lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/TestLazyLoading.py
  lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/main.cpp

Index: lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/main.cpp
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/main.cpp
@@ -0,0 +1,69 @@
+////
+// Struct loading declarations.
+
+struct StructFirstMember { int i; };
+struct StructBehindPointer { int i; };
+struct StructBehindRef { int i; };
+struct StructMember { int i; };
+
+StructBehindRef struct_instance;
+
+struct SomeStruct {
+  StructFirstMember *first;
+  StructBehindPointer *ptr;
+  StructMember member;
+  StructBehindRef  = struct_instance;
+};
+
+struct OtherStruct {
+  int member_int;
+};
+
+////
+// Class loading declarations.
+
+struct ClassMember { int i; };
+struct UnusedClassMember { int i; };
+struct UnusedClassMemberPtr { int i; };
+
+namespace NS {
+class ClassInNamespace {
+  int i;
+};
+class ClassWeEnter {
+public:
+  int dummy; // Prevent bug where LLDB always completes first member.
+  ClassMember member;
+  UnusedClassMember unused_member;
+  UnusedClassMemberPtr *unused_member_ptr;
+  int enteredFunction() {
+return member.i; // Location: class function
+  }
+};
+};
+
+////
+// Function we can stop in.
+
+int functionWithOtherStruct() {
+  OtherStruct other_struct_var;
+  other_struct_var.member_int++; // Location: other struct function
+  return other_struct_var.member_int;
+}
+
+int functionWithMultipleLocals() {
+  SomeStruct struct_var;
+  OtherStruct other_struct_var;
+  NS::ClassInNamespace namespace_class;
+  other_struct_var.member_int++; // Location: multiple locals function
+  return other_struct_var.member_int;
+}
+
+int main(int argc, char **argv) {
+  NS::ClassWeEnter c;
+  c.enteredFunction();
+
+  functionWithOtherStruct();
+  functionWithMultipleLocals();
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/TestLazyLoading.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/lazy-loading/TestLazyLoading.py
@@ -0,0 +1,231 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+"""
+This test ensures that we only create Clang AST nodes in our module AST
+when we actually need them.
+
+All tests in this file behave like this:
+  1. Use LLDB to do something (expression evaluation, breakpoint setting, etc.).
+  2. Check that certain Clang AST nodes were not loaded during 

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/CMakeLists.txt:114
+  COMMAND ${CMAKE_COMMAND} -E make_directory ${ARG_DEST_DIR})
+foreach(src_file ${ARG_FILES})
+  if(ARG_DEST_FILE_NAME)

tatyana-krasnukha wrote:
> This is quite off-topic, but it would be great if this function could handle 
> whole directories as well as separate files. So adding a script will not 
> require to change this file.
It isn't really off topic, but I don't think it's really feasible, or a good 
idea even :P.

Think of this as the list of cpp files in the cmake `add_library` calls. Though 
there are ways to avoid listing the files there (via some sort of globbing, 
generally), they usually come with their own problems. Either the glob is 
evaluated at configuration time, and then you have to remember to re-run cmake 
to pick up the newly added files; or the glob is done at build time, which 
means that even a noop build needs to reglob everything...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589



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

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486
+  size_t ret = data.SetData(m_data, offset, length);
+  // DataExtractor::SetData copies the address byte size from m_data, but
+  // m_data's address byte size is only set from sizeof(void*), and we can't
+  // access subclasses GetAddressByteSize() when setting up m_data in the
+  // constructor.
+  data.SetAddressByteSize(GetAddressByteSize());
+  return ret;

mstorsjo wrote:
> mstorsjo wrote:
> > labath wrote:
> > > mstorsjo wrote:
> > > > clayborg wrote:
> > > > > I would vote to make this happen within DataExtractor::SetData(const 
> > > > > DataExtractor &...)
> > > > Do you mean that we'd extend `DataExtractor::SetData(const 
> > > > DataExtractor &...)` to take a byte address size parameter, or that 
> > > > we'd update `m_data`'s byte address size before doing `data.SetData()` 
> > > > here?
> > > > 
> > > > Ideally I'd set the right byte address size in `m_data` as soon as it 
> > > > is known and available. It's not possible to do this in `ObjectFile`'s 
> > > > constructor, as that is called before the subclass is constructed and 
> > > > its virtual methods are available, but is there a better point in the 
> > > > lifetime where we could update it?
> > > I too would prefer that, but I couldn't see a way to achieve that (which 
> > > is why I stamped this version).
> > > 
> > > Today, with a fresh set of eyes, I think it may be reasonable to have 
> > > this happen in `ObjectFile::ParseHeader`. After the header has been 
> > > parsed, all object formats (I hope) should be able to determine their 
> > > address size and endianness, and the operating invariant would be that 
> > > the address size is only valid after the ParseHeader has been called. 
> > > WDYT?
> > Sounds sensible! I'll give it a shot.
> `ObjectFile::ParseHeader` is currently pure virtual, and thus the subclass 
> concrete implementations of it don't call the base class version of the 
> method. Do you want me to make the base class method non-pure and add calls 
> to it in the subclasses, or add calls to 
> `m_data.SetAddressByteSize(GetAddressByteSize());` in all of the subclasses 
> `ParseHeader`?
Yeah.. I don't know... I am starting to wonder if this really is a good idea.. 
From the looks of it, there doesn't seem to be anyone (outside of the object 
file classes themselves) who is calling this method, so it's not clear to me 
why is it even present on the base class...

Overall, the scheme that I think I'd like the most would be something like:
```
class ObjectFile {
  /*nonvirtual*/ uint32_t GetAddressByteSize() { ParseHeader(); return 
m_data.GetAddressByteSize(); }
  // same for byte order
  
  /*nonvirtual*/ void ParseHeader() { std::tie(address_size, byte_order) = 
DoParseHeader(); m_data.SetAddressByteSize(address_size); 
m_data.SetByteOrder(byte_order); }
  virtual ??? DoParseHeader() = 0;
};
```

However, I don't know how hard would it be to get to that point, so I think I'd 
settle for just making sure that each subclass calls `m_data.SetByteOrder` 
internally. It looks like ObjectFileELF does that already. I haven't checked, 
but I wouldn't be surprised if MachO does the same...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70848



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

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added inline comments.



Comment at: lldb/CMakeLists.txt:114
+  COMMAND ${CMAKE_COMMAND} -E make_directory ${ARG_DEST_DIR})
+foreach(src_file ${ARG_FILES})
+  if(ARG_DEST_FILE_NAME)

This is quite off-topic, but it would be great if this function could handle 
whole directories as well as separate files. So adding a script will not 
require to change this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589



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

[Lldb-commits] [PATCH] D71021: [lldb/DWARF] Switch to llvm debug_rnglists parser

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5767e284bea: [lldb/DWARF] Switch to llvm debug_rnglists 
parser (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71021

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
===
--- lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
+++ lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
@@ -1,12 +1,19 @@
 # REQUIRES: x86
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -s lookup_rnglists" -o exit | FileCheck %s
+# RUN: %lldb %t -o "image lookup -v -s lookup_rnglists" \
+# RUN:   -o "image lookup -v -s lookup_rnglists2" -o exit | FileCheck %s
 
+# CHECK-LABEL: image lookup -v -s lookup_rnglists
 # CHECK:  Function: id = {0x7fff0030}, name = "rnglists", range = [0x-0x0004)
 # CHECK:Blocks: id = {0x7fff0030}, range = [0x-0x0004)
 # CHECK-NEXT:   id = {0x7fff0046}, ranges = [0x0001-0x0002)[0x0003-0x0004)
 
+# CHECK-LABEL: image lookup -v -s lookup_rnglists2
+# CHECK:  Function: id = {0x7fff007a}, name = "rnglists2", range = [0x0004-0x0007)
+# CHECK:Blocks: id = {0x7fff007a}, range = [0x0004-0x0007)
+# CHECK-NEXT:   id = {0x7fff0091}, range = [0x0005-0x0007)
+
 .text
 .p2align 12
 rnglists:
@@ -21,6 +28,15 @@
 .Lblock2_end:
 .Lrnglists_end:
 
+rnglists2:
+nop
+.Lblock3_begin:
+lookup_rnglists2:
+nop
+nop
+.Lblock3_end:
+.Lrnglists2_end:
+
 .section.debug_abbrev,"",@progbits
 .byte   1   # Abbreviation Code
 .byte   17  # DW_TAG_compile_unit
@@ -78,6 +94,28 @@
 .byte   0   # End Of Children Mark
 .Ldebug_info_end0:
 
+.Lcu_begin1:
+.long   .Ldebug_info_end1-.Ldebug_info_start1 # Length of Unit
+.Ldebug_info_start1:
+.short  5   # DWARF version number
+.byte   1   # DWARF Unit Type
+.byte   8   # Address Size (in bytes)
+.long   .debug_abbrev   # Offset Into Abbrev. Section
+.byte   1   # Abbrev [1] 0xc:0x5f DW_TAG_compile_unit
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.quad   rnglists2   # DW_AT_low_pc
+.long   .Lrnglists2_end-rnglists2 # DW_AT_high_pc
+.long   .Lrnglists_table_base1  # DW_AT_rnglists_base
+.byte   2   # Abbrev [2] 0x2b:0x37 DW_TAG_subprogram
+.quad   rnglists2   # DW_AT_low_pc
+.long   .Lrnglists2_end-rnglists2 # DW_AT_high_pc
+.asciz  "rnglists2" # DW_AT_name
+.byte   5   # Abbrev [5] 0x52:0xf DW_TAG_lexical_block
+.byte   0   # DW_AT_ranges
+.byte   0   # End Of Children Mark
+.byte   0   # End Of Children Mark
+.Ldebug_info_end1:
+
 .section.debug_rnglists,"",@progbits
 .long   .Ldebug_rnglist_table_end0-.Ldebug_rnglist_table_start0 # Length
 .Ldebug_rnglist_table_start0:
@@ -96,3 +134,18 @@
 .uleb128 .Lblock2_end-rnglists#   ending offset
 .byte   0   # DW_RLE_end_of_list
 .Ldebug_rnglist_table_end0:
+
+.long   .Ldebug_rnglist_table_end1-.Ldebug_rnglist_table_start1 # Length
+.Ldebug_rnglist_table_start1:
+.short  5   # Version
+.byte   8   # Address size
+.byte   0   # Segment selector size
+.long   1   # Offset entry count
+.Lrnglists_table_base1:
+.long   .Ldebug_ranges1-.Lrnglists_table_base1
+.Ldebug_ranges1:
+.byte   4   # DW_RLE_offset_pair
+.uleb128 .Lblock3_begin-rnglists2 #   starting offset
+.uleb128 .Lblock3_end-rnglists2   #   ending offset
+.byte   0   # DW_RLE_end_of_list
+.Ldebug_rnglist_table_end1:
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===

[Lldb-commits] [lldb] f5767e2 - [lldb/DWARF] Switch to llvm debug_rnglists parser

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-05T13:02:03+01:00
New Revision: f5767e284beaff4e5eb35f0f64270a070b47f6d3

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

LOG: [lldb/DWARF] Switch to llvm debug_rnglists parser

Summary:
Our rnglist support was working only for the trivial cases (one CU),
because we only ever parsed one contribution out of the debug_rnglists
section. This means we were never able to resolve range lists for the
second and subsequent units (DW_FORM_sec_offset references came out
blang, and DW_FORM_rnglistx references always used the ranges lists from
the first unit).

Since both llvm and lldb rnglist parsers are sufficiently
self-contained, and operate similarly, we can fix this problem by
switching to the llvm parser instead. Besides the changes which are due
to variations in the interface, the main thing is that now the range
list object is a member of the DWARFUnit, instead of the entire symbol
file. This ensures that each unit can get it's own private set of range
list indices, and is consistent with how llvm's DWARFUnit does it
(overall, I've tried to structure the code the same way as the llvm
version).

I've also added a test case for the two unit scenario.

Reviewers: JDevlieghere, aprantl, clayborg

Subscribers: dblaikie, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index 8c0fbeb4b717..1bab4e9db634 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -200,7 +200,7 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor 
,
   return false;
 }
 
-static DWARFRangeList GetRangesOrReportError(const DWARFUnit ,
+static DWARFRangeList GetRangesOrReportError(DWARFUnit ,
  const DWARFDebugInfoEntry ,
  const DWARFFormValue ) {
   llvm::Expected expected_ranges =
@@ -223,7 +223,7 @@ static DWARFRangeList GetRangesOrReportError(const 
DWARFUnit ,
 // Gets the valid address ranges for a given DIE by looking for a
 // DW_AT_low_pc/DW_AT_high_pc pair, DW_AT_entry_pc, or DW_AT_ranges attributes.
 bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
-const DWARFUnit *cu, const char *, const char *,
+DWARFUnit *cu, const char *, const char *,
 DWARFRangeList , int _file, int _line, int _column,
 int _file, int _line, int _column,
 DWARFExpression *frame_base) const {
@@ -766,7 +766,7 @@ bool DWARFDebugInfoEntry::GetAttributeAddressRange(
 }
 
 size_t DWARFDebugInfoEntry::GetAttributeAddressRanges(
-const DWARFUnit *cu, DWARFRangeList , bool check_hi_lo_pc,
+DWARFUnit *cu, DWARFRangeList , bool check_hi_lo_pc,
 bool check_specification_or_abstract_origin) const {
   ranges.Clear();
 
@@ -1012,8 +1012,7 @@ DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
   return storage.c_str();
 }
 
-bool DWARFDebugInfoEntry::LookupAddress(const dw_addr_t address,
-const DWARFUnit *cu,
+bool DWARFDebugInfoEntry::LookupAddress(const dw_addr_t address, DWARFUnit *cu,
 DWARFDebugInfoEntry **function_die,
 DWARFDebugInfoEntry **block_die) {
   bool found_address = false;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index f3952ae9598b..f35af6e7d498 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -50,7 +50,7 @@ class DWARFDebugInfoEntry {
   bool Extract(const lldb_private::DWARFDataExtractor ,
const DWARFUnit *cu, lldb::offset_t *offset_ptr);
 
-  bool LookupAddress(const dw_addr_t address, const DWARFUnit *cu,
+  bool LookupAddress(const dw_addr_t address, DWARFUnit *cu,
  DWARFDebugInfoEntry **function_die,
  DWARFDebugInfoEntry **block_die);
 
@@ -91,7 +91,7 @@ class DWARFDebugInfoEntry {
  

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream

[Pavel Labath via Phabricator via lldb-commits]

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

Let's ship it.




Comment at: lldb/source/Utility/Stream.cpp:88
+  std::string f = "{0}0x{2,0+" + size + ":x-}{3}";
+  s << llvm::formatv(f.c_str(), prefix, addr_size * 2, addr, suffix);
 }

The "formatv" way of doing this would be via something like 
`formatv("{0}0x{1:x}{2}", prefix, fmt_align(addr, AlignStyle::Right, 
addr_size*2, '0'), suffix)`, but that's quite a mouthful so I'd probably go for 
just `s << prefix << format_hex(addr, 2+2*addr_size) << suffix;`


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

https://reviews.llvm.org/D71052



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

[Lldb-commits] [PATCH] D70721: [lldb/cpluspluslanguage] Add constructor substitutor

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rGc16f0b18c13e: [lldb/cpluspluslanguage] Add constructor 
substitutor (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D70721?vs=232148=232302#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70721

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -191,6 +191,8 @@
   EXPECT_THAT(FindAlternate("_ZN1A1fEx"), Contains("_ZN1A1fEl"));
   EXPECT_THAT(FindAlternate("_ZN1A1fEy"), Contains("_ZN1A1fEm"));
   EXPECT_THAT(FindAlternate("_ZN1A1fEai"), Contains("_ZN1A1fEci"));
+  EXPECT_THAT(FindAlternate("_ZN1AC1Ev"), Contains("_ZN1AC2Ev"));
+  EXPECT_THAT(FindAlternate("_ZN1AD1Ev"), Contains("_ZN1AD2Ev"));
   EXPECT_THAT(FindAlternate("_bogus"), IsEmpty());
 }
 
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -284,46 +284,34 @@
   }
 };
 
-/// Given a mangled function `Mangled`, replace all the primitive function type
-/// arguments of `Search` with type `Replace`.
-class TypeSubstitutor
-: public llvm::itanium_demangle::AbstractManglingParser
+class ManglingSubstitutor
+: public llvm::itanium_demangle::AbstractManglingParser {
-  /// Input character until which we have constructed the respective output
-  /// already
-  const char *Written;
+  using Base =
+  llvm::itanium_demangle::AbstractManglingParser;
 
-  llvm::StringRef Search;
-  llvm::StringRef Replace;
-  llvm::SmallString<128> Result;
+public:
+  ManglingSubstitutor() : Base(nullptr, nullptr) {}
 
-  /// Whether we have performed any substitutions.
-  bool Substituted;
+  template
+  ConstString substitute(llvm::StringRef Mangled, Ts &&... Vals) {
+this->getDerived().reset(Mangled, std::forward(Vals)...);
+return substituteImpl(Mangled);
+  }
 
-  void reset(llvm::StringRef Mangled, llvm::StringRef Search,
- llvm::StringRef Replace) {
-AbstractManglingParser::reset(Mangled.begin(), Mangled.end());
+
+protected:
+  void reset(llvm::StringRef Mangled) {
+Base::reset(Mangled.begin(), Mangled.end());
 Written = Mangled.begin();
-this->Search = Search;
-this->Replace = Replace;
 Result.clear();
 Substituted = false;
   }
 
-  void appendUnchangedInput() {
-Result += llvm::StringRef(Written, First - Written);
-Written = First;
-  }
-
-public:
-  TypeSubstitutor() : AbstractManglingParser(nullptr, nullptr) {}
-
-  ConstString substitute(llvm::StringRef Mangled, llvm::StringRef From,
- llvm::StringRef To) {
+  ConstString substituteImpl(llvm::StringRef Mangled) {
 Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
-
-reset(Mangled, From, To);
-if (parse() == nullptr) {
+if (this->parse() == nullptr) {
   LLDB_LOG(log, "Failed to substitute mangling in {0}", Mangled);
   return ConstString();
 }
@@ -336,20 +324,69 @@
 return ConstString(Result);
   }
 
+  void trySubstitute(llvm::StringRef From, llvm::StringRef To) {
+if (!llvm::StringRef(currentParserPos(), this->numLeft()).startswith(From))
+  return;
+
+// We found a match. Append unmodified input up to this point.
+appendUnchangedInput();
+
+// And then perform the replacement.
+Result += To;
+Written += From.size();
+Substituted = true;
+  }
+
+private:
+  /// Input character until which we have constructed the respective output
+  /// already.
+  const char *Written;
+
+  llvm::SmallString<128> Result;
+
+  /// Whether we have performed any substitutions.
+  bool Substituted;
+
+  const char *currentParserPos() const { return this->First; }
+
+  void appendUnchangedInput() {
+Result +=
+llvm::StringRef(Written, std::distance(Written, currentParserPos()));
+Written = currentParserPos();
+  }
+
+};
+
+/// Given a mangled function `Mangled`, replace all the primitive function type
+/// arguments of `Search` with type `Replace`.
+class TypeSubstitutor : public ManglingSubstitutor {
+  llvm::StringRef Search;
+  llvm::StringRef Replace;
+
+public:
+  void reset(llvm::StringRef Mangled, llvm::StringRef Search,
+ llvm::StringRef Replace) {
+ManglingSubstitutor::reset(Mangled);
+this->Search = Search;
+this->Replace = Replace;
+  }
+
   llvm::itanium_demangle::Node *parseType() {
-if (llvm::StringRef(First, 

[Lldb-commits] [PATCH] D70721: [lldb/cpluspluslanguage] Add constructor substitutor

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:352
+  void appendUnchangedInput() {
+Result += llvm::StringRef(Written, this->First - Written);
+Written = this->First;

shafik wrote:
> labath wrote:
> > shafik wrote:
> > > `this->First - Written` feels awkward, I feel like given the names they 
> > > should be reversed :-(
> > Yeah, it's a bit weird, but I'm not sure what to do about it.. do you have 
> > any specific suggestion? `std::distance(Written, First)` ? adding a member 
> > function like `currentParserPos()` to wrap the `First`?
> Yes, I think `currentParserPos()` would be helpful, it would at least clarify 
> the intent and it makes it more obvious it is doing the correct thing.
I'll do that. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70721



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

[Lldb-commits] [lldb] c16f0b1 - [lldb/cpluspluslanguage] Add constructor substitutor

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-05T12:44:51+01:00
New Revision: c16f0b18c13e88fedaa510bc2442bb693a6230c8

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

LOG: [lldb/cpluspluslanguage] Add constructor substitutor

Summary:
This patch adds code which will substitute references to the full object
constructors/destructors with their base object versions.

Like all substitutions in this category, this operation is not really
sound, but doing this in a more precise way allows us to get rid of a
much larger hack -- matching function according to their demangled
names, which effectively does the same thing, but also much more.

This is a (very late) follow-up to D54074.

Background: clang has an optimization which can eliminate full object
structors completely, if they are found to be equivalent to their base
object versions. It does this because it assumes they can be regenerated
on demand in the compile unit that needs them (e.g., because they are
declared inline). However, this doesn't work for the debugging scenario,
where we don't have the structor bodies available -- we pretend all
constructors are defined out-of-line as far as clang is concerned. This
causes clang to emit references to the (nonexisting) full object
structors during expression evaluation.

Fun fact: This is not a problem on darwin, because the relevant
optimization is disabled to work around a linker bug.

Reviewers: teemperor, JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index c22f4ae9e41a..4385a60f5862 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -284,46 +284,34 @@ class NodeAllocator {
   }
 };
 
-/// Given a mangled function `Mangled`, replace all the primitive function type
-/// arguments of `Search` with type `Replace`.
-class TypeSubstitutor
-: public llvm::itanium_demangle::AbstractManglingParser
+class ManglingSubstitutor
+: public llvm::itanium_demangle::AbstractManglingParser {
-  /// Input character until which we have constructed the respective output
-  /// already
-  const char *Written;
+  using Base =
+  llvm::itanium_demangle::AbstractManglingParser;
 
-  llvm::StringRef Search;
-  llvm::StringRef Replace;
-  llvm::SmallString<128> Result;
+public:
+  ManglingSubstitutor() : Base(nullptr, nullptr) {}
 
-  /// Whether we have performed any substitutions.
-  bool Substituted;
+  template
+  ConstString substitute(llvm::StringRef Mangled, Ts &&... Vals) {
+this->getDerived().reset(Mangled, std::forward(Vals)...);
+return substituteImpl(Mangled);
+  }
 
-  void reset(llvm::StringRef Mangled, llvm::StringRef Search,
- llvm::StringRef Replace) {
-AbstractManglingParser::reset(Mangled.begin(), Mangled.end());
+
+protected:
+  void reset(llvm::StringRef Mangled) {
+Base::reset(Mangled.begin(), Mangled.end());
 Written = Mangled.begin();
-this->Search = Search;
-this->Replace = Replace;
 Result.clear();
 Substituted = false;
   }
 
-  void appendUnchangedInput() {
-Result += llvm::StringRef(Written, First - Written);
-Written = First;
-  }
-
-public:
-  TypeSubstitutor() : AbstractManglingParser(nullptr, nullptr) {}
-
-  ConstString substitute(llvm::StringRef Mangled, llvm::StringRef From,
- llvm::StringRef To) {
+  ConstString substituteImpl(llvm::StringRef Mangled) {
 Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
-
-reset(Mangled, From, To);
-if (parse() == nullptr) {
+if (this->parse() == nullptr) {
   LLDB_LOG(log, "Failed to substitute mangling in {0}", Mangled);
   return ConstString();
 }
@@ -336,20 +324,69 @@ class TypeSubstitutor
 return ConstString(Result);
   }
 
+  void trySubstitute(llvm::StringRef From, llvm::StringRef To) {
+if (!llvm::StringRef(currentParserPos(), this->numLeft()).startswith(From))
+  return;
+
+// We found a match. Append unmodified input up to this point.
+appendUnchangedInput();
+
+// And then perform the replacement.
+Result += To;
+Written += From.size();
+Substituted = true;
+  }
+
+private:
+  /// Input character until which we have constructed the respective output
+  /// already.
+  const char *Written;
+
+  llvm::SmallString<128> Result;
+
+  /// Whether we have performed any substitutions.
+  bool Substituted;
+
+  const char 

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 232301.
teemperor retitled this revision from "[lldb][NFC] Move Address and 
AddressRange functions out of Stream into namespace and let them take 
raw_ostream" to "[lldb][NFC] Move Address and AddressRange functions out of 
Stream and let them take raw_ostream".
teemperor edited the summary of this revision.
teemperor added a comment.

- Removed unrelated change to std-vector test.
- Moved from namespace to Dump* prefix.
- Renamed some function in FormatEntity that already had that name and made 
Clang very unhappy.


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

https://reviews.llvm.org/D71052

Files:
  lldb/include/lldb/Utility/Stream.h
  lldb/source/Core/Address.cpp
  lldb/source/Core/AddressRange.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Target/ThreadPlanRunToAddress.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Target/ThreadPlanStepInstruction.cpp
  lldb/source/Target/ThreadPlanStepOverRange.cpp
  lldb/source/Target/ThreadPlanStepThrough.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/VMRange.cpp
  lldb/unittests/Utility/StreamTest.cpp

Index: lldb/unittests/Utility/StreamTest.cpp
===
--- lldb/unittests/Utility/StreamTest.cpp
+++ lldb/unittests/Utility/StreamTest.cpp
@@ -37,94 +37,94 @@
 }
 
 TEST_F(StreamTest, AddressPrefix) {
-  s.Address(0x1, 1, "foo");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "foo");
   EXPECT_EQ("foo0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressEmptyPrefix) {
-  s.Address(0x1, 1, nullptr);
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, "");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSuffix) {
-  s.Address(0x1, 1, nullptr, "foo");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, "foo");
   EXPECT_EQ("0x01foo", TakeValue());
 }
 
 TEST_F(StreamTest, AddressNoSuffix) {
-  s.Address(0x1, 1, nullptr, nullptr);
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, nullptr, "");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, nullptr, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressPrefixAndSuffix) {
-  s.Address(0x1, 1, "foo", "bar");
+  DumpAddress(s.AsRawOstream(), 0x1, 1, "foo", "bar");
   EXPECT_EQ("foo0x01bar", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSize) {
-  s.Address(0x0, 0);
+  DumpAddress(s.AsRawOstream(), 0x0, 0);
   EXPECT_EQ("0x0", TakeValue());
-  s.Address(0x1, 0);
+  DumpAddress(s.AsRawOstream(), 0x1, 0);
   EXPECT_EQ("0x1", TakeValue());
 
-  s.Address(0x1, 1);
+  DumpAddress(s.AsRawOstream(), 0x1, 1);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0xf1, 1);
+  DumpAddress(s.AsRawOstream(), 0xf1, 1);
   EXPECT_EQ("0xf1", TakeValue());
-  s.Address(0xff, 1);
+  DumpAddress(s.AsRawOstream(), 0xff, 1);
   EXPECT_EQ("0xff", TakeValue());
-  s.Address(0x100, 1);
+  DumpAddress(s.AsRawOstream(), 0x100, 1);
   EXPECT_EQ("0x100", TakeValue());
 
-  s.Address(0xf00, 4);
+  DumpAddress(s.AsRawOstream(), 0xf00, 4);
   EXPECT_EQ("0x0f00", TakeValue());
-  s.Address(0x100, 8);
+  DumpAddress(s.AsRawOstream(), 0x100, 8);
   EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x100, 10);
+  DumpAddress(s.AsRawOstream(), 0x100, 10);
   EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x1234, 10);
+  DumpAddress(s.AsRawOstream(), 0x1234, 10);
   EXPECT_EQ("0x1234", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRange) {
-  s.AddressRange(0x100, 0x101, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeEmptyRange) {
-  s.AddressRange(0x100, 0x100, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x100, 2);
   EXPECT_EQ("[0x0100-0x0100)", TakeValue());
-  s.AddressRange(0x0, 0x0, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x0, 0x0, 2);
   EXPECT_EQ("[0x-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeInvalidRange) {
-  s.AddressRange(0x100, 0x0FF, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x0FF, 2);
   EXPECT_EQ("[0x0100-0x00ff)", TakeValue());
-  s.AddressRange(0x100, 0x0, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x0, 2);
   EXPECT_EQ("[0x0100-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeSize) {
-  s.AddressRange(0x100, 0x101, 0);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 0);
   EXPECT_EQ("[0x100-0x101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 2);
+  DumpAddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream

[Raphael Isemann via Phabricator via lldb-commits]

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



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/TestBasicVector.py:23-53
-self.expect("expr (size_t)a.size()", substrs=['(size_t) $0 = 3'])
-self.expect("expr (int)a.front()", substrs=['(int) $1 = 3'])
-self.expect("expr (int)a[1]", substrs=['(int) $2 = 1'])
-self.expect("expr (int)a.back()", substrs=['(int) $3 = 2'])
+self.expect("expr a.size()", substrs=[' $0 = 3'])
+self.expect("expr a.front()", substrs=[' $1 = 3'])
+self.expect("expr a[1]", substrs=[' $2 = 1'])
+self.expect("expr a.back()", substrs=[' $3 = 2'])
 
 self.expect("expr std::sort(a.begin(), a.end())")
-self.expect("expr (int)a.front()", substrs=['(int) $4 = 1'])

labath wrote:
> huh ?
Just checking if you actually look at my patches :)


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

https://reviews.llvm.org/D71052



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

[Lldb-commits] [PATCH] D70954: [lldb] Don't put compile unit name into the support file list and support DWARF5 line tables

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG57f8a998ceaf: [lldb] Dont put compile unit name into 
the support file list and support… (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70954

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
@@ -0,0 +1,129 @@
+# Test handling of DWARF5 line tables. In particular, test that we handle files
+# which are present in the line table more than once.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "source info -f file0.c" -o "source info -f file1.c" \
+# RUN:   -o "breakpoint set -f file0.c -l 42" \
+# RUN:   -o "breakpoint set -f file0.c -l 47" \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK-LABEL: source info -f file0.c
+# CHECK: [0x-0x0001): /file0.c:42
+# CHECK-LABEL: source info -f file1.c
+# CHECK: [0x0001-0x0002): /file1.c:47
+# CHECK-LABEL: breakpoint set -f file0.c -l 42
+# CHECK: Breakpoint 1: {{.*}}`foo,
+# CHECK-LABEL: breakpoint set -f file0.c -l 47
+# CHECK: Breakpoint 2: {{.*}}`foo + 2,
+
+.text
+.globl  foo
+foo:
+nop
+nop
+nop
+.Lfoo_end:
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   0   # DW_CHILDREN_no
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   19  # DW_AT_language
+.byte   5   # DW_FORM_data2
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   16  # DW_AT_stmt_list
+.byte   23  # DW_FORM_sec_offset
+.byte   27  # DW_AT_comp_dir
+.byte   8   # DW_FORM_string
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Lcu_begin0:
+.long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+.short  5   # DWARF version number
+.byte   1   # DWARF Unit Type
+.byte   8   # Address Size (in bytes)
+.long   .debug_abbrev   # Offset Into Abbrev. Section
+.byte   1   # Abbrev [1] 0xc:0x23 DW_TAG_compile_unit
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.short  12  # DW_AT_language
+.asciz  "file0.c"   # DW_AT_name
+.long   .Lline_table_begin  # DW_AT_stmt_list
+.asciz  "/" # DW_AT_comp_dir
+.quad   foo # DW_AT_low_pc
+.long   .Lfoo_end-foo   # DW_AT_high_pc
+.Ldebug_info_end0:
+
+.section.debug_line,"",@progbits
+.Lline_table_begin:
+.long .Lline_end-.Lline_start
+.Lline_start:
+.short  5   # DWARF version number
+.byte   8   # Address Size (in bytes)
+.byte   0   # Segment Selector Size
+.long   .Lheader_end-.Lheader_start
+.Lheader_start:
+.byte   1   # Minimum Instruction Length
+.byte   1   # Maximum Operations per Instruction
+.byte   1   # Default is_stmt
+.byte   0   # Line Base
+.byte   0   # Line Range
+.byte   5   # Opcode Base
+.byte   0, 1, 1, 1  # Standard Opcode Lengths
+
+# Directory table format
+.byte   1   # One element per directory entry
+.byte   1   # DW_LNCT_path
+.byte   0x08# DW_FORM_string
+
+# Directory table entries
+.byte   1   # 1 directory
+

[Lldb-commits] [lldb] 57f8a99 - [lldb] Don't put compile unit name into the support file list and support DWARF5 line tables

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-05T11:37:18+01:00
New Revision: 57f8a998ceaf36e021878e8810bb57a00452c07d

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

LOG: [lldb] Don't put compile unit name into the support file list and support 
DWARF5 line tables

Summary:
Lldb's "format-independent" debug info made use of the fact that DWARF
(<=4) did not use the file index zero, and reused the support file index
zero for storing the compile unit name.

While this provided some convenience for DWARF<=4, it meant that the PDB
plugin needed to artificially remap file indices in order to free up
index 0. Furthermore, DWARF v5 make file index 0 legal, which meant that
similar remapping would be needed in the dwarf plugin too.

What this patch does instead is remove the requirement of having the
compile unit name in the index 0. It is not that useful since the name
can always be fetched from the CompileUnit object. Remapping code in the
pdb plugin(s) has been removed or simplified.

DWARF plugin has started inserting an empty FileSpec at index 0 to
ensure the indices keep matching up (in case of DWARF<=4). For DWARF5,
we insert the file 0 from the line table.

I add a test to ensure we can correctly lookup line table entries
referencing file 0, and in particular the case where the file 0 is also
duplicated in another file entry, as this is how clang produces line
tables in some circumstances (see pr44170). Though this is probably a
bug in clang, this is not forbidden by DWARF, and lldb already has
support for that in some (but not all) cases -- this adds a test for the
code path which was not fixed in this patch.

Reviewers: clayborg, JDevlieghere, jdoerfert

Subscribers: aprantl, lldb-commits

Tags: #lldb

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/CompileUnit.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index b8575d13d457..fc8fe30101cb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -198,15 +198,23 @@ GetFileByIndex(const llvm::DWARFDebugLine::Prologue 
, size_t idx,
   return std::move(rel_path);
 }
 
-static FileSpecList ParseSupportFilesFromPrologue(
-const lldb::ModuleSP ,
-const llvm::DWARFDebugLine::Prologue , FileSpec::Style style,
-llvm::StringRef compile_dir = {}, FileSpec first_file = {}) {
+static FileSpecList
+ParseSupportFilesFromPrologue(const lldb::ModuleSP ,
+  const llvm::DWARFDebugLine::Prologue ,
+  FileSpec::Style style,
+  llvm::StringRef compile_dir = {}) {
   FileSpecList support_files;
-  support_files.Append(first_file);
+  size_t first_file = 0;
+  if (prologue.getVersion() <= 4) {
+// File index 0 is not valid before DWARF v5. Add a dummy entry to ensure
+// support file list indices match those we get from the debug info and 
line
+// tables.
+support_files.Append(FileSpec());
+first_file = 1;
+  }
 
   const size_t number_of_files = prologue.FileNames.size();
-  for (size_t idx = 1; idx <= number_of_files; ++idx) {
+  for (size_t idx = first_file; idx <= number_of_files; ++idx) {
 std::string remapped_file;
 if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
   if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
@@ -1046,8 +1054,7 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit 
_unit) {
 
   comp_unit.SetSupportFiles(ParseSupportFilesFromPrologue(
   comp_unit.GetModule(), line_table->Prologue, dwarf_cu->GetPathStyle(),
-  dwarf_cu->GetCompilationDirectory().GetCString(),
-  comp_unit.GetPrimaryFile()));
+  dwarf_cu->GetCompilationDirectory().GetCString()));
 
   return true;
 }

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index f0308e23c9d7..22d1b08ea9e7 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1110,9 +1110,7 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit 
_unit) {
   // LLDB wants the index of the file in the list of support files.
   auto fn_iter = llvm::find(cci->m_file_list, *efn);
   lldbassert(fn_iter != cci->m_file_list.end());
-  // LLDB support 

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream into namespace and let them take raw_ostream

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

All our namespace names are `snake_case`, so this new namespace should be too. 
Though, honestly, I don't think this needs to be a namespace -- I'd probably 
just make the function names a bit more descriptive... Maybe 
DumpAddress/DumpAddressRange, to match the existing 
DumpRegisterValue/DumpDataExtractor ?




Comment at: 
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/TestBasicVector.py:23-53
-self.expect("expr (size_t)a.size()", substrs=['(size_t) $0 = 3'])
-self.expect("expr (int)a.front()", substrs=['(int) $1 = 3'])
-self.expect("expr (int)a[1]", substrs=['(int) $2 = 1'])
-self.expect("expr (int)a.back()", substrs=['(int) $3 = 2'])
+self.expect("expr a.size()", substrs=[' $0 = 3'])
+self.expect("expr a.front()", substrs=[' $1 = 3'])
+self.expect("expr a[1]", substrs=[' $2 = 1'])
+self.expect("expr a.back()", substrs=[' $3 = 2'])
 
 self.expect("expr std::sort(a.begin(), a.end())")
-self.expect("expr (int)a.front()", substrs=['(int) $4 = 1'])

huh ?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71052



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

[Lldb-commits] [PATCH] D71052: [lldb][NFC] Move Address and AddressRange functions out of Stream into namespace and let them take raw_ostream

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: labath.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

Yet another step on the long road towards getting rid of lldb's Stream class.

We probably should just make this some kind of member of Address/AddressRange, 
but it seems quite often we just push
in random integers in there and this is just about getting rid of Stream and 
not improving arbitrary APIs.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71052

Files:
  lldb/include/lldb/Utility/Stream.h
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/TestBasicVector.py
  lldb/source/Core/Address.cpp
  lldb/source/Core/AddressRange.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Target/ThreadPlanRunToAddress.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Target/ThreadPlanStepInstruction.cpp
  lldb/source/Target/ThreadPlanStepOverRange.cpp
  lldb/source/Target/ThreadPlanStepThrough.cpp
  lldb/source/Utility/Stream.cpp
  lldb/source/Utility/VMRange.cpp
  lldb/unittests/Utility/StreamTest.cpp

Index: lldb/unittests/Utility/StreamTest.cpp
===
--- lldb/unittests/Utility/StreamTest.cpp
+++ lldb/unittests/Utility/StreamTest.cpp
@@ -37,94 +37,94 @@
 }
 
 TEST_F(StreamTest, AddressPrefix) {
-  s.Address(0x1, 1, "foo");
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, "foo");
   EXPECT_EQ("foo0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressEmptyPrefix) {
-  s.Address(0x1, 1, nullptr);
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, "");
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSuffix) {
-  s.Address(0x1, 1, nullptr, "foo");
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, nullptr, "foo");
   EXPECT_EQ("0x01foo", TakeValue());
 }
 
 TEST_F(StreamTest, AddressNoSuffix) {
-  s.Address(0x1, 1, nullptr, nullptr);
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, nullptr, nullptr);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0x1, 1, nullptr, "");
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, nullptr, "");
   EXPECT_EQ("0x01", TakeValue());
 }
 
 TEST_F(StreamTest, AddressPrefixAndSuffix) {
-  s.Address(0x1, 1, "foo", "bar");
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1, "foo", "bar");
   EXPECT_EQ("foo0x01bar", TakeValue());
 }
 
 TEST_F(StreamTest, AddressSize) {
-  s.Address(0x0, 0);
+  StreamUtils::Address(s.AsRawOstream(), 0x0, 0);
   EXPECT_EQ("0x0", TakeValue());
-  s.Address(0x1, 0);
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 0);
   EXPECT_EQ("0x1", TakeValue());
 
-  s.Address(0x1, 1);
+  StreamUtils::Address(s.AsRawOstream(), 0x1, 1);
   EXPECT_EQ("0x01", TakeValue());
-  s.Address(0xf1, 1);
+  StreamUtils::Address(s.AsRawOstream(), 0xf1, 1);
   EXPECT_EQ("0xf1", TakeValue());
-  s.Address(0xff, 1);
+  StreamUtils::Address(s.AsRawOstream(), 0xff, 1);
   EXPECT_EQ("0xff", TakeValue());
-  s.Address(0x100, 1);
+  StreamUtils::Address(s.AsRawOstream(), 0x100, 1);
   EXPECT_EQ("0x100", TakeValue());
 
-  s.Address(0xf00, 4);
+  StreamUtils::Address(s.AsRawOstream(), 0xf00, 4);
   EXPECT_EQ("0x0f00", TakeValue());
-  s.Address(0x100, 8);
+  StreamUtils::Address(s.AsRawOstream(), 0x100, 8);
   EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x100, 10);
+  StreamUtils::Address(s.AsRawOstream(), 0x100, 10);
   EXPECT_EQ("0x0100", TakeValue());
-  s.Address(0x1234, 10);
+  StreamUtils::Address(s.AsRawOstream(), 0x1234, 10);
   EXPECT_EQ("0x1234", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRange) {
-  s.AddressRange(0x100, 0x101, 2);
+  StreamUtils::AddressRange(s.AsRawOstream(), 0x100, 0x101, 2);
   EXPECT_EQ("[0x0100-0x0101)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeEmptyRange) {
-  s.AddressRange(0x100, 0x100, 2);
+  StreamUtils::AddressRange(s.AsRawOstream(), 0x100, 0x100, 2);
   EXPECT_EQ("[0x0100-0x0100)", TakeValue());
-  s.AddressRange(0x0, 0x0, 2);
+  StreamUtils::AddressRange(s.AsRawOstream(), 0x0, 0x0, 2);
   EXPECT_EQ("[0x-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeInvalidRange) {
-  s.AddressRange(0x100, 0x0FF, 2);
+  StreamUtils::AddressRange(s.AsRawOstream(), 0x100, 0x0FF, 2);
   EXPECT_EQ("[0x0100-0x00ff)", TakeValue());
-  s.AddressRange(0x100, 0x0, 2);
+  StreamUtils::AddressRange(s.AsRawOstream(), 0x100, 0x0, 2);
   EXPECT_EQ("[0x0100-0x)", TakeValue());
 }
 
 TEST_F(StreamTest, AddressRangeSize) {
-  s.AddressRange(0x100, 0x101, 0);
+  StreamUtils::AddressRange(s.AsRawOstream(), 0x100, 0x101, 0);
   EXPECT_EQ("[0x100-0x101)", TakeValue());
-  s.AddressRange(0x100, 0x101, 

[Lldb-commits] [lldb] 4b4ede4 - Reland "[LiveDebugValues] Introduce entry values of unmodified params"

[Djordje Todorovic via lldb-commits]

Author: Djordje Todorovic
Date: 2019-12-05T11:10:49+01:00
New Revision: 4b4ede440a2ad51b150cb913775eee76189aac38

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

LOG: Reland "[LiveDebugValues] Introduce entry values of unmodified params"

Relanding this after resolving the cause of the test failure.

Added: 
llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/kill-entry-value-after-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir

Modified: 

lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py

lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
llvm/lib/CodeGen/LiveDebugValues.cpp
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
index 1adf3fc44a69..a49ffa84c547 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
@@ -1,5 +1,5 @@
 LEVEL = ../../../make
 CXX_SOURCES := main.cpp
 include $(LEVEL)/Makefile.rules
-CXXFLAGS_EXTRAS := -O1 -glldb -Xclang -femit-debug-entry-values
+CXXFLAGS_EXTRAS := -O2 -glldb -Xclang -femit-debug-entry-values
 include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
index 1192c2b672f6..e0285e6d626d 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
@@ -6,8 +6,7 @@
 supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())
 
 lldbinline.MakeInlineTest(__file__, globals(),
-[decorators.skipIf(bugnumber="llvm.org/pr44059"),
- decorators.skipUnlessPlatform(supported_platforms),
+[decorators.skipUnlessPlatform(supported_platforms),
  decorators.skipIf(compiler="clang", compiler_version=['<', '10.0']),
  decorators.skipUnlessArch('x86_64'),
  decorators.skipUnlessHasCallSiteInfo,

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
index ff72a81c6b29..9aac6e947838 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
@@ -18,6 +18,14 @@ template __attribute__((noinline)) void use(T x) 
{
   /* Clobbers */ : "rsi" \
   );
 
+// Destroy %rbx in the current frame.
+#define DESTROY_RBX \
+  asm volatile ("xorq %%rbx, %%rbx" \
+  /* Outputs */  : \
+  /* Inputs */   : \
+  /* Clobbers */ : "rbx" \
+  );
+
 struct S1 {
   int field1 = 123;
   int *field2 = 
@@ -30,10 +38,17 @@ void func1(int , int x) {
   // Destroy 'x' in the current frame.
   DESTROY_RSI;
 
-  //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=FUNC1-DESC")
-  // FUNC1-DESC: name = "x", type = "int", location = 
DW_OP_entry_value(DW_OP_reg4 RSI)
+  // NOTE: Currently, we do not generate DW_OP_entry_value for the 'x',
+  // since it gets copied into a register that is not callee saved,
+  // and we can not guarantee that its value has not changed.
 
   ++sink;
+
+  // Destroy 'sink' in the current frame.
+  DESTROY_RBX;
+
+  //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=FUNC1-DESC")
+  // FUNC1-DESC: name = "sink", type = "int &", location = 
DW_OP_entry_value(DW_OP_reg5 RDI)
 }
 
 __attribute__((noinline))
@@ -43,10 +58,16 @@ void func2(int , int x) {
   // Destroy 'x' in the current frame.
   DESTROY_RSI;
 
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR")
-  // FUNC2-EXPR: (int) ${{.*}} = 123
+  //% self.filecheck("expr x", "main.cpp", 

[Lldb-commits] [PATCH] D70155: [LLDB] Avoid triple corruption while merging core info from platform and target triples

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b8185bb1b45: Avoid triple corruption while merging core 
info (authored by omjavaid).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70155

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/Utility/ArchSpecTest.cpp


Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -216,6 +216,41 @@
 EXPECT_EQ(llvm::Triple::EnvironmentType::UnknownEnvironment,
   A.GetTriple().getEnvironment());
   }
+  {
+ArchSpec A("arm--linux-eabihf");
+ArchSpec B("armv8l--linux-gnueabihf");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::ArchType::arm, B.GetTriple().getArch());
+
+EXPECT_EQ(ArchSpec::eCore_arm_generic, A.GetCore());
+EXPECT_EQ(ArchSpec::eCore_arm_armv8l, B.GetCore());
+
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  B.GetTriple().getVendor());
+
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+
+EXPECT_EQ(llvm::Triple::EnvironmentType::EABIHF,
+  A.GetTriple().getEnvironment());
+EXPECT_EQ(llvm::Triple::EnvironmentType::GNUEABIHF,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
+EXPECT_EQ(ArchSpec::eCore_arm_armv8l, A.GetCore());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::EABIHF,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -868,7 +868,7 @@
   IsCompatibleMatch(other) && GetCore() == ArchSpec::eCore_arm_generic &&
   other.GetCore() != ArchSpec::eCore_arm_generic) {
 m_core = other.GetCore();
-CoreUpdated(true);
+CoreUpdated(false);
   }
   if (GetFlags() == 0) {
 SetFlags(other.GetFlags());


Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -216,6 +216,41 @@
 EXPECT_EQ(llvm::Triple::EnvironmentType::UnknownEnvironment,
   A.GetTriple().getEnvironment());
   }
+  {
+ArchSpec A("arm--linux-eabihf");
+ArchSpec B("armv8l--linux-gnueabihf");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::ArchType::arm, B.GetTriple().getArch());
+
+EXPECT_EQ(ArchSpec::eCore_arm_generic, A.GetCore());
+EXPECT_EQ(ArchSpec::eCore_arm_armv8l, B.GetCore());
+
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  B.GetTriple().getVendor());
+
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+
+EXPECT_EQ(llvm::Triple::EnvironmentType::EABIHF,
+  A.GetTriple().getEnvironment());
+EXPECT_EQ(llvm::Triple::EnvironmentType::GNUEABIHF,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
+EXPECT_EQ(ArchSpec::eCore_arm_armv8l, A.GetCore());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::EABIHF,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -868,7 +868,7 @@
   IsCompatibleMatch(other) && GetCore() == ArchSpec::eCore_arm_generic &&
   other.GetCore() != ArchSpec::eCore_arm_generic) {
 m_core = other.GetCore();
-CoreUpdated(true);
+CoreUpdated(false);
   }
   if (GetFlags() == 0) {
 SetFlags(other.GetFlags());
___
lldb-commits mailing list

[Lldb-commits] [lldb] 8b8185b - Avoid triple corruption while merging core info

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2019-12-05T13:10:04+05:00
New Revision: 8b8185bb1b456e0ccf7b1ed1f00bc646853ab004

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

LOG: Avoid triple corruption while merging core info

Summary:
This patch fixes a bug where when target triple created from elf information
is arm-*-linux-eabihf and platform triple is armv8l-*-linux-gnueabihf. Merging
both triple results in armv8l--unknown-unknown.

This happens because we order a triple update while calling CoreUpdated and
CoreUpdated creates a new triple with no vendor or environment information.

Making sure we do not update triple and just update to more specific core
fixes the issue.

Reviewers: labath, jasonmolenda, clayborg

Reviewed By: jasonmolenda

Subscribers: jankratochvil, kristof.beyls, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Utility/ArchSpec.cpp
lldb/unittests/Utility/ArchSpecTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index 2bebecb2c677..bbfa5cf61d01 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -868,7 +868,7 @@ void ArchSpec::MergeFrom(const ArchSpec ) {
   IsCompatibleMatch(other) && GetCore() == ArchSpec::eCore_arm_generic &&
   other.GetCore() != ArchSpec::eCore_arm_generic) {
 m_core = other.GetCore();
-CoreUpdated(true);
+CoreUpdated(false);
   }
   if (GetFlags() == 0) {
 SetFlags(other.GetFlags());

diff  --git a/lldb/unittests/Utility/ArchSpecTest.cpp 
b/lldb/unittests/Utility/ArchSpecTest.cpp
index 0186ff05ead8..9115808c1258 100644
--- a/lldb/unittests/Utility/ArchSpecTest.cpp
+++ b/lldb/unittests/Utility/ArchSpecTest.cpp
@@ -216,6 +216,41 @@ TEST(ArchSpecTest, MergeFrom) {
 EXPECT_EQ(llvm::Triple::EnvironmentType::UnknownEnvironment,
   A.GetTriple().getEnvironment());
   }
+  {
+ArchSpec A("arm--linux-eabihf");
+ArchSpec B("armv8l--linux-gnueabihf");
+
+EXPECT_TRUE(A.IsValid());
+EXPECT_TRUE(B.IsValid());
+
+EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
+EXPECT_EQ(llvm::Triple::ArchType::arm, B.GetTriple().getArch());
+
+EXPECT_EQ(ArchSpec::eCore_arm_generic, A.GetCore());
+EXPECT_EQ(ArchSpec::eCore_arm_armv8l, B.GetCore());
+
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  B.GetTriple().getVendor());
+
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+
+EXPECT_EQ(llvm::Triple::EnvironmentType::EABIHF,
+  A.GetTriple().getEnvironment());
+EXPECT_EQ(llvm::Triple::EnvironmentType::GNUEABIHF,
+  B.GetTriple().getEnvironment());
+
+A.MergeFrom(B);
+EXPECT_EQ(llvm::Triple::ArchType::arm, A.GetTriple().getArch());
+EXPECT_EQ(ArchSpec::eCore_arm_armv8l, A.GetCore());
+EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+  A.GetTriple().getVendor());
+EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+EXPECT_EQ(llvm::Triple::EnvironmentType::EABIHF,
+  A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {



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