[Lldb-commits] [PATCH] D12079: [MIPS] microMIPS breakpoints, disassembly and compressed addresses

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Open issues:

- In FormatEntity.cpp we probably don't need any changes to DumpAddress()
- Remove MIPS comment from generic code and let the "Target::GetOpcodeLoadAddress (...) const" document what is happening.
- Call Target::GetOpcodeLoadAddress (lldb::addr_t load_addr, AddressClass addr_class) const to fixup the PC.


================
Comment at: source/Core/FormatEntity.cpp:421-429
@@ -420,6 +420,11 @@
     addr_t vaddr = LLDB_INVALID_ADDRESS;
+
+    // If the address belongs to eAddressClassCodeAlternateISA then
+    // dump its callable form.
+    Address callable_addr (addr.GetCallableLoadAddress(target));
+
     if (exe_ctx && !target->GetSectionLoadList().IsEmpty())
-        vaddr = addr.GetLoadAddress (target);
+        vaddr = callable_addr.GetLoadAddress (target);
     if (vaddr == LLDB_INVALID_ADDRESS)
-        vaddr = addr.GetFileAddress ();
+        vaddr = callable_addr.GetFileAddress ();
 
----------------
I repeat this concern: do we still need to do this? There should be no changes needed for this function if the bit #0 has been stripped. 

================
Comment at: source/Target/RegisterContext.cpp:110-116
@@ +109,9 @@
+    {
+        /*
+         * MIPS:
+         * When a breakpoint is hit in microMIPS address space, bit #0 of the PC
+         * is set by the target (CallableLoadAddress). However there is no trace
+         * of bit #0 elsewhere in the debugger. Clear bit #0 so that we can find
+         * breakpoints etc. set using OpcodeLoadAddress.
+        */
+        TargetSP target_sp = m_thread.CalculateTarget();
----------------
Probably no need for this MIPS specific comment in here, it should be documented once in the Target functions that strip the bit zero.

================
Comment at: source/Target/RegisterContext.cpp:117-120
@@ +116,6 @@
+        */
+        TargetSP target_sp = m_thread.CalculateTarget();
+        Target *target = target_sp.get();
+        Address addr (pc);
+        pc = addr.GetOpcodeLoadAddress (target);
+    }
----------------
We don't need to make a section + offset address here, we can just use:
```
lldb::addr_t
Target::GetOpcodeLoadAddress (lldb::addr_t load_addr, AddressClass addr_class) const;
```

So this code should be:

```
TargetSP target_sp = m_thread.CalculateTarget();
if (target_sp)
    pc = target->GetOpcodeLoadAddress (pc, eAddressClassCode);
```



Repository:
  rL LLVM

http://reviews.llvm.org/D12079