date:20190909

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

2019-09-09 Thread Sam McCall via Phabricator via lldb-commits

sammccall added a subscriber: benlangmuir.
sammccall added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1051
 std::error_code
 RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
+  // Don't change the working directory if the path doesn't exist.

This is a change in behavior, in that the WD of the underlying filesystem is no 
longer set, so e.g. in overlay mode RedirectingFileSystem::status() will look 
for non-overlaid relative paths in the wrong directory.

e.g. if we're using an overlay VFS with no files mapped and external WD is 
/foo, `cd /bar; stat baz` reads /bar/baz before this patch and reads /foo/baz 
after it. The old behavior seems more logical.

I think the right thing to do is to have setCurrentWorkingDirectory() call 
ExternalFS->setCurrentWorkingDirectory() if overlay mode is on. It shouldn't 
propagate failure, but rather record it `bool ExternalFSValidWD`, and only 
allow fallthrough operations when this flag is true.
This means cd into a dir which is invalid in the underlying FS would put it 
into a "broken" state where it's no longer consulted, but an (absolute) cd to a 
valid directory would fix it. (Variations are possible, depending on how you 
want to handle relative cd when the CWD is invalid in the underlying FS).

If you would rather just change the behavior here, you probably need to talk to 
someone familiar with the existing clients. It seems mostly used by clang's 
`-ivfsoverlay`, which was added by @benlangmuir way back when. I don't really 
know what it's used for.


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

2019-09-09 Thread Sam McCall via Phabricator via lldb-commits

sammccall added a comment.

BTW, This approach does look much better to me, it fits well into the VFS 
abstraction and I'd argue is fixing a bug in RedirectingFileSystem.
We do need to be careful about introducing new bugs, though. (or avoid the 
generality and just implement the parts LLDB needs, as a separate vfs::FS)


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

2019-09-09 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

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

labath, please see my answer to your question and maybe help me out why you 
think I don't test what I want to test.




Comment at: lldb/lit/Breakpoint/minidebuginfo-set-and-hit-breakpoint.test:50
+
+# RUN: LD_LIBRARY_PATH=$LD_LIBRARY_PATH:%T %lldb -x -b -o 'b multiplyByThree' 
-o 'b multiplyByFour' -o 'breakpoint list -v' -o 'run' -o 'continue' %t.binary 
| FileCheck %s
+

