[MIPS] Handle false positives for MIPS hardware watchpoints

[jaydeep]

Summary

Last 3bits of the watchpoint address are masked by the kernel. For example, 'n' is at 0x120010d00 and 'm' is 0x120010d04. When a watchpoint is set at 'm', then watch exception is generated even when 'n' is read/written. To handle this case, instruction at PC is emulated to find the base address of the load/store instruction. This address is then appended to the description of the stop-info packet. Client then reads this information to check whether the user has set a watchpoint on this address.

[jingham]

This looks fine. Please add a comment to the declaration of GetWatchpointHitAddress. The meaning is really only explained in StopInfo.cpp, and I don't think the name is clear enough (we would say the watchpoint was NOT hit if the WatchpointHitAddress didn't equal the set address) to stand without some clarification. Other than that it looks good.

[jaydeep]

Addressed review comments

[jaydeep]

Could you please find some time to review this?
Thanks.

[clayborg]

Looks fine.

[jaydeep]

Hi jingham,
Could you please review this?
Thanks.

[jingham]

Yes, this looks fine.

[jaydeep]

Closed by commit rL244864

include/lldb/Host/common/NativeRegisterContext.h

public:
     GetWatchpointHitIndex(uint32_t &wp_index, lldb::addr_t trap_addr);
 
     virtual Error
     IsWatchpointVacant (uint32_t wp_index, bool &is_vacant);
 
     virtual lldb::addr_t
     GetWatchpointAddress (uint32_t wp_index);
 
+    virtual lldb::addr_t
+    GetWatchpointHitAddress (uint32_t wp_index);
+
     virtual bool
     HardwareSingleStep (bool enable);
 
     virtual Error
     ReadRegisterValueFromMemory (const lldb_private::RegisterInfo *reg_info, lldb::addr_t src_addr, size_t src_len, RegisterValue &reg_value);
 
     virtual Error
     WriteRegisterValueToMemory (const lldb_private::RegisterInfo *reg_info, lldb::addr_t dst_addr, size_t dst_len, const RegisterValue &reg_value);

include/lldb/Target/StopInfo.h

public:
     static lldb::StopInfoSP
     CreateStopReasonWithBreakpointSiteID (Thread &thread, lldb::break_id_t break_id);
 
     // This creates a StopInfo for the thread where the should_stop is already set, and won't be recalculated.
     static lldb::StopInfoSP
     CreateStopReasonWithBreakpointSiteID (Thread &thread, lldb::break_id_t break_id, bool should_stop);
 
     static lldb::StopInfoSP
-    CreateStopReasonWithWatchpointID (Thread &thread, lldb::break_id_t watch_id);
+    CreateStopReasonWithWatchpointID (Thread &thread, lldb::break_id_t watch_id, lldb::addr_t watch_hit_addr = LLDB_INVALID_ADDRESS);
 
     static lldb::StopInfoSP
     CreateStopReasonWithSignal (Thread &thread, int signo, const char *description = nullptr);
 
     static lldb::StopInfoSP
     CreateStopReasonToTrace (Thread &thread);
 
     static lldb::StopInfoSP

source/Host/common/NativeRegisterContext.cpp

 }
 
 lldb::addr_t
 NativeRegisterContext::GetWatchpointAddress (uint32_t wp_index)
 {
     return LLDB_INVALID_ADDRESS;
 }
 
