diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h index 7594ad91369b..dda3eaead956 100644 --- a/lldb/include/lldb/Breakpoint/Watchpoint.h +++ b/lldb/include/lldb/Breakpoint/Watchpoint.h @@ -32,8 +32,7 @@ class Watchpoint : { public: - Watchpoint (lldb::addr_t addr, size_t size, bool hardware = true); - + Watchpoint (Target& target, lldb::addr_t addr, size_t size, const ClangASTType *type, bool hardware = true); ~Watchpoint (); void @@ -63,21 +62,13 @@ public: // Snapshot management interface. bool IsWatchVariable() const; void SetWatchVariable(bool val); - std::string GetOldSnapshot() const; - void SetOldSnapshot (const std::string &str); - std::string GetNewSnapshot() const; - void SetNewSnapshot (const std::string &str); - uint64_t GetOldSnapshotVal() const; - void SetOldSnapshotVal (uint64_t val); - uint64_t GetNewSnapshotVal() const; - void SetNewSnapshotVal (uint64_t val); - void ClearSnapshots(); + bool CaptureWatchedValue (const ExecutionContext &exe_ctx); void GetDescription (Stream *s, lldb::DescriptionLevel level); void Dump (Stream *s) const; void DumpSnapshots (Stream *s, const char * prefix = NULL) const; void DumpWithLevel (Stream *s, lldb::DescriptionLevel description_level) const; - Target &GetTarget() { return *m_target; } + Target &GetTarget() { return m_target; } const Error &GetError() { return m_error; } //------------------------------------------------------------------ @@ -158,15 +149,21 @@ public: bool IsDisabledDuringEphemeralMode(); + + const ClangASTType & + GetClangASTType() + { + return m_type; + } + private: friend class Target; friend class WatchpointList; - void SetTarget(Target *target_ptr) { m_target = target_ptr; } void ResetHitCount() { m_hit_count = 0; } - Target *m_target; + Target &m_target; bool m_enabled; // Is this watchpoint enabled bool m_is_hardware; // Is this a hardware watchpoint bool m_is_watch_variable; // True if set via 'watchpoint set variable'. @@ -185,10 +182,9 @@ private: uint32_t m_false_alarms; // Number of false alarms. std::string m_decl_str; // Declaration information, if any. std::string m_watch_spec_str; // Spec for the watchpoint. - std::string m_snapshot_old_str; // Old snapshot for the watchpoint value as by ValueObject::DumpValueObject(). - std::string m_snapshot_new_str; // New Snapshot for the watchpoint value as by ValueObject::DumpValueObject(). - uint64_t m_snapshot_old_val; // Old snapshot for the watchpoint bytes. - uint64_t m_snapshot_new_val; // New Snapshot for the watchpoint bytes. + lldb::ValueObjectSP m_old_value_sp; + lldb::ValueObjectSP m_new_value_sp; + ClangASTType m_type; Error m_error; // An error object describing errors associated with this watchpoint. WatchpointOptions m_options; // Settable watchpoint options, which is a delegate to handle // the callback machinery. diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index c66d61b513f7..65cdaa1e2790 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -522,7 +522,8 @@ public: lldb::WatchpointSP CreateWatchpoint (lldb::addr_t addr, size_t size, - uint32_t type, + const ClangASTType *type, + uint32_t kind, Error &error); lldb::WatchpointSP diff --git a/lldb/lldb.xcodeproj/project.pbxproj b/lldb/lldb.xcodeproj/project.pbxproj index 1fa2dbbde93f..fe0ae1c38cb2 100644 --- a/lldb/lldb.xcodeproj/project.pbxproj +++ b/lldb/lldb.xcodeproj/project.pbxproj @@ -2732,8 +2732,8 @@ 26BC7CFB10F1B71400F91463 /* StoppointLocation.h */, 26BC7E1710F1B83100F91463 /* StoppointLocation.cpp */, 26BC7CFC10F1B71400F91463 /* Watchpoint.h */, - B27318431416AC43006039C8 /* WatchpointList.h */, 26BC7E1810F1B83100F91463 /* Watchpoint.cpp */, + B27318431416AC43006039C8 /* WatchpointList.h */, B27318411416AC12006039C8 /* WatchpointList.cpp */, B2B7CCED15D1BFB700EEFB57 /* WatchpointOptions.h */, B2B7CCEF15D1C20F00EEFB57 /* WatchpointOptions.cpp */, diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 569e6fe2c4f2..071c4b5a983c 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -1783,7 +1783,9 @@ SBTarget::WatchAddress (lldb::addr_t addr, size_t size, bool read, bool write, S watch_type |= LLDB_WATCH_TYPE_WRITE; // Target::CreateWatchpoint() is thread safe. Error cw_error; - watchpoint_sp = target_sp->CreateWatchpoint(addr, size, watch_type, cw_error); + // This API doesn't take in a type, so we can't figure out what it is. + ClangASTType *type = NULL; + watchpoint_sp = target_sp->CreateWatchpoint(addr, size, type, watch_type, cw_error); error.SetError(cw_error); sb_watchpoint.SetSP (watchpoint_sp); } diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 9e80310f0457..fb66cb799135 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -1930,7 +1930,8 @@ SBValue::Watch (bool resolve_location, bool read, bool write, SBError &error) watch_type |= LLDB_WATCH_TYPE_WRITE; Error rc; - WatchpointSP watchpoint_sp = target_sp->CreateWatchpoint(addr, byte_size, watch_type, rc); + ClangASTType type (value_sp->GetClangAST(), value_sp->GetClangType()); + WatchpointSP watchpoint_sp = target_sp->CreateWatchpoint(addr, byte_size, &type, watch_type, rc); error.SetError(rc); if (watchpoint_sp) diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp index ca5503ff2d34..2012eef8785c 100644 --- a/lldb/source/Breakpoint/Watchpoint.cpp +++ b/lldb/source/Breakpoint/Watchpoint.cpp @@ -15,6 +15,10 @@ // Project includes #include "lldb/Breakpoint/StoppointCallbackContext.h" #include "lldb/Core/Stream.h" +#include "lldb/Core/Value.h" +#include "lldb/Core/ValueObject.h" +#include "lldb/Core/ValueObjectMemory.h" +#include "lldb/Symbol/ClangASTContext.h" #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" #include "lldb/Target/ThreadSpec.h" @@ -23,9 +27,9 @@ using namespace lldb; using namespace lldb_private; -Watchpoint::Watchpoint (lldb::addr_t addr, size_t size, bool hardware) : +Watchpoint::Watchpoint (Target& target, lldb::addr_t addr, size_t size, const ClangASTType *type, bool hardware) : StoppointLocation (0, addr, size, hardware), - m_target(NULL), + m_target(target), m_enabled(false), m_is_hardware(hardware), m_is_watch_variable(false), @@ -39,13 +43,27 @@ Watchpoint::Watchpoint (lldb::addr_t addr, size_t size, bool hardware) : m_false_alarms(0), m_decl_str(), m_watch_spec_str(), - m_snapshot_old_str(), - m_snapshot_new_str(), - m_snapshot_old_val(0), - m_snapshot_new_val(0), + m_type(), m_error(), m_options () { + if (type && type->IsValid()) + m_type = *type; + else + { + // If we don't have a known type, then we force it to unsigned int of the right size. + ClangASTContext *ast_context = target.GetScratchClangASTContext(); + clang_type_t clang_type = ast_context->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, size); + m_type.SetClangType(ast_context->getASTContext(), clang_type); + } + + // Set the initial value of the watched variable: + if (m_target.GetProcessSP()) + { + ExecutionContext exe_ctx; + m_target.GetProcessSP()->CalculateExecutionContext(exe_ctx); + CaptureWatchedValue (exe_ctx); + } } Watchpoint::~Watchpoint() @@ -97,78 +115,6 @@ Watchpoint::SetWatchSpec (const std::string &str) return; } -// Strip at most one character from the end of the string. -static inline std::string -RStripOnce(const std::string &str, const char c) -{ - std::string res = str; - size_t len = res.length(); - if (len && res.at(len - 1) == '\n') - res.resize(len - 1); - return res; -} - -std::string -Watchpoint::GetOldSnapshot() const -{ - return m_snapshot_old_str; -} - -void -Watchpoint::SetOldSnapshot (const std::string &str) -{ - m_snapshot_old_str = RStripOnce(str, '\n'); -} - -std::string -Watchpoint::GetNewSnapshot() const -{ - return m_snapshot_new_str; -} - -void -Watchpoint::SetNewSnapshot (const std::string &str) -{ - m_snapshot_old_str = m_snapshot_new_str; - m_snapshot_new_str = RStripOnce(str, '\n'); -} - -uint64_t -Watchpoint::GetOldSnapshotVal() const -{ - return m_snapshot_old_val; -} - -void -Watchpoint::SetOldSnapshotVal (uint64_t val) -{ - m_snapshot_old_val = val; - return; -} - -uint64_t -Watchpoint::GetNewSnapshotVal() const -{ - return m_snapshot_new_val; -} - -void -Watchpoint::SetNewSnapshotVal (uint64_t val) -{ - m_snapshot_old_val = m_snapshot_new_val; - m_snapshot_new_val = val; - return; -} - -void -Watchpoint::ClearSnapshots() -{ - m_snapshot_old_str.clear(); - m_snapshot_new_str.clear(); - m_snapshot_old_val = 0; - m_snapshot_new_val = 0; -} - // Override default impl of StoppointLocation::IsHardware() since m_is_hardware // member field is more accurate. bool @@ -189,6 +135,20 @@ Watchpoint::SetWatchVariable(bool val) m_is_watch_variable = val; } +bool +Watchpoint::CaptureWatchedValue (const ExecutionContext &exe_ctx) +{ + ConstString watch_name("$__lldb__watch_value"); + m_old_value_sp = m_new_value_sp; + Address watch_address(GetLoadAddress()); + m_new_value_sp = ValueObjectMemory::Create (exe_ctx.GetBestExecutionContextScope(), watch_name.AsCString(), watch_address, m_type); + m_new_value_sp = m_new_value_sp->CreateConstantValue(watch_name); + if (m_new_value_sp && m_new_value_sp->GetError().Success()) + return true; + else + return false; +} + void Watchpoint::IncrementFalseAlarmsAndReviseHitCount() { @@ -247,19 +207,14 @@ Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const s->Printf("\nWatchpoint %u hit:", GetID()); prefix = ""; } - - if (IsWatchVariable()) + + if (m_old_value_sp) { - if (!m_snapshot_old_str.empty()) - s->Printf("\n%sold value: %s", prefix, m_snapshot_old_str.c_str()); - if (!m_snapshot_new_str.empty()) - s->Printf("\n%snew value: %s", prefix, m_snapshot_new_str.c_str()); + s->Printf("\n%sold value: %s", prefix, m_old_value_sp->GetValueAsCString()); } - else + if (m_new_value_sp) { - uint32_t num_hex_digits = GetByteSize() * 2; - s->Printf("\n%sold value: 0x%0*.*llx", prefix, num_hex_digits, num_hex_digits, m_snapshot_old_val); - s->Printf("\n%snew value: 0x%0*.*llx", prefix, num_hex_digits, num_hex_digits, m_snapshot_new_val); + s->Printf("\n%snew value: %s", prefix, m_new_value_sp->GetValueAsCString()); } } @@ -345,7 +300,6 @@ Watchpoint::SetEnabled(bool enabled) // Don't clear the snapshots for now. // Within StopInfo.cpp, we purposely do disable/enable watchpoint while performing watchpoint actions. - //ClearSnapshots(); } m_enabled = enabled; } diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp index 63275f9d303c..d3b22439586a 100644 --- a/lldb/source/Commands/CommandObjectWatchpoint.cpp +++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp @@ -1056,6 +1056,8 @@ protected: valobj_sp = valobj_list.GetValueObjectAtIndex(0); } + ClangASTType type; + if (valobj_sp) { AddressType addr_type; addr = valobj_sp->GetAddressOf(false, &addr_type); @@ -1065,6 +1067,7 @@ protected: size = m_option_watchpoint.watch_size == 0 ? valobj_sp->GetByteSize() : m_option_watchpoint.watch_size; } + type.SetClangType(valobj_sp->GetClangAST(), valobj_sp->GetClangType()); } else { const char *error_cstr = error.AsCString(NULL); if (error_cstr) @@ -1078,7 +1081,7 @@ protected: // Now it's time to create the watchpoint. uint32_t watch_type = m_option_watchpoint.watch_type; error.Clear(); - Watchpoint *wp = target->CreateWatchpoint(addr, size, watch_type, error).get(); + Watchpoint *wp = target->CreateWatchpoint(addr, size, &type, watch_type, error).get(); if (wp) { wp->SetWatchSpec(command.GetArgumentAtIndex(0)); wp->SetWatchVariable(true); @@ -1088,9 +1091,6 @@ protected: var_sp->GetDeclaration().DumpStopContext(&ss, true); wp->SetDeclInfo(ss.GetString()); } - StreamString ss; - ValueObject::DumpValueObject(ss, valobj_sp.get()); - wp->SetNewSnapshot(ss.GetString()); output_stream.Printf("Watchpoint created: "); wp->GetDescription(&output_stream, lldb::eDescriptionLevelFull); output_stream.EOL(); @@ -1265,8 +1265,15 @@ protected: // Now it's time to create the watchpoint. uint32_t watch_type = m_option_watchpoint.watch_type; + + // Fetch the type from the value object, the type of the watched object is the pointee type + /// of the expression, so convert to that if we found a valid type. + ClangASTType type(valobj_sp->GetClangAST(), valobj_sp->GetClangType()); + if (type.IsValid()) + type.SetClangType(type.GetASTContext(), type.GetPointeeType()); + Error error; - Watchpoint *wp = target->CreateWatchpoint(addr, size, watch_type, error).get(); + Watchpoint *wp = target->CreateWatchpoint(addr, size, &type, watch_type, error).get(); if (wp) { if (var_sp && var_sp->GetDeclaration().GetFile()) { StreamString ss; @@ -1275,11 +1282,6 @@ protected: wp->SetDeclInfo(ss.GetString()); } output_stream.Printf("Watchpoint created: "); - uint64_t val = target->GetProcessSP()->ReadUnsignedIntegerFromMemory(addr, size, 0, error); - if (error.Success()) - wp->SetNewSnapshotVal(val); - else - output_stream.Printf("watchpoint snapshot failed: %s", error.AsCString()); wp->GetDescription(&output_stream, lldb::eDescriptionLevelFull); output_stream.EOL(); result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index c5c9c7051f4b..1537b4db2232 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -550,75 +550,6 @@ public: } } - // Record the snapshot of our watchpoint. - VariableSP var_sp; - ValueObjectSP valobj_sp; - StackFrame *frame = exe_ctx.GetFramePtr(); - if (frame) - { - bool snapshot_taken = true; - if (!wp_sp->IsWatchVariable()) - { - // We are not watching a variable, just read from the process memory for the watched location. - assert (process); - Error error; - uint64_t val = process->ReadUnsignedIntegerFromMemory(wp_sp->GetLoadAddress(), - wp_sp->GetByteSize(), - 0, - error); - if (log) - { - if (error.Success()) - log->Printf("Watchpoint snapshot val taken: 0x%llx\n", val); - else - log->Printf("Watchpoint snapshot val taking failed.\n"); - } - wp_sp->SetNewSnapshotVal(val); - } - else if (!wp_sp->GetWatchSpec().empty()) - { - // Use our frame to evaluate the variable expression. - Error error; - uint32_t expr_path_options = StackFrame::eExpressionPathOptionCheckPtrVsMember | - StackFrame::eExpressionPathOptionsAllowDirectIVarAccess; - valobj_sp = frame->GetValueForVariableExpressionPath (wp_sp->GetWatchSpec().c_str(), - eNoDynamicValues, - expr_path_options, - var_sp, - error); - if (valobj_sp) - { - // We're in business. - StreamString ss; - ValueObject::DumpValueObject(ss, valobj_sp.get()); - wp_sp->SetNewSnapshot(ss.GetString()); - } - else - { - // The variable expression has become out of scope? - // Let us forget about this stop info. - if (log) - log->Printf("Snapshot attempt failed. Variable expression has become out of scope?"); - snapshot_taken = false; - m_should_stop = false; - wp_sp->IncrementFalseAlarmsAndReviseHitCount(); - } - - if (log && snapshot_taken) - log->Printf("Watchpoint snapshot taken: '%s'\n", wp_sp->GetNewSnapshot().c_str()); - } - - // Now dump the snapshots we have taken. - if (snapshot_taken) - { - Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger(); - StreamSP output_sp = debugger.GetAsyncOutputStream (); - wp_sp->DumpSnapshots(output_sp.get()); - output_sp->EOL(); - output_sp->Flush(); - } - } - 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 @@ -703,6 +634,18 @@ public: m_should_stop = false; } } + // Finally, if we are going to stop, print out the new & old values: + if (m_should_stop) + { + wp_sp->CaptureWatchedValue(exe_ctx); + + Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger(); + StreamSP output_sp = debugger.GetAsyncOutputStream (); + wp_sp->DumpSnapshots(output_sp.get()); + output_sp->EOL(); + output_sp->Flush(); + } + } else { @@ -713,6 +656,7 @@ public: } if (log) log->Printf ("Process::%s returning from action with m_should_stop: %d.", __FUNCTION__, m_should_stop); + } virtual const char * diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 330537301afb..33d0ffd16183 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -523,12 +523,12 @@ CheckIfWatchpointsExhausted(Target *target, Error &error) // See also Watchpoint::SetWatchpointType(uint32_t type) and // the OptionGroupWatchpoint::WatchType enum type. WatchpointSP -Target::CreateWatchpoint(lldb::addr_t addr, size_t size, uint32_t type, Error &error) +Target::CreateWatchpoint(lldb::addr_t addr, size_t size, const ClangASTType *type, uint32_t kind, Error &error) { LogSP log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_WATCHPOINTS)); if (log) log->Printf("Target::%s (addr = 0x%8.8llx size = %llu type = %u)\n", - __FUNCTION__, addr, (uint64_t)size, type); + __FUNCTION__, addr, (uint64_t)size, kind); WatchpointSP wp_sp; if (!ProcessIsValid()) @@ -559,7 +559,7 @@ Target::CreateWatchpoint(lldb::addr_t addr, size_t size, uint32_t type, Error &e (matched_sp->WatchpointRead() ? LLDB_WATCH_TYPE_READ : 0) | (matched_sp->WatchpointWrite() ? LLDB_WATCH_TYPE_WRITE : 0); // Return the existing watchpoint if both size and type match. - if (size == old_size && type == old_type) { + if (size == old_size && kind == old_type) { wp_sp = matched_sp; wp_sp->SetEnabled(false); } else { @@ -570,13 +570,12 @@ Target::CreateWatchpoint(lldb::addr_t addr, size_t size, uint32_t type, Error &e } if (!wp_sp) { - Watchpoint *new_wp = new Watchpoint(addr, size); + Watchpoint *new_wp = new Watchpoint(*this, addr, size, type); if (!new_wp) { printf("Watchpoint ctor failed, out of memory?\n"); return wp_sp; } - new_wp->SetWatchpointType(type); - new_wp->SetTarget(this); + new_wp->SetWatchpointType(kind); wp_sp.reset(new_wp); m_watchpoint_list.Add(wp_sp); } diff --git a/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py b/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py index 9dc5986336b0..81a1e0b88c7c 100644 --- a/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py +++ b/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py @@ -12,6 +12,15 @@ class WatchedVariableHitWhenInScopeTestCase(TestBase): mydir = os.path.join("functionalities", "watchpoint", "variable_out_of_scope") + # + # This test depends on not tracking watchpoint expression hits if we have + # left the watchpoint scope. We will provide such an ability at some point + # but the way this was done was incorrect, and it is unclear that for the + # most part that's not what folks mostly want, so we have to provide a + # clearer API to express this. + # + + @unittest2.expectedFailure @dsym_test def test_watched_var_should_only_hit_when_in_scope_with_dsym(self): """Test that a variable watchpoint should only hit when in scope.""" @@ -19,6 +28,7 @@ class WatchedVariableHitWhenInScopeTestCase(TestBase): self.setTearDownCleanup(dictionary=self.d) self.watched_var() + @unittest2.expectedFailure @dwarf_test def test_watched_var_should_only_hit_when_in_scope_with_dwarf(self): """Test that a variable watchpoint should only hit when in scope.""" diff --git a/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py b/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py index ecad0102eb55..543aad7174b8 100644 --- a/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py +++ b/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py @@ -97,8 +97,8 @@ class WatchpointLLDBCommandTestCase(TestBase): # Check that the watchpoint snapshoting mechanism is working. self.expect("watchpoint list -v", - substrs = ['old value:', 'global = 0', - 'new value:', 'global = 1']) + substrs = ['old value:', ' = 0', + 'new value:', ' = 1']) # The watchpoint command "forced" our global variable 'cookie' to become 777. self.expect("frame variable -g cookie", diff --git a/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py b/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py index 0fa6de171d9a..055697add15e 100644 --- a/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py +++ b/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py @@ -85,8 +85,8 @@ class WatchpointPythonCommandTestCase(TestBase): # Check that the watchpoint snapshoting mechanism is working. self.expect("watchpoint list -v", - substrs = ['old value:', 'global = 0', - 'new value:', 'global = 1']) + substrs = ['old value:', ' = 0', + 'new value:', ' = 1']) # The watchpoint command "forced" our global variable 'cookie' to become 777. self.expect("frame variable -g cookie",