labath wrote:
> Instead of messing with the environment, it might be better to just set the 
> rpath (`-Wl,-rpath`) of the executable appropriately (but I would still like 
> to know what you're exactly trying to test with that extra shared library).
labath, I'd like to test the following scenario: Set a breakpoint on a symbol 
(`multiplyByThree`) in the `.dynsym` section and one on a symbol 
(`multiplyByFour`) from the .`symtab` that's embedded within the 
`.gnu_debugdata` section. I have no clue how to get those symbols without using 
a shared lib. Do you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

[Lldb-commits] [PATCH] D67347: [Windows] Use EH info from the PE32 exceptions directory to construct unwind plans

2019-09-09 Thread Aleksandr Urakov via Phabricator via lldb-commits

aleksandr.urakov created this revision.
Herald added subscribers: llvm-commits, lldb-commits, JDevlieghere, MaskRay, 
arichardson, aprantl, mgorny, emaste.
Herald added a reviewer: espindola.
Herald added projects: LLDB, LLVM.

This patch adds an implementation of reconstructing of unwind plans from PE EH 
info.

To achieve the goal the `ICallFrameInfo` abstraction was made. It is based on 
the `DWARFCallFrameInfo` class interface with a few changes to make it less 
DWARF-specific.

To implement the new interface for PECOFF object files the class 
`PECallFrameInfo` was written. It uses the next helper classes:

- `UnwindCodesIterator` helps to iterate through `UnwindCode` structures (and 
processes chained infos transparently);
- `EHProgramBuilder` with the use of `UnwindCodesIterator` constructs 
`EHProgram`;


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67347

Files:
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/include/lldb/Symbol/ICallFrameInfo.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Symbol/UnwindTable.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Symbol/FuncUnwinders.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/source/Symbol/UnwindTable.cpp
  lldb/unittests/Symbol/CMakeLists.txt
  lldb/unittests/Symbol/TestPECallFrameInfo.cpp
  llvm/include/llvm/Support/Win64EH.h

Index: llvm/include/llvm/Support/Win64EH.h
===
--- llvm/include/llvm/Support/Win64EH.h
+++ llvm/include/llvm/Support/Win64EH.h
@@ -30,7 +30,9 @@
   UOP_SetFPReg,
   UOP_SaveNonVol,
   UOP_SaveNonVolBig,
-  UOP_SaveXMM128 = 8,
+  UOP_Epilog,
+  UOP_SpareCode,
+  UOP_SaveXMM128,
   UOP_SaveXMM128Big,
   UOP_PushMachFrame,
   // The following set of unwind opcodes is for ARM64.  They are documented at
Index: lldb/unittests/Symbol/TestPECallFrameInfo.cpp
===
--- /dev/null
+++ lldb/unittests/Symbol/TestPECallFrameInfo.cpp
@@ -0,0 +1,344 @@
+//===-- TestPECallFrameInfo.cpp --*- C++ -*-===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
+
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ICallFrameInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
+
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+class PECallFrameInfoTest : public testing::Test {
+public:
+  void SetUp() override {
+FileSystem::Initialize();
+HostInfo::Initialize();
+ObjectFilePECOFF::Initialize();
+SymbolFileSymtab::Initialize();
+  }
+
+  void TearDown() override {
+SymbolFileSymtab::Terminate();
+ObjectFilePECOFF::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+
+protected:
+  void GetUnwindPlan(addr_t file_addr, UnwindPlan &plan) const;
+};
+
+void PECallFrameInfoTest::GetUnwindPlan(addr_t file_addr, UnwindPlan &plan) const {
+  llvm::Expected ExpectedFile = TestFile::fromYaml(
+  R"(
+--- !COFF
+OptionalHeader:  
+  AddressOfEntryPoint: 0
+  ImageBase:   16777216
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable: 
+RelativeVirtualAddress: 0
+Size:0
+  ImportTable: 
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:   
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:  
+RelativeVirtualAddress: 12288
+Size:60
+

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

2019-09-09 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

At the object file level I would love to see much less of this specific unwind 
info making it into the ObjectFile interface. Why can't we just ask the 
ObjectFile to provide an UnwindPlan for a given address. There is so much 
complexity within the UnwindPlan where it is managing all these specific unwind 
plans. IMHO we should just ask the ObjectFile for an UnwindPlan and let the 
ObjectFile decide which one is best. Right now UnwindPlan manages a ton of 
different kinds of unwind alternatives and UnwindPlan has all sorts of 
knowledge about each specific unwind plan type (ARM compact unwind, Apple 
compact unwind, EH frame, .debug_frame, arch default, and more). We seem to be 
adding to this complexity here by making all ObjectFile instances having to 
know how to create each different type when UnwindPlan already has the all the 
abstraction we need in UnwindPlan::Row.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67347



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

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

2019-09-09 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

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



Comment at: lldb/lit/Breakpoint/minidebuginfo-set-and-hit-breakpoint.test:50
+
+# RUN: LD_LIBRARY_PATH=$LD_LIBRARY_PATH:%T %lldb -x -b -o 'b multiplyByThree' 
-o 'b multiplyByFour' -o 'breakpoint list -v' -o 'run' -o 'continue' %t.binary 
| FileCheck %s
+

kwk wrote:
> labath wrote:
> > Instead of messing with the environment, it might be better to just set the 
> > rpath (`-Wl,-rpath`) of the executable appropriately (but I would still 
> > like to know what you're exactly trying to test with that extra shared 
> > library).
> labath, I'd like to test the following scenario: Set a breakpoint on a symbol 
> (`multiplyByThree`) in the `.dynsym` section and one on a symbol 
> (`multiplyByFour`) from the .`symtab` that's embedded within the 
> `.gnu_debugdata` section. I have no clue how to get those symbols without 
> using a shared lib. Do you?
I found a better and simpler way thanks to the help from @jankratochvil . Will 
implement it soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

2019-09-09 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added inline comments.



Comment at: lldb/source/Host/common/LZMA.cpp:124
+   llvm::SmallVectorImpl &Uncompressed) {
+  if (InputBuffer.size() == 0)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),

empty



Comment at: lldb/source/Host/common/LZMA.cpp:129
+  uint64_t uncompressedSize = 0;
+  auto err = getUncompressedSize(InputBuffer, uncompressedSize);
+  if (err)

if (auto err = ...)
  return err;



Comment at: lldb/source/Host/common/LZMA.cpp:136
+  // Decompress xz buffer to buffer.
+  uint64_t memlimit(UINT64_MAX);
+  size_t inpos = 0;

= is more common



Comment at: lldb/source/Host/common/LZMA.cpp:138
+  size_t inpos = 0;
+  inpos = 0;
+  size_t outpos = 0;

stray `inpos = 0;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

[Lldb-commits] [lldb] r371417 - LLDB - Simplify GetProgramFileSpec

2019-09-09 Thread David Carlier via lldb-commits

Author: devnexen
Date: Mon Sep  9 09:10:14 2019
New Revision: 371417

URL: http://llvm.org/viewvc/llvm-project?rev=371417&view=rev
Log:
LLDB - Simplify GetProgramFileSpec

Reviewers: zturner, emaste

Reviewed By: emaste

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

Modified:
lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp

Modified: lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp?rev=371417&r1=371416&r2=371417&view=diff
==
--- lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp (original)
+++ lldb/trunk/source/Host/freebsd/HostInfoFreeBSD.cpp Mon Sep  9 09:10:14 2019
@@ -64,13 +64,10 @@ FileSpec HostInfoFreeBSD::GetProgramFile
   static FileSpec g_program_filespec;
   if (!g_program_filespec) {
 int exe_path_mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, getpid()};
-size_t exe_path_size;
-if (sysctl(exe_path_mib, 4, NULL, &exe_path_size, NULL, 0) == 0) {
-  char *exe_path = new char[exe_path_size];
-  if (sysctl(exe_path_mib, 4, exe_path, &exe_path_size, NULL, 0) == 0)
-g_program_filespec.SetFile(exe_path, FileSpec::Style::native);
-  delete[] exe_path;
-}
+char exe_path[PATH_MAX];
+size_t exe_path_size = sizeof(exe_path);
+if (sysctl(exe_path_mib, 4, exe_path, &exe_path_size, NULL, 0) == 0)
+  g_program_filespec.SetFile(exe_path, false);
   }
   return g_program_filespec;
 }


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

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

2019-09-09 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

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

- Remove -x from %lldb because that expands to the command which already 
includes it
- Improve and fix the minidebuginfo-set-and-hit-breakpoint.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/LZMA.h
  lldb/lit/CMakeLists.txt
  lldb/lit/Modules/ELF/Inputs/minidebuginfo-main.c
  lldb/lit/Modules/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/lit/Modules/ELF/minidebuginfo-find-symbols.yaml
  lldb/lit/Modules/ELF/minidebuginfo-no-lzma.yaml
  lldb/lit/Modules/ELF/minidebuginfo-set-and-hit-breakpoint.test
  lldb/lit/lit.cfg.py
  lldb/lit/lit.site.cfg.py.in
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/LZMA.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -208,6 +208,10 @@
   /// Collection of symbols from the dynamic table.
   DynamicSymbolColl m_dynamic_symbols;
 
+  /// Object file parsed from .gnu_debugdata section (\sa
+  /// GetGnuDebugDataObjectFile())
+  std::shared_ptr m_gnu_debug_data_object_file;
+
   /// List of file specifications corresponding to the modules (shared
   /// libraries) on which this object file depends.
   mutable std::unique_ptr m_filespec_up;
@@ -383,6 +387,14 @@
   lldb_private::UUID &uuid);
 
   bool AnySegmentHasPhysicalAddress();
+  
+  /// Takes the .gnu_debugdata and returns the decompressed object file that is
+  /// stored within that section.
+  ///
+  /// \returns either the decompressed object file stored within the
+  /// .gnu_debugdata section or \c nullptr if an error occured or if there's no
+  /// section with that name.
+  std::shared_ptr GetGnuDebugDataObjectFile();
 };
 
 #endif // liblldb_ObjectFileELF_h_
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Host/LZMA.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/SectionLoadList.h"
@@ -1842,6 +1843,70 @@
   // unified section list.
   if (GetType() != eTypeDebugInfo)
 unified_section_list = *m_sections_up;
+  
+  // If there's a .gnu_debugdata section, we'll try to read the .symtab that's
+  // embedded in there and replace the one in the original object file (if any).
+  // If there's none in the orignal object file, we add it to it.
+  if (auto gdd_obj_file = GetGnuDebugDataObjectFile()) {
+if (auto gdd_objfile_section_list = gdd_obj_file->GetSectionList()) {
+  if (SectionSP symtab_section_sp =
+  gdd_objfile_section_list->FindSectionByType(
+  eSectionTypeELFSymbolTable, true)) {
+SectionSP module_section_sp = unified_section_list.FindSectionByType(
+eSectionTypeELFSymbolTable, true);
+if (module_section_sp)
+  unified_section_list.ReplaceSection(module_section_sp->GetID(),
+  symtab_section_sp);
+else
+  unified_section_list.AddSection(symtab_section_sp);
+  }
+}
+  }  
+}
+
+std::shared_ptr ObjectFileELF::GetGnuDebugDataObjectFile() {
+  if (m_gnu_debug_data_object_file != nullptr)
+return m_gnu_debug_data_object_file;
+
+  SectionSP section =
+  GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata"));
+  if (!section)
+return nullptr;
+
+  if (!lldb_private::lzma::isAvailable()) {
+GetModule()->ReportWarning(
+"No LZMA support found for reading .gnu_debugdata section");
+return nullptr;
+  }
+
+  // Uncompress the data
+  DataExtractor data;
+  section->GetSectionData(data);
+  llvm::SmallVector uncompressedData;
+  auto err = lldb_private::lzma::uncompress(data.GetData(), uncompressedData);
+  if (err) {
+GetModule()->ReportWarning(
+"An error occurred while decompression the section %s: %s",
+section->GetName().AsCString(), llvm::toString(std::move(err)).c_str());
+return nullptr;
+  }
+
+  // Construct ObjectFileELF object from decompressed buffer
+  DataBufferSP gdd_data_buf(
+  new DataBufferHeap(uncompressedData.data(), uncompressedData.size()));
+  auto fspec = GetFileSpec().CopyByAppendingPathComponent(
+  llvm::StringRef("gnu_debugdata"));
+  m_gnu_debug_data_object_file.reset(new ObjectFileELF(
+

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

2019-09-09 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk added a comment.

@labath I have greatly improved the set and hit breakpoint test. Thank you 
again @jankratochvil for bringing my attention to `-Wl,--dynamic-list`. That 
helped to put one symbol in `.dynsym` explicitly. I will see if I can utilize 
that knowledge to separate out the patch this comment of yours @labath :

> Let's put this bit into a separate patch. As I said over IRC, I view this bit 
> as functionally independent of the debugdata stuff (it's definitely 
> independent in it's current form, as it will kick in for non-debugdata files 
> too, which I think is fine).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

[Lldb-commits] [PATCH] D67022: Enhance SymbolFileDWARF::ParseDeclsForContext performance

2019-09-09 Thread Guilherme Andrade via Phabricator via lldb-commits

guiandrade updated this revision to Diff 219372.
guiandrade edited the summary of this revision.
guiandrade added a comment.
Herald added subscribers: kadircet, ilya-biryukov.

Falling back to the previous workaround to test 
EnsureAllDIEsInDeclContextHaveBeenParsed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67022

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include "Plugins/SymbolFile/DWARF/DWARFASTParserClang.h"
@@ -19,29 +20,36 @@
 public:
   using DWARFASTParserClang::DWARFASTParserClang;
   using DWARFASTParserClang::LinkDeclContextToDIE;
+
+  std::vector GetDeclContextToDIEMapKeys() {
+std::vector keys;
+for (const auto &it : m_decl_ctx_to_die)
+  keys.push_back(it.first);
+return keys;
+  }
 };
 } // namespace
 
 // If your implementation needs to dereference the dummy pointers we are
 // defining here, causing this test to fail, feel free to delete it.
 TEST(DWARFASTParserClangTests,
- TestGetDIEForDeclContextReturnsOnlyMatchingEntries) {
+ EnsureAllDIEsInDeclContextHaveBeenParsedParsesOnlyMatchingEntries) {
   ClangASTContext ast_ctx;
   DWARFASTParserClangStub ast_parser(ast_ctx);
 
   DWARFUnit *unit = nullptr;
-  DWARFDIE die1(unit, (DWARFDebugInfoEntry *)1LL);
-  DWARFDIE die2(unit, (DWARFDebugInfoEntry *)2LL);
-  DWARFDIE die3(unit, (DWARFDebugInfoEntry *)3LL);
-  DWARFDIE die4(unit, (DWARFDebugInfoEntry *)4LL);
-  ast_parser.LinkDeclContextToDIE((clang::DeclContext *)1LL, die1);
-  ast_parser.LinkDeclContextToDIE((clang::DeclContext *)2LL, die2);
-  ast_parser.LinkDeclContextToDIE((clang::DeclContext *)2LL, die3);
-  ast_parser.LinkDeclContextToDIE((clang::DeclContext *)3LL, die4);
-
-  auto die_list = ast_parser.GetDIEForDeclContext(
-  CompilerDeclContext(nullptr, (clang::DeclContext *)2LL));
-  ASSERT_EQ(2u, die_list.size());
-  ASSERT_EQ(die2, die_list[0]);
-  ASSERT_EQ(die3, die_list[1]);
+  std::vector dies = {DWARFDIE(unit, (DWARFDebugInfoEntry *)1LL),
+DWARFDIE(unit, (DWARFDebugInfoEntry *)2LL),
+DWARFDIE(unit, (DWARFDebugInfoEntry *)3LL),
+DWARFDIE(unit, (DWARFDebugInfoEntry *)4LL)};
+  std::vector decl_ctxs = {
+  (clang::DeclContext *)1LL, (clang::DeclContext *)2LL,
+  (clang::DeclContext *)2LL, (clang::DeclContext *)3LL};
+  for (int i = 0; i < 4; ++i)
+ast_parser.LinkDeclContextToDIE(decl_ctxs[i], dies[i]);
+  ast_parser.EnsureAllDIEsInDeclContextHaveBeenParsed(
+  CompilerDeclContext(nullptr, decl_ctxs[1]));
+
+  EXPECT_THAT(ast_parser.GetDeclContextToDIEMapKeys(),
+  testing::UnorderedElementsAre(decl_ctxs[0], decl_ctxs[3]));
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1204,16 +1204,9 @@
 
 void SymbolFileDWARF::ParseDeclsForContext(CompilerDeclContext decl_ctx) {
   auto *type_system = decl_ctx.GetTypeSystem();
-  if (!type_system)
-return;
-  DWARFASTParser *ast_parser = type_system->GetDWARFParser();
-  std::vector decl_ctx_die_list =
-  ast_parser->GetDIEForDeclContext(decl_ctx);
-
-  for (DWARFDIE decl_ctx_die : decl_ctx_die_list)
-for (DWARFDIE decl = decl_ctx_die.GetFirstChild(); decl;
- decl = decl.GetSibling())
-  ast_parser->GetDeclForUIDFromDWARF(decl);
+  if (type_system != nullptr)
+type_system->GetDWARFParser()->EnsureAllDIEsInDeclContextHaveBeenParsed(
+decl_ctx);
 }
 
 user_id_t SymbolFileDWARF::GetUID(DIERef ref) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -51,8 +51,8 @@
   lldb_private::CompilerDecl
   GetDeclForUIDFromDWARF(const DWARFDIE &die) override;
 
-  std::vector
-  GetDIEForDeclContext(lldb_private::CompilerDeclContext decl_context) override;
+  void EnsureAllDIEsInDeclContextHaveBeenParsed(
+  lldb_private::CompilerDeclContext decl_context) override;
 
   lldb_pr

[Lldb-commits] [PATCH] D67022: Enhance SymbolFileDWARF::ParseDeclsForContext performance

2019-09-09 Thread Guilherme Andrade via Phabricator via lldb-commits

guiandrade added a comment.

Maybe we could use the previous workaround until we find something better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67022



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

[Lldb-commits] [PATCH] D67022: Enhance SymbolFileDWARF::ParseDeclsForContext performance

2019-09-09 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

LGTM. Test seems about as good as we can do. Pavel, you ok with this now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67022



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 219398.
JDevlieghere added a comment.

Implement Sam's suggestion for changing the external file system's working 
directory.


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

https://reviews.llvm.org/D65677

Files:
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1994,3 +1994,120 @@
   EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),
 errc::no_such_file_or_directory);
 }
+
+TEST_F(VFSFromYAMLTest, WorkingDirectory) {
+  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/");
+  Lower->addDirectory("//root/foo");
+  Lower->addRegularFile("//root/foo/a");
+  Lower->addRegularFile("//root/foo/b");
+  IntrusiveRefCntPtr FS = getFromYAMLString(
+  "{ 'use-external-names': false,\n"
+  "  'roots': [\n"
+  "{\n"
+  "  'type': 'directory',\n"
+  "  'name': '//root/',\n"
+  "  'contents': [ {\n"
+  "  'type': 'file',\n"
+  "  'name': 'bar/a',\n"
+  "  'external-contents': '//root/foo/a'\n"
+  "}\n"
+  "  ]\n"
+  "}\n"
+  "]\n"
+  "}",
+  Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+  std::error_code EC = FS->setCurrentWorkingDirectory("//root/bar/");
+  ASSERT_FALSE(EC);
+
+  llvm::ErrorOr WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+
+  llvm::ErrorOr Status = FS->status("./a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->isStatusKnown());
+  EXPECT_FALSE(Status->isDirectory());
+  EXPECT_TRUE(Status->isRegularFile());
+  EXPECT_FALSE(Status->isSymlink());
+  EXPECT_FALSE(Status->isOther());
+  EXPECT_TRUE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("bogus");
+  ASSERT_TRUE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+
+  EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/");
+
+  EC = FS->setCurrentWorkingDirectory("bar/");
+  ASSERT_FALSE(EC);
+  WorkingDir = FS->getCurrentWorkingDirectory();
+  ASSERT_TRUE(WorkingDir);
+  EXPECT_EQ(*WorkingDir, "//root/bar/");
+}
+
+TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
+  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("//root/");
+  Lower->addDirectory("//root/foo");
+  Lower->addRegularFile("//root/foo/a");
+  Lower->addRegularFile("//root/foo/b");
+  Lower->addRegularFile("//root/c");
+  IntrusiveRefCntPtr FS = getFromYAMLString(
+  "{ 'use-external-names': false,\n"
+  "  'roots': [\n"
+  "{\n"
+  "  'type': 'directory',\n"
+  "  'name': '//root/',\n"
+  "  'contents': [ {\n"
+  "  'type': 'file',\n"
+  "  'name': 'bar/a',\n"
+  "  'external-contents': '//root/foo/a'\n"
+  "}\n"
+  "  ]\n"
+  "}\n"
+  "]\n"
+  "}",
+  Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+  std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+  ASSERT_TRUE(FS.get() != nullptr);
+
+  llvm::ErrorOr Status = FS->status("./bar/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("./foo/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("//root/bar/");
+  ASSERT_FALSE(EC);
+
+  Status = FS->status("./a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("./b");
+  ASSERT_TRUE(Status.getError());
+  EXPECT_FALSE(Status->exists());
+
+  Status = FS->status("./c");
+  ASSERT_TRUE(Status.getError());
+  EXPECT_FALSE(Status->exists());
+
+  EC = FS->setCurrentWorkingDirectory("//root/");
+  ASSERT_FALSE(EC);
+
+  Status = FS->status("./c");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+}
Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -989,6 +989,17 @@
 // RedirectingFileSystem implementation
 //===---===/
 
+RedirectingFileSystem::RedirectingFileSystem(
+IntrusiveRefCntPtr ExternalFS)
+: ExternalFS(std::move(ExternalFS)) {
+  if (ExternalFS)
+if (auto ExternalWorkingDirectory =
+ExternalFS->getCurrentWorkingDirec

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl created this revision.
aprantl added reviewers: jasonmolenda, davide, labath, clayborg.
Herald added a project: LLDB.
aprantl added a reviewer: jakubjelinek.

This patch adds basic support for DW_OP_convert[1] for integer types. Recent 
versions of LLVM's optimizer may insert this opcode into DWARF expressions. 
DW_OP_convert is effectively a type cast operation that takes a reference to a 
base type DIE (or zero) and then casts the value at the top of the DWARF stack 
to that type. Internally this works by changing the bit size of the APInt that 
is used as backing storage for LLDB's DWARF stack.

I managed to write a unit test for this by implementing a mock YAML object file 
/ module that takes debug info sections in yaml2obj format.

[1] Typed DWARF stack. http://www.dwarfstd.org/ShowIssue.php?issue=140425.1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67369

Files:
  lldb/include/lldb/Core/Section.h
  lldb/include/lldb/Utility/Scalar.h
  lldb/source/Core/Section.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Utility/Scalar.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp

Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -7,24 +7,105 @@
 //===--===//
 
 #include "lldb/Expression/DWARFExpression.h"
+#include "../../source/Plugins/SymbolFile/DWARF/DWARFUnit.h"
+#include "../../source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/dwarf.h"
+#include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ObjectYAML/DWARFEmitter.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
 
-static llvm::Expected Evaluate(llvm::ArrayRef expr) {
+/// A mock module holding an object file parsed from YAML.
+class YAMLModule : public lldb_private::Module {
+public:
+  YAMLModule(ArchSpec &arch) : Module(FileSpec("test"), arch) {}
+  void SetObjectFile(lldb::ObjectFileSP obj_file) { m_objfile_sp = obj_file; }
+  ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
+};
+
+/// A mock object file that can be parsed from YAML.
+class YAMLObjectFile : public lldb_private::ObjectFile {
+  const lldb::ModuleSP &m_module_sp;
+  llvm::StringMap> &m_section_map;
+  /// Because there is only one DataExtractor in the ObjectFile
+  /// interface, all sections are copied into a contiguous buffer.
+  std::vector m_buffer;
+
+public:
+  YAMLObjectFile(const lldb::ModuleSP &module_sp,
+ llvm::StringMap> &map)
+  : ObjectFile(module_sp, &module_sp->GetFileSpec(), /*file_offset*/ 0,
+   /*length*/ 0, /*data_sp*/ nullptr, /*data_offset*/ 0),
+m_module_sp(module_sp), m_section_map(map) {}
+  ConstString GetPluginName() override { return ConstString("YAMLObjectFile"); }
+  uint32_t GetPluginVersion() override { return 0; }
+  void Dump(Stream *s) override {}
+  uint32_t GetAddressByteSize() const override { return 8; }
+  uint32_t GetDependentModules(FileSpecList &file_list) override { return 0; }
+  bool IsExecutable() const override { return 0; }
+  ArchSpec GetArchitecture() override { return {}; }
+
+  void CreateSections(SectionList &unified_section_list) override {
+lldb::offset_t total_bytes = 0;
+for (auto &entry : m_section_map)
+  total_bytes += entry.getValue()->getBufferSize();
+m_buffer.reserve(total_bytes);
+m_data =
+DataExtractor(m_buffer.data(), total_bytes, lldb::eByteOrderLittle, 4);
+
+lldb::user_id_t sect_id = 1;
+for (auto &entry : m_section_map) {
+  llvm::StringRef name = entry.getKey();
+  lldb::SectionType sect_type =
+  llvm::StringSwitch(name)
+  .Case("debug_info", lldb::eSectionTypeDWARFDebugInfo)
+  .Case("debug_abbrev", lldb::eSectionTypeDWARFDebugAbbrev);
+  auto &membuf = entry.getValue();
+  lldb::addr_t file_vm_addr = 0;
+  lldb::addr_t vm_size = 0;
+  lldb::offset_t file_offset = m_buffer.size();
+  lldb::offset_t file_size = membuf->getBufferSize();
+  m_buffer.resize(file_offset + file_size);
+  memcpy(m_buffer.data() + file_offset, membuf->getBufferStart(),
+ file_size);
+  uint32_t log2align = 0;
+  uint32_t flags = 0;
+  auto section_sp = std::make_shared(
+  m_module_sp, this, sect_id++, ConstString(name), sect_type,
+  file_vm_addr, vm_size, file_offset, file_size, log2align, flags);
+  unified_section_list.AddSection(section_sp);
+}
+  }
+
+  Symtab *GetSymtab() override { return nullptr; }
+  bool IsStripped() override { return false; }
+  UUID GetUUID() override { r

[Lldb-commits] [PATCH] D67370: Fix ELF core file memory reading for PT_LOAD program headers with no p_filesz

2019-09-09 Thread Greg Clayton via Phabricator via lldb-commits

clayborg created this revision.
clayborg added reviewers: labath, aprantl.

Prior to this fix, ELF files might contain PT_LOAD program headers that had a 
valid p_vaddr, and a valid file p_offset, but the p_filesz would be zero. For 
example in 
llvm-project/lldb/test/testcases/functionalities/postmortem/elf-core/thread_crash/linux-i386.core
 we see:

  Program Headers:
  Index   p_type   p_flagsp_offset   p_vaddr
p_paddrp_filesz   p_memszp_align
  ===  -- -- -- 
-- -- -- --
  [0] PT_NOTE  0x 0x0474 0x 
0x 0x1940 0x 0x
  [1] PT_LOAD  0x0005 0x2000 0x08048000 
0x 0x 0x3000 0x1000
  [2] PT_LOAD  0x0004 0x2000 0x0804b000 
0x 0x 0x1000 0x1000
  [3] PT_LOAD  0x0006 0x2000 0x0804c000 
0x 0x 0x1000 0x1000
  [4] PT_LOAD  0x0006 0x2000 0x09036000 
0x 0x 0x00025000 0x1000
  [5] PT_LOAD  0x 0x2000 0xf63a1000 
0x 0x 0x1000 0x1000
  [6] PT_LOAD  0x0006 0x2000 0xf63a2000 
0x 0x 0x0080 0x1000
  [7] PT_LOAD  0x 0x2000 0xf6ba2000 
0x 0x 0x1000 0x1000
  [8] PT_LOAD  0x0006 0x2000 0xf6ba3000 
0x 0x 0x00804000 0x1000
  [9] PT_LOAD  0x0005 0x2000 0xf73a7000 
0x 0x 0x001b1000 0x1000
  [   10] PT_LOAD  0x0004 0x2000 0xf7558000 
0x 0x 0x2000 0x1000
  [   11] PT_LOAD  0x0006 0x2000 0xf755a000 
0x 0x 0x1000 0x1000
  [   12] PT_LOAD  0x0006 0x2000 0xf755b000 
0x 0x 0x3000 0x1000
  [   13] PT_LOAD  0x0005 0x2000 0xf755e000 
0x 0x 0x00019000 0x1000
  [   14] PT_LOAD  0x0004 0x2000 0xf7577000 
0x 0x 0x1000 0x1000
  [   15] PT_LOAD  0x0006 0x2000 0xf7578000 
0x 0x 0x1000 0x1000
  [   16] PT_LOAD  0x0006 0x2000 0xf7579000 
0x 0x 0x2000 0x1000
  [   17] PT_LOAD  0x0005 0x2000 0xf757b000 
0x 0x 0x0001c000 0x1000
  [   18] PT_LOAD  0x0004 0x2000 0xf7597000 
0x 0x 0x1000 0x1000
  [   19] PT_LOAD  0x0006 0x2000 0xf7598000 
0x 0x 0x1000 0x1000
  [   20] PT_LOAD  0x0005 0x2000 0xf7599000 
0x 0x 0x00053000 0x1000
  [   21] PT_LOAD  0x0004 0x2000 0xf75ec000 
0x 0x 0x1000 0x1000
  [   22] PT_LOAD  0x0006 0x2000 0xf75ed000 
0x 0x 0x1000 0x1000
  [   23] PT_LOAD  0x0005 0x2000 0xf75ee000 
0x 0x 0x00176000 0x1000
  [   24] PT_LOAD  0x0004 0x2000 0xf7764000 
0x 0x 0x6000 0x1000
  [   25] PT_LOAD  0x0006 0x2000 0xf776a000 
0x 0x 0x1000 0x1000
  [   26] PT_LOAD  0x0006 0x2000 0xf776b000 
0x 0x 0x3000 0x1000
  [   27] PT_LOAD  0x0006 0x2000 0xf778a000 
0x 0x 0x2000 0x1000
  [   28] PT_LOAD  0x0004 0x000

[Lldb-commits] [lldb] r371457 - Fix ELF core file memory reading for PT_LOAD program headers with no p_filesz

2019-09-09 Thread Greg Clayton via lldb-commits

Author: gclayton
Date: Mon Sep  9 14:45:49 2019
New Revision: 371457

URL: http://llvm.org/viewvc/llvm-project?rev=371457&view=rev
Log:
Fix ELF core file memory reading for PT_LOAD program headers with no p_filesz

Prior to this fix, ELF files might contain PT_LOAD program headers that had a 
valid p_vaddr, and a valid file p_offset, but the p_filesz would be zero. For 
example in 
llvm-project/lldb/test/testcases/functionalities/postmortem/elf-core/thread_crash/linux-i386.core
 we see:

Program Headers:
Index   p_type   p_flagsp_offset   p_vaddr
p_paddrp_filesz   p_memszp_align
===  -- -- -- 
-- -- -- --
[0] PT_NOTE  0x 0x0474 0x 
0x 0x1940 0x 0x
[1] PT_LOAD  0x0005 0x2000 0x08048000 
0x 0x 0x3000 0x1000
[2] PT_LOAD  0x0004 0x2000 0x0804b000 
0x 0x 0x1000 0x1000
[3] PT_LOAD  0x0006 0x2000 0x0804c000 
0x 0x 0x1000 0x1000
[4] PT_LOAD  0x0006 0x2000 0x09036000 
0x 0x 0x00025000 0x1000
[5] PT_LOAD  0x 0x2000 0xf63a1000 
0x 0x 0x1000 0x1000
[6] PT_LOAD  0x0006 0x2000 0xf63a2000 
0x 0x 0x0080 0x1000
[7] PT_LOAD  0x 0x2000 0xf6ba2000 
0x 0x 0x1000 0x1000
[8] PT_LOAD  0x0006 0x2000 0xf6ba3000 
0x 0x 0x00804000 0x1000
[9] PT_LOAD  0x0005 0x2000 0xf73a7000 
0x 0x 0x001b1000 0x1000
[   10] PT_LOAD  0x0004 0x2000 0xf7558000 
0x 0x 0x2000 0x1000
[   11] PT_LOAD  0x0006 0x2000 0xf755a000 
0x 0x 0x1000 0x1000
[   12] PT_LOAD  0x0006 0x2000 0xf755b000 
0x 0x 0x3000 0x1000
[   13] PT_LOAD  0x0005 0x2000 0xf755e000 
0x 0x 0x00019000 0x1000
[   14] PT_LOAD  0x0004 0x2000 0xf7577000 
0x 0x 0x1000 0x1000
[   15] PT_LOAD  0x0006 0x2000 0xf7578000 
0x 0x 0x1000 0x1000
[   16] PT_LOAD  0x0006 0x2000 0xf7579000 
0x 0x 0x2000 0x1000
[   17] PT_LOAD  0x0005 0x2000 0xf757b000 
0x 0x 0x0001c000 0x1000
[   18] PT_LOAD  0x0004 0x2000 0xf7597000 
0x 0x 0x1000 0x1000
[   19] PT_LOAD  0x0006 0x2000 0xf7598000 
0x 0x 0x1000 0x1000
[   20] PT_LOAD  0x0005 0x2000 0xf7599000 
0x 0x 0x00053000 0x1000
[   21] PT_LOAD  0x0004 0x2000 0xf75ec000 
0x 0x 0x1000 0x1000
[   22] PT_LOAD  0x0006 0x2000 0xf75ed000 
0x 0x 0x1000 0x1000
[   23] PT_LOAD  0x0005 0x2000 0xf75ee000 
0x 0x 0x00176000 0x1000
[   24] PT_LOAD  0x0004 0x2000 0xf7764000 
0x 0x 0x6000 0x1000
[   25] PT_LOAD  0x0006 0x2000 0xf776a000 
0x 0x 0x1000 0x1000
[   26] PT_LOAD  0x0006 0x2000 0xf776b000 
0x 0x 0x3000 0x1000
[   27] PT_LOAD  0x0006 0x2000 0xf778a000 
0x 0x 0x00

[Lldb-commits] [PATCH] D67370: Fix ELF core file memory reading for PT_LOAD program headers with no p_filesz

2019-09-09 Thread Phabricator via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371457: Fix ELF core file memory reading for PT_LOAD program 
headers with no p_filesz (authored by gclayton, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67370?vs=219423&id=219436#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67370

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp


Index: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -118,16 +118,20 @@
   FileRange file_range(header.p_offset, header.p_filesz);
   VMRangeToFileOffset::Entry range_entry(addr, header.p_memsz, file_range);
 
-  VMRangeToFileOffset::Entry *last_entry = m_core_aranges.Back();
-  if (last_entry && last_entry->GetRangeEnd() == range_entry.GetRangeBase() &&
-  last_entry->data.GetRangeEnd() == range_entry.data.GetRangeBase() &&
-  last_entry->GetByteSize() == last_entry->data.GetByteSize()) {
-last_entry->SetRangeEnd(range_entry.GetRangeEnd());
-last_entry->data.SetRangeEnd(range_entry.data.GetRangeEnd());
-  } else {
-m_core_aranges.Append(range_entry);
+  // Only add to m_core_aranges if the file size is non zero. Some core files
+  // have PT_LOAD segments for all address ranges, but set f_filesz to zero for
+  // the .text sections since they can be retrieved from the object files.
+  if (header.p_filesz > 0) {
+VMRangeToFileOffset::Entry *last_entry = m_core_aranges.Back();
+if (last_entry && last_entry->GetRangeEnd() == range_entry.GetRangeBase() 
&&
+last_entry->data.GetRangeEnd() == range_entry.data.GetRangeBase() &&
+last_entry->GetByteSize() == last_entry->data.GetByteSize()) {
+  last_entry->SetRangeEnd(range_entry.GetRangeEnd());
+  last_entry->data.SetRangeEnd(range_entry.data.GetRangeEnd());
+} else {
+  m_core_aranges.Append(range_entry);
+}
   }
-
   // Keep a separate map of permissions that that isn't coalesced so all ranges
   // are maintained.
   const uint32_t permissions =
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py
@@ -51,6 +51,17 @@
 self.assertEqual(process.GetProcessID(), pid)
 
 for thread in process:
+# Verify that if we try to read memory from a PT_LOAD that has
+# p_filesz of zero that we don't get bytes from the next section
+# that actually did have bytes. The addresses below were found by
+# dumping the program headers of linux-i386.core and
+# linux-x86_64.core and verifying that they had a p_filesz of zero.
+mem_err = lldb.SBError()
+if process.GetAddressByteSize() == 4:
+bytes_read = process.ReadMemory(0x8048000, 4, mem_err)
+else:
+bytes_read = process.ReadMemory(0x40, 4, mem_err)
+self.assertEqual(bytes_read, None)
 reason = thread.GetStopReason()
 if( thread.GetThreadID() == tid ):
 self.assertEqual(reason, lldb.eStopReasonSignal)


Index: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -118,16 +118,20 @@
   FileRange file_range(header.p_offset, header.p_filesz);
   VMRangeToFileOffset::Entry range_entry(addr, header.p_memsz, file_range);
 
-  VMRangeToFileOffset::Entry *last_entry = m_core_aranges.Back();
-  if (last_entry && last_entry->GetRangeEnd() == range_entry.GetRangeBase() &&
-  last_entry->data.GetRangeEnd() == range_entry.data.GetRangeBase() &&
-  last_entry->GetByteSize() == last_entry->data.GetByteSize()) {
-last_entry->SetRangeEnd(range_entry.GetRangeEnd());
-last_entry->data.SetRangeEnd(range_entry.data.GetRangeEnd());
-  } else {
-m_core_aranges.Append(range_entry);
+  // Only add to m_core_aranges if the file size is non zero. Some core files
+  // have PT_LOAD segments for all address ranges, but set f_filesz to zero for
+  // the .text sections since they can be retrieved from the object fil

[Lldb-commits] [PATCH] D67370: Fix ELF core file memory reading for PT_LOAD program headers with no p_filesz

2019-09-09 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

$ git llvm push
Pushing 1 monorepo commit:

  f54a49a8133 Fix ELF core file memory reading for PT_LOAD program headers with 
no p_filesz

Sending
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py
Sendinglldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
Transmitting file data ..done
Committing transaction...
Committed revision 371457.
Committed f54a49a8133 to svn.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67370



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

[Lldb-commits] [lldb] r371459 - [Reproducer] Disconnect when the replay server is out of packets.

2019-09-09 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Sep  9 15:05:48 2019
New Revision: 371459

URL: http://llvm.org/viewvc/llvm-project?rev=371459&view=rev
Log:
[Reproducer] Disconnect when the replay server is out of packets.

This is a fix for the issue described in r371144.

> On more than one occasion I've found this test got stuck during replay
> while waiting for a packet from debugserver when the debugger was in
> the process of being destroyed.

When the replay server is out of packets we should just disconnect so
the debugger doesn't have to do any cleanup that it wouldn't do during
capture.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp?rev=371459&r1=371458&r2=371459&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 Mon Sep  9 15:05:48 2019
@@ -9,6 +9,7 @@
 #include 
 
 #include "lldb/Host/Config.h"
+#include "llvm/ADT/ScopeExit.h"
 
 #include "GDBRemoteCommunicationReplayServer.h"
 #include "ProcessGDBRemoteLog.h"
@@ -256,11 +257,10 @@ void GDBRemoteCommunicationReplayServer:
 thread_result_t GDBRemoteCommunicationReplayServer::AsyncThread(void *arg) {
   GDBRemoteCommunicationReplayServer *server =
   (GDBRemoteCommunicationReplayServer *)arg;
-
+  auto D = make_scope_exit([&]() { server->Disconnect(); });
   EventSP event_sp;
   bool done = false;
-
-  while (true) {
+  while (!done) {
 if (server->m_async_listener_sp->GetEvent(event_sp, llvm::None)) {
   const uint32_t event_type = event_sp->GetType();
   if (event_sp->BroadcasterIs(&server->m_async_broadcaster)) {


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

[Lldb-commits] [lldb] r371460 - Revert "[Reproducer] Add a `cont` to ModuleCXX.test"

2019-09-09 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Sep  9 15:07:45 2019
New Revision: 371460

URL: http://llvm.org/viewvc/llvm-project?rev=371460&view=rev
Log:
Revert "[Reproducer] Add a `cont` to ModuleCXX.test"

This should no longer be necessary after r371459.

Modified:
lldb/trunk/lit/Reproducer/Modules/Inputs/ModuleCXX.in

Modified: lldb/trunk/lit/Reproducer/Modules/Inputs/ModuleCXX.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/Modules/Inputs/ModuleCXX.in?rev=371460&r1=371459&r2=371460&view=diff
==
--- lldb/trunk/lit/Reproducer/Modules/Inputs/ModuleCXX.in (original)
+++ lldb/trunk/lit/Reproducer/Modules/Inputs/ModuleCXX.in Mon Sep  9 15:07:45 
2019
@@ -3,5 +3,4 @@ run
 expr -l Objective-C++ -- @import Foo
 expr -l Objective-C++ -- @import Bar
 expr -- Bar()
-cont
 reproducer generate


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

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Jason Molenda via Phabricator via lldb-commits

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

LGTM, nice job with the test case, I see myself copying that in the future. :)




Comment at: lldb/source/Utility/Scalar.cpp:423
+if (bit_size <= sizeof(int)*8) return Scalar::e_sint;
+if (bit_size <= sizeof(long)*8) return Scalar::e_slong;
+if (bit_size <= sizeof(long long)*8) return Scalar::e_slonglong;

What about an ILP32 target like arm64_32, with lldb running on an LP64 system - 
are we mixing target & host type sizes here?  I'm not sure if Scalar's types 
(e_slong etc) are in host or target terms.  I mean this code might as well be 
if bit_size <= 32 return Scalar::e_Signed32BitType, but I wanted to check in 
because we're using host type sizes here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-09 Thread Vedant Kumar via Phabricator via lldb-commits

vsk created this revision.
vsk added reviewers: aprantl, friss, jasonmolenda, jingham.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Add support for evaluating DW_OP_entry_value. This involves:

- Teaching clang to emit debug entry values when the debugger tuning is set to 
lldb.
- Parsing DW_TAG_call_site_parameter.
- Updating the expression evaluator.

rdar://54496008


https://reviews.llvm.org/D67376

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/TestEntryValsDumpLocationDescriptions1.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/StackFrameList.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -790,9 +790,9 @@
 CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, IsTail, PCAddr,
  PCOffset, CallReg);
 
-  // For now only GDB supports call site parameter debug info.
+  // For now only GDB and LLDB support call site parameter debug info.
   if (Asm->TM.Options.EnableDebugEntryValues &&
-  tuneForGDB()) {
+  (tuneForGDB() || tuneForLLDB())) {
 ParamSet Params;
 // Try to interpret values of call site parameters.
 collectCallSiteParameters(&MI, Params);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -250,26 +250,19 @@
begin.GetDisplayName(), end.GetDisplayName(), return_pc);
 
   // Find a non-tail calling edge with the correct return PC.
-  auto first_level_edges = begin.GetCallEdges();
   if (log)
-for (const CallEdge &edge : first_level_edges)
+for (const CallEdge &edge : begin.GetCallEdges())
   LLDB_LOG(log, "FindInterveningFrames: found call with retn-PC = {0:x}",
edge.GetReturnPCAddress(begin, target));
-  auto first_edge_it = std::lower_bound(
-  first_level_edges.begin(), first_level_edges.end(), return_pc,
-  [&](const CallEdge &edge, addr_t target_pc) {
-return edge.GetReturnPCAddress(begin, target) < target_pc;
-  });
-  if (first_edge_it == first_level_edges.end() ||
-  first_edge_it->GetReturnPCAddress(begin, target) != return_pc) {
+  CallEdge *first_edge = begin.GetCallEdgeForReturnAddress(return_pc, target);
+  if (!first_edge) {
 LLDB_LOG(log, "No call edge outgoing from {0} with retn-PC == {1:x}",
  begin.GetDisplayName(), return_pc);
 return;
   }
-  CallEdge &first_edge = const_cast(*first_edge_it);
 
   // The first callee may not be resolved, or there may be nothing to fill in.
-  Function *first_callee = first_edge.GetCallee(images);
+  Function *first_callee = first_edge->GetCallee(images);
   if (!first_callee) {
 LLDB_LOG(log, "Could not resolve callee");
 return;
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -127,11 +127,18 @@
 }
 
 //
-CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc)
-: return_pc(return_pc), resolved(false) {
+CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc,
+   CallSiteParameterArray parameters)
+: return_pc(return_pc), parameters(std::move(parameters)), resolved(false) {
   lazy_callee.symbol_name = symbol_name;
 }
 
+llvm::ArrayRef CallEdge::GetCallSiteParameters() const {
+  if (parameters)
+return *parameters.get();
+  return {};
+}
+
 void CallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   if (resolved)
 return;
@@ -276,6 +283,20 @@
   });
 }
 
+CallEdge *Function::GetCallEdgeForReturnAddress(addr_t return_pc,
+Target &target) {
+  auto edges = GetCallEdges();
+  auto edge_it =
+  std::lower_bound(edges.begin(), edges.end(), return_pc,
+   [&](const CallEdge &edge, addr_t pc) {
+ return edge.GetReturnPCAddress(*this, target) < pc;
+   });
+  if (edge_it == edges.end() ||
+  edge_it->GetReturnPCAddress(*this, target) != return_pc)
+return nullptr;
+  return &const_cast(*edge_it);
+}
+

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-09 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 219442.
vsk added a comment.

- Clean up the test.


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

https://reviews.llvm.org/D67376

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/TestEntryValsDumpLocationDescriptions1.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/StackFrameList.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -790,9 +790,9 @@
 CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, IsTail, PCAddr,
  PCOffset, CallReg);
 
-  // For now only GDB supports call site parameter debug info.
+  // For now only GDB and LLDB support call site parameter debug info.
   if (Asm->TM.Options.EnableDebugEntryValues &&
-  tuneForGDB()) {
+  (tuneForGDB() || tuneForLLDB())) {
 ParamSet Params;
 // Try to interpret values of call site parameters.
 collectCallSiteParameters(&MI, Params);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -250,26 +250,19 @@
begin.GetDisplayName(), end.GetDisplayName(), return_pc);
 
   // Find a non-tail calling edge with the correct return PC.
-  auto first_level_edges = begin.GetCallEdges();
   if (log)
-for (const CallEdge &edge : first_level_edges)
+for (const CallEdge &edge : begin.GetCallEdges())
   LLDB_LOG(log, "FindInterveningFrames: found call with retn-PC = {0:x}",
edge.GetReturnPCAddress(begin, target));
-  auto first_edge_it = std::lower_bound(
-  first_level_edges.begin(), first_level_edges.end(), return_pc,
-  [&](const CallEdge &edge, addr_t target_pc) {
-return edge.GetReturnPCAddress(begin, target) < target_pc;
-  });
-  if (first_edge_it == first_level_edges.end() ||
-  first_edge_it->GetReturnPCAddress(begin, target) != return_pc) {
+  CallEdge *first_edge = begin.GetCallEdgeForReturnAddress(return_pc, target);
+  if (!first_edge) {
 LLDB_LOG(log, "No call edge outgoing from {0} with retn-PC == {1:x}",
  begin.GetDisplayName(), return_pc);
 return;
   }
-  CallEdge &first_edge = const_cast(*first_edge_it);
 
   // The first callee may not be resolved, or there may be nothing to fill in.
-  Function *first_callee = first_edge.GetCallee(images);
+  Function *first_callee = first_edge->GetCallee(images);
   if (!first_callee) {
 LLDB_LOG(log, "Could not resolve callee");
 return;
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -127,11 +127,18 @@
 }
 
 //
-CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc)
-: return_pc(return_pc), resolved(false) {
+CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc,
+   CallSiteParameterArray parameters)
+: return_pc(return_pc), parameters(std::move(parameters)), resolved(false) {
   lazy_callee.symbol_name = symbol_name;
 }
 
+llvm::ArrayRef CallEdge::GetCallSiteParameters() const {
+  if (parameters)
+return *parameters.get();
+  return {};
+}
+
 void CallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   if (resolved)
 return;
@@ -276,6 +283,20 @@
   });
 }
 
+CallEdge *Function::GetCallEdgeForReturnAddress(addr_t return_pc,
+Target &target) {
+  auto edges = GetCallEdges();
+  auto edge_it =
+  std::lower_bound(edges.begin(), edges.end(), return_pc,
+   [&](const CallEdge &edge, addr_t pc) {
+ return edge.GetReturnPCAddress(*this, target) < pc;
+   });
+  if (edge_it == edges.end() ||
+  edge_it->GetReturnPCAddress(*this, target) != return_pc)
+return nullptr;
+  return &const_cast(*edge_it);
+}
+
 Block &Function::GetBlock(bool can_create) {
   if (!m_block.BlockInfoHasBeenParsed() && can_create) {
 ModuleSP module_sp = CalculateSymbolContextModule();
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
=

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Adrian Prantl via Phabricator via lldb-commits

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



Comment at: lldb/source/Utility/Scalar.cpp:423
+if (bit_size <= sizeof(int)*8) return Scalar::e_sint;
+if (bit_size <= sizeof(long)*8) return Scalar::e_slong;
+if (bit_size <= sizeof(long long)*8) return Scalar::e_slonglong;

jasonmolenda wrote:
> What about an ILP32 target like arm64_32, with lldb running on an LP64 system 
> - are we mixing target & host type sizes here?  I'm not sure if Scalar's 
> types (e_slong etc) are in host or target terms.  I mean this code might as 
> well be if bit_size <= 32 return Scalar::e_Signed32BitType, but I wanted to 
> check in because we're using host type sizes here.
As the comment on line 420 above says: 
`// Scalar types are always host types, hence the sizeof().`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-09 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

This is very exciting!




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/Makefile:4
+include $(LEVEL)/Makefile.rules
+CXXFLAGS += -g -O1 -glldb -Xclang -femit-debug-entry-values

The -g should be redundant, and if we leave out -glldb (which should be the 
default for clang anyway) then this test in theory could also work with other 
compilers such as GCC (which I think implements the same option under the same 
-f option). I have no idea whether anyone runs such a bot, but it's still nice.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp:7
+//
+//===--===//
+

I would just drop the header for testcases.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp:35
+  /* Clobbers */ : "rsi"
+  );
+

Cool.



Comment at: lldb/source/Expression/DWARFExpression.cpp:497
+
+  finish_subexpressions_to(end_offset);
 }

We should probably just let llvm's libDebugInfo do the printing here, but this 
works fine for now.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1117
+  if ((!exe_ctx || !exe_ctx->HasTargetScope()) || !reg_ctx) {
+LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no exe/reg context");
+return false;

The Evaluate function allows setting an Error with a message. Should we do the 
same thing here so we can return the error to the caller?



Comment at: lldb/source/Expression/DWARFExpression.cpp:2856
+  error_ptr, log)) {
+LLDB_LOGF(log, "Could not evaluate %s.", DW_OP_value_to_name(op));
+return false;

See above, we can set `error_ptr` here.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp:62
 DRC_class DW_OP_value_to_class(uint32_t val) {
+  // FIXME: This switch should be simplified by using DW_OP_* names.
   switch (val) {

Don't spend too much time on that, we should just use the llvm DWARFExpression 
iterator instead.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3712
 
+/// Collect call site parameters in a TAG_call_site DIE.
+static CallSiteParameterArray

DW_TAG_call_site



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3752
+LocationInCaller = parse_simple_location(i);
+break;
+  }

default?



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:795
   if (Asm->TM.Options.EnableDebugEntryValues &&
-  tuneForGDB()) {
+  (tuneForGDB() || tuneForLLDB())) {
 ParamSet Params;

This should also be tested inside of LLVM itself and probably should be a 
separate commit.


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

https://reviews.llvm.org/D67376



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

[Lldb-commits] [lldb] r371470 - [Symbol] Give ClangASTContext a PersistentExpressionState instead of a ClangPersistentVariables

2019-09-09 Thread Alex Langford via lldb-commits

Author: xiaobai
Date: Mon Sep  9 16:11:43 2019
New Revision: 371470

URL: http://llvm.org/viewvc/llvm-project?rev=371470&view=rev
Log:
[Symbol] Give ClangASTContext a PersistentExpressionState instead of a 
ClangPersistentVariables

ClangASTContext doesn't use m_persistent_variables in a way specific to
ClangPersistentVariables. Therefore, it should hold a unique pointer to
PersistentExpressionState instead of a ClangPersistentVariablesUP.
This also prevents you from pulling in a plugin header when including
ClangASTContext.h

Doing this exposed an implicit dependency in ObjCLanguage that was
corrected by including ClangModulesDeclVendor.h

Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=371470&r1=371469&r2=371470&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Mon Sep  9 16:11:43 2019
@@ -26,8 +26,8 @@
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
 
-#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "lldb/Core/ClangForward.h"
+#include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Utility/ConstString.h"
@@ -1042,13 +1042,9 @@ public:
   }
 private:
   lldb::TargetWP m_target_wp;
-  lldb::ClangPersistentVariablesUP m_persistent_variables; ///< These are the
-   ///persistent
-   ///variables
-   ///associated with
-   ///this process for
-   ///the expression
-   ///parser.
+  std::unique_ptr
+  m_persistent_variables; // These are the persistent variables associated
+  // with this process for the expression parser
 };
 
 } // namespace lldb_private