+lldb::addr_t
+NativeRegisterContext::GetWatchpointHitAddress (uint32_t wp_index)
+{
+    return LLDB_INVALID_ADDRESS;
+}
+
 bool
 NativeRegisterContext::HardwareSingleStep (bool enable)
 {
     return false;
 }
 
 Error
 NativeRegisterContext::ReadRegisterValueFromMemory (

source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h

protected:
 
     bool
     Emulate_DADDiu (llvm::MCInst& insn);
 
     bool
     Emulate_SD (llvm::MCInst& insn);
 
     bool
+    Emulate_SW (llvm::MCInst& insn);
+
+    bool
+    Emulate_LW (llvm::MCInst& insn);
+
+    bool
     Emulate_LD (llvm::MCInst& insn);
 
     bool
     Emulate_BEQ (llvm::MCInst& insn);
 
     bool
     Emulate_BNE (llvm::MCInst& insn);
 

source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp

EmulateInstructionMIPS64::GetOpcodeForInstruction (const char *op_name)
     {
         //----------------------------------------------------------------------
         // Prologue/Epilogue instructions
         //----------------------------------------------------------------------
         { "DADDiu",     &EmulateInstructionMIPS64::Emulate_DADDiu,      "DADDIU rt,rs,immediate"    },
         { "SD",         &EmulateInstructionMIPS64::Emulate_SD,          "SD rt,offset(rs)"          },
         { "LD",         &EmulateInstructionMIPS64::Emulate_LD,          "LD rt,offset(base)"        },
 
+        { "SW",         &EmulateInstructionMIPS64::Emulate_SW,          "SW rt,offset(rs)"          },
+        { "LW",         &EmulateInstructionMIPS64::Emulate_LW,          "LW rt,offset(rs)"          },
+
         //----------------------------------------------------------------------
         // Branch instructions
         //----------------------------------------------------------------------
         { "BEQ",        &EmulateInstructionMIPS64::Emulate_BEQ,         "BEQ rs,rt,offset"          },
         { "BNE",        &EmulateInstructionMIPS64::Emulate_BNE,         "BNE rs,rt,offset"          },
         { "BEQL",       &EmulateInstructionMIPS64::Emulate_BEQL,        "BEQL rs,rt,offset"         },
         { "BNEL",       &EmulateInstructionMIPS64::Emulate_BNEL,        "BNEL rs,rt,offset"         },
         { "BGEZALL",    &EmulateInstructionMIPS64::Emulate_BGEZALL,     "BGEZALL rt,offset"         },
if (dst == gcc_dwarf_sp_mips64)
 
         WriteRegisterUnsigned (context, eRegisterKindDWARF, gcc_dwarf_sp_mips64, result);
     }
     
     return true;
 }
 
 bool
-EmulateInstructionMIPS64::Emulate_SD (llvm::MCInst& insn)
+EmulateInstructionMIPS64::Emulate_SW (llvm::MCInst& insn)
 {
     bool success = false;
-    uint32_t imm16 = insn.getOperand(2).getImm();
-    uint64_t imm = SignedBits(imm16, 15, 0);
-    uint32_t src, base;
+    uint32_t base;
+    int64_t imm, address;
+    Context bad_vaddr_context;
 
-    src = m_reg_info->getEncodingValue (insn.getOperand(0).getReg());
     base = m_reg_info->getEncodingValue (insn.getOperand(1).getReg());
+    imm = insn.getOperand(2).getImm();
 
-    /* We look for sp based non-volatile register stores */
-    if (base == gcc_dwarf_sp_mips64 && nonvolatile_reg_p (src))
-    {
-        uint64_t address;
-        RegisterInfo reg_info_base;
-        RegisterInfo reg_info_src;
+    RegisterInfo reg_info_base;
+    if (!GetRegisterInfo (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + base, reg_info_base))
+        return false;
+
+    /* read base register */
+    address = ReadRegisterUnsigned (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + base, 0, &success);
+    if (!success)
+        return false;
+
+    /* destination address */
+    address = address + imm;
+
+    /* Set the bad_vaddr register with base address used in the instruction */
+    bad_vaddr_context.type = eContextInvalid;
+    WriteRegisterUnsigned (bad_vaddr_context, eRegisterKindDWARF, gcc_dwarf_bad_mips64, address);
+
+    return true;
+}
+
+bool
+EmulateInstructionMIPS64::Emulate_LW (llvm::MCInst& insn)
+{
+    bool success = false;
+    uint32_t base;
+    int64_t imm, address;
+    Context bad_vaddr_context;
+
+    base = m_reg_info->getEncodingValue (insn.getOperand(1).getReg());
+    imm = insn.getOperand(2).getImm();
+
+    RegisterInfo reg_info_base;
+    if (!GetRegisterInfo (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + base, reg_info_base))
+        return false;
+
+    /* read base register */
+    address = ReadRegisterUnsigned (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + base, 0, &success);
+    if (!success)
+        return false;
+
+    /* destination address */
+    address = address + imm;
+
+    /* Set the bad_vaddr register with base address used in the instruction */
+    bad_vaddr_context.type = eContextInvalid;
+    WriteRegisterUnsigned (bad_vaddr_context, eRegisterKindDWARF, gcc_dwarf_bad_mips64, address);
+
+    return true;
+}
+
+bool
+EmulateInstructionMIPS64::Emulate_SD (llvm::MCInst& insn)
+    bool success = false;
+    uint32_t imm16 = insn.getOperand(2).getImm();
+    uint64_t imm = SignedBits(imm16, 15, 0);
+    uint32_t src, base;
+    Context bad_vaddr_context;
+
+    src = m_reg_info->getEncodingValue (insn.getOperand(0).getReg());
+    base = m_reg_info->getEncodingValue (insn.getOperand(1).getReg());
 
-        if (!GetRegisterInfo (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + base, reg_info_base)
-            || !GetRegisterInfo (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + src, reg_info_src))
-            return false;
 
-        /* read SP */
-        address = ReadRegisterUnsigned (eRegisterKindDWARF, gcc_dwarf_zero_mips64 + base, 0, &success);
-        if (!success)
-            return false;
 
-        /* destination address */
-        address = address + imm;
 
+    /* We look for sp based non-volatile register stores */
+    if (base == gcc_dwarf_sp_mips64 && nonvolatile_reg_p (src))
+    {
         Context context;
         RegisterValue data_src;
         context.type = eContextPushRegisterOnStack;
         context.SetRegisterToRegisterPlusOffset (reg_info_src, reg_info_base, 0);
 
         uint8_t buffer [RegisterValue::kMaxRegisterByteSize];
         Error error;
 
         if (!ReadRegister (&reg_info_base, data_src))
             return false;
 
         if (data_src.GetAsMemoryData (&reg_info_src, buffer, reg_info_src.byte_size, eByteOrderLittle, error) == 0)
             return false;
 
         if (!WriteMemory (context, address, buffer, reg_info_src.byte_size))
             return false;
-
-        return true;
     }
 
-    return false;
+    /* Set the bad_vaddr register with base address used in the instruction */
+    bad_vaddr_context.type = eContextInvalid;
+    WriteRegisterUnsigned (bad_vaddr_context, eRegisterKindDWARF, gcc_dwarf_bad_mips64, address);
+
+    return true;
 }
 
 bool
 EmulateInstructionMIPS64::Emulate_LD (llvm::MCInst& insn)
 {
     uint32_t src, base;
 
     src = m_reg_info->getEncodingValue (insn.getOperand(0).getReg());

source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h

public:
                                            uint32_t concrete_frame_idx);
 
         uint32_t
         GetRegisterSetCount () const override;
 
         lldb::addr_t
         GetPCfromBreakpointLocation (lldb::addr_t fail_value = LLDB_INVALID_ADDRESS) override;
 
