Reland "[lldb] Don't send invalid region addresses to lldb server"

This reverts commit c65627a1fe.

The test immediately after the new invalid symbol test was
failing on Windows. This was because when we called
VirtualQueryEx to get the region info for 0x0,
even if it succeeded we would call GetLastError.

Which must have picked up the last error that was set while
trying to lookup "not_an_address". Which happened to be 2.
("The system cannot find the file specified.")

To fix this only call GetLastError when we know VirtualQueryEx
has failed. (when it returns 0, which we were also checking for anyway)

Also convert memory region to an early return style
to make the logic clearer.

Reviewed By: labath, stella.stamenova

Differential Revision: https://reviews.llvm.org/D88229
This commit is contained in:
David Spickett 2020-09-24 14:50:41 +01:00
parent 6a089ce0e4
commit 71cf97e95b
3 changed files with 65 additions and 56 deletions

View File

@ -1687,63 +1687,66 @@ public:
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;
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());
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);
}
}
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());
}
}
} else {
if (!process_sp) {
m_prev_end_addr = LLDB_INVALID_ADDRESS;
result.AppendError("invalid process");
result.SetStatus(eReturnStatusFailed);
return false;
}
return result.Succeeded();
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());
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);
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);
return true;
}
result.SetStatus(eReturnStatusFailed);
result.AppendErrorWithFormat("%s\n", error.AsCString());
return false;
}
const char *GetRepeatCommand(Args &current_command_args,

View File

@ -405,7 +405,8 @@ Status ProcessDebugger::GetMemoryRegionInfo(lldb::addr_t vm_addr,
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 @@ Status ProcessDebugger::GetMemoryRegionInfo(lldb::addr_t vm_addr,
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 @@ Status ProcessDebugger::GetMemoryRegionInfo(lldb::addr_t vm_addr,
info.SetMapped(MemoryRegionInfo::eNo);
}
error.SetError(::GetLastError(), eErrorTypeWin32);
LLDB_LOGV(log,
"Memory region info for address {0}: readable={1}, "
"executable={2}, writable={3}",

View File

@ -41,6 +41,12 @@ class MemoryCommandRegion(TestBase):
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())