Modified: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp?rev=371470&r1=371469&r2=371470&view=diff
==
--- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp (original)
+++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp Mon Sep  9 
16:11:43 2019
@@ -22,6 +22,7 @@
 
 #include "llvm/Support/Threading.h"
 
+#include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 
 #include "CF.h"

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=371470&r1=371469&r2=371470&view=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Mon Sep  9 16:11:43 2019
@@ -65,6 +65,7 @@
 #include "llvm/Support/Threading.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangFunctionCaller.h"
+#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "Plugins/ExpressionParser/Clang/ClangUserExpression.h"
 #include "Plugins/ExpressionParser/Clang/ClangUtilityFunction.h"
 #include "lldb/Utility/ArchSpec.h"


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

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Adrian Prantl via Phabricator via lldb-commits

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



Comment at: lldb/source/Utility/Scalar.cpp:423
+if (bit_size <= sizeof(int)*8) return Scalar::e_sint;
+if (bit_size <= sizeof(long)*8) return Scalar::e_slong;
+if (bit_size <= sizeof(long long)*8) return Scalar::e_slonglong;

aprantl wrote:
> jasonmolenda wrote:
> > What about an ILP32 target like arm64_32, with lldb running on an LP64 
> > system - are we mixing target & host type sizes here?  I'm not sure if 
> > Scalar's types (e_slong etc) are in host or target terms.  I mean this code 
> > might as well be if bit_size <= 32 return Scalar::e_Signed32BitType, but I 
> > wanted to check in because we're using host type sizes here.
> As the comment on line 420 above says: 
> `// Scalar types are always host types, hence the sizeof().`
Note that I think the decision of making Scalar types host types very 
questionable and it would make much more sense for it to be target types, but 
the way the Scalar constructors are implemented they are definitely host types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: vsk, davide, teemperor, labath.
JDevlieghere added a project: LLDB.
Herald added subscribers: abidh, mgorny, emaste.
JDevlieghere updated this revision to Diff 219449.