+        lldb::addr_t
+        GetWatchpointHitAddress (uint32_t wp_index) override;
+
         const RegisterSet *
         GetRegisterSet (uint32_t set_index) const override;
 
         Error
         ReadRegister (const RegisterInfo *reg_info, RegisterValue &reg_value) override;
 
         Error
         WriteRegister (const RegisterInfo *reg_info, const RegisterValue &reg_value) override;

source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp

 // C++ Includes
 
 // Other libraries and framework includes
 #include "lldb/Core/Error.h"
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Log.h"
 #include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Host/HostInfo.h"
-
+#include "lldb/Core/EmulateInstruction.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-enumerations.h"
 #include "Plugins/Process/Linux/NativeProcessLinux.h"
 #include "Plugins/Process/Linux/Procfs.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_mips64.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_mips.h"
 
 #include <sys/ptrace.h>
 #include <asm/ptrace.h>
 

 NativeRegisterContextLinux_mips64::GetWatchpointAddress (uint32_t wp_index)
 {
     if (wp_index >= NumSupportedHardwareWatchpoints())
         return LLDB_INVALID_ADDRESS;
 
     return hw_addr_map[wp_index];
 }
 
+struct EmulatorBaton
+{
+    lldb::addr_t m_watch_hit_addr;
+    NativeProcessLinux* m_process;
+    NativeRegisterContext* m_reg_context;
+
+    EmulatorBaton(NativeProcessLinux* process, NativeRegisterContext* reg_context) :
+            m_watch_hit_addr(LLDB_INVALID_ADDRESS), 
+            m_process(process),
+            m_reg_context(reg_context) 
+            {}
+};
+
+static size_t
+ReadMemoryCallback (EmulateInstruction *instruction, void *baton,
+                    const EmulateInstruction::Context &context, lldb::addr_t addr, 
+                    void *dst, size_t length)
+{
+    size_t bytes_read;
+    EmulatorBaton* emulator_baton = static_cast<EmulatorBaton*>(baton);
+    emulator_baton->m_process->ReadMemory(addr, dst, length, bytes_read);
+    return bytes_read;
+}
+
+static size_t
+WriteMemoryCallback (EmulateInstruction *instruction, void *baton,
+                     const EmulateInstruction::Context &context, 
+                     lldb::addr_t addr, const void *dst, size_t length)
+{
+    return length;
+}
+
+static bool
+ReadRegisterCallback (EmulateInstruction *instruction, void *baton,
+                      const RegisterInfo *reg_info, RegisterValue &reg_value)
+{
+    EmulatorBaton* emulator_baton = static_cast<EmulatorBaton*>(baton);
+
+    const RegisterInfo* full_reg_info = emulator_baton->m_reg_context->GetRegisterInfo(
+            lldb::eRegisterKindDWARF, reg_info->kinds[lldb::eRegisterKindDWARF]);
+
+    Error error = emulator_baton->m_reg_context->ReadRegister(full_reg_info, reg_value);
+    if (error.Success())
+        return true;
+
+    return false;
+}
+
+static bool
+WriteRegisterCallback (EmulateInstruction *instruction, void *baton,
+                       const EmulateInstruction::Context &context,
+                       const RegisterInfo *reg_info, const RegisterValue &reg_value)
+{
+    if (reg_info->kinds[lldb::eRegisterKindDWARF] == gcc_dwarf_bad_mips64)
+    {
+        EmulatorBaton* emulator_baton = static_cast<EmulatorBaton*>(baton);
+        emulator_baton->m_watch_hit_addr = reg_value.GetAsUInt64 ();
+    }
+
+    return true;
+}
+
+/*
+ * MIPS Linux kernel returns a masked address (last 3bits are masked)
+ * when a HW watchpoint is hit. However user may not have set a watchpoint
+ * on this address. Emulate instruction at PC and find the base address of
+ * the load/store instruction. This will give the exact address used to
+ * read/write the variable. Send this exact address to client so that
+ * it can decide to stop or continue the thread.
+*/
+lldb::addr_t
+NativeRegisterContextLinux_mips64::GetWatchpointHitAddress (uint32_t wp_index)
+{
+    if (wp_index >= NumSupportedHardwareWatchpoints())
+        return LLDB_INVALID_ADDRESS;
+
+    lldb_private::ArchSpec arch;
+    arch = GetRegisterInfoInterface().GetTargetArchitecture();
+    std::unique_ptr<EmulateInstruction> emulator_ap(
+        EmulateInstruction::FindPlugin(arch, lldb_private::eInstructionTypeAny, nullptr));
+
+    if (emulator_ap == nullptr)
+        return LLDB_INVALID_ADDRESS;
+    
+    EmulatorBaton baton(static_cast<NativeProcessLinux*>(m_thread.GetProcess().get()), this);
+    emulator_ap->SetBaton (&baton);
+    emulator_ap->SetReadMemCallback (&ReadMemoryCallback);
+    emulator_ap->SetReadRegCallback (&ReadRegisterCallback);
+    emulator_ap->SetWriteMemCallback (&WriteMemoryCallback);
+    emulator_ap->SetWriteRegCallback (&WriteRegisterCallback);
+
+    if (!emulator_ap->ReadInstruction())
+        return LLDB_INVALID_ADDRESS;
+
+    if (emulator_ap->EvaluateInstruction(lldb::eEmulateInstructionOptionNone))
+        return baton.m_watch_hit_addr;
+
+    return LLDB_INVALID_ADDRESS;
+}
+
 uint32_t
 NativeRegisterContextLinux_mips64::NumSupportedHardwareWatchpoints ()
 {
     Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
     struct pt_watch_regs regs;
     static int num_valid = 0;
     if (!num_valid)
     {

source/Plugins/Process/Linux/NativeThreadLinux.cpp

NativeThreadLinux::SetStoppedByWatchpoint (uint32_t wp_index)
     m_stop_description.clear ();
 
     lldbassert(wp_index != LLDB_INVALID_INDEX32 &&
                "wp_index cannot be invalid");
 
     std::ostringstream ostr;
     ostr << GetRegisterContext()->GetWatchpointAddress(wp_index) << " ";
     ostr << wp_index;
+
+    /*
+     * MIPS: Last 3bits of the watchpoint address are masked by the kernel. For example:
+     * 'n' is at 0x120010d00 and 'm' is 0x120010d04. When a watchpoint is set at 'm', then
+     * watch exception is generated even when 'n' is read/written. To handle this case,
+     * find the base address of the load/store instruction and append it in the stop-info 
+     * packet.
+    */
+    ostr << " " << GetRegisterContext()->GetWatchpointHitAddress(wp_index);
+
     m_stop_description = ostr.str();
 
     m_stop_info.reason = StopReason::eStopReasonWatchpoint;
     m_stop_info.details.signal.signo = SIGTRAP;
 }
 
 bool
 NativeThreadLinux::IsStoppedAtBreakpoint ()

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

if (tid != LLDB_INVALID_THREAD_ID)
                         {
                             // Let the trap just use the standard signal stop reason below...
                         }
                         else if (reason.compare("watchpoint") == 0)
                         {
                             StringExtractor desc_extractor(description.c_str());
                             addr_t wp_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
                             uint32_t wp_index = desc_extractor.GetU32(LLDB_INVALID_INDEX32);
+                            addr_t wp_hit_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
                             watch_id_t watch_id = LLDB_INVALID_WATCH_ID;
                             if (wp_addr != LLDB_INVALID_ADDRESS)
                             {
                                 WatchpointSP wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
                                 if (wp_sp)
                                 {
                                     wp_sp->SetHardwareIndex(wp_index);
                                     watch_id = wp_sp->GetID();
                                 }
                             }
                             if (watch_id == LLDB_INVALID_WATCH_ID)
                             {
                                 Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_WATCHPOINTS));
                                 if (log) log->Printf ("failed to find watchpoint");
                             }
