Don't fail step out if remote server doesn't implement qMemoryRegionInfo

[ted]

Summary

The return address validation in D71372 will fail if the memory permissions can't be determined. Many embedded stubs either don't implement the qMemoryRegionInfo packet, or don't have memory permissions at all.

Remove the return from the if clause that calls GetLoadAddressPermissions, so this call failing doesn't cause the step out to abort. Instead, assume that the memory permission check doesn't apply to this type of target.

[labath]

This seems fine to me, though I do get a feeling that without the testcase, some refactor will break this sooner or later. Making a test case for this would be tricky (though not impossible) and this it wasn't even your patch that broke this, so I don't think we should vehemently require a test to go with this. However, creating something might still be worthwhile, since you're the one who's going to have to debug any breakages...

[jingham]

I don't think you want to put anything in m_constructor_errors in this case either. It's fine to log something out, but ValidatePlan uses m_constructor_errors to help report a problem. If the breakpoint failed to take for some other reason, then we would report a permissions problem which is not the reason for the failure.

[ted]

Wow @jingham you posted your comment and I landed the patch at the same time!

I'll remove the m_constructor_errors.Printf line as requested.

[ted]

Removed the code that sets m_constructor_errors when GetLoadAddressPermissions returns False, as requested by @jingham .

[ted]

@jingham friendly ping

[jingham]

Oops, sorry. Looks fine.

lldb/source/Target/ThreadPlanStepOut.cpp

if (immediate_return_from_sp->IsInlined()) {
 
     if (m_return_addr == LLDB_INVALID_ADDRESS)
       return;
 
     // Perform some additional validation on the return address.
     uint32_t permissions = 0;
     if (!m_thread.GetProcess()->GetLoadAddressPermissions(m_return_addr,
                                                           permissions)) {
-      m_constructor_errors.Printf("Return address (0x%" PRIx64
-                                  ") permissions not found.",
-                                  m_return_addr);
-      LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast<void *>(this),
-                m_constructor_errors.GetData());
+      LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
+                ") permissions not found.", static_cast<void *>(this),
     } else if (!(permissions & ePermissionsExecutable)) {
       m_constructor_errors.Printf("Return address (0x%" PRIx64
                                   ") did not point to executable memory.",
                                   m_return_addr);
       LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast<void *>(this),
                 m_constructor_errors.GetData());
       return;
     }