This removes the CleanUp class and replaces its usages with llvm's ScopeExit 
which has similar semantics.


https://reviews.llvm.org/D67378

Files:
  lldb/include/lldb/Utility/CleanUp.h
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/CleanUpTest.cpp

Index: lldb/unittests/Utility/CleanUpTest.cpp
===
--- lldb/unittests/Utility/CleanUpTest.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//===-- CleanUpTest.cpp -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Utility/CleanUp.h"
-#include "gtest/gtest.h"
-
-using namespace lldb_private;
-
-TEST(CleanUpTest, no_args) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-  }
-  ASSERT_TRUE(f);
-}
-
-TEST(CleanUpTest, multiple_args) {
-  bool f1 = false;
-  bool f2 = false;
-  bool f3 = false;
-  {
-CleanUp cleanup(
-[](bool arg1, bool *arg2, bool &arg3) {
-  ASSERT_FALSE(arg1);
-  *arg2 = true;
-  arg3 = true;
-},
-f1, &f2, f3);
-  }
-  ASSERT_TRUE(f2);
-  ASSERT_FALSE(f3);
-}
-
-TEST(CleanUpTest, disable) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-cleanup.disable();
-  }
-  ASSERT_FALSE(f);
-}
Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -4,7 +4,6 @@
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   BroadcasterTest.cpp
-  CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   DataExtractorTest.cpp
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -29,12 +29,12 @@
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/IntervalMap.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -1035,7 +1035,8 @@
 return 1;
   }
 