-                            thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithWatchpointID (*thread_sp, watch_id));
+                            thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithWatchpointID (*thread_sp, watch_id, wp_hit_addr));
                             handled = true;
                         }
                         else if (reason.compare("exception") == 0)
                         {
                             thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithException(*thread_sp, description.c_str()));
                             handled = true;
                         }
                         else if (reason.compare("exec") == 0)

source/Target/StopInfo.cpp

public:
                 watchpoint->TurnOffEphemeralMode();
             }
         }
     private:
         Process *process;
         Watchpoint *watchpoint;
     };
 
-    StopInfoWatchpoint (Thread &thread, break_id_t watch_id) :
+    StopInfoWatchpoint (Thread &thread, break_id_t watch_id, lldb::addr_t watch_hit_addr) :
         StopInfo(thread, watch_id),
         m_should_stop(false),
-        m_should_stop_is_valid(false)
+        m_should_stop_is_valid(false),
+        m_watch_hit_addr(watch_hit_addr)
     {
     }
     
     virtual ~StopInfoWatchpoint ()
     {
     }
 
     virtual StopReason
PerformAction (Event *event_ptr)
                                 process->ResumeSynchronous(NULL);
                                 process->GetThreadList().SetSelectedThreadByID (thread_sp->GetID());
                                 thread_sp->SetStopInfo(stored_stop_info_sp);
                             }
                         }
                     }
                 }
 
