DavidSpickett updated this revision to Diff 294266.
DavidSpickett added a comment.
- Convert memory region to early return style to make the logic clearer
- Only call ::GetLastError once when VirtualQueryEx fails
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88229/new/
https://reviews.llvm.org/D88229
Files:
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===================================================================
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -41,6 +41,12 @@
self.assertFalse(result.Succeeded())
self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+ # Test that when the address fails to parse, we show an error and do not continue
+ interp.HandleCommand("memory region not_an_address", result)
+ self.assertFalse(result.Succeeded())
+ self.assertEqual(result.GetError(),
+ "error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")
+
# Now let's print the memory region starting at 0 which should always work.
interp.HandleCommand("memory region 0x0", result)
self.assertTrue(result.Succeeded())
Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -405,7 +405,8 @@
MEMORY_BASIC_INFORMATION mem_info = {};
SIZE_T result = ::VirtualQueryEx(handle, addr, &mem_info, sizeof(mem_info));
if (result == 0) {
- if (::GetLastError() == ERROR_INVALID_PARAMETER) {
+ DWORD last_error = ::GetLastError();
+ if (last_error == ERROR_INVALID_PARAMETER) {
// ERROR_INVALID_PARAMETER is returned if VirtualQueryEx is called with
// an address past the highest accessible address. We should return a
// range from the vm_addr to LLDB_INVALID_ADDRESS
@@ -417,7 +418,7 @@
info.SetMapped(MemoryRegionInfo::eNo);
return error;
} else {
- error.SetError(::GetLastError(), eErrorTypeWin32);
+ error.SetError(last_error, eErrorTypeWin32);
LLDB_LOG(log,
"VirtualQueryEx returned error {0} while getting memory "
"region info for address {1:x}",
@@ -460,7 +461,6 @@
info.SetMapped(MemoryRegionInfo::eNo);
}
- error.SetError(::GetLastError(), eErrorTypeWin32);
LLDB_LOGV(log,
"Memory region info for address {0}: readable={1}, "
"executable={2}, writable={3}",
Index: lldb/source/Commands/CommandObjectMemory.cpp
===================================================================
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1687,63 +1687,67 @@
protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
ProcessSP process_sp = m_exe_ctx.GetProcessSP();
- if (process_sp) {
- Status error;
- lldb::addr_t load_addr = m_prev_end_addr;
+ if (!process_sp) {
m_prev_end_addr = LLDB_INVALID_ADDRESS;
+ result.AppendError("invalid process");
+ result.SetStatus(eReturnStatusFailed);
+ return false;
+ }
+
+ Status error;
+ lldb::addr_t load_addr = m_prev_end_addr;
+ m_prev_end_addr = LLDB_INVALID_ADDRESS;
- const size_t argc = command.GetArgumentCount();
- if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS)) {
- result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",
- m_cmd_name.c_str(), m_cmd_syntax.c_str());
+ const size_t argc = command.GetArgumentCount();
+ if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS)) {
+ result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",
+ m_cmd_name.c_str(), m_cmd_syntax.c_str());
+ result.SetStatus(eReturnStatusFailed);
+ return false;
+ }
+
+ if (argc == 1) {
+ auto load_addr_str = command[0].ref();
+ load_addr = OptionArgParser::ToAddress(&m_exe_ctx, load_addr_str,
+ LLDB_INVALID_ADDRESS, &error);
+ if (error.Fail() || load_addr == LLDB_INVALID_ADDRESS) {
+ result.AppendErrorWithFormat(
+ "invalid address argument \"%s\": %s\n", command[0].c_str(),
+ error.AsCString());
result.SetStatus(eReturnStatusFailed);
- } else {
- if (command.GetArgumentCount() == 1) {
- auto load_addr_str = command[0].ref();
- load_addr = OptionArgParser::ToAddress(&m_exe_ctx, load_addr_str,
- LLDB_INVALID_ADDRESS, &error);
- if (error.Fail() || load_addr == LLDB_INVALID_ADDRESS) {
- result.AppendErrorWithFormat(
- "invalid address argument \"%s\": %s\n", command[0].c_str(),
- error.AsCString());
- result.SetStatus(eReturnStatusFailed);
- }
- }
+ return false;
+ }
+ }
- lldb_private::MemoryRegionInfo range_info;
- error = process_sp->GetMemoryRegionInfo(load_addr, range_info);
- if (error.Success()) {
- lldb_private::Address addr;
- ConstString name = range_info.GetName();
- ConstString section_name;
- if (process_sp->GetTarget().ResolveLoadAddress(load_addr, addr)) {
- SectionSP section_sp(addr.GetSection());
- if (section_sp) {
- // Got the top most section, not the deepest section
- while (section_sp->GetParent())
- section_sp = section_sp->GetParent();
- section_name = section_sp->GetName();
- }
- }
- result.AppendMessageWithFormatv(
- "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
- range_info.GetRange().GetRangeBase(),
- range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
- range_info.GetWritable(), range_info.GetExecutable(),
- name ? " " : "", name, section_name ? " " : "", section_name);
- m_prev_end_addr = range_info.GetRange().GetRangeEnd();
- result.SetStatus(eReturnStatusSuccessFinishResult);
- } else {
- result.SetStatus(eReturnStatusFailed);
- result.AppendErrorWithFormat("%s\n", error.AsCString());
+ lldb_private::MemoryRegionInfo range_info;
+ error = process_sp->GetMemoryRegionInfo(load_addr, range_info);
+ if (error.Success()) {
+ lldb_private::Address addr;
+ ConstString name = range_info.GetName();
+ ConstString section_name;
+ if (process_sp->GetTarget().ResolveLoadAddress(load_addr, addr)) {
+ SectionSP section_sp(addr.GetSection());
+ if (section_sp) {
+ // Got the top most section, not the deepest section
+ while (section_sp->GetParent())
+ section_sp = section_sp->GetParent();
+ section_name = section_sp->GetName();
}
}
- } else {
- m_prev_end_addr = LLDB_INVALID_ADDRESS;
- result.AppendError("invalid process");
- result.SetStatus(eReturnStatusFailed);
+ result.AppendMessageWithFormatv(
+ "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
+ range_info.GetRange().GetRangeBase(),
+ range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
+ range_info.GetWritable(), range_info.GetExecutable(),
+ name ? " " : "", name, section_name ? " " : "", section_name);
+ m_prev_end_addr = range_info.GetRange().GetRangeEnd();
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ return true;
}
- return result.Succeeded();
+
+ result.SetStatus(eReturnStatusFailed);
+ result.AppendErrorWithFormat("%s\n", error.AsCString());
+ return false;
}
const char *GetRepeatCommand(Args ¤t_command_args,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
