[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
aleksandr.urakov added a comment. Ping! Can you review this, please? I have just implemented the UDT types completion for PDB symbol files and I am preparing a patch for that, but it makes no sense without the part implemented in this one :) https://reviews.llvm.org/D49018 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
asmith accepted this revision. asmith added a comment. This revision is now accepted and ready to land. Except for minor formatting LGTM Comment at: lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp:7 + + return; +} Please remove the return Comment at: source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h:26 +//-- +/// Converts a location information from a PDB symbol to a DWARF expression +/// Are you sure about the /// ? Should these be // ? https://reviews.llvm.org/D49018 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
aleksandr.urakov marked an inline comment as done. aleksandr.urakov added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h:26 +//-- +/// Converts a location information from a PDB symbol to a DWARF expression +/// asmith wrote: > Are you sure about the /// ? > > Should these be // ? Yes, I think that /// should be there, it's a Doxygen comment (e.g. Visual Studio highlights info about a function and its parameters after that). https://reviews.llvm.org/D49018 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
aleksandr.urakov updated this revision to Diff 155154.
aleksandr.urakov added a comment.
Thanks a lot, I have updated the patch!
I also have added a test case for a non-zeroth frame (it became possible after
https://reviews.llvm.org/D49111).
If it's all right, can you commit this for me, please? I have no commit access.
https://reviews.llvm.org/D49018
Files:
lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
lit/SymbolFile/PDB/variables-locations.test
source/Plugins/SymbolFile/PDB/CMakeLists.txt
source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -47,6 +47,7 @@
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
#include "Plugins/SymbolFile/PDB/PDBASTParser.h"
+#include "Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h"
#include
@@ -869,11 +870,14 @@
auto mangled = GetMangledForPDBData(pdb_data);
auto mangled_cstr = mangled.empty() ? nullptr : mangled.c_str();
- DWARFExpression location(nullptr);
+ bool is_constant;
+ DWARFExpression location = ConvertPDBLocationToDWARFExpression(
+ GetObjectFile()->GetModule(), pdb_data, is_constant);
var_sp = std::make_shared(
var_uid, var_name.c_str(), mangled_cstr, type_sp, scope, context_scope,
ranges, &decl, location, is_external, is_artificial, is_static_member);
+ var_sp->SetLocationIsConstantValueData(is_constant);
m_variables.insert(std::make_pair(var_uid, var_sp));
return var_sp;
Index: source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h
===
--- /dev/null
+++ source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h
@@ -0,0 +1,45 @@
+//===-- PDBLocationToDWARFExpression.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef lldb_Plugins_SymbolFile_PDB_PDBLocationToDWARFExpression_h_
+#define lldb_Plugins_SymbolFile_PDB_PDBLocationToDWARFExpression_h_
+
+#include "lldb/Core/Module.h"
+
+namespace lldb_private {
+class DWARFExpression;
+}
+
+namespace llvm {
+namespace pdb {
+class PDBSymbolData;
+}
+} // namespace llvm
+
+//--
+/// Converts a location information from a PDB symbol to a DWARF expression
+///
+/// @param[in] module
+/// The module \a symbol belongs to.
+///
+/// @param[in] symbol
+/// The symbol with a location information to convert.
+///
+/// @param[out] is_constant
+/// Set to \b true if the result expression is a constant value data,
+/// and \b false if it is a DWARF bytecode.
+///
+/// @return
+/// The DWARF expression corresponding to the location data of \a symbol.
+//--
+lldb_private::DWARFExpression
+ConvertPDBLocationToDWARFExpression(lldb::ModuleSP module,
+const llvm::pdb::PDBSymbolData &symbol,
+bool &is_constant);
+#endif
Index: source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- /dev/null
+++ source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -0,0 +1,585 @@
+//===-- PDBLocationToDWARFExpression.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "PDBLocationToDWARFExpression.h"
+
+#include "lldb/Core/Section.h"
+#include "lldb/Core/StreamBuffer.h"
+#include "lldb/Core/dwarf.h"
+#include "lldb/Expression/DWARFExpression.h"
+#include "lldb/Utility/DataBufferHeap.h"
+
+#include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolData.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace llvm::pdb;
+
+namespace {
+const uint32_t g_code_view_to_lldb_registers_x86[] = {
+LLDB_INVALID_REGNUM, // CVRegNONE
+lldb_al_i386,// CVRegAL
+lldb_cl_i386,// CVRegCL
+lldb_dl_i386,// CVRegDL
+lldb_bl_i386,// CVRegBL
+lldb_ah_i386,// CVRegAH
+lldb_ch_i386,
[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks
amccarth accepted this revision. amccarth added a comment. This revision is now accepted and ready to land. LGTM if you don't want to consider my remaining suggestion for this patch. Thanks for the extra tests. Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:52 ASSERT_GT(parser->GetData().size(), 0UL); +auto result = parser->Initialize(); +if (expect_failure) lemo wrote: > amccarth wrote: > > lemo wrote: > > > amccarth wrote: > > > > I would rather see this function return the result of the Initialize > > > > and let the individual tests check the expectation explicitly. > > > > > > > > I know that will lead to a little bit of duplication in the tests, but > > > > it will make the individual tests easier to understand in isolation. > > > > It also makes it possible for each test to decide whether to ASSERT or > > > > EXPECT. And it eliminates the need for the bool parameter which is > > > > hard to decipher at the call sites. > > > Good idea, although gunit doesn't let us ASSERT in non-void returning > > > functions. > > > > > > I agree that the bool parameter is ugly, so I created a separate > > > TruncateMinidump() helper (which cleans up the SetUpData since the > > > load_size was only used for truncation) > > This isn't quit what I meant. I'd rather not have the ASSERTs in helper > > functions at all (regardless of return type). The helpers should return a > > status and the actual test code should do whatever ASSERT or EXPECT is > > appropriate. > So how would we handle the existing checks in SetupData()? SetUpData would just return an error status instead of ASSERTing. The actual ASSERTs would be placed in the tests that call SetUpData. As I said, that would add some duplication, because individual tests would have to ASSERT (or EXPECT) on the result of SetUpData, but it makes the tests easier to read and maintain the tests. Since there are other tests using SetUpData, they would have to be updated, so maybe you want to declare this suggestion as out-of-scope for this patch. Anyway, I'm happy that you eliminated the boolean argument. https://reviews.llvm.org/D49202 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks
lemo added a comment. Thanks Adrian for the thorough review. Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:52 ASSERT_GT(parser->GetData().size(), 0UL); +auto result = parser->Initialize(); +if (expect_failure) amccarth wrote: > lemo wrote: > > amccarth wrote: > > > lemo wrote: > > > > amccarth wrote: > > > > > I would rather see this function return the result of the Initialize > > > > > and let the individual tests check the expectation explicitly. > > > > > > > > > > I know that will lead to a little bit of duplication in the tests, > > > > > but it will make the individual tests easier to understand in > > > > > isolation. It also makes it possible for each test to decide whether > > > > > to ASSERT or EXPECT. And it eliminates the need for the bool > > > > > parameter which is hard to decipher at the call sites. > > > > Good idea, although gunit doesn't let us ASSERT in non-void returning > > > > functions. > > > > > > > > I agree that the bool parameter is ugly, so I created a separate > > > > TruncateMinidump() helper (which cleans up the SetUpData since the > > > > load_size was only used for truncation) > > > This isn't quit what I meant. I'd rather not have the ASSERTs in helper > > > functions at all (regardless of return type). The helpers should return > > > a status and the actual test code should do whatever ASSERT or EXPECT is > > > appropriate. > > So how would we handle the existing checks in SetupData()? > SetUpData would just return an error status instead of ASSERTing. The actual > ASSERTs would be placed in the tests that call SetUpData. As I said, that > would add some duplication, because individual tests would have to ASSERT (or > EXPECT) on the result of SetUpData, but it makes the tests easier to read and > maintain the tests. > > Since there are other tests using SetUpData, they would have to be updated, > so maybe you want to declare this suggestion as out-of-scope for this patch. > Anyway, I'm happy that you eliminated the boolean argument. I think I understand the idea and I agree it's tempting. The problem is that not all the checks in SetupData() are status based, so there's no direct mapping from each operation to a return value. It doesn't seem terribly bad to let the helper encapsulate some of the checks. https://reviews.llvm.org/D49202 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute
teemperor added a comment. @jingham I thought the same at first, but giving an error `command.empty()` turns out to be a problem. The command string contains the arguments (but not the command name itself). So when calling for example `bt` we would hit this error (as there are no args, so `command` is empty). I think this code was written with the assumption that `command` contains the whole command line or something like that. https://reviews.llvm.org/D49207 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Thanks. https://reviews.llvm.org/D48865 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute
jingham added a comment. Yeah, you are right about that. The command string presented to commands never contained the command name, that always got stripped off. But I don't think that ever left the command string to be nullptr... So you are right, that was just dead code. https://reviews.llvm.org/D49207 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r336918 - Restructure the minidump loading path and add early & explicit consistency checks
Author: lemo
Date: Thu Jul 12 10:27:18 2018
New Revision: 336918
URL: http://llvm.org/viewvc/llvm-project?rev=336918&view=rev
Log:
Restructure the minidump loading path and add early & explicit consistency
checks
Corrupted minidumps was leading to unpredictable behavior.
This change adds explicit consistency checks for the minidump early on. The
checks are not comprehensive but they should catch obvious structural
violations:
streams with type == 0
duplicate streams (same type)
overlapping streams
truncated minidumps
Another early check is to make sure we actually support the minidump
architecture
instead of crashing at a random place deep inside LLDB.
Differential Revision: https://reviews.llvm.org/D49202
Added:
lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
(with props)
lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
(with props)
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/trunk/unittests/Process/minidump/CMakeLists.txt
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=336918&r1=336917&r2=336918&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Thu Jul 12
10:27:18 2018
@@ -14,10 +14,13 @@
// Other libraries and framework includes
#include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Utility/LLDBAssert.h"
// C includes
// C++ includes
+#include
#include
+#include
using namespace lldb_private;
using namespace minidump;
@@ -27,46 +30,11 @@ MinidumpParser::Create(const lldb::DataB
if (data_buf_sp->GetByteSize() < sizeof(MinidumpHeader)) {
return llvm::None;
}
-
- llvm::ArrayRef header_data(data_buf_sp->GetBytes(),
- sizeof(MinidumpHeader));
- const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
-
- if (header == nullptr) {
-return llvm::None;
- }
-
- lldb::offset_t directory_list_offset = header->stream_directory_rva;
- // check if there is enough data for the parsing of the directory list
- if ((directory_list_offset +
- sizeof(MinidumpDirectory) * header->streams_count) >
- data_buf_sp->GetByteSize()) {
-return llvm::None;
- }
-
- const MinidumpDirectory *directory = nullptr;
- Status error;
- llvm::ArrayRef directory_data(
- data_buf_sp->GetBytes() + directory_list_offset,
- sizeof(MinidumpDirectory) * header->streams_count);
- llvm::DenseMap directory_map;
-
- for (uint32_t i = 0; i < header->streams_count; ++i) {
-error = consumeObject(directory_data, directory);
-if (error.Fail()) {
- return llvm::None;
-}
-directory_map[static_cast(directory->stream_type)] =
-directory->location;
- }
-
- return MinidumpParser(data_buf_sp, std::move(directory_map));
+ return MinidumpParser(data_buf_sp);
}
-MinidumpParser::MinidumpParser(
-const lldb::DataBufferSP &data_buf_sp,
-llvm::DenseMap &&directory_map)
-: m_data_sp(data_buf_sp), m_directory_map(directory_map) {}
+MinidumpParser::MinidumpParser(const lldb::DataBufferSP &data_buf_sp)
+: m_data_sp(data_buf_sp) {}
llvm::ArrayRef MinidumpParser::GetData() {
return llvm::ArrayRef(m_data_sp->GetBytes(),
@@ -480,3 +448,109 @@ MinidumpParser::GetMemoryRegionInfo(lldb
// appear truncated.
return info;
}
+
+Status MinidumpParser::Initialize() {
+ Status error;
+
+ lldbassert(m_directory_map.empty());
+
+ llvm::ArrayRef header_data(m_data_sp->GetBytes(),
+ sizeof(MinidumpHeader));
+ const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
+ if (header == nullptr) {
+error.SetErrorString("invalid minidump: can't parse the header");
+return error;
+ }
+
+ // A minidump without at least one stream is clearly ill-formed
+ if (header->streams_count == 0) {
+error.SetErrorString("invalid minidump: no streams present");
+return error;
+ }
+
+ struct FileRange {
+uint32_t offset = 0;
+uint32_t size = 0;
+
+FileRange(uint32_t offset, uint32_t size) : offset(offset), size(size) {}
+uint32_t end() const { return offset + size; }
+ };
+
+ const uint32_t file_size = m_data_sp->GetByteSize();
+
+ // Build a global minidump file map, checking for:
+ // - overlapping streams/data structures
+ // - truncation (streams pointing past the end of file)
+ std::vector minidump_map;
+
+ // Add the minidump header to the file map
+ if (sizeof
[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336918: Restructure the minidump loading path and add early
& explicit consistency… (authored by lemo, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49202?vs=155080&id=155212#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49202
Files:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/trunk/unittests/Process/minidump/CMakeLists.txt
lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
Index: lldb/trunk/unittests/Process/minidump/CMakeLists.txt
===
--- lldb/trunk/unittests/Process/minidump/CMakeLists.txt
+++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt
@@ -17,6 +17,8 @@
linux-x86_64.dmp
linux-x86_64_not_crashed.dmp
fizzbuzz_no_heap.dmp
- fizzbuzz_wow64.dmp)
+ fizzbuzz_wow64.dmp
+ bad_duplicate_streams.dmp
+ bad_overlapping_streams.dmp)
add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
Index: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -38,16 +38,32 @@
class MinidumpParserTest : public testing::Test {
public:
- void SetUpData(const char *minidump_filename,
- uint64_t load_size = UINT64_MAX) {
+ void SetUpData(const char *minidump_filename) {
std::string filename = GetInputFilePath(minidump_filename);
-auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0);
+auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, -1, 0);
+ASSERT_NE(BufferPtr, nullptr);
+llvm::Optional optional_parser =
+MinidumpParser::Create(BufferPtr);
+ASSERT_TRUE(optional_parser.hasValue());
+parser.reset(new MinidumpParser(optional_parser.getValue()));
+ASSERT_GT(parser->GetData().size(), 0UL);
+auto result = parser->Initialize();
+ASSERT_TRUE(result.Success()) << result.AsCString();
+ }
+
+ void InvalidMinidump(const char *minidump_filename, uint64_t load_size) {
+std::string filename = GetInputFilePath(minidump_filename);
+auto BufferPtr =
+DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0);
+ASSERT_NE(BufferPtr, nullptr);
llvm::Optional optional_parser =
MinidumpParser::Create(BufferPtr);
ASSERT_TRUE(optional_parser.hasValue());
parser.reset(new MinidumpParser(optional_parser.getValue()));
ASSERT_GT(parser->GetData().size(), 0UL);
+auto result = parser->Initialize();
+ASSERT_TRUE(result.Fail());
}
std::unique_ptr parser;
@@ -68,12 +84,15 @@
EXPECT_EQ(1232UL, context.size());
}
-TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
- SetUpData("linux-x86_64.dmp", 200);
- llvm::ArrayRef thread_list;
+TEST_F(MinidumpParserTest, TruncatedMinidumps) {
+ InvalidMinidump("linux-x86_64.dmp", 32);
+ InvalidMinidump("linux-x86_64.dmp", 100);
+ InvalidMinidump("linux-x86_64.dmp", 20 * 1024);
+}
- thread_list = parser->GetThreads();
- ASSERT_EQ(0UL, thread_list.size());
+TEST_F(MinidumpParserTest, IllFormedMinidumps) {
+ InvalidMinidump("bad_duplicate_streams.dmp", -1);
+ InvalidMinidump("bad_overlapping_streams.dmp", -1);
}
TEST_F(MinidumpParserTest, GetArchitecture) {
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
@@ -33,9 +33,6 @@
version != MinidumpHeaderConstants::Version)
return nullptr;
- // TODO check for max number of streams ?
- // TODO more sanity checks ?
-
return header;
}
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
@@ -89,13 +89,15 @@
llvm::Optional GetMemoryRegionInfo(lldb::addr_t);
+ // Perform consistency checks and initialize internal data structures
+ Status Initialize();
+
+private:
+ MinidumpParser(const lldb::DataBufferSP &data_buf_sp);
+
private:
lldb::DataBufferSP m_data_sp;
llvm::DenseMap m_directory_map;
-
- MinidumpParser(
- const lldb::DataBufferSP &data_buf_sp,
- llvm::DenseMap &&directory_map);
};
} //
[Lldb-commits] [lldb] r336948 - [process] Update the documentation for ReadMemory and DoReadMemory to include the error parameter
Author: stella.stamenova Date: Thu Jul 12 14:27:56 2018 New Revision: 336948 URL: http://llvm.org/viewvc/llvm-project?rev=336948&view=rev Log: [process] Update the documentation for ReadMemory and DoReadMemory to include the error parameter Summary: The current documentation does not include the error parameter. Reviewers: jingham, asmith Reviewed By: jingham Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D49251 Modified: lldb/trunk/include/lldb/Target/Process.h Modified: lldb/trunk/include/lldb/Target/Process.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=336948&r1=336947&r2=336948&view=diff == --- lldb/trunk/include/lldb/Target/Process.h (original) +++ lldb/trunk/include/lldb/Target/Process.h Thu Jul 12 14:27:56 2018 @@ -1696,8 +1696,15 @@ public: /// A byte buffer that is at least \a size bytes long that /// will receive the memory bytes. /// + /// @param[out] error + /// An error that indicates the success or failure of this + /// operation. If error indicates success (error.Success()), + /// then the value returned can be trusted, otherwise zero + /// will be returned. + /// /// @return /// The number of bytes that were actually read into \a buf. + /// Zero is returned in the case of an error. //-- virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status &error) = 0; @@ -1723,12 +1730,18 @@ public: /// @param[in] size /// The number of bytes to read. /// + /// @param[out] error + /// An error that indicates the success or failure of this + /// operation. If error indicates success (error.Success()), + /// then the value returned can be trusted, otherwise zero + /// will be returned. + /// /// @return /// The number of bytes that were actually read into \a buf. If /// the returned number is greater than zero, yet less than \a /// size, then this function will get called again with \a /// vm_addr, \a buf, and \a size updated appropriately. Zero is - /// returned to indicate an error. + /// returned in the case of an error. //-- virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status &error); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute
teemperor accepted this revision. teemperor added a comment. Davide also approved it offline (thx). https://reviews.llvm.org/D49207 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r336955 - Get rid of the C-string parameter in DoExecute
Author: teemperor
Date: Thu Jul 12 15:28:52 2018
New Revision: 336955
URL: http://llvm.org/viewvc/llvm-project?rev=336955&view=rev
Log:
Get rid of the C-string parameter in DoExecute
Summary:
This patch gets rid of the C-string parameter in the
RawCommandObject::DoExecute function,
making the code simpler and less memory unsafe.
There seems to be a assumption in some command objects that this parameter
could be a nullptr,
but from what I can see the rest of the API doesn't actually allow this (and
other command
objects and related code pieces dereference this parameter without any checks).
Especially CommandObjectRegexCommand has error handling code for a nullptr that
is now gone.
Reviewers: davide, jingham, teemperor
Reviewed By: teemperor
Subscribers: jingham, lldb-commits
Differential Revision: https://reviews.llvm.org/D49207
Modified:
lldb/trunk/include/lldb/Interpreter/CommandObject.h
lldb/trunk/include/lldb/Interpreter/CommandObjectRegexCommand.h
lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
lldb/trunk/source/Commands/CommandObjectCommands.cpp
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Commands/CommandObjectExpression.h
lldb/trunk/source/Commands/CommandObjectPlatform.cpp
lldb/trunk/source/Commands/CommandObjectSettings.cpp
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Commands/CommandObjectType.cpp
lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp
lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp
lldb/trunk/source/Interpreter/CommandObjectScript.cpp
lldb/trunk/source/Interpreter/CommandObjectScript.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
Modified: lldb/trunk/include/lldb/Interpreter/CommandObject.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandObject.h?rev=336955&r1=336954&r2=336955&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandObject.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandObject.h Thu Jul 12 15:28:52 2018
@@ -434,7 +434,8 @@ public:
bool Execute(const char *args_string, CommandReturnObject &result) override;
protected:
- virtual bool DoExecute(const char *command, CommandReturnObject &result) = 0;
+ virtual bool DoExecute(llvm::StringRef command,
+ CommandReturnObject &result) = 0;
bool WantsRawCommandString() override { return true; }
};
Modified: lldb/trunk/include/lldb/Interpreter/CommandObjectRegexCommand.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandObjectRegexCommand.h?rev=336955&r1=336954&r2=336955&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandObjectRegexCommand.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandObjectRegexCommand.h Thu Jul 12
15:28:52 2018
@@ -43,7 +43,7 @@ public:
int HandleCompletion(CompletionRequest &request) override;
protected:
- bool DoExecute(const char *command, CommandReturnObject &result) override;
+ bool DoExecute(llvm::StringRef command, CommandReturnObject &result)
override;
struct Entry {
RegularExpression regex;
Modified: lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h?rev=336955&r1=336954&r2=336955&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h (original)
+++ lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h Thu Jul 12 15:28:52
2018
@@ -96,13 +96,13 @@ public:
virtual bool Interrupt() { return false; }
virtual bool ExecuteOneLine(
- const char *command, CommandReturnObject *result,
+ llvm::StringRef command, CommandReturnObject *result,
const ExecuteScriptOptions &options = ExecuteScriptOptions()) = 0;
virtual void ExecuteInterpreterLoop() = 0;
virtual bool ExecuteOneLineWithReturn(
- const char *in_string, ScriptReturnType return_type, void *ret_value,
+ llvm::StringRef in_string, ScriptReturnType return_type, void *ret_value,
const ExecuteScriptOptions &options = ExecuteScriptOptions()) {
return true;
}
@@ -343,7 +343,7 @@ public:
}
virtual bool
- RunScriptBasedCommand(const char *impl_function, const char *args,
+ RunScriptBasedCommand(const char *impl_function, llvm::StringRef args,
ScriptedCommandSynchronicity syn
[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute
This revision was not accepted when it landed; it landed in state "Needs
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336955: Get rid of the C-string parameter in DoExecute
(authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49207?vs=155079&id=155286#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49207
Files:
lldb/trunk/include/lldb/Interpreter/CommandObject.h
lldb/trunk/include/lldb/Interpreter/CommandObjectRegexCommand.h
lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
lldb/trunk/source/Commands/CommandObjectCommands.cpp
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Commands/CommandObjectExpression.h
lldb/trunk/source/Commands/CommandObjectPlatform.cpp
lldb/trunk/source/Commands/CommandObjectSettings.cpp
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Commands/CommandObjectType.cpp
lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp
lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp
lldb/trunk/source/Interpreter/CommandObjectScript.cpp
lldb/trunk/source/Interpreter/CommandObjectScript.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
Index: lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp
===
--- lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp
+++ lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp
@@ -35,53 +35,47 @@
//--
CommandObjectRegexCommand::~CommandObjectRegexCommand() {}
-bool CommandObjectRegexCommand::DoExecute(const char *command,
+bool CommandObjectRegexCommand::DoExecute(llvm::StringRef command,
CommandReturnObject &result) {
- if (command) {
-EntryCollection::const_iterator pos, end = m_entries.end();
-for (pos = m_entries.begin(); pos != end; ++pos) {
- RegularExpression::Match regex_match(m_max_matches);
-
- if (pos->regex.Execute(command, ®ex_match)) {
-std::string new_command(pos->command);
-std::string match_str;
-char percent_var[8];
-size_t idx, percent_var_idx;
-for (uint32_t match_idx = 1; match_idx <= m_max_matches; ++match_idx) {
- if (regex_match.GetMatchAtIndex(command, match_idx, match_str)) {
-const int percent_var_len =
-::snprintf(percent_var, sizeof(percent_var), "%%%u", match_idx);
-for (idx = 0; (percent_var_idx = new_command.find(
- percent_var, idx)) != std::string::npos;) {
- new_command.erase(percent_var_idx, percent_var_len);
- new_command.insert(percent_var_idx, match_str);
- idx += percent_var_idx + match_str.size();
-}
+ EntryCollection::const_iterator pos, end = m_entries.end();
+ for (pos = m_entries.begin(); pos != end; ++pos) {
+RegularExpression::Match regex_match(m_max_matches);
+
+if (pos->regex.Execute(command, ®ex_match)) {
+ std::string new_command(pos->command);
+ std::string match_str;
+ char percent_var[8];
+ size_t idx, percent_var_idx;
+ for (uint32_t match_idx = 1; match_idx <= m_max_matches; ++match_idx) {
+if (regex_match.GetMatchAtIndex(command, match_idx, match_str)) {
+ const int percent_var_len =
+ ::snprintf(percent_var, sizeof(percent_var), "%%%u", match_idx);
+ for (idx = 0; (percent_var_idx = new_command.find(
+ percent_var, idx)) != std::string::npos;) {
+new_command.erase(percent_var_idx, percent_var_len);
+new_command.insert(percent_var_idx, match_str);
+idx += percent_var_idx + match_str.size();
}
}
-// Interpret the new command and return this as the result!
-if (m_interpreter.GetExpandRegexAliases())
- result.GetOutputStream().Printf("%s\n", new_command.c_str());
-// Pass in true for "no context switching". The command that called us
-// should have set up the context appropriately, we shouldn't have to
-// redo that.
-return m_interpreter.HandleCommand(new_command.c_str(),
- eLazyBoolCalculate, result, nullptr,
- true, true);
}
+ // Interpret the new command and return this as the result!
+ if (m_interpreter.GetExpandRegexAliases())
+result.GetOutput
[Lldb-commits] [lldb] r336956 - Remove incorrect thread-pc-values clearing
Author: jmolenda
Date: Thu Jul 12 15:45:41 2018
New Revision: 336956
URL: http://llvm.org/viewvc/llvm-project?rev=336956&view=rev
Log:
Remove incorrect thread-pc-values clearing
from ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue.
Patch by Venkata Ramanaiah.
Differential Revision: https://reviews.llvm.org/D48868
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=336956&r1=336955&r2=336956&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu Jul
12 15:45:41 2018
@@ -1523,7 +1523,6 @@ void ProcessGDBRemote::ClearThreadIDList
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value)
{
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336956: Remove incorrect thread-pc-values clearing (authored
by jmolenda, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48868?vs=154947&id=155291#toc
Repository:
rL LLVM
https://reviews.llvm.org/D48868
Files:
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1523,7 +1523,6 @@
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value)
{
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1523,7 +1523,6 @@
size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value) {
m_thread_ids.clear();
- m_thread_pcs.clear();
size_t comma_pos;
lldb::tid_t tid;
while ((comma_pos = value.find(',')) != std::string::npos) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
shafik created this revision.
shafik added reviewers: jingham, friss, jasonmolenda.
Herald added subscribers: christof, mgorny.
Herald added a reviewer: EricWF.
Add formattors for libc++ std::optional and tests to verify the new formattors.
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/LibCxx.cpp
source/Plugins/Language/CPlusPlus/LibCxx.h
source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,91 @@
+//===-- LibCxxOptional.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd: public SyntheticChildrenFrontEnd {
+public:
+ OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+ }
+
+ size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+ }
+
+ bool MightHaveChildren() override { return true; }
+ bool Update() override;
+ size_t CalculateNumChildren() override { return m_elements.size(); }
+ ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+ std::vector m_elements;
+ ValueObjectSP m_base_sp;
+};
+}
+
+bool OptionalFrontEnd::Update() {
+ m_elements.clear();
+
+ ValueObjectSP engaged_sp( m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+ if (!engaged_sp) {
+return false;
+ }
+
+ uint64_t num_elements = engaged_sp->GetValueAsSigned(0) ;
+
+ m_elements.assign(num_elements, ValueObjectSP());
+ return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+ if (idx >= m_elements.size()) {
+return ValueObjectSP();
+ }
+
+ //ValueObjectSP val_sp( m_backend.GetChildMemberWithName(ConstString("__val_"), true));
+ ValueObjectSP val_sp( m_backend.GetChildMemberWithName(ConstString("__engaged_") , true)->GetParent()->GetChildAtIndex(0,true)->GetChildMemberWithName(ConstString("__val_"), true) );
+
+ if (!val_sp) {
+ fprintf( stderr, "no __val_!\n" ) ;
+return ValueObjectSP();
+ }
+
+ if (m_elements[idx]) {
+return m_elements[idx];
+ }
+
+ CompilerType holder_type =
+val_sp->GetCompilerType() ;
+
+ if (!holder_type) {
+ fprintf( stderr, "no holder_type!\n" ) ;
+return ValueObjectSP();
+ }
+
+m_elements[idx] =
+val_sp->Clone(ConstString(llvm::formatv("[{0}]", idx).str()));
+
+ return m_elements[idx];
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP valobj_sp) {
+ if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+ return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,12 @@
ValueObject &valobj, Stream &stream,
const TypeSummaryOptions &options); // libc++ std::wstring
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions
+&options); // libc++ std::optional<>
+
+
bool LibcxxSmartPointerSummaryProvider(
ValueObject &valobj, Stream &stream,
const TypeSummaryOptions
@@ -133,6 +139,9 @@
SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
lldb::ValueObjectSP);
+SyntheticChildrenFrontEnd *LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+ lldb::ValueObjectSP valobj_sp) ;
+
} // namespace formatters
} // namespace lldb_private
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,25 @@
using namespace lldb_private;
using nam
[Lldb-commits] [lldb] r336960 - [lldbsuite] The test inside TestOverloadedFunctions.py has the wrong class name
Author: stella.stamenova Date: Thu Jul 12 16:02:33 2018 New Revision: 336960 URL: http://llvm.org/viewvc/llvm-project?rev=336960&view=rev Log: [lldbsuite] The test inside TestOverloadedFunctions.py has the wrong class name Summary: It looks like the test file was copied from TestCPPStaticMethods.py because they have the same name. This means that the two tests will try to write to the same output files and will either overwrite each other's output or occasionally cause failures because they can't both access the same file. Reviewers: asmith, zturner Reviewed By: zturner Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D49261 Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/overloaded-functions/TestOverloadedFunctions.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/overloaded-functions/TestOverloadedFunctions.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/overloaded-functions/TestOverloadedFunctions.py?rev=336960&r1=336959&r2=336960&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/overloaded-functions/TestOverloadedFunctions.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/overloaded-functions/TestOverloadedFunctions.py Thu Jul 12 16:02:33 2018 @@ -8,7 +8,7 @@ from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil -class CPPStaticMethodsTestCase(TestBase): +class OverloadedFunctionsTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
davide added a comment.
Some comments. Jonas looked at many formatters recently so he's in a good
position to take a look.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile:9
+CXXFLAGS += -std=c++17
+#CFLAGS += -std=c++17
commented out code?
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21-22
+@add_test_categories(["libc++"])
+##@skipIf(debug_info="gmodules",
+##bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
+def test_with_run_command(self):
ditto
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:14-38
+optional_int number ;
+
+printf( "%d\n", number.has_value() ) ; // break here
+
+number = 42 ;
+
+printf( "%d\n", *number ) ; // break here
You don't really need to se all these breakpoints.
You just need set one on the return and print all the types.
Also, an `lldbInline` python test here should suffice and it's probably more
readable (grep for lldbInline).
Comment at: source/Plugins/Language/CPlusPlus/CMakeLists.txt:14-15
LibCxxTuple.cpp
+ LibCxxOptional.cpp
LibCxxUnorderedMap.cpp
LibCxxVector.cpp
Please sort this.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:48-51
+ const char *engaged_as_cstring = engaged_sp->GetValueAsUnsigned(0) == 1 ?
"true" : "false" ;
+
+ stream.Printf(" engaged=%s", engaged_as_cstring );
+
Can you use StringRef?
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.h:30-33
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions
+&options); // libc++ std::optional<>
Please clang-format this patch.
Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:59
+
+ //ValueObjectSP val_sp(
m_backend.GetChildMemberWithName(ConstString("__val_"), true));
+ ValueObjectSP val_sp(
m_backend.GetChildMemberWithName(ConstString("__engaged_") ,
true)->GetParent()->GetChildAtIndex(0,true)->GetChildMemberWithName(ConstString("__val_"),
true) );
Please remove this commented out code.
Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:63
+ if (!val_sp) {
+ fprintf( stderr, "no __val_!\n" ) ;
+return ValueObjectSP();
if you want to log diagnostics, you might consider using the `LOG` facility and
then add these to the `data formatters` channel.
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
jingham requested changes to this revision.
jingham added a comment.
It seems a little odd to choose to represent this as an array with 0 elements.
I think I would be confused by that. You can maybe choose a better name like
"value" for your cloned object. But it looks like it is also possible to make
a Synthetic Child Provider that just replaces the value of the object with the
value provided by the synthetic child provider. That's done in the Python
front end by saying you have no children and then providing a the "get_value"
method. It must be possible to do this from C++ as well, but it will take a
little thought to figure out how that works. But that would be the most
natural way to present this, I think.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:26-38
+self.runCmd("file " + self.getBuildArtifact("a.out"),
CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "break here"))
+
+self.runCmd("run", RUN_SUCCEEDED)
you can do this more cleanly with:
(self.target, _, _, bkpt) = lldbutil.run_to_source_breakpoint(self, "break
here", lldb.SBFileSpec("main.cpp"))
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:39-48
+# This is the function to remove the custom formats in order to have a
+# clean slate for the next test case.
+def cleanup():
+self.runCmd('type format clear', check=False)
+self.runCmd('type summary clear', check=False)
+self.runCmd('type filter clear', check=False)
+self.runCmd('type synth clear', check=False)
I don't think you need this cleanup, you aren't adding any formatters, you're
just testing built-in ones.
Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:63
+ if (!val_sp) {
+ fprintf( stderr, "no __val_!\n" ) ;
+return ValueObjectSP();
If you want to log this, get the formatters log channel and put the text there.
Then somebody will see this when they turn on the log. An fprintf like this
won't help anybody.
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
Sure, I will do it Monday if someone hasn’t done it already. > On Jul 12, 2018, at 9:22 PM, Aleksandr Urakov via Phabricator > wrote: > > aleksandr.urakov updated this revision to Diff 155154. > aleksandr.urakov added a comment. > > Thanks a lot, I have updated the patch! > > I also have added a test case for a non-zeroth frame (it became possible > after https://reviews.llvm.org/D49111). > > If it's all right, can you commit this for me, please? I have no commit > access. > > > https://reviews.llvm.org/D49018 > > Files: > lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp > lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script > lit/SymbolFile/PDB/variables-locations.test > source/Plugins/SymbolFile/PDB/CMakeLists.txt > source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp > source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h > source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
asmith added a subscriber: aleksandr.urakov. asmith added a comment. Sure, I will do it Monday if someone hasn’t done it already. https://reviews.llvm.org/D49018 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server
xiaobai created this revision.
xiaobai added reviewers: zturner, labath, sas.
Herald added a subscriber: mgorny.
There is currently a way to skip the debugserver build. See how the CMake
variables SKIP_DEBUGSERVER and LLDB_CODESIGN_IDENTITY are used if you're
interested in that.
I'd like the option to skip buildng lldb-server as well. I work on a debug
server called ds2 and I often don't need to build lldb-server when I build lldb.
https://reviews.llvm.org/D49282
Files:
cmake/modules/LLDBConfig.cmake
tools/CMakeLists.txt
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -5,7 +5,7 @@
add_subdirectory(argdumper)
add_subdirectory(driver)
add_subdirectory(lldb-mi)
-if (LLDB_CAN_USE_LLDB_SERVER)
+if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD)
add_subdirectory(lldb-server)
endif()
add_subdirectory(intel-features)
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -358,6 +358,8 @@
list(APPEND system_libs ${CMAKE_DL_LIBS})
+SET(SKIP_LLDB_SERVER_BUILD OFF CACHE BOOL "Skip building lldb-server")
+
# Figure out if lldb could use lldb-server. If so, then we'll
# ensure we build lldb-server when an lldb target is being built.
if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -5,7 +5,7 @@
add_subdirectory(argdumper)
add_subdirectory(driver)
add_subdirectory(lldb-mi)
-if (LLDB_CAN_USE_LLDB_SERVER)
+if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD)
add_subdirectory(lldb-server)
endif()
add_subdirectory(intel-features)
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -358,6 +358,8 @@
list(APPEND system_libs ${CMAKE_DL_LIBS})
+SET(SKIP_LLDB_SERVER_BUILD OFF CACHE BOOL "Skip building lldb-server")
+
# Figure out if lldb could use lldb-server. If so, then we'll
# ensure we build lldb-server when an lldb target is being built.
if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