+                /*
+                 * MIPS: Last 3bits of the watchpoint address are masked by the kernel. For example:
+                 * 'n' is at 0x120010d00 and 'm' is 0x120010d04. When a watchpoint is set at 'm', then
+                 * watch exception is generated even when 'n' is read/written. To handle this case,
+                 * server emulates the instruction at PC and finds the base address of the load/store
+                 * instruction and appends it in the description of the stop-info packet. If watchpoint
+                 * is not set on this address by user then this do not stop.
+                */
+                if (m_watch_hit_addr != LLDB_INVALID_ADDRESS)
+                {
+                    WatchpointSP wp_hit_sp = thread_sp->CalculateTarget()->GetWatchpointList().FindByAddress(m_watch_hit_addr);
+                    if (!wp_hit_sp)
+                        m_should_stop = false;
+                }
+                
                 if (m_should_stop && wp_sp->GetConditionText() != NULL)
                 {
                     // We need to make sure the user sees any parse errors in their condition, so we'll hook the
                     // constructor errors up to the debugger's Async I/O.
                     ExpressionResults result_code;
                     EvaluateExpressionOptions expr_options;
                     expr_options.SetUnwindOnError(true);
                     expr_options.SetIgnoreBreakpoints(true);
PerformAction (Event *event_ptr)
             
             m_should_stop_is_valid = true;
         }
     }
         
 private:
     bool m_should_stop;
     bool m_should_stop_is_valid;
+    lldb::addr_t m_watch_hit_addr;
 };
 
 
 
 //----------------------------------------------------------------------
 // StopInfoUnixSignal
 //----------------------------------------------------------------------
 

 
 StopInfoSP
 StopInfo::CreateStopReasonWithBreakpointSiteID (Thread &thread, break_id_t break_id, bool should_stop)
 {
     return StopInfoSP (new StopInfoBreakpoint (thread, break_id, should_stop));
 }
 
 StopInfoSP
-StopInfo::CreateStopReasonWithWatchpointID (Thread &thread, break_id_t watch_id)
+StopInfo::CreateStopReasonWithWatchpointID (Thread &thread, break_id_t watch_id, lldb::addr_t watch_hit_addr)
 {
-    return StopInfoSP (new StopInfoWatchpoint (thread, watch_id));
+    return StopInfoSP (new StopInfoWatchpoint (thread, watch_id, watch_hit_addr));
 }
 
 StopInfoSP
 StopInfo::CreateStopReasonWithSignal (Thread &thread, int signo, const char *description)
 {
     return StopInfoSP (new StopInfoUnixSignal (thread, signo, description));
 }