-  CleanUp TerminateDebugger([&] { DebuggerLifetime.Terminate(); });
+  auto TerminateDebugger =
+  llvm::make_scope_exit([&] { DebuggerLifetime.Terminate(); });
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +32,7 @@
 #include "lldb/Utility/UUID.h"
 #include "mach/machine.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
@@ -264,7 +264,7 @@
 return {};
 
   // Make sure we close the directory before exiting this scope.
-  CleanUp cleanup_dir(closedir, dirp);
+  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
 
   FileSpec dsym_fspec;
   dsym_fspec.GetDirectory().SetCString(path);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -63,7 +63,6 @@
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Utility/Args.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
@@ -81,6 +80,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
+#include "llvm/ADT

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 219449.

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

https://reviews.llvm.org/D67378

Files:
  lldb/include/lldb/Utility/CleanUp.h
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/CleanUpTest.cpp

Index: lldb/unittests/Utility/CleanUpTest.cpp
===
--- lldb/unittests/Utility/CleanUpTest.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//===-- CleanUpTest.cpp -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Utility/CleanUp.h"
-#include "gtest/gtest.h"
-
-using namespace lldb_private;
-
-TEST(CleanUpTest, no_args) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-  }
-  ASSERT_TRUE(f);
-}
-
-TEST(CleanUpTest, multiple_args) {
-  bool f1 = false;
-  bool f2 = false;
-  bool f3 = false;
-  {
-CleanUp cleanup(
-[](bool arg1, bool *arg2, bool &arg3) {
-  ASSERT_FALSE(arg1);
-  *arg2 = true;
-  arg3 = true;
-},
-f1, &f2, f3);
-  }
-  ASSERT_TRUE(f2);
-  ASSERT_FALSE(f3);
-}
-
-TEST(CleanUpTest, disable) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-cleanup.disable();
-  }
-  ASSERT_FALSE(f);
-}
Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -4,7 +4,6 @@
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   BroadcasterTest.cpp
-  CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   DataExtractorTest.cpp
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -29,12 +29,12 @@
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/IntervalMap.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -1035,7 +1035,8 @@
 return 1;
   }
 
