[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
ramana-nvr updated this revision to Diff 154947.
ramana-nvr added a comment.
For now, leaving the m_thread_pcs.clear() in UpdateThreadIDList() unchanged as
it is not causing any problems.
https://reviews.llvm.org/D48868
Files:
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ 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: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ 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] D49192: [FileCheck] Add -allow-deprecated-dag-overlap to failing lldb tests
jdenny created this revision. jdenny added a reviewer: probinson. Herald added a subscriber: JDevlieghere. See https://reviews.llvm.org/D47106 for details. https://reviews.llvm.org/D49192 Files: lldb/lit/SymbolFile/DWARF/find-basic-function.cpp Index: lldb/lit/SymbolFile/DWARF/find-basic-function.cpp === --- lldb/lit/SymbolFile/DWARF/find-basic-function.cpp +++ lldb/lit/SymbolFile/DWARF/find-basic-function.cpp @@ -7,7 +7,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -21,7 +21,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -36,7 +36,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ Index: lldb/lit/SymbolFile/DWARF/find-basic-function.cpp === --- lldb/lit/SymbolFile/DWARF/find-basic-function.cpp +++ lldb/lit/SymbolFile/DWARF/find-basic-function.cpp @@ -7,7 +7,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -21,7 +21,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -36,7 +36,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r336824 - Allow specifying an exit code for the 'quit' command
Author: teemperor Date: Wed Jul 11 10:18:01 2018 New Revision: 336824 URL: http://llvm.org/viewvc/llvm-project?rev=336824&view=rev Log: Allow specifying an exit code for the 'quit' command Summary: This patch adds the possibility to specify an exit code when calling quit. We accept any int, even though it depends on the user what happens if the int is out of the range of what the operating system supports as exit codes. Fixes rdar://problem/38452312 Reviewers: davide, jingham, clayborg Reviewed By: jingham Subscribers: clayborg, jingham, lldb-commits Differential Revision: https://reviews.llvm.org/D48659 Added: lldb/trunk/lit/Quit/ lldb/trunk/lit/Quit/TestQuitExitCode-30.test lldb/trunk/lit/Quit/TestQuitExitCode0.test lldb/trunk/lit/Quit/TestQuitExitCode30.test lldb/trunk/lit/Quit/TestQuitExitCodeHex0.test lldb/trunk/lit/Quit/TestQuitExitCodeHexA.test lldb/trunk/lit/Quit/TestQuitExitCodeImplicit0.test lldb/trunk/lit/Quit/TestQuitExitCodeNonInt.test lldb/trunk/lit/Quit/TestQuitExitCodeTooManyArgs.test lldb/trunk/lit/Quit/expect_exit_code.py (with props) lldb/trunk/lit/Quit/lit.local.cfg lldb/trunk/packages/Python/lldbsuite/test/quit/ lldb/trunk/packages/Python/lldbsuite/test/quit/TestQuit.py Modified: lldb/trunk/include/lldb/API/SBCommandInterpreter.h lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h lldb/trunk/scripts/interface/SBCommandInterpreter.i lldb/trunk/source/API/SBCommandInterpreter.cpp lldb/trunk/source/Commands/CommandObjectQuit.cpp lldb/trunk/source/Interpreter/CommandInterpreter.cpp lldb/trunk/tools/driver/Driver.cpp lldb/trunk/tools/driver/Driver.h Modified: lldb/trunk/include/lldb/API/SBCommandInterpreter.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBCommandInterpreter.h?rev=336824&r1=336823&r2=336824&view=diff == --- lldb/trunk/include/lldb/API/SBCommandInterpreter.h (original) +++ lldb/trunk/include/lldb/API/SBCommandInterpreter.h Wed Jul 11 10:18:01 2018 @@ -208,6 +208,25 @@ public: void SetPromptOnQuit(bool b); //-- + /// Sets whether the command interpreter should allow custom exit codes + /// for the 'quit' command. + //-- + void AllowExitCodeOnQuit(bool allow); + + //-- + /// Returns true if the user has called the 'quit' command with a custom exit + /// code. + //-- + bool HasCustomQuitExitCode(); + + //-- + /// Returns the exit code that the user has specified when running the + /// 'quit' command. Returns 0 if the user hasn't called 'quit' at all or + /// without a custom exit code. + //-- + int GetQuitStatus(); + + //-- /// Resolve the command just as HandleCommand would, expanding abbreviations /// and aliases. If successful, result->GetOutput has the full expansion. //-- Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=336824&r1=336823&r2=336824&view=diff == --- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original) +++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h Wed Jul 11 10:18:01 2018 @@ -455,6 +455,30 @@ public: void SetPromptOnQuit(bool b); + //-- + /// Specify if the command interpreter should allow that the user can + /// specify a custom exit code when calling 'quit'. + //-- + void AllowExitCodeOnQuit(bool allow); + + //-- + /// Sets the exit code for the quit command. + /// @param[in] exit_code + /// The exit code that the driver should return on exit. + /// @return True if the exit code was successfully set; false if the + /// interpreter doesn't allow custom exit codes. + /// @see AllowExitCodeOnQuit + //-- + LLVM_NODISCARD bool SetQuitExitCode(int exit_code); + + //-- + /// Returns the exit code that the user has specified when running the + /// 'quit' command. + /// @param[out] exited + /// Set to true if the user has ca
[Lldb-commits] [PATCH] D48659: Allow specifying an exit code for the 'quit' command
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336824: Allow specifying an exit code for the
'quit' command (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48659?vs=154318&id=155030#toc
Repository:
rL LLVM
https://reviews.llvm.org/D48659
Files:
lldb/trunk/include/lldb/API/SBCommandInterpreter.h
lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
lldb/trunk/lit/Quit/TestQuitExitCode-30.test
lldb/trunk/lit/Quit/TestQuitExitCode0.test
lldb/trunk/lit/Quit/TestQuitExitCode30.test
lldb/trunk/lit/Quit/TestQuitExitCodeHex0.test
lldb/trunk/lit/Quit/TestQuitExitCodeHexA.test
lldb/trunk/lit/Quit/TestQuitExitCodeImplicit0.test
lldb/trunk/lit/Quit/TestQuitExitCodeNonInt.test
lldb/trunk/lit/Quit/TestQuitExitCodeTooManyArgs.test
lldb/trunk/lit/Quit/expect_exit_code.py
lldb/trunk/lit/Quit/lit.local.cfg
lldb/trunk/packages/Python/lldbsuite/test/quit/TestQuit.py
lldb/trunk/scripts/interface/SBCommandInterpreter.i
lldb/trunk/source/API/SBCommandInterpreter.cpp
lldb/trunk/source/Commands/CommandObjectQuit.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/tools/driver/Driver.cpp
lldb/trunk/tools/driver/Driver.h
Index: lldb/trunk/source/API/SBCommandInterpreter.cpp
===
--- lldb/trunk/source/API/SBCommandInterpreter.cpp
+++ lldb/trunk/source/API/SBCommandInterpreter.cpp
@@ -379,6 +379,23 @@
m_opaque_ptr->SetPromptOnQuit(b);
}
+void SBCommandInterpreter::AllowExitCodeOnQuit(bool allow) {
+ if (m_opaque_ptr)
+m_opaque_ptr->AllowExitCodeOnQuit(allow);
+}
+
+bool SBCommandInterpreter::HasCustomQuitExitCode() {
+ bool exited = false;
+ if (m_opaque_ptr)
+m_opaque_ptr->GetQuitExitCode(exited);
+ return exited;
+}
+
+int SBCommandInterpreter::GetQuitStatus() {
+ bool exited = false;
+ return (m_opaque_ptr ? m_opaque_ptr->GetQuitExitCode(exited) : 0);
+}
+
void SBCommandInterpreter::ResolveCommand(const char *command_line,
SBCommandReturnObject &result) {
result.Clear();
Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp
@@ -144,6 +144,26 @@
m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
}
+void CommandInterpreter::AllowExitCodeOnQuit(bool allow) {
+ m_allow_exit_code = allow;
+ if (!allow)
+m_quit_exit_code.reset();
+}
+
+bool CommandInterpreter::SetQuitExitCode(int exit_code) {
+ if (!m_allow_exit_code)
+return false;
+ m_quit_exit_code = exit_code;
+ return true;
+}
+
+int CommandInterpreter::GetQuitExitCode(bool &exited) const {
+ exited = m_quit_exit_code.hasValue();
+ if (exited)
+return *m_quit_exit_code;
+ return 0;
+}
+
void CommandInterpreter::ResolveCommand(const char *command_line,
CommandReturnObject &result) {
std::string command = command_line;
Index: lldb/trunk/source/Commands/CommandObjectQuit.cpp
===
--- lldb/trunk/source/Commands/CommandObjectQuit.cpp
+++ lldb/trunk/source/Commands/CommandObjectQuit.cpp
@@ -16,6 +16,7 @@
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Target/Process.h"
+#include "lldb/Utility/StreamString.h"
using namespace lldb;
using namespace lldb_private;
@@ -26,7 +27,7 @@
CommandObjectQuit::CommandObjectQuit(CommandInterpreter &interpreter)
: CommandObjectParsed(interpreter, "quit", "Quit the LLDB debugger.",
- "quit") {}
+ "quit [exit-code]") {}
CommandObjectQuit::~CommandObjectQuit() {}
@@ -77,6 +78,41 @@
return false;
}
}
+
+ if (command.GetArgumentCount() > 1) {
+result.AppendError("Too many arguments for 'quit'. Only an optional exit "
+ "code is allowed");
+result.SetStatus(eReturnStatusFailed);
+return false;
+ }
+
+ if (command.GetArgumentCount() > 1) {
+result.AppendError("Too many arguments for 'quit'. Only an optional exit "
+ "code is allowed");
+result.SetStatus(eReturnStatusFailed);
+return false;
+ }
+
+ // We parse the exit code argument if there is one.
+ if (command.GetArgumentCount() == 1) {
+llvm::StringRef arg = command.GetArgumentAtIndex(0);
+int exit_code;
+if (arg.getAsInteger(/*autodetect radix*/ 0, exit_code)) {
+ lldb_private::StreamString s;
+ std::string arg_str = arg.str();
+ s.Printf("Couldn't parse '%s' as integer for exit code.", arg_str.data());
+ result.AppendError(
[Lldb-commits] [PATCH] D49192: [FileCheck] Add -allow-deprecated-dag-overlap to failing lldb tests
probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D49192 ___ 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 created this revision.
lemo added reviewers: amccarth, labath.
lemo added a project: LLDB.
Herald added a subscriber: mgrang.
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.
https://reviews.llvm.org/D49202
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -39,15 +39,21 @@
class MinidumpParserTest : public testing::Test {
public:
void SetUpData(const char *minidump_filename,
- uint64_t load_size = UINT64_MAX) {
+ uint64_t load_size = UINT64_MAX,
+ bool expect_failure = false) {
std::string filename = GetInputFilePath(minidump_filename);
auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0);
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();
+if (expect_failure)
+ ASSERT_TRUE(result.Fail());
+else
+ ASSERT_TRUE(result.Success()) << result.AsCString();
}
std::unique_ptr parser;
@@ -68,12 +74,10 @@
EXPECT_EQ(1232UL, context.size());
}
-TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
- SetUpData("linux-x86_64.dmp", 200);
- llvm::ArrayRef thread_list;
-
- thread_list = parser->GetThreads();
- ASSERT_EQ(0UL, thread_list.size());
+TEST_F(MinidumpParserTest, TruncatedMinidumps) {
+ SetUpData("linux-x86_64.dmp", 32, true);
+ SetUpData("linux-x86_64.dmp", 100, true);
+ SetUpData("linux-x86_64.dmp", 20 * 1024, true);
}
TEST_F(MinidumpParserTest, GetArchitecture) {
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -105,7 +105,7 @@
if (!DataPtr)
return nullptr;
- assert(DataPtr->GetByteSize() == header_size);
+ lldbassert(DataPtr->GetByteSize() == header_size);
// first, only try to parse the header, beacuse we need to be fast
llvm::ArrayRef HeaderBytes = DataPtr->GetData();
@@ -164,14 +164,33 @@
Status ProcessMinidump::DoLoadCore() {
Status error;
+ // Minidump parser initialization & consistency checks
+ error = m_minidump_parser.Initialize();
+ if (error.Fail())
+return error;
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
+ switch (arch.GetMachine()) {
+ case llvm::Triple::x86:
+ case llvm::Triple::x86_64:
+// supported
+break;
+
+ default:
+error.SetErrorStringWithFormat("Unsupported minidump architecture: %s",
+ arch.GetArchitectureName());
+return error;
+ }
+
m_thread_list = m_minidump_parser.GetThreads();
m_active_exception = m_minidump_parser.GetExceptionStream();
ReadModuleList();
- GetTarget().SetArchitecture(GetArchitecture());
+ GetTarget().SetArchitecture(arch);
llvm::Optional pid = m_minidump_parser.GetPid();
if (!pid) {
-error.SetErrorString("failed to parse PID");
+error.SetErrorString("Failed to parse PID");
return error;
}
SetID(pid.getValue());
Index: source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ 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: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -89,14 +89,16 @@
llvm::Optional GetMemoryRegionInfo(lldb::addr_t);
+ // Perform consistency checks and initialize internal data structures
+ Status Initialize();
+
+private:
+ MinidumpParser(const lldb::DataBufferSP &data_bu
[Lldb-commits] [PATCH] D49192: [FileCheck] Add -allow-deprecated-dag-overlap to failing lldb tests
This revision was automatically updated to reflect the committed changes. Closed by commit rL336846: [FileCheck] Add -allow-deprecated-dag-overlap to failing lldb tests (authored by jdenny, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49192?vs=155008&id=155056#toc Repository: rL LLVM https://reviews.llvm.org/D49192 Files: lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp Index: lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp === --- lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp +++ lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp @@ -7,7 +7,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -21,7 +21,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -36,7 +36,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ Index: lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp === --- lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp +++ lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp @@ -7,7 +7,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -21,7 +21,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ @@ -36,7 +36,7 @@ // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \ // RUN: FileCheck --check-prefix=METHOD %s // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \ -// RUN: FileCheck --check-prefix=FULL %s +// RUN: FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \ // RUN: FileCheck --check-prefix=FULL-MANGLED %s // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \ ___ 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
jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. looks good. https://reviews.llvm.org/D48868 ___ 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
amccarth added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:23
#include
+#include
+#include
Why add ``? It looks like your new map is just a vector.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:367
// appropriate range linearly each time is stupid. Perhaps we should build
- // an index for faster lookups.
+ // an i for faster lookups.
llvm::Optional range = FindMemoryRange(addr);
This looks like an inadvertent change. Please change "i" back to "index".
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:454
+Status MinidumpParser::Initialize() {
+ Status result;
+
For consistency, please consider renaming `result` to `error`.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:558
+ return result;
+}
This is a long function (which, unfortunately, is not unusual in LLVM/LLDB),
but the comments here are really good, so I'm OK with it.
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:93
+ // Perform consistency checks and initialize internal data structures
+ Status Initialize();
+
I'm not a big fan of 2-step initialization, but that seems to be the way of
LLDB. :-(
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:172
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
Should the architecture check be in the MinidumpParser::Initialize with the
other checks?
I don't know the answer; I'm just asking for your thinking about this.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:193
if (!pid) {
-error.SetErrorString("failed to parse PID");
+error.SetErrorString("Failed to parse PID");
return error;
I realize nothing is perfectly consistent, but I think LLVM tends to start
error strings with a lowercase letter (except for proper nouns). Can you
revert this case change and make sure your new error strings follow that
pattern?
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:52
ASSERT_GT(parser->GetData().size(), 0UL);
+auto result = parser->Initialize();
+if (expect_failure)
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.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:80
+ SetUpData("linux-x86_64.dmp", 100, true);
+ SetUpData("linux-x86_64.dmp", 20 * 1024, true);
}
See comment on SetUpData. As much as practical, I'd rather have the EXPECTs
and ASSERTs directly in the tests rather than hiding them in a helper function.
Also note that, while the first two arguments are pretty intuitive, there's no
way to understand the true/false in the third argument without going to look up
to see what SetUpData does.
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 inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:23
#include
+#include
+#include
amccarth wrote:
> Why add ``? It looks like your new map is just a vector.
Good catch
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:558
+ return result;
+}
amccarth wrote:
> This is a long function (which, unfortunately, is not unusual in LLVM/LLDB),
> but the comments here are really good, so I'm OK with it.
I agree size-wise it's borderline. It's also a bit smelly in that it does two
things (checks + initialization).
I originally had a separate "ConsistencyCheck", but there was significant
duplication (the directory traversal) and the combined version won by a tiny
margin :) If this grows with more checks I'd be happy to revisit and refactor
it.
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:93
+ // Perform consistency checks and initialize internal data structures
+ Status Initialize();
+
amccarth wrote:
> I'm not a big fan of 2-step initialization, but that seems to be the way of
> LLDB. :-(
Agreed. I don't see how to avoid this w/o much bigger changes though (we don't
have a chance to return meaningful errors during process creation).
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:172
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
amccarth wrote:
> Should the architecture check be in the MinidumpParser::Initialize with the
> other checks?
>
> I don't know the answer; I'm just asking for your thinking about this.
Good question, here's my take: the checks are for consistency and a minidump
with a currently unsupported architecture is a valid minidump.
So I think it's better to have this check external since the architecture
support is not a minidump parser concern. WDYT?
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:193
if (!pid) {
-error.SetErrorString("failed to parse PID");
+error.SetErrorString("Failed to parse PID");
return error;
amccarth wrote:
> I realize nothing is perfectly consistent, but I think LLVM tends to start
> error strings with a lowercase letter (except for proper nouns). Can you
> revert this case change and make sure your new error strings follow that
> pattern?
Sigh, sure. I was hoping it's the other way around.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:52
ASSERT_GT(parser->GetData().size(), 0UL);
+auto result = parser->Initialize();
+if (expect_failure)
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)
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 updated this revision to Diff 155069.
lemo marked 9 inline comments as done.
lemo added a comment.
Incorporating CR feedback
https://reviews.llvm.org/D49202
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -38,16 +38,30 @@
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);
+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 TruncateMinidump(const char *minidump_filename, uint64_t load_size) {
+std::string filename = GetInputFilePath(minidump_filename);
+auto BufferPtr =
+DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0);
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 +82,10 @@
EXPECT_EQ(1232UL, context.size());
}
-TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
- SetUpData("linux-x86_64.dmp", 200);
- llvm::ArrayRef thread_list;
-
- thread_list = parser->GetThreads();
- ASSERT_EQ(0UL, thread_list.size());
+TEST_F(MinidumpParserTest, TruncatedMinidumps) {
+ TruncateMinidump("linux-x86_64.dmp", 32);
+ TruncateMinidump("linux-x86_64.dmp", 100);
+ TruncateMinidump("linux-x86_64.dmp", 20 * 1024);
}
TEST_F(MinidumpParserTest, GetArchitecture) {
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -105,7 +105,7 @@
if (!DataPtr)
return nullptr;
- assert(DataPtr->GetByteSize() == header_size);
+ lldbassert(DataPtr->GetByteSize() == header_size);
// first, only try to parse the header, beacuse we need to be fast
llvm::ArrayRef HeaderBytes = DataPtr->GetData();
@@ -164,10 +164,29 @@
Status ProcessMinidump::DoLoadCore() {
Status error;
+ // Minidump parser initialization & consistency checks
+ error = m_minidump_parser.Initialize();
+ if (error.Fail())
+return error;
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
+ switch (arch.GetMachine()) {
+ case llvm::Triple::x86:
+ case llvm::Triple::x86_64:
+// supported
+break;
+
+ default:
+error.SetErrorStringWithFormat("unsupported minidump architecture: %s",
+ arch.GetArchitectureName());
+return error;
+ }
+
m_thread_list = m_minidump_parser.GetThreads();
m_active_exception = m_minidump_parser.GetExceptionStream();
ReadModuleList();
- GetTarget().SetArchitecture(GetArchitecture());
+ GetTarget().SetArchitecture(arch);
llvm::Optional pid = m_minidump_parser.GetPid();
if (!pid) {
Index: source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ 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: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -89,14 +89,16 @@
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;
- const MinidumpHeader *m_header;
+ const
[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks
amccarth added a comment. Also, I'm not seeing tests for the other consistency checks you're adding (like whether there are any streams or whether the streams overlap). Is it difficult to create such malformed minidumps? 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: > > 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. 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
amccarth added inline comments. Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:172 + + // Do we support the minidump's architecture? + ArchSpec arch = GetArchitecture(); lemo wrote: > amccarth wrote: > > Should the architecture check be in the MinidumpParser::Initialize with the > > other checks? > > > > I don't know the answer; I'm just asking for your thinking about this. > Good question, here's my take: the checks are for consistency and a minidump > with a currently unsupported architecture is a valid minidump. > > So I think it's better to have this check external since the architecture > support is not a minidump parser concern. WDYT? Yes, this makes sense to me. 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. Regarding test for the other checks, I'll try to fabricate a few invalid minidumps (although it would obviously provide limited coverage) 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: > > > 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()? 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] [lldb] r336865 - [windows] Fix out-of-memory failure in some of the tests
Author: stella.stamenova
Date: Wed Jul 11 15:47:35 2018
New Revision: 336865
URL: http://llvm.org/viewvc/llvm-project?rev=336865&view=rev
Log:
[windows] Fix out-of-memory failure in some of the tests
Summary: When ReadProcessMemory fails, bytes_read is sometimes set to a large
garbage value. In that case, we need to set it back to zero before returning or
the garbage value will be used to allocate memory later causing LLDB to crash
with an out of memory error.
Reviewers: asmith, zturner
Reviewed By: zturner
Subscribers: zturner, asmith, stella.stamenova, llvm-commits
Differential Revision: https://reviews.llvm.org/D49159
Modified:
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
Modified: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp?rev=336865&r1=336864&r2=336865&view=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp Wed Jul
11 15:47:35 2018
@@ -240,7 +240,7 @@ Status ProcessWindows::DoLaunch(Module *
if (working_dir && (!working_dir.ResolvePath() ||
!fs::is_directory(working_dir.GetPath( {
result.SetErrorStringWithFormat("No such file or directory: %s",
- working_dir.GetCString());
+working_dir.GetCString());
return result;
}
@@ -365,7 +365,10 @@ Status ProcessWindows::DoResume() {
Status result = thread->DoResume();
if (result.Fail()) {
failed = true;
-LLDB_LOG(log, "Trying to resume thread at index {0}, but failed with
error {1}.", i, result);
+LLDB_LOG(
+log,
+"Trying to resume thread at index {0}, but failed with error {1}.",
+i, result);
}
}
@@ -473,8 +476,9 @@ void ProcessWindows::RefreshStateAfterSt
m_session_data->m_debugger->GetActiveException();
ExceptionRecordSP active_exception = exception_record.lock();
if (!active_exception) {
-LLDB_LOG(log, "there is no active exception in process {0}. Why is the "
- "process stopped?",
+LLDB_LOG(log,
+ "there is no active exception in process {0}. Why is the "
+ "process stopped?",
m_session_data->m_debugger->GetProcess().GetProcessId());
return;
}
@@ -491,8 +495,9 @@ void ProcessWindows::RefreshStateAfterSt
const uint64_t pc = register_context->GetPC();
BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
if (site && site->ValidForThisThread(stop_thread.get())) {
- LLDB_LOG(log, "Single-stepped onto a breakpoint in process {0} at "
-"address {1:x} with breakpoint site {2}",
+ LLDB_LOG(log,
+ "Single-stepped onto a breakpoint in process {0} at "
+ "address {1:x} with breakpoint site {2}",
m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
site->GetID());
stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(*stop_thread,
@@ -514,22 +519,25 @@ void ProcessWindows::RefreshStateAfterSt
BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
if (site) {
- LLDB_LOG(log, "detected breakpoint in process {0} at address {1:x} with "
-"breakpoint site {2}",
+ LLDB_LOG(log,
+ "detected breakpoint in process {0} at address {1:x} with "
+ "breakpoint site {2}",
m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
site->GetID());
if (site->ValidForThisThread(stop_thread.get())) {
-LLDB_LOG(log, "Breakpoint site {0} is valid for this thread ({1:x}), "
- "creating stop info.",
+LLDB_LOG(log,
+ "Breakpoint site {0} is valid for this thread ({1:x}), "
+ "creating stop info.",
site->GetID(), stop_thread->GetID());
stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(
*stop_thread, site->GetID());
register_context->SetPC(pc);
} else {
-LLDB_LOG(log, "Breakpoint site {0} is not valid for this thread, "
- "creating empty stop info.",
+LLDB_LOG(log,
+ "Breakpoint site {0} is not valid for this thread, "
+ "creating empty stop info.",
site->GetID());
}
stop_thread->SetStopInfo(stop_info);
@@ -651,8 +659,13 @@ size_t ProcessWindows::DoReadMemory(lldb
SIZE_T bytes_read = 0;
if (!ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &bytes_read)) {
+// Reading from the process can fail for a
[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks
lemo updated this revision to Diff 155076.
lemo added a comment.
Adding a few ill-formed minidumps for testing the new checks
https://reviews.llvm.org/D49202
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -38,16 +38,30 @@
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);
+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);
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 +82,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: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -105,7 +105,7 @@
if (!DataPtr)
return nullptr;
- assert(DataPtr->GetByteSize() == header_size);
+ lldbassert(DataPtr->GetByteSize() == header_size);
// first, only try to parse the header, beacuse we need to be fast
llvm::ArrayRef HeaderBytes = DataPtr->GetData();
@@ -164,10 +164,29 @@
Status ProcessMinidump::DoLoadCore() {
Status error;
+ // Minidump parser initialization & consistency checks
+ error = m_minidump_parser.Initialize();
+ if (error.Fail())
+return error;
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
+ switch (arch.GetMachine()) {
+ case llvm::Triple::x86:
+ case llvm::Triple::x86_64:
+// supported
+break;
+
+ default:
+error.SetErrorStringWithFormat("unsupported minidump architecture: %s",
+ arch.GetArchitectureName());
+return error;
+ }
+
m_thread_list = m_minidump_parser.GetThreads();
m_active_exception = m_minidump_parser.GetExceptionStream();
ReadModuleList();
- GetTarget().SetArchitecture(GetArchitecture());
+ GetTarget().SetArchitecture(arch);
llvm::Optional pid = m_minidump_parser.GetPid();
if (!pid) {
Index: source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ 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: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -89,14 +89,16 @@
llvm::Optional GetMemoryRegi
[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks
lemo updated this revision to Diff 155080.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D49202
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ 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: unittests/Process/minidump/CMakeLists.txt
===
--- unittests/Process/minidump/CMakeLists.txt
+++ 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: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -105,7 +105,7 @@
if (!DataPtr)
return nullptr;
- assert(DataPtr->GetByteSize() == header_size);
+ lldbassert(DataPtr->GetByteSize() == header_size);
// first, only try to parse the header, beacuse we need to be fast
llvm::ArrayRef HeaderBytes = DataPtr->GetData();
@@ -164,10 +164,29 @@
Status ProcessMinidump::DoLoadCore() {
Status error;
+ // Minidump parser initialization & consistency checks
+ error = m_minidump_parser.Initialize();
+ if (error.Fail())
+return error;
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
+ switch (arch.GetMachine()) {
+ case llvm::Triple::x86:
+ case llvm::Triple::x86_64:
+// supported
+break;
+
+ default:
+error.SetErrorStringWithFormat("unsupported minidump architecture: %s",
+ arch.GetArchitectureName());
+return error;
+ }
+
m_thread_list = m_minidump_parser.GetThreads();
m_active_exception = m_minidump_parser.GetExceptionStream();
ReadModuleList();
- GetTarget().SetArchitecture(GetArchitecture());
+ GetTarget().SetArchitecture(arch);
llvm::Optional pid = m_minidump_parser.GetPid();
if (!pid) {
Index: source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- source/Plugins/Process/minidump/MinidumpT
[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute
teemperor created this revision.
teemperor added a reviewer: davide.
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.
https://reviews.llvm.org/D49207
Files:
include/lldb/Interpreter/CommandObject.h
include/lldb/Interpreter/CommandObjectRegexCommand.h
include/lldb/Interpreter/ScriptInterpreter.h
source/Commands/CommandObjectCommands.cpp
source/Commands/CommandObjectExpression.cpp
source/Commands/CommandObjectExpression.h
source/Commands/CommandObjectPlatform.cpp
source/Commands/CommandObjectSettings.cpp
source/Commands/CommandObjectThread.cpp
source/Commands/CommandObjectType.cpp
source/Commands/CommandObjectWatchpoint.cpp
source/Interpreter/CommandObjectRegexCommand.cpp
source/Interpreter/CommandObjectScript.cpp
source/Interpreter/CommandObjectScript.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -151,14 +151,14 @@
bool Interrupt() override;
bool ExecuteOneLine(
- const char *command, CommandReturnObject *result,
+ llvm::StringRef command, CommandReturnObject *result,
const ExecuteScriptOptions &options = ExecuteScriptOptions()) override;
void ExecuteInterpreterLoop() override;
bool ExecuteOneLineWithReturn(
- const char *in_string, ScriptInterpreter::ScriptReturnType return_type,
- void *ret_value,
+ llvm::StringRef in_string,
+ ScriptInterpreter::ScriptReturnType return_type, void *ret_value,
const ExecuteScriptOptions &options = ExecuteScriptOptions()) override;
lldb_private::Status ExecuteMultipleLines(
@@ -259,18 +259,17 @@
GetSyntheticTypeName(const StructuredData::ObjectSP &implementor) override;
bool
- RunScriptBasedCommand(const char *impl_function, const char *args,
+ RunScriptBasedCommand(const char *impl_function, llvm::StringRef args,
ScriptedCommandSynchronicity synchronicity,
lldb_private::CommandReturnObject &cmd_retobj,
Status &error,
const lldb_private::ExecutionContext &exe_ctx) override;
- bool
- RunScriptBasedCommand(StructuredData::GenericSP impl_obj_sp, const char *args,
-ScriptedCommandSynchronicity synchronicity,
-lldb_private::CommandReturnObject &cmd_retobj,
-Status &error,
-const lldb_private::ExecutionContext &exe_ctx) override;
+ bool RunScriptBasedCommand(
+ StructuredData::GenericSP impl_obj_sp, llvm::StringRef args,
+ ScriptedCommandSynchronicity synchronicity,
+ lldb_private::CommandReturnObject &cmd_retobj, Status &error,
+ const lldb_private::ExecutionContext &exe_ctx) override;
Status GenerateFunction(const char *signature,
const StringList &input) override;
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -751,12 +751,12 @@
}
bool ScriptInterpreterPython::ExecuteOneLine(
-const char *command, CommandReturnObject *result,
+llvm::StringRef command, CommandReturnObject *result,
const ExecuteScriptOptions &options) {
if (!m_valid_session)
return false;
- if (command && command[0]) {
+ if (!command.empty()) {
// We want to call run_one_line, passing in the dictionary and the command
// string. We cannot do this through PyRun_SimpleString here because the
// command string may contain escaped characters, and putting it inside
@@ -894,9 +894,11 @@
return true;
// The one-liner failed. Append the error message.
-if (result)
+if (result) {
+ std::string command_str = command.str();
result->AppendErrorWithFormat(
- "python failed attempting to evaluate '%s'\n", command);
+ "python failed att
[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
I think the CommandObjectRegex check that you took out wasn't something devious
or clever, but was just an incomplete (and so buggy) version of the check:
if (command && command[0])
that you've been replacing with:
if (command.empty())
everywhere else in the patch. After all, the error message if you take the
failing branch was:
result.AppendError("empty command passed to regular expression command");
So I think instead of taking this line out, you should also convert it to an
if(command.empty()) and early return it with the error message you removed.
Other than that, this change looks fine.
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] D49155: [IRInterpreter] Fix misevaluation of interpretation expressions with `urem`
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Ack'ed by Jim offline. https://reviews.llvm.org/D49155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r336872 - [IRInterpreter] Fix misevaluation of interpretation expressions with `urem`.
Author: davide
Date: Wed Jul 11 17:31:04 2018
New Revision: 336872
URL: http://llvm.org/viewvc/llvm-project?rev=336872&view=rev
Log:
[IRInterpreter] Fix misevaluation of interpretation expressions with `urem`.
Scalar::MakeUnsigned was implemented incorrectly so it didn't
really change the sign of the type (leaving signed types signed).
This showed up as a misevaluation when IR-interpreting urem but
it's likely to arise in other contexts.
This commit fixes the definition, and adds a test to make
sure this won't regress in future (hopefully).
Fixes rdar://problem/42038760 and LLVM PR38076
Differential Revision: https://reviews.llvm.org/D49155
Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
Modified:
lldb/trunk/source/Core/Scalar.cpp
Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile?rev=336872&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
Wed Jul 11 17:31:04 2018
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules
Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py?rev=336872&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
Wed Jul 11 17:31:04 2018
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)
Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c?rev=336872&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
Wed Jul 11 17:31:04 2018
@@ -0,0 +1,19 @@
+// Make sure we IR-interpret the expression correctly.
+
+typedef unsigned int uint32_t;
+struct S0 {
+ signed f2;
+};
+static g_463 = 0x1561983AL;
+void func_1(void)
+{
+ struct S0 l_19;
+ l_19.f2 = 419;
+ uint32_t l_4037 = 4294967295UL;
+ l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)",
substrs=['(unsigned int) $0 = 358717883'])
+}
+int main()
+{
+ func_1();
+ return 0;
+}
Modified: lldb/trunk/source/Core/Scalar.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Scalar.cpp?rev=336872&r1=336871&r2=336872&view=diff
==
--- lldb/trunk/source/Core/Scalar.cpp (original)
+++ lldb/trunk/source/Core/Scalar.cpp Wed Jul 11 17:31:04 2018
@@ -1184,38 +1184,38 @@ bool Scalar::MakeUnsigned() {
case e_void:
break;
case e_sint:
+m_type = e_uint;
success = true;
break;
case e_uint:
-m_type = e_uint;
success = true;
break;
case e_slong:
+m_type = e_ulong;
success = true;
break;
case e_ulong:
-m_type = e_ulong;
success = true;
break;
case e_slonglong:
+m_type = e_ulonglong;
success = true;
break;
case e_ulonglong:
-m_type = e_ulonglong;
success = true;
break;
case e_sint128:
+m_type = e_uint128;
success = true;
break;
case e_uint128:
-m_type = e_uint128;
success = true;
break;
case e_sint256:
+m_type = e_uint256;
success = true;
break;
case e_uint256:
-m_type = e_uint256;
success = true;
break;
case e_float:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49155: [IRInterpreter] Fix misevaluation of interpretation expressions with `urem`
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336872: [IRInterpreter] Fix misevaluation of interpretation
expressions with `urem`. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49155?vs=154884&id=155096#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49155
Files:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
lldb/trunk/source/Core/Scalar.cpp
Index: lldb/trunk/source/Core/Scalar.cpp
===
--- lldb/trunk/source/Core/Scalar.cpp
+++ lldb/trunk/source/Core/Scalar.cpp
@@ -1184,38 +1184,38 @@
case e_void:
break;
case e_sint:
+m_type = e_uint;
success = true;
break;
case e_uint:
-m_type = e_uint;
success = true;
break;
case e_slong:
+m_type = e_ulong;
success = true;
break;
case e_ulong:
-m_type = e_ulong;
success = true;
break;
case e_slonglong:
+m_type = e_ulonglong;
success = true;
break;
case e_ulonglong:
-m_type = e_ulonglong;
success = true;
break;
case e_sint128:
+m_type = e_uint128;
success = true;
break;
case e_uint128:
-m_type = e_uint128;
success = true;
break;
case e_sint256:
+m_type = e_uint256;
success = true;
break;
case e_uint256:
-m_type = e_uint256;
success = true;
break;
case e_float:
Index:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
===
---
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
+++
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules
Index:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
===
---
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
+++
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
@@ -0,0 +1,19 @@
+// Make sure we IR-interpret the expression correctly.
+
+typedef unsigned int uint32_t;
+struct S0 {
+ signed f2;
+};
+static g_463 = 0x1561983AL;
+void func_1(void)
+{
+ struct S0 l_19;
+ l_19.f2 = 419;
+ uint32_t l_4037 = 4294967295UL;
+ l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)",
substrs=['(unsigned int) $0 = 358717883'])
+}
+int main()
+{
+ func_1();
+ return 0;
+}
Index:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
===
---
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
+++
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)
Index: lldb/trunk/source/Core/Scalar.cpp
===
--- lldb/trunk/source/Core/Scalar.cpp
+++ lldb/trunk/source/Core/Scalar.cpp
@@ -1184,38 +1184,38 @@
case e_void:
break;
case e_sint:
+m_type = e_uint;
success = true;
break;
case e_uint:
-m_type = e_uint;
success = true;
break;
case e_slong:
+m_type = e_ulong;
success = true;
break;
case e_ulong:
-m_type = e_ulong;
success = true;
break;
case e_slonglong:
+m_type = e_ulonglong;
success = true;
break;
case e_ulonglong:
-m_type = e_ulonglong;
success = true;
break;
case e_sint128:
+m_type = e_uint128;
success = true;
break;
case e_uint128:
-m_type = e_uint128;
success = true;
break;
case e_sint256:
+m_type = e_uint256;
success = true;
break;
case e_uint256:
-m_type = e_uint256;
success = true;
break;
case e_float:
Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
Re: [Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet
I do not have commit access to LLDB. Could someone please commit the below changes? On Thu, Jul 12, 2018 at 2:32 AM, Jason Molenda via Phabricator < revi...@reviews.llvm.org> wrote: > jasonmolenda accepted this revision. > jasonmolenda added a comment. > This revision is now accepted and ready to land. > > looks good. > > > https://reviews.llvm.org/D48868 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r336884 - Remove unused variable m_header as it hasn't been used since it was
Author: echristo
Date: Wed Jul 11 20:52:45 2018
New Revision: 336884
URL: http://llvm.org/viewvc/llvm-project?rev=336884&view=rev
Log:
Remove unused variable m_header as it hasn't been used since it was
added in 2016.
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
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=336884&r1=336883&r2=336884&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Wed Jul 11
20:52:45 2018
@@ -60,14 +60,13 @@ MinidumpParser::Create(const lldb::DataB
directory->location;
}
- return MinidumpParser(data_buf_sp, header, std::move(directory_map));
+ return MinidumpParser(data_buf_sp, std::move(directory_map));
}
MinidumpParser::MinidumpParser(
-const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
+const lldb::DataBufferSP &data_buf_sp,
llvm::DenseMap &&directory_map)
-: m_data_sp(data_buf_sp), m_header(header), m_directory_map(directory_map)
{
-}
+: m_data_sp(data_buf_sp), m_directory_map(directory_map) {}
llvm::ArrayRef MinidumpParser::GetData() {
return llvm::ArrayRef(m_data_sp->GetBytes(),
Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h?rev=336884&r1=336883&r2=336884&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h Wed Jul 11
20:52:45 2018
@@ -91,11 +91,10 @@ public:
private:
lldb::DataBufferSP m_data_sp;
- const MinidumpHeader *m_header;
llvm::DenseMap m_directory_map;
MinidumpParser(
- const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
+ const lldb::DataBufferSP &data_buf_sp,
llvm::DenseMap &&directory_map);
};
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r336885 - Remove the unused m_signal member variable, but leave the code that gets it out of the json.
Author: echristo
Date: Wed Jul 11 20:52:46 2018
New Revision: 336885
URL: http://llvm.org/viewvc/llvm-project?rev=336885&view=rev
Log:
Remove the unused m_signal member variable, but leave the code that gets it out
of the json.
Modified:
lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h
Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp?rev=336885&r1=336884&r2=336885&view=diff
==
--- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp Wed Jul 11
20:52:46 2018
@@ -58,9 +58,9 @@ support::endianness ProcessInfo::GetEndi
//== ThreadInfo
ThreadInfo::ThreadInfo(StringRef name, StringRef reason, RegisterMap registers,
- unsigned int signal)
+ unsigned int)
: m_name(name.str()), m_reason(reason.str()),
- m_registers(std::move(registers)), m_signal(signal) {}
+ m_registers(std::move(registers)) {}
const RegisterValue *ThreadInfo::ReadRegister(unsigned int Id) const {
auto Iter = m_registers.find(Id);
Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h?rev=336885&r1=336884&r2=336885&view=diff
==
--- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h Wed Jul 11
20:52:46 2018
@@ -60,7 +60,6 @@ private:
std::string m_name;
std::string m_reason;
RegisterMap m_registers;
- unsigned int m_signal;
};
class JThreadsInfo : public Parser {
___
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
ramana-nvr updated this revision to Diff 155114.
ramana-nvr added a comment.
The error messages now refer to the thread ID instead of the thread index.
https://reviews.llvm.org/D48865
Files:
source/Commands/CommandObjectThread.cpp
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1183,8 +1183,8 @@
thread->GetStackFrameAtIndex(m_options.m_frame_idx).get();
if (frame == nullptr) {
result.AppendErrorWithFormat(
-"Frame index %u is out of range for thread %u.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+"Frame index %u is out of range for thread id %" PRIu64 ".\n",
+m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1201,9 +1201,8 @@
if (line_table == nullptr) {
result.AppendErrorWithFormat("Failed to resolve the line table for "
- "frame %u of thread index %u.\n",
- m_options.m_frame_idx,
- m_options.m_thread_idx);
+ "frame %u of thread id %" PRIu64 ".\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1279,13 +1278,13 @@
new_plan_sp->SetOkayToDiscard(false);
} else {
result.AppendErrorWithFormat(
-"Frame index %u of thread %u has no debug information.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+ "Frame index %u of thread id %" PRIu64 " has no debug
information.\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
- process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx);
+ process->GetThreadList().SetSelectedThreadByID(thread->GetID());
StreamString stream;
Status error;
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1183,8 +1183,8 @@
thread->GetStackFrameAtIndex(m_options.m_frame_idx).get();
if (frame == nullptr) {
result.AppendErrorWithFormat(
-"Frame index %u is out of range for thread %u.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+"Frame index %u is out of range for thread id %" PRIu64 ".\n",
+m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1201,9 +1201,8 @@
if (line_table == nullptr) {
result.AppendErrorWithFormat("Failed to resolve the line table for "
- "frame %u of thread index %u.\n",
- m_options.m_frame_idx,
- m_options.m_thread_idx);
+ "frame %u of thread id %" PRIu64 ".\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -1279,13 +1278,13 @@
new_plan_sp->SetOkayToDiscard(false);
} else {
result.AppendErrorWithFormat(
-"Frame index %u of thread %u has no debug information.\n",
-m_options.m_frame_idx, m_options.m_thread_idx);
+ "Frame index %u of thread id %" PRIu64 " has no debug information.\n",
+ m_options.m_frame_idx, thread->GetID());
result.SetStatus(eReturnStatusFailed);
return false;
}
- process->GetThreadList().SetSelectedThreadByID(m_options.m_thread_idx);
+ process->GetThreadList().SetSelectedThreadByID(thread->GetID());
StreamString stream;
Status error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49062: [lldb-mi] Re-implement data-info-line command.
aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py:22 @skipIfDarwin # pexpect is known to be unreliable on Darwin @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread races def test_lldbmi_data_disassemble(self): apolyakov wrote: > aprantl wrote: > > It would be *awesome* if we could also convert this entire to a > > lit/FileCheck. Looks like all the tests in this file are basically skipped > > everywhere because it's so unreliable... > It's problematic to convert `data-info-line` test to a lit one since we don't > know addresses of a source lines. It means that we can't do this: > `-data-info-line *0xsome_address` I see. You are right, we can't change make one command input depend on previous output in a LIT-based test. But it looks like this is only needed for testing one form of the data-info-line command. We could extract everything else into a LIT-based test that runs on every platform and still greatly improve the test coverage. https://reviews.llvm.org/D49062 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