-  CleanUp TerminateDebugger([&] { DebuggerLifetime.Terminate(); });
+  auto TerminateDebugger =
+  llvm::make_scope_exit([&] { DebuggerLifetime.Terminate(); });
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +32,7 @@
 #include "lldb/Utility/UUID.h"
 #include "mach/machine.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
@@ -264,7 +264,7 @@
 return {};
 
   // Make sure we close the directory before exiting this scope.
-  CleanUp cleanup_dir(closedir, dirp);
+  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
 
   FileSpec dsym_fspec;
   dsym_fspec.GetDirectory().SetCString(path);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -63,7 +63,6 @@
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Utility/Args.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
@@ -81,6 +80,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
@@ -3519,8 +3519,8 @@
 
 int our_socket = sockets[0];
 int gdb_socket = sockets[1];
-Cle

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:901-902
 // Make sure we deallocate the buffer memory:
-buffer_cleanup.emplace([process, buffer_addr] { 
-process->DeallocateMemory(buffer_addr); 
-});
+buffer_cleanup.emplace(
+[process, buffer_addr] { process->DeallocateMemory(buffer_addr); });
   }

This seems an undesirable cleanup.  Now it is going to be hard to just put a 
breakpoint on the cleanup code, which seems worth the one extra line (and not 
related to replacing CleanUp with scope_exit.  Ditto the one on 826.  Is 
clang-format doing this for you?


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

https://reviews.llvm.org/D67378



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

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/Scalar.h:107
+  /// Return the most efficient Scalar::Type for the requested size.
+  static Type GetBestType(size_t bit_size, bool sign);
+

How about `GetTypeForBitSize`? 



Comment at: lldb/source/Expression/DWARFExpression.cpp:2571
+  if (stack.size() < 1) {
+if (error_ptr)
+  error_ptr->SetErrorString(

Can we wrap this in a lambda? 



Comment at: lldb/source/Utility/Scalar.cpp:442
+  default:
+assert(false && "Promoting a Scalar to a specific number of bits is only "
+"supported for integer types.");

llvm::unreachable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Adrian Prantl via Phabricator via lldb-commits

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



Comment at: lldb/source/Expression/DWARFExpression.cpp:2571
+  if (stack.size() < 1) {
+if (error_ptr)
+  error_ptr->SetErrorString(

JDevlieghere wrote:
> Can we wrap this in a lambda? 
Which part specifically do you mean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 219454.
JDevlieghere added a comment.

Add a comment so you can easily break on the cleanup code.


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

https://reviews.llvm.org/D67378

Files:
  lldb/include/lldb/Utility/CleanUp.h
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/CleanUpTest.cpp

Index: lldb/unittests/Utility/CleanUpTest.cpp
===
--- lldb/unittests/Utility/CleanUpTest.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//===-- CleanUpTest.cpp -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Utility/CleanUp.h"
-#include "gtest/gtest.h"
-
-using namespace lldb_private;
-
-TEST(CleanUpTest, no_args) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-  }
-  ASSERT_TRUE(f);
-}
-
-TEST(CleanUpTest, multiple_args) {
-  bool f1 = false;
-  bool f2 = false;
-  bool f3 = false;
-  {
-CleanUp cleanup(
-[](bool arg1, bool *arg2, bool &arg3) {
-  ASSERT_FALSE(arg1);
-  *arg2 = true;
-  arg3 = true;
-},
-f1, &f2, f3);
-  }
-  ASSERT_TRUE(f2);
-  ASSERT_FALSE(f3);
-}
-
-TEST(CleanUpTest, disable) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-cleanup.disable();
-  }
-  ASSERT_FALSE(f);
-}
Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -4,7 +4,6 @@
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   BroadcasterTest.cpp
-  CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   DataExtractorTest.cpp
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -29,12 +29,12 @@
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/IntervalMap.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -1035,7 +1035,8 @@
 return 1;
   }
 
-  CleanUp TerminateDebugger([&] { DebuggerLifetime.Terminate(); });
+  auto TerminateDebugger =
+  llvm::make_scope_exit([&] { DebuggerLifetime.Terminate(); });
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +32,7 @@
 #include "lldb/Utility/UUID.h"
 #include "mach/machine.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
@@ -264,7 +264,7 @@
 return {};
 
   // Make sure we close the directory before exiting this scope.
-  CleanUp cleanup_dir(closedir, dirp);
+  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
 
   FileSpec dsym_fspec;
   dsym_fspec.GetDirectory().SetCString(path);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -63,7 +63,6 @@
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Utility/Args.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
@@ -81,6 +80,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
@@ -3519,

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2571
+  if (stack.size() < 1) {
+if (error_ptr)
+  error_ptr->SetErrorString(

aprantl wrote:
> JDevlieghere wrote:
> > Can we wrap this in a lambda? 
> Which part specifically do you mean?
```auto SetErrorString = [&](llvm::StringRef msg) { if (error_ptr) 
error_ptr->SetErrorString(msg); })```

We could deduplicate even more code by using a macro that also covers the check 
around it and does the return, but I'm not sure if that's really worth it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

This pattern repeated a couple of times north or the one you fixed.  Can you 
get those too?


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

https://reviews.llvm.org/D67378



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

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 219457.
JDevlieghere added a comment.

Add more comments to keep the original formatting.


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

https://reviews.llvm.org/D67378

Files:
  lldb/include/lldb/Utility/CleanUp.h
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/CleanUpTest.cpp

Index: lldb/unittests/Utility/CleanUpTest.cpp
===
--- lldb/unittests/Utility/CleanUpTest.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//===-- CleanUpTest.cpp -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Utility/CleanUp.h"
-#include "gtest/gtest.h"
-
-using namespace lldb_private;
-
-TEST(CleanUpTest, no_args) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-  }
-  ASSERT_TRUE(f);
-}
-
-TEST(CleanUpTest, multiple_args) {
-  bool f1 = false;
-  bool f2 = false;
-  bool f3 = false;
-  {
-CleanUp cleanup(
-[](bool arg1, bool *arg2, bool &arg3) {
-  ASSERT_FALSE(arg1);
-  *arg2 = true;
-  arg3 = true;
-},
-f1, &f2, f3);
-  }
-  ASSERT_TRUE(f2);
-  ASSERT_FALSE(f3);
-}
-
-TEST(CleanUpTest, disable) {
-  bool f = false;
-  {
-CleanUp cleanup([&] { f = true; });
-cleanup.disable();
-  }
-  ASSERT_FALSE(f);
-}
Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -4,7 +4,6 @@
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   BroadcasterTest.cpp
-  CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   DataExtractorTest.cpp
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -29,12 +29,12 @@
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/IntervalMap.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -1035,7 +1035,8 @@
 return 1;
   }
 
-  CleanUp TerminateDebugger([&] { DebuggerLifetime.Terminate(); });
+  auto TerminateDebugger =
+  llvm::make_scope_exit([&] { DebuggerLifetime.Terminate(); });
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +32,7 @@
 #include "lldb/Utility/UUID.h"
 #include "mach/machine.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
@@ -264,7 +264,7 @@
 return {};
 
   // Make sure we close the directory before exiting this scope.
-  CleanUp cleanup_dir(closedir, dirp);
+  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
 
   FileSpec dsym_fspec;
   dsym_fspec.GetDirectory().SetCString(path);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -63,7 +63,6 @@
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Utility/Args.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
@@ -81,6 +80,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
@@ -3519,8 +3519,

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D67378#1664070 , @jingham wrote:

> This pattern repeated a couple of times north or the one you fixed.  Can you 
> get those too?


Thanks, I missed those. I believe I covered all the cases that were multi-line 
originally.


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

https://reviews.llvm.org/D67378



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

[Lldb-commits] [lldb] r371472 - [Expression] Remove unused header from LLVMUserExpression

2019-09-09 Thread Alex Langford via lldb-commits

Author: xiaobai
Date: Mon Sep  9 16:59:54 2019
New Revision: 371472

URL: http://llvm.org/viewvc/llvm-project?rev=371472&view=rev
Log:
[Expression] Remove unused header from LLVMUserExpression

Modified:
lldb/trunk/source/Expression/LLVMUserExpression.cpp

Modified: lldb/trunk/source/Expression/LLVMUserExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/LLVMUserExpression.cpp?rev=371472&r1=371471&r2=371472&view=diff
==
--- lldb/trunk/source/Expression/LLVMUserExpression.cpp (original)
+++ lldb/trunk/source/Expression/LLVMUserExpression.cpp Mon Sep  9 16:59:54 2019
@@ -18,7 +18,6 @@
 #include "lldb/Expression/Materializer.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/Block.h"
-#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolVendor.h"


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

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Vedant Kumar via Phabricator via lldb-commits

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

Thanks a lot!


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

https://reviews.llvm.org/D67378



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

[Lldb-commits] [lldb] r371474 - [Utility] Replace `lldb_private::CleanUp` by `llvm::scope_exit`

2019-09-09 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Sep  9 17:20:50 2019
New Revision: 371474

URL: http://llvm.org/viewvc/llvm-project?rev=371474&view=rev
Log:
[Utility] Replace `lldb_private::CleanUp` by `llvm::scope_exit`

This removes the CleanUp class and replaces its usages with llvm's
ScopeExit, which has similar semantics.

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

Removed:
lldb/trunk/include/lldb/Utility/CleanUp.h
lldb/trunk/unittests/Utility/CleanUpTest.cpp
Modified:
lldb/trunk/source/Host/macosx/objcxx/Host.mm
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Symbol/LocateSymbolFileMacOSX.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp
lldb/trunk/unittests/Utility/CMakeLists.txt

Removed: lldb/trunk/include/lldb/Utility/CleanUp.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CleanUp.h?rev=371473&view=auto
==
--- lldb/trunk/include/lldb/Utility/CleanUp.h (original)
+++ lldb/trunk/include/lldb/Utility/CleanUp.h (removed)
@@ -1,42 +0,0 @@
-//===-- CleanUp.h ---*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef liblldb_CleanUp_h_
-#define liblldb_CleanUp_h_
-
-#include "lldb/lldb-public.h"
-#include 
-
-namespace lldb_private {
-
-/// Run a cleanup function on scope exit unless it's explicitly disabled.
-class CleanUp {
-  std::function Clean;
-
-public:
-  /// Register a cleanup function which applies \p Func to a list of arguments.
-  /// Use caution with arguments which are references: they will be copied.
-  template 
-  CleanUp(F &&Func, Args &&... args)
-  : Clean(std::bind(std::forward(Func), std::forward(args)...)) {}
-
-  ~CleanUp() {
-if (Clean)
-  Clean();
-  }
-
-  /// Disable the cleanup.
-  void disable() { Clean = nullptr; }
-
-  // Prevent cleanups from being run more than once.
-  DISALLOW_COPY_AND_ASSIGN(CleanUp);
-};
-
-} // namespace lldb_private
-
-#endif // #ifndef liblldb_CleanUp_h_

Modified: lldb/trunk/source/Host/macosx/objcxx/Host.mm
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/objcxx/Host.mm?rev=371474&r1=371473&r2=371474&view=diff
==
--- lldb/trunk/source/Host/macosx/objcxx/Host.mm (original)
+++ lldb/trunk/source/Host/macosx/objcxx/Host.mm Mon Sep  9 17:20:50 2019
@@ -59,7 +59,6 @@
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -71,8 +70,9 @@
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-defines.h"
 
-#include "llvm/Support/FileSystem.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
 
 #include "../cfcpp/CFCBundle.h"
 #include "../cfcpp/CFCMutableArray.h"
@@ -1092,7 +1092,8 @@ static Status LaunchProcessPosixSpawn(co
   }
 
   // Make sure we clean up the posix spawn attributes before exiting this 
scope.
-  CleanUp cleanup_attr(posix_spawnattr_destroy, &attr);
+  auto cleanup_attr =
+  llvm::make_scope_exit([&]() { posix_spawnattr_destroy(&attr); });
 
   sigset_t no_signals;
   sigset_t all_signals;
@@ -1195,7 +1196,8 @@ static Status LaunchProcessPosixSpawn(co
 }
 
 // Make sure we clean up the posix file actions before exiting this scope.
-CleanUp cleanup_fileact(posix_spawn_file_actions_destroy, &file_actions);
+auto cleanup_fileact = llvm::make_scope_exit(
+[&]() { posix_spawn_file_actions_destroy(&file_actions); });
 
 for (size_t i = 0; i < num_file_actions; ++i) {
   const FileAction *launch_file_action =

Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp?rev=371474&r1=371473&r2=371474&view=diff
==
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp Mon Sep  9 
17:20:50 2019
@@ -27,11 +27,11 @@
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/ScopeExit.h"

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

2019-09-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371474: [Utility] Replace `lldb_private::CleanUp` by 
`llvm::scope_exit` (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67378?vs=219457&id=219460#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67378

Files:
  lldb/trunk/include/lldb/Utility/CleanUp.h
  lldb/trunk/source/Host/macosx/objcxx/Host.mm
  lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/trunk/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp
  lldb/trunk/unittests/Utility/CMakeLists.txt
  lldb/trunk/unittests/Utility/CleanUpTest.cpp

Index: lldb/trunk/unittests/Utility/CMakeLists.txt
===
--- lldb/trunk/unittests/Utility/CMakeLists.txt
+++ lldb/trunk/unittests/Utility/CMakeLists.txt
@@ -4,7 +4,6 @@
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   BroadcasterTest.cpp
-  CleanUpTest.cpp
   ConstStringTest.cpp
   CompletionRequestTest.cpp
   DataExtractorTest.cpp
Index: lldb/trunk/source/Host/macosx/objcxx/Host.mm
===
--- lldb/trunk/source/Host/macosx/objcxx/Host.mm
+++ lldb/trunk/source/Host/macosx/objcxx/Host.mm
@@ -59,7 +59,6 @@
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -71,8 +70,9 @@
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-defines.h"
 
-#include "llvm/Support/FileSystem.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
 
 #include "../cfcpp/CFCBundle.h"
 #include "../cfcpp/CFCMutableArray.h"
@@ -1092,7 +1092,8 @@
   }
 
   // Make sure we clean up the posix spawn attributes before exiting this scope.
-  CleanUp cleanup_attr(posix_spawnattr_destroy, &attr);
+  auto cleanup_attr =
+  llvm::make_scope_exit([&]() { posix_spawnattr_destroy(&attr); });
 
   sigset_t no_signals;
   sigset_t all_signals;
@@ -1195,7 +1196,8 @@
 }
 
 // Make sure we clean up the posix file actions before exiting this scope.
-CleanUp cleanup_fileact(posix_spawn_file_actions_destroy, &file_actions);
+auto cleanup_fileact = llvm::make_scope_exit(
+[&]() { posix_spawn_file_actions_destroy(&file_actions); });
 
 for (size_t i = 0; i < num_file_actions; ++i) {
   const FileAction *launch_file_action =
Index: lldb/trunk/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/trunk/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/trunk/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
@@ -33,6 +32,7 @@
 #include "lldb/Utility/UUID.h"
 #include "mach/machine.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
@@ -264,7 +264,7 @@
 return {};
 
   // Make sure we close the directory before exiting this scope.
-  CleanUp cleanup_dir(closedir, dirp);
+  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
 
   FileSpec dsym_fspec;
   dsym_fspec.GetDirectory().SetCString(path);
Index: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -27,11 +27,11 @@
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Thread.h"
-#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/ScopeExit.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -798,12 +798,13 @@
 "for path: %s", utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
-  
+
   // Make sure we deallocate the input string memory:
-  CleanUp path_cleanup([process, path_addr] { 
-  process->DeallocateMemory(path_addr); 
+  auto path_cleanup = llvm::make_scope_exit([process, path_addr] {
+// Deallocate the buffer.
+process->DeallocateMemory(path_addr);
   });
-  
+
   process->WriteMemory(path_addr, path.c_str(), path_len, utility

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

2019-09-09 Thread Vedant Kumar via Phabricator via lldb-commits

vsk added a comment.

Looks good overall, especially the testing.




Comment at: lldb/include/lldb/Utility/Scalar.h:107
+  /// Return the most efficient Scalar::Type for the requested size.
+  static Type GetBestType(size_t bit_size, bool sign);
+

JDevlieghere wrote:
> How about `GetTypeForBitSize`? 
nit: bool signed ?



Comment at: lldb/source/Expression/DWARFExpression.cpp:2571
+  if (stack.size() < 1) {
+if (error_ptr)
+  error_ptr->SetErrorString(

JDevlieghere wrote:
> aprantl wrote:
> > JDevlieghere wrote:
> > > Can we wrap this in a lambda? 
> > Which part specifically do you mean?
> ```auto SetErrorString = [&](llvm::StringRef msg) { if (error_ptr) 
> error_ptr->SetErrorString(msg); })```
> 
> We could deduplicate even more code by using a macro that also covers the 
> check around it and does the return, but I'm not sure if that's really worth 
> it. 
Oh, like LLDB_LOG? Seems like a nice idea. I think I need this for D67376, so I 
might do it if no one else has started already. It'd be great to use that here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67369



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

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-09 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 219462.
vsk marked 7 inline comments as done.
vsk added a comment.

- Partially address review feedback.


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

https://reviews.llvm.org/D67376

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Utility/Status.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/TestEntryValsDumpLocationDescriptions1.py
  
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -790,9 +790,9 @@
 CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, IsTail, PCAddr,
  PCOffset, CallReg);
 
-  // For now only GDB supports call site parameter debug info.
+  // For now only GDB and LLDB support call site parameter debug info.
   if (Asm->TM.Options.EnableDebugEntryValues &&
-  tuneForGDB()) {
+  (tuneForGDB() || tuneForLLDB())) {
 ParamSet Params;
 // Try to interpret values of call site parameters.
 collectCallSiteParameters(&MI, Params);
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -127,11 +127,18 @@
 }
 
 //
-CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc)
-: return_pc(return_pc), resolved(false) {
+CallEdge::CallEdge(const char *symbol_name, lldb::addr_t return_pc,
+   CallSiteParameterArray parameters)
+: return_pc(return_pc), parameters(std::move(parameters)), resolved(false) {
   lazy_callee.symbol_name = symbol_name;
 }
 
+llvm::ArrayRef CallEdge::GetCallSiteParameters() const {
+  if (parameters)
+return *parameters.get();
+  return {};
+}
+
 void CallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   if (resolved)
 return;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -8,6 +8,7 @@
 
 #include "SymbolFileDWARF.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Threading.h"
 
@@ -3708,9 +3709,63 @@
   return vars_added;
 }
 
+/// Collect call site parameters in a DW_TAG_call_site DIE.
+static CallSiteParameterArray
+CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
+  CallSiteParameterArray parameters = nullptr;
+  for (DWARFDIE child = call_site_die.GetFirstChild(); child.IsValid();
+   child = child.GetSibling()) {
+if (child.Tag() != DW_TAG_call_site_parameter)
+  continue;
+
+llvm::Optional LocationInCallee = {};
+llvm::Optional LocationInCaller = {};
+
+DWARFAttributes attributes;
+const size_t num_attributes = child.GetAttributes(attributes);
+
+// Parse the location at index \p attr_index within this call site parameter
+// DIE, or return None on failure.
+auto parse_simple_location =
+[&](int attr_index) -> llvm::Optional {
+  DWARFFormValue form_value;
+  if (!attributes.ExtractFormValueAtIndex(attr_index, form_value))
+return {};
+  if (!DWARFFormValue::IsBlockForm(form_value.Form()))
+return {};
+  auto data = child.GetData();
+  uint32_t block_offset = form_value.BlockData() - data.GetDataStart();
+  uint32_t block_length = form_value.Unsigned();
+  return DWARFExpression(module,
+ DataExtractor(data, block_offset, block_length),
+ child.GetCU());
+};
+
+for (size_t i = 0; i < num_attributes; ++i) {
+  dw_attr_t attr = attributes.AttributeAtIndex(i);
+  switch (attr) {
+  case DW_AT_location:
+LocationInCallee = parse_simple_location(i);
+break;
+  case DW_AT_call_value:
+LocationInCaller = parse_simple_location(i);
+break;
+  }
+}
+
+if (LocationInCallee && LocationInCaller) {
+  if (!parameters)
+parameters.reset(new std::vector);
+  CallSiteParameter param = {*LocationInCallee, *LocationInCaller};
+  parameters->push_back(param);
+}
+  }
+  return parameters;
+}
+
 /// Collect cal

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

2019-09-09 Thread Vedant Kumar via Phabricator via lldb-commits

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

TODO:

- Split out llvm change.
- Add a test to validate that lldb skips inline frames when evaluating 
DW_OP_entry_value.




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/Makefile:4
+include $(LEVEL)/Makefile.rules
+CXXFLAGS += -g -O1 -glldb -Xclang -femit-debug-entry-values

aprantl wrote:
> The -g should be redundant, and if we leave out -glldb (which should be the 
> default for clang anyway) then this test in theory could also work with other 
> compilers such as GCC (which I think implements the same option under the 
> same -f option). I have no idea whether anyone runs such a bot, but it's 
> still nice.
llvm entry value support requires one of -glldb or -ggdb at the moment, but -g 
can be dropped.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1117
+  if ((!exe_ctx || !exe_ctx->HasTargetScope()) || !reg_ctx) {
+LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no exe/reg context");
+return false;

aprantl wrote:
> The Evaluate function allows setting an Error with a message. Should we do 
> the same thing here so we can return the error to the caller?
I worry that these log messages are too specific to be meaningful to the 
caller. The immediate caller should be able to issue an error, however.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp:62
 DRC_class DW_OP_value_to_class(uint32_t val) {
+  // FIXME: This switch should be simplified by using DW_OP_* names.
   switch (val) {

aprantl wrote:
> Don't spend too much time on that, we should just use the llvm 
> DWARFExpression iterator instead.
Updated the fixme.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3752
+LocationInCaller = parse_simple_location(i);
+break;
+  }

aprantl wrote:
> default?
I don't believe any action is needed in the default case: we do not want to log 
or report an error.


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

https://reviews.llvm.org/D67376



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

45 matches

Mail list logo