diff options
| author | Adrian Prantl <aprantl@apple.com> | 2018-04-30 16:49:04 +0000 |
|---|---|---|
| committer | Adrian Prantl <aprantl@apple.com> | 2018-04-30 16:49:04 +0000 |
| commit | 05097246f352eca76207c9ebb08656c88bdf751a (patch) | |
| tree | bfc4ec8250a939aaf4ade6fc6c528726183e5367 /lldb/source/Target | |
| parent | add59c052dd6768fd54431e6a3bf045e7f25cb59 (diff) | |
| download | bcm5719-llvm-05097246f352eca76207c9ebb08656c88bdf751a.tar.gz bcm5719-llvm-05097246f352eca76207c9ebb08656c88bdf751a.zip | |
Reflow paragraphs in comments.
This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to read.
FYI, the script I used was:
import textwrap
import commands
import os
import sys
import re
tmp = "%s.tmp"%sys.argv[1]
out = open(tmp, "w+")
with open(sys.argv[1], "r") as f:
header = ""
text = ""
comment = re.compile(r'^( *//) ([^ ].*)$')
special = re.compile(r'^((([A-Z]+[: ])|([0-9]+ )).*)|(.*;)$')
for line in f:
match = comment.match(line)
if match and not special.match(match.group(2)):
# skip intentionally short comments.
if not text and len(match.group(2)) < 40:
out.write(line)
continue
if text:
text += " " + match.group(2)
else:
header = match.group(1)
text = match.group(2)
continue
if text:
filled = textwrap.wrap(text, width=(78-len(header)),
break_long_words=False)
for l in filled:
out.write(header+" "+l+'\n')
text = ""
out.write(line)
os.rename(tmp, sys.argv[1])
Differential Revision: https://reviews.llvm.org/D46144
llvm-svn: 331197
Diffstat (limited to 'lldb/source/Target')
37 files changed, 1209 insertions, 1508 deletions
diff --git a/lldb/source/Target/ABI.cpp b/lldb/source/Target/ABI.cpp index 72f58be5a3d..ecf7aed858e 100644 --- a/lldb/source/Target/ABI.cpp +++ b/lldb/source/Target/ABI.cpp @@ -193,18 +193,18 @@ bool ABI::PrepareTrivialCall(Thread &thread, lldb::addr_t sp, bool ABI::GetFallbackRegisterLocation( const RegisterInfo *reg_info, UnwindPlan::Row::RegisterLocation &unwind_regloc) { - // Did the UnwindPlan fail to give us the caller's stack pointer? - // The stack pointer is defined to be the same as THIS frame's CFA, so return - // the CFA value as - // the caller's stack pointer. This is true on x86-32/x86-64 at least. + // Did the UnwindPlan fail to give us the caller's stack pointer? The stack + // pointer is defined to be the same as THIS frame's CFA, so return the CFA + // value as the caller's stack pointer. This is true on x86-32/x86-64 at + // least. if (reg_info->kinds[eRegisterKindGeneric] == LLDB_REGNUM_GENERIC_SP) { unwind_regloc.SetIsCFAPlusOffset(0); return true; } // If a volatile register is being requested, we don't want to forward the - // next frame's register contents - // up the stack -- the register is not retrievable at this frame. + // next frame's register contents up the stack -- the register is not + // retrievable at this frame. if (RegisterIsVolatile(reg_info)) { unwind_regloc.SetUndefined(); return true; diff --git a/lldb/source/Target/ExecutionContext.cpp b/lldb/source/Target/ExecutionContext.cpp index 3643c159042..73c64916cf1 100644 --- a/lldb/source/Target/ExecutionContext.cpp +++ b/lldb/source/Target/ExecutionContext.cpp @@ -369,15 +369,14 @@ ExecutionContext &ExecutionContext::operator=(const ExecutionContext &rhs) { bool ExecutionContext::operator==(const ExecutionContext &rhs) const { // Check that the frame shared pointers match, or both are valid and their - // stack - // IDs match since sometimes we get new objects that represent the same + // stack IDs match since sometimes we get new objects that represent the same // frame within a thread. if ((m_frame_sp == rhs.m_frame_sp) || (m_frame_sp && rhs.m_frame_sp && m_frame_sp->GetStackID() == rhs.m_frame_sp->GetStackID())) { - // Check that the thread shared pointers match, or both are valid and - // their thread IDs match since sometimes we get new objects that - // represent the same thread within a process. + // Check that the thread shared pointers match, or both are valid and their + // thread IDs match since sometimes we get new objects that represent the + // same thread within a process. if ((m_thread_sp == rhs.m_thread_sp) || (m_thread_sp && rhs.m_thread_sp && m_thread_sp->GetID() == rhs.m_thread_sp->GetID())) { @@ -596,9 +595,9 @@ lldb::ThreadSP ExecutionContextRef::GetThreadSP() const { lldb::ThreadSP thread_sp(m_thread_wp.lock()); if (m_tid != LLDB_INVALID_THREAD_ID) { - // We check if the thread has been destroyed in cases where clients - // might still have shared pointer to a thread, but the thread is - // not valid anymore (not part of the process) + // We check if the thread has been destroyed in cases where clients might + // still have shared pointer to a thread, but the thread is not valid + // anymore (not part of the process) if (!thread_sp || !thread_sp->IsValid()) { lldb::ProcessSP process_sp(GetProcessSP()); if (process_sp && process_sp->IsValid()) { @@ -608,9 +607,8 @@ lldb::ThreadSP ExecutionContextRef::GetThreadSP() const { } } - // Check that we aren't about to return an invalid thread sp. We might return - // a nullptr thread_sp, - // but don't return an invalid one. + // Check that we aren't about to return an invalid thread sp. We might + // return a nullptr thread_sp, but don't return an invalid one. if (thread_sp && !thread_sp->IsValid()) thread_sp.reset(); diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp index ced35941868..ad1b4093155 100644 --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -133,12 +133,12 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len, Status &error) { size_t bytes_left = dst_len; - // Check the L1 cache for a range that contain the entire memory read. - // If we find a range in the L1 cache that does, we use it. Else we fall - // back to reading memory in m_L2_cache_line_byte_size byte sized chunks. - // The L1 cache contains chunks of memory that are not required to be - // m_L2_cache_line_byte_size bytes in size, so we don't try anything - // tricky when reading from them (no partial reads from the L1 cache). + // Check the L1 cache for a range that contain the entire memory read. If we + // find a range in the L1 cache that does, we use it. Else we fall back to + // reading memory in m_L2_cache_line_byte_size byte sized chunks. The L1 + // cache contains chunks of memory that are not required to be + // m_L2_cache_line_byte_size bytes in size, so we don't try anything tricky + // when reading from them (no partial reads from the L1 cache). std::lock_guard<std::recursive_mutex> guard(m_mutex); if (!m_L1_cache.empty()) { @@ -155,11 +155,11 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len, } } - // If this memory read request is larger than the cache line size, then - // we (1) try to read as much of it at once as possible, and (2) don't - // add the data to the memory cache. We don't want to split a big read - // up into more separate reads than necessary, and with a large memory read - // request, it is unlikely that the caller function will ask for the next + // If this memory read request is larger than the cache line size, then we + // (1) try to read as much of it at once as possible, and (2) don't add the + // data to the memory cache. We don't want to split a big read up into more + // separate reads than necessary, and with a large memory read request, it is + // unlikely that the caller function will ask for the next // 4 bytes after the large memory read - so there's little benefit to saving // it in the cache. if (dst && dst_len > m_L2_cache_line_byte_size) { @@ -218,9 +218,9 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len, bytes_left -= curr_read_size; curr_addr += curr_read_size; - // We have a cache page that succeeded to read some bytes - // but not an entire page. If this happens, we must cap - // off how much data we are able to read... + // We have a cache page that succeeded to read some bytes but not + // an entire page. If this happens, we must cap off how much data + // we are able to read... if (pos->second->GetByteSize() != cache_line_byte_size) return dst_len - bytes_left; } @@ -277,8 +277,8 @@ lldb::addr_t AllocatedBlock::ReserveBlock(uint32_t size) { if (range_size >= size) { // We found a free block that is big enough for our data. Figure out how - // many chunks we will need and calculate the resulting block size we will - // reserve. + // many chunks we will need and calculate the resulting block size we + // will reserve. addr_t addr = free_block.GetRangeBase(); size_t num_chunks = CalculateChunksNeededForSize(size); lldb::addr_t block_size = num_chunks * m_chunk_size; @@ -296,8 +296,8 @@ lldb::addr_t AllocatedBlock::ReserveBlock(uint32_t size) { // Make the new allocated range and add it to the allocated ranges. Range<lldb::addr_t, uint32_t> reserved_block(free_block); reserved_block.SetByteSize(block_size); - // Insert the reserved range and don't combine it with other blocks - // in the reserved blocks list. + // Insert the reserved range and don't combine it with other blocks in + // the reserved blocks list. m_reserved_blocks.Insert(reserved_block, false); // Adjust the free range in place since we won't change the sorted // ordering of the m_free_blocks list. diff --git a/lldb/source/Target/ModuleCache.cpp b/lldb/source/Target/ModuleCache.cpp index 2b654772639..19adfbabe27 100644 --- a/lldb/source/Target/ModuleCache.cpp +++ b/lldb/source/Target/ModuleCache.cpp @@ -312,9 +312,8 @@ Status ModuleCache::GetAndPut(const FileSpec &root_dir_spec, llvm::FileRemover tmp_symfile_remover(tmp_download_sym_file_spec.GetPath()); if (error.Fail()) // Failed to download a symfile but fetching the module was successful. The - // module might - // contain the necessary symbols and the debugging is also possible without - // a symfile. + // module might contain the necessary symbols and the debugging is also + // possible without a symfile. return Status(); error = Put(root_dir_spec, escaped_hostname.c_str(), module_spec, diff --git a/lldb/source/Target/ObjCLanguageRuntime.cpp b/lldb/source/Target/ObjCLanguageRuntime.cpp index e50167dcb58..b1fcee6db63 100644 --- a/lldb/source/Target/ObjCLanguageRuntime.cpp +++ b/lldb/source/Target/ObjCLanguageRuntime.cpp @@ -170,8 +170,7 @@ ObjCLanguageRuntime::GetDescriptorIterator(const ConstString &name) { UpdateISAToDescriptorMap(); if (m_hash_to_isa_map.empty()) { // No name hashes were provided, we need to just linearly power through - // the - // names and find a match + // the names and find a match for (ISAToDescriptorIterator pos = m_isa_to_descriptor.begin(); pos != end; ++pos) { if (pos->second->GetClassName() == name) @@ -239,9 +238,9 @@ ObjCLanguageRuntime::GetClassDescriptorFromClassName( ObjCLanguageRuntime::ClassDescriptorSP ObjCLanguageRuntime::GetClassDescriptor(ValueObject &valobj) { ClassDescriptorSP objc_class_sp; - // if we get an invalid VO (which might still happen when playing around - // with pointers returned by the expression parser, don't consider this - // a valid ObjC object) + // if we get an invalid VO (which might still happen when playing around with + // pointers returned by the expression parser, don't consider this a valid + // ObjC object) if (valobj.GetCompilerType().IsValid()) { addr_t isa_pointer = valobj.GetPointerValue(); if (isa_pointer != LLDB_INVALID_ADDRESS) { diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index 0ed2b510fb0..e5d1f47347f 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -46,8 +46,8 @@ #include "llvm/Support/FileSystem.h" -// Define these constants from POSIX mman.h rather than include the file -// so that they will be correct even when compiled on Linux. +// Define these constants from POSIX mman.h rather than include the file so +// that they will be correct even when compiled on Linux. #define MAP_PRIVATE 2 #define MAP_ANON 0x1000 @@ -56,8 +56,8 @@ using namespace lldb_private; static uint32_t g_initialize_count = 0; -// Use a singleton function for g_local_platform_sp to avoid init -// constructors since LLDB is often part of a shared library +// Use a singleton function for g_local_platform_sp to avoid init constructors +// since LLDB is often part of a shared library static PlatformSP &GetHostPlatformSP() { static PlatformSP g_platform_sp; return g_platform_sp; @@ -468,10 +468,9 @@ bool Platform::GetOSVersion(uint32_t &major, uint32_t &minor, uint32_t &update, bool fetch = false; if (success) { - // We have valid OS version info, check to make sure it wasn't - // manually set prior to connecting. If it was manually set prior - // to connecting, then lets fetch the actual OS version info - // if we are now connected. + // We have valid OS version info, check to make sure it wasn't manually + // set prior to connecting. If it was manually set prior to connecting, + // then lets fetch the actual OS version info if we are now connected. if (is_connected && !m_os_version_set_while_connected) fetch = true; } else { @@ -490,8 +489,8 @@ bool Platform::GetOSVersion(uint32_t &major, uint32_t &minor, uint32_t &update, minor = m_minor_os_version; update = m_update_os_version; } else if (process) { - // Check with the process in case it can answer the question if - // a process was provided + // Check with the process in case it can answer the question if a process + // was provided return process->GetHostOSVersion(major, minor, update); } return success; @@ -578,8 +577,8 @@ RecurseCopy_Callback(void *baton, llvm::sys::fs::file_type ft, // now recurse std::string src_dir_path(src.GetPath()); - // Make a filespec that only fills in the directory of a FileSpec so - // when we enumerate we can quickly fill in the filename for dst copies + // Make a filespec that only fills in the directory of a FileSpec so when + // we enumerate we can quickly fill in the filename for dst copies FileSpec recurse_dst; recurse_dst.GetDirectory().SetCString(dst_dir.GetPath().c_str()); RecurseCopyBaton rc_baton2 = {recurse_dst, rc_baton->platform_ptr, @@ -657,9 +656,9 @@ Status Platform::Install(const FileSpec &src, const FileSpec &dst) { if (first_dst_dir_char == '/' || first_dst_dir_char == '\\') { fixed_dst.GetDirectory() = dst.GetDirectory(); } - // If the fixed destination file doesn't have a directory yet, - // then we must have a relative path. We will resolve this relative - // path against the platform's working directory + // If the fixed destination file doesn't have a directory yet, then we + // must have a relative path. We will resolve this relative path against + // the platform's working directory if (!fixed_dst.GetDirectory()) { FileSpec relative_spec; std::string path; @@ -862,21 +861,19 @@ const char *Platform::GetGroupName(uint32_t gid) { bool Platform::SetOSVersion(uint32_t major, uint32_t minor, uint32_t update) { if (IsHost()) { - // We don't need anyone setting the OS version for the host platform, - // we should be able to figure it out by calling - // HostInfo::GetOSVersion(...). + // We don't need anyone setting the OS version for the host platform, we + // should be able to figure it out by calling HostInfo::GetOSVersion(...). return false; } else { - // We have a remote platform, allow setting the target OS version if - // we aren't connected, since if we are connected, we should be able to + // We have a remote platform, allow setting the target OS version if we + // aren't connected, since if we are connected, we should be able to // request the remote OS version from the connected platform. if (IsConnected()) return false; else { - // We aren't connected and we might want to set the OS version - // ahead of time before we connect so we can peruse files and - // use a local SDK or PDK cache of support files to disassemble - // or do other things. + // We aren't connected and we might want to set the OS version ahead of + // time before we connect so we can peruse files and use a local SDK or + // PDK cache of support files to disassemble or do other things. m_major_os_version = major; m_minor_os_version = minor; m_update_os_version = update; @@ -897,9 +894,9 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec, module_search_paths_ptr, nullptr, nullptr); } else { - // No valid architecture was specified, ask the platform for - // the architectures that we should be using (in the correct order) - // and see if we can find a match that way + // No valid architecture was specified, ask the platform for the + // architectures that we should be using (in the correct order) and see + // if we can find a match that way ModuleSpec arch_module_spec(module_spec); for (uint32_t idx = 0; GetSupportedArchitectureAtIndex( idx, arch_module_spec.GetArchitecture()); @@ -943,18 +940,17 @@ const ArchSpec &Platform::GetSystemArchitecture() { m_system_arch_set_while_connected = m_system_arch.IsValid(); } } else { - // We have a remote platform. We can only fetch the remote - // system architecture if we are connected, and we don't want to do it - // more than once. + // We have a remote platform. We can only fetch the remote system + // architecture if we are connected, and we don't want to do it more than + // once. const bool is_connected = IsConnected(); bool fetch = false; if (m_system_arch.IsValid()) { - // We have valid OS version info, check to make sure it wasn't - // manually set prior to connecting. If it was manually set prior - // to connecting, then lets fetch the actual OS version info - // if we are now connected. + // We have valid OS version info, check to make sure it wasn't manually + // set prior to connecting. If it was manually set prior to connecting, + // then lets fetch the actual OS version info if we are now connected. if (is_connected && !m_system_arch_set_while_connected) fetch = true; } else { @@ -1026,8 +1022,8 @@ Status Platform::DisconnectRemote() { bool Platform::GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &process_info) { - // Take care of the host case so that each subclass can just - // call this function to get the host functionality. + // Take care of the host case so that each subclass can just call this + // function to get the host functionality. if (IsHost()) return Host::GetProcessInfo(pid, process_info); return false; @@ -1035,8 +1031,8 @@ bool Platform::GetProcessInfo(lldb::pid_t pid, uint32_t Platform::FindProcesses(const ProcessInstanceInfoMatch &match_info, ProcessInstanceInfoList &process_infos) { - // Take care of the host case so that each subclass can just - // call this function to get the host functionality. + // Take care of the host case so that each subclass can just call this + // function to get the host functionality. uint32_t match_count = 0; if (IsHost()) match_count = Host::FindProcesses(match_info, process_infos); @@ -1049,8 +1045,8 @@ Status Platform::LaunchProcess(ProcessLaunchInfo &launch_info) { if (log) log->Printf("Platform::%s()", __FUNCTION__); - // Take care of the host case so that each subclass can just - // call this function to get the host functionality. + // Take care of the host case so that each subclass can just call this + // function to get the host functionality. if (IsHost()) { if (::getenv("LLDB_LAUNCH_FLAG_LAUNCH_IN_TTY")) launch_info.GetFlags().Set(eLaunchFlagLaunchInTTY); @@ -1107,8 +1103,7 @@ Status Platform::KillProcess(const lldb::pid_t pid) { log->Printf("Platform::%s, pid %" PRIu64, __FUNCTION__, pid); // Try to find a process plugin to handle this Kill request. If we can't, - // fall back to - // the default OS implementation. + // fall back to the default OS implementation. size_t num_debuggers = Debugger::GetNumDebuggers(); for (size_t didx = 0; didx < num_debuggers; ++didx) { DebuggerSP debugger = Debugger::GetDebuggerAtIndex(didx); @@ -1143,23 +1138,22 @@ Platform::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, // Make sure we stop at the entry point launch_info.GetFlags().Set(eLaunchFlagDebug); // We always launch the process we are going to debug in a separate process - // group, since then we can handle ^C interrupts ourselves w/o having to worry - // about the target getting them as well. + // group, since then we can handle ^C interrupts ourselves w/o having to + // worry about the target getting them as well. launch_info.SetLaunchInSeparateProcessGroup(true); // Allow any StructuredData process-bound plugins to adjust the launch info // if needed size_t i = 0; bool iteration_complete = false; - // Note iteration can't simply go until a nullptr callback is returned, as - // it is valid for a plugin to not supply a filter. + // Note iteration can't simply go until a nullptr callback is returned, as it + // is valid for a plugin to not supply a filter. auto get_filter_func = PluginManager::GetStructuredDataFilterCallbackAtIndex; for (auto filter_callback = get_filter_func(i, iteration_complete); !iteration_complete; filter_callback = get_filter_func(++i, iteration_complete)) { if (filter_callback) { - // Give this ProcessLaunchInfo filter a chance to adjust the launch - // info. + // Give this ProcessLaunchInfo filter a chance to adjust the launch info. error = (*filter_callback)(launch_info, target); if (!error.Success()) { if (log) @@ -1192,10 +1186,10 @@ Platform::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, // process if this happens. process_sp->SetShouldDetach(false); - // If we didn't have any file actions, the pseudo terminal might - // have been used where the slave side was given as the file to - // open for stdin/out/err after we have already opened the master - // so we can read/write stdin/out/err. + // If we didn't have any file actions, the pseudo terminal might have + // been used where the slave side was given as the file to open for + // stdin/out/err after we have already opened the master so we can + // read/write stdin/out/err. int pty_fd = launch_info.GetPTY().ReleaseMasterFileDescriptor(); if (pty_fd != PseudoTerminal::invalid_fd) { process_sp->SetSTDIOFileDescriptor(pty_fd); @@ -1314,8 +1308,8 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, offset += bytes_written; if (bytes_written != bytes_read) { - // We didn't write the correct number of bytes, so adjust - // the file position in the source file we are reading from... + // We didn't write the correct number of bytes, so adjust the file + // position in the source file we are reading from... source_file.SeekFromStart(offset); } } @@ -1600,9 +1594,9 @@ Status Platform::GetRemoteSharedModule(const ModuleSpec &module_spec, if (module_spec.GetArchitecture().IsValid() == false) { Status error; - // No valid architecture was specified, ask the platform for - // the architectures that we should be using (in the correct order) - // and see if we can find a match that way + // No valid architecture was specified, ask the platform for the + // architectures that we should be using (in the correct order) and see if + // we can find a match that way ModuleSpec arch_module_spec(module_spec); for (uint32_t idx = 0; GetSupportedArchitectureAtIndex( idx, arch_module_spec.GetArchitecture()); @@ -1853,8 +1847,8 @@ size_t Platform::GetSoftwareBreakpointTrapOpcode(Target &target, // TODO: support big-endian arm and thumb trap codes. case llvm::Triple::arm: { - // The ARM reference recommends the use of 0xe7fddefe and 0xdefe - // but the linux kernel does otherwise. + // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the + // linux kernel does otherwise. static const uint8_t g_arm_breakpoint_opcode[] = {0xf0, 0x01, 0xf0, 0xe7}; static const uint8_t g_thumb_breakpoint_opcode[] = {0x01, 0xde}; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 2b16afd5811..1f223dfc444 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -90,19 +90,18 @@ public: ProcessOptionValueProperties(const ConstString &name) : OptionValueProperties(name) {} - // This constructor is used when creating ProcessOptionValueProperties when it - // is part of a new lldb_private::Process instance. It will copy all current - // global property values as needed + // This constructor is used when creating ProcessOptionValueProperties when + // it is part of a new lldb_private::Process instance. It will copy all + // current global property values as needed ProcessOptionValueProperties(ProcessProperties *global_properties) : OptionValueProperties(*global_properties->GetValueProperties()) {} const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx, bool will_modify, uint32_t idx) const override { - // When getting the value for a key from the process options, we will always - // try and grab the setting from the current process if there is one. Else - // we just - // use the one from this instance. + // When getting the value for a key from the process options, we will + // always try and grab the setting from the current process if there is + // one. Else we just use the one from this instance. if (exe_ctx) { Process *process = exe_ctx->GetProcessPtr(); if (process) { @@ -809,8 +808,8 @@ Process::~Process() { StopPrivateStateThread(); // ThreadList::Clear() will try to acquire this process's mutex, so - // explicitly clear the thread list here to ensure that the mutex - // is not destroyed before the thread list. + // explicitly clear the thread list here to ensure that the mutex is not + // destroyed before the thread list. m_thread_list.Clear(); } @@ -848,13 +847,12 @@ void Process::Finalize() { // Clear our broadcaster before we proceed with destroying Broadcaster::Clear(); - // Do any cleanup needed prior to being destructed... Subclasses - // that override this method should call this superclass method as well. + // Do any cleanup needed prior to being destructed... Subclasses that + // override this method should call this superclass method as well. // We need to destroy the loader before the derived Process class gets - // destroyed - // since it is very likely that undoing the loader will require access to the - // real process. + // destroyed since it is very likely that undoing the loader will require + // access to the real process. m_dynamic_checkers_ap.reset(); m_abi_sp.reset(); m_os_ap.reset(); @@ -874,8 +872,8 @@ void Process::Finalize() { m_language_runtimes.clear(); m_instrumentation_runtimes.clear(); m_next_event_action_ap.reset(); - // Clear the last natural stop ID since it has a strong - // reference to this process + // Clear the last natural stop ID since it has a strong reference to this + // process m_mod_id.SetStopEventForLastNaturalStopID(EventSP()); //#ifdef LLDB_CONFIGURATION_DEBUG // StreamFile s(stdout, false); @@ -931,14 +929,11 @@ void Process::SynchronouslyNotifyStateChanged(StateType state) { // FIXME: We need to do some work on events before the general Listener sees // them. // For instance if we are continuing from a breakpoint, we need to ensure that -// we do -// the little "insert real insn, step & stop" trick. But we can't do that when -// the -// event is delivered by the broadcaster - since that is done on the thread that -// is -// waiting for new events, so if we needed more than one event for our handling, -// we would -// stall. So instead we do it when we fetch the event off of the queue. +// we do the little "insert real insn, step & stop" trick. But we can't do +// that when the event is delivered by the broadcaster - since that is done on +// the thread that is waiting for new events, so if we needed more than one +// event for our handling, we would stall. So instead we do it when we fetch +// the event off of the queue. // StateType Process::GetNextEvent(EventSP &event_sp) { @@ -974,15 +969,13 @@ StateType Process::WaitForProcessToStop(const Timeout<std::micro> &timeout, ListenerSP hijack_listener_sp, Stream *stream, bool use_run_lock) { // We can't just wait for a "stopped" event, because the stopped event may - // have restarted the target. - // We have to actually check each event, and in the case of a stopped event - // check the restarted flag - // on the event. + // have restarted the target. We have to actually check each event, and in + // the case of a stopped event check the restarted flag on the event. if (event_sp_ptr) event_sp_ptr->reset(); StateType state = GetState(); - // If we are exited or detached, we won't ever get back to any - // other valid state... + // If we are exited or detached, we won't ever get back to any other valid + // state... if (state == eStateDetached || state == eStateExited) return state; @@ -1148,10 +1141,8 @@ bool Process::HandleProcessStateChangedEvent(const EventSP &event_sp, case eStopReasonSignal: { // Don't select a signal thread if we weren't going to stop at - // that - // signal. We have to have had another reason for stopping here, - // and - // the user doesn't want to see this thread. + // that signal. We have to have had another reason for stopping + // here, and the user doesn't want to see this thread. uint64_t signo = thread->GetStopInfo()->GetValue(); if (process_sp->GetUnixSignals()->GetShouldStop(signo)) { if (!other_thread) @@ -1191,10 +1182,9 @@ bool Process::HandleProcessStateChangedEvent(const EventSP &event_sp, } } // Drop the ThreadList mutex by here, since GetThreadStatus below might - // have to run code, - // e.g. for Data formatters, and if we hold the ThreadList mutex, then the - // process is going to - // have a hard time restarting the process. + // have to run code, e.g. for Data formatters, and if we hold the + // ThreadList mutex, then the process is going to have a hard time + // restarting the process. if (stream) { Debugger &debugger = process_sp->GetTarget().GetDebugger(); if (debugger.GetTargetList().GetSelectedTarget().get() == @@ -1382,8 +1372,8 @@ bool Process::SetExitStatus(int status, const char *cstr) { else m_exit_string.clear(); - // Clear the last natural stop ID since it has a strong - // reference to this process + // Clear the last natural stop ID since it has a strong reference to this + // process m_mod_id.SetStopEventForLastNaturalStopID(EventSP()); SetPrivateState(eStateExited); @@ -1410,9 +1400,9 @@ bool Process::IsAlive() { } } -// This static callback can be used to watch for local child processes on -// the current host. The child process exits, the process will be -// found in the global target list (we want to be completely sure that the +// This static callback can be used to watch for local child processes on the +// current host. The child process exits, the process will be found in the +// global target list (we want to be completely sure that the // lldb_private::Process doesn't go away before we can deliver the signal. bool Process::SetProcessExitStatus( lldb::pid_t pid, bool exited, @@ -1449,20 +1439,19 @@ void Process::UpdateThreadListIfNeeded() { const StateType state = GetPrivateState(); if (StateIsStoppedState(state, true)) { std::lock_guard<std::recursive_mutex> guard(m_thread_list.GetMutex()); - // m_thread_list does have its own mutex, but we need to - // hold onto the mutex between the call to UpdateThreadList(...) - // and the os->UpdateThreadList(...) so it doesn't change on us + // m_thread_list does have its own mutex, but we need to hold onto the + // mutex between the call to UpdateThreadList(...) and the + // os->UpdateThreadList(...) so it doesn't change on us ThreadList &old_thread_list = m_thread_list; ThreadList real_thread_list(this); ThreadList new_thread_list(this); - // Always update the thread list with the protocol specific - // thread list, but only update if "true" is returned + // Always update the thread list with the protocol specific thread list, + // but only update if "true" is returned if (UpdateThreadList(m_thread_list_real, real_thread_list)) { // Don't call into the OperatingSystem to update the thread list if we - // are shutting down, since - // that may call back into the SBAPI's, requiring the API lock which is - // already held by whoever is - // shutting us down, causing a deadlock. + // are shutting down, since that may call back into the SBAPI's, + // requiring the API lock which is already held by whoever is shutting + // us down, causing a deadlock. OperatingSystem *os = GetOperatingSystem(); if (os && !m_destroy_in_process) { // Clear any old backing threads where memory threads might have been @@ -1472,12 +1461,9 @@ void Process::UpdateThreadListIfNeeded() { old_thread_list.GetThreadAtIndex(i, false)->ClearBackingThread(); // Turn off dynamic types to ensure we don't run any expressions. - // Objective C - // can run an expression to determine if a SBValue is a dynamic type - // or not - // and we need to avoid this. OperatingSystem plug-ins can't run - // expressions - // that require running code... + // Objective C can run an expression to determine if a SBValue is a + // dynamic type or not and we need to avoid this. OperatingSystem + // plug-ins can't run expressions that require running code... Target &target = GetTarget(); const lldb::DynamicValueType saved_prefer_dynamic = @@ -1586,9 +1572,9 @@ void Process::SetPublicState(StateType new_state, bool restarted) { const StateType old_state = m_public_state.GetValue(); m_public_state.SetValue(new_state); - // On the transition from Run to Stopped, we unlock the writer end of the - // run lock. The lock gets locked in Resume, which is the public API - // to tell the program to run. + // On the transition from Run to Stopped, we unlock the writer end of the run + // lock. The lock gets locked in Resume, which is the public API to tell the + // program to run. if (!StateChangedIsExternallyHijacked()) { if (new_state == eStateDetached) { if (log) @@ -1701,17 +1687,16 @@ void Process::SetPrivateState(StateType new_state) { new Event(eBroadcastBitStateChanged, new ProcessEventData(shared_from_this(), new_state))); if (StateIsStoppedState(new_state, false)) { - // Note, this currently assumes that all threads in the list - // stop when the process stops. In the future we will want to - // support a debugging model where some threads continue to run - // while others are stopped. When that happens we will either need - // a way for the thread list to identify which threads are stopping - // or create a special thread list containing only threads which - // actually stopped. + // Note, this currently assumes that all threads in the list stop when + // the process stops. In the future we will want to support a debugging + // model where some threads continue to run while others are stopped. + // When that happens we will either need a way for the thread list to + // identify which threads are stopping or create a special thread list + // containing only threads which actually stopped. // - // The process plugin is responsible for managing the actual - // behavior of the threads and should have stopped any threads - // that are going to stop before we get here. + // The process plugin is responsible for managing the actual behavior of + // the threads and should have stopped any threads that are going to stop + // before we get here. m_thread_list.DidStop(); m_mod_id.BumpStopID(); @@ -1884,8 +1869,8 @@ Process::CreateBreakpointSite(const BreakpointLocationSP &owner, break; } - // Reset the IsIndirect flag here, in case the location changes from - // pointing to a indirect symbol to a regular symbol. + // Reset the IsIndirect flag here, in case the location changes from pointing + // to a indirect symbol to a regular symbol. owner->SetIsIndirect(false); if (owner->ShouldResolveIndirectFunctions()) { @@ -1915,8 +1900,7 @@ Process::CreateBreakpointSite(const BreakpointLocationSP &owner, BreakpointSiteSP bp_site_sp; // Look up this breakpoint site. If it exists, then add this new owner, - // otherwise - // create a new breakpoint site and add it. + // otherwise create a new breakpoint site and add it. bp_site_sp = m_breakpoint_site_list.FindByAddress(load_addr); @@ -2164,7 +2148,8 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite *bp_site) { return error; } -// Uncomment to verify memory caching works after making changes to caching code +// Uncomment to verify memory caching works after making changes to caching +// code //#define VERIFY_MEMORY_READS size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) { @@ -2218,8 +2203,8 @@ size_t Process::ReadCStringFromMemory(addr_t addr, std::string &out_str, if (length == 0) break; out_str.append(buf, length); - // If we got "length - 1" bytes, we didn't get the whole C string, we - // need to read some more characters + // If we got "length - 1" bytes, we didn't get the whole C string, we need + // to read some more characters if (length == sizeof(buf) - 1) curr_addr += length; else @@ -2232,7 +2217,8 @@ size_t Process::ReadStringFromMemory(addr_t addr, char *dst, size_t max_bytes, Status &error, size_t type_width) { size_t total_bytes_read = 0; if (dst && max_bytes && type_width && max_bytes >= type_width) { - // Ensure a null terminator independent of the number of bytes that is read. + // Ensure a null terminator independent of the number of bytes that is + // read. memset(dst, 0, max_bytes); size_t bytes_left = max_bytes - type_width; @@ -2279,8 +2265,7 @@ size_t Process::ReadStringFromMemory(addr_t addr, char *dst, size_t max_bytes, } // Deprecated in favor of ReadStringFromMemory which has wchar support and -// correct code to find -// null terminators. +// correct code to find null terminators. size_t Process::ReadCStringFromMemory(addr_t addr, char *dst, size_t dst_max_len, Status &result_error) { @@ -2454,16 +2439,16 @@ size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size, // Check for bytes before this breakpoint const addr_t curr_addr = addr + bytes_written; if (intersect_addr > curr_addr) { - // There are some bytes before this breakpoint that we need to - // just write to memory + // There are some bytes before this breakpoint that we need to just + // write to memory size_t curr_size = intersect_addr - curr_addr; size_t curr_bytes_written = WriteMemoryPrivate( curr_addr, ubuf + bytes_written, curr_size, error); bytes_written += curr_bytes_written; if (curr_bytes_written != curr_size) { - // We weren't able to write all of the requested bytes, we - // are done looping and will return the number of bytes that - // we have written so far. + // We weren't able to write all of the requested bytes, we are + // done looping and will return the number of bytes that we have + // written so far. if (error.Success()) error.SetErrorToGenericError(); } @@ -2697,8 +2682,8 @@ StateType Process::WaitForProcessStopPrivate(EventSP &event_sp, const Timeout<std::micro> &timeout) { StateType state; - // Now wait for the process to launch and return control to us, and then - // call DidLaunch: + // Now wait for the process to launch and return control to us, and then call + // DidLaunch: while (true) { event_sp.reset(); state = GetStateChangedEventsPrivate(event_sp, timeout); @@ -2779,8 +2764,8 @@ Status Process::Launch(ProcessLaunchInfo &launch_info) { StateType state = WaitForProcessStopPrivate(event_sp, seconds(10)); if (state == eStateInvalid || !event_sp) { - // We were able to launch the process, but we failed to - // catch the initial stop. + // We were able to launch the process, but we failed to catch the + // initial stop. error.SetErrorString("failed to catch stop after launch"); SetExitStatus(0, "failed to catch stop after launch"); Destroy(false); @@ -2800,18 +2785,15 @@ Status Process::Launch(ProcessLaunchInfo &launch_info) { if (!m_os_ap) LoadOperatingSystemPlugin(false); - // We successfully launched the process and stopped, - // now it the right time to set up signal filters before resuming. + // We successfully launched the process and stopped, now it the + // right time to set up signal filters before resuming. UpdateAutomaticSignalFiltering(); // Note, the stop event was consumed above, but not handled. This - // was done - // to give DidLaunch a chance to run. The target is either stopped - // or crashed. - // Directly set the state. This is done to prevent a stop message - // with a bunch - // of spurious output on thread status, as well as not pop a - // ProcessIOHandler. + // was done to give DidLaunch a chance to run. The target is either + // stopped or crashed. Directly set the state. This is done to + // prevent a stop message with a bunch of spurious output on thread + // status, as well as not pop a ProcessIOHandler. SetPublicState(state, false); if (PrivateStateThreadIsValid()) @@ -2820,15 +2802,13 @@ Status Process::Launch(ProcessLaunchInfo &launch_info) { StartPrivateStateThread(); // Target was stopped at entry as was intended. Need to notify the - // listeners - // about it. + // listeners about it. if (state == eStateStopped && launch_info.GetFlags().Test(eLaunchFlagStopAtEntry)) HandlePrivateEvent(event_sp); } else if (state == eStateExited) { // We exited while trying to launch somehow. Don't call DidLaunch - // as that's - // not likely to work, and return an invalid pid. + // as that's not likely to work, and return an invalid pid. HandlePrivateEvent(event_sp); } } @@ -2867,8 +2847,7 @@ Status Process::LoadCore() { LoadOperatingSystemPlugin(false); // We successfully loaded a core file, now pretend we stopped so we can - // show all of the threads in the core file and explore the crashed - // state. + // show all of the threads in the core file and explore the crashed state. SetPrivateState(eStateStopped); // Wait for a stopped event since we just posted one above... @@ -2944,8 +2923,8 @@ Process::AttachCompletionHandler::PerformAction(lldb::EventSP &event_sp) { // During attach, prior to sending the eStateStopped event, // lldb_private::Process subclasses must set the new process ID. assert(m_process->GetID() != LLDB_INVALID_PROCESS_ID); - // We don't want these events to be reported, so go set the ShouldReportStop - // here: + // We don't want these events to be reported, so go set the + // ShouldReportStop here: m_process->GetThreadList().SetShouldReportStop(eVoteNo); if (m_exec_count > 0) { @@ -3139,8 +3118,8 @@ void Process::CompleteAttach() { } // We just attached. If we have a platform, ask it for the process - // architecture, and if it isn't - // the same as the one we've already set, switch architectures. + // architecture, and if it isn't the same as the one we've already set, + // switch architectures. PlatformSP platform_sp(GetTarget().GetPlatform()); assert(platform_sp); if (platform_sp) { @@ -3277,21 +3256,20 @@ Status Process::PrivateResume() { m_mod_id.GetStopID(), StateAsCString(m_public_state.GetValue()), StateAsCString(m_private_state.GetValue())); - // If signals handing status changed we might want to update - // our signal filters before resuming. + // If signals handing status changed we might want to update our signal + // filters before resuming. UpdateAutomaticSignalFiltering(); Status error(WillResume()); // Tell the process it is about to resume before the thread list if (error.Success()) { - // Now let the thread list know we are about to resume so it - // can let all of our threads know that they are about to be - // resumed. Threads will each be called with - // Thread::WillResume(StateType) where StateType contains the state - // that they are supposed to have when the process is resumed - // (suspended/running/stepping). Threads should also check - // their resume signal in lldb::Thread::GetResumeSignal() - // to see if they are supposed to start back up with a signal. + // Now let the thread list know we are about to resume so it can let all of + // our threads know that they are about to be resumed. Threads will each be + // called with Thread::WillResume(StateType) where StateType contains the + // state that they are supposed to have when the process is resumed + // (suspended/running/stepping). Threads should also check their resume + // signal in lldb::Thread::GetResumeSignal() to see if they are supposed to + // start back up with a signal. if (m_thread_list.WillResume()) { // Last thing, do the PreResumeActions. if (!RunPreResumeActions()) { @@ -3308,11 +3286,10 @@ Status Process::PrivateResume() { } } } else { - // Somebody wanted to run without running (e.g. we were faking a step from - // one frame of a set of inlined - // frames that share the same PC to another.) So generate a continue & a - // stopped event, - // and let the world handle them. + // Somebody wanted to run without running (e.g. we were faking a step + // from one frame of a set of inlined frames that share the same PC to + // another.) So generate a continue & a stopped event, and let the world + // handle them. if (log) log->Printf( "Process::PrivateResume() asked to simulate a start & stop."); @@ -3330,9 +3307,8 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) { if (!StateIsRunningState(m_public_state.GetValue())) return Status("Process is not running."); - // Don't clear the m_clear_thread_plans_on_stop, only set it to true if - // in case it was already set and some thread plan logic calls halt on its - // own. + // Don't clear the m_clear_thread_plans_on_stop, only set it to true if in + // case it was already set and some thread plan logic calls halt on its own. m_clear_thread_plans_on_stop |= clear_thread_plans; ListenerSP halt_listener_sp( @@ -3344,8 +3320,8 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) { SendAsyncInterrupt(); if (m_public_state.GetValue() == eStateAttaching) { - // Don't hijack and eat the eStateExited as the code that was doing - // the attach will be waiting for this event... + // Don't hijack and eat the eStateExited as the code that was doing the + // attach will be waiting for this event... RestoreProcessEvents(); SetExitStatus(SIGKILL, "Cancelled async attach."); Destroy(false); @@ -3371,8 +3347,8 @@ Status Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) { Status error; // Check both the public & private states here. If we're hung evaluating an - // expression, for instance, then - // the public state will be stopped, but we still need to interrupt. + // expression, for instance, then the public state will be stopped, but we + // still need to interrupt. if (m_public_state.GetValue() == eStateRunning || m_private_state.GetValue() == eStateRunning) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS)); @@ -3392,10 +3368,9 @@ Status Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) { RestoreProcessEvents(); // If the process exited while we were waiting for it to stop, put the - // exited event into - // the shared pointer passed in and return. Our caller doesn't need to do - // anything else, since - // they don't have a process anymore... + // exited event into the shared pointer passed in and return. Our caller + // doesn't need to do anything else, since they don't have a process + // anymore... if (state == eStateExited || m_private_state.GetValue() == eStateExited) { if (log) @@ -3410,9 +3385,8 @@ Status Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) { log->Printf("Process::%s() failed to stop, state is: %s", __FUNCTION__, StateAsCString(state)); // If we really couldn't stop the process then we should just error out - // here, but if the - // lower levels just bobbled sending the event and we really are stopped, - // then continue on. + // here, but if the lower levels just bobbled sending the event and we + // really are stopped, then continue on. StateType private_state = m_private_state.GetValue(); if (private_state != eStateStopped) { return Status( @@ -3460,20 +3434,18 @@ Status Process::Detach(bool keep_stopped) { } m_destroy_in_process = false; - // If we exited when we were waiting for a process to stop, then - // forward the event here so we don't lose the event + // If we exited when we were waiting for a process to stop, then forward the + // event here so we don't lose the event if (exit_event_sp) { - // Directly broadcast our exited event because we shut down our - // private state thread above + // Directly broadcast our exited event because we shut down our private + // state thread above BroadcastEvent(exit_event_sp); } // If we have been interrupted (to kill us) in the middle of running, we may - // not end up propagating - // the last events through the event system, in which case we might strand the - // write lock. Unlock - // it here so when we do to tear down the process we don't get an error - // destroying the lock. + // not end up propagating the last events through the event system, in which + // case we might strand the write lock. Unlock it here so when we do to tear + // down the process we don't get an error destroying the lock. m_public_run_lock.SetStopped(); return error; @@ -3482,9 +3454,8 @@ Status Process::Detach(bool keep_stopped) { Status Process::Destroy(bool force_kill) { // Tell ourselves we are in the process of destroying the process, so that we - // don't do any unnecessary work - // that might hinder the destruction. Remember to set this back to false when - // we are done. That way if the attempt + // don't do any unnecessary work that might hinder the destruction. Remember + // to set this back to false when we are done. That way if the attempt // failed and the process stays around for some reason it won't be in a // confused state. @@ -3507,12 +3478,11 @@ Status Process::Destroy(bool force_kill) { } if (m_public_state.GetValue() != eStateRunning) { - // Ditch all thread plans, and remove all our breakpoints: in case we have - // to restart the target to - // kill it, we don't want it hitting a breakpoint... - // Only do this if we've stopped, however, since if we didn't manage to - // halt it above, then - // we're not going to have much luck doing this now. + // Ditch all thread plans, and remove all our breakpoints: in case we + // have to restart the target to kill it, we don't want it hitting a + // breakpoint... Only do this if we've stopped, however, since if we + // didn't manage to halt it above, then we're not going to have much luck + // doing this now. m_thread_list.DiscardThreadPlans(); DisableAllBreakpointSites(); } @@ -3532,20 +3502,18 @@ Status Process::Destroy(bool force_kill) { m_process_input_reader.reset(); } - // If we exited when we were waiting for a process to stop, then - // forward the event here so we don't lose the event + // If we exited when we were waiting for a process to stop, then forward + // the event here so we don't lose the event if (exit_event_sp) { - // Directly broadcast our exited event because we shut down our - // private state thread above + // Directly broadcast our exited event because we shut down our private + // state thread above BroadcastEvent(exit_event_sp); } - // If we have been interrupted (to kill us) in the middle of running, we may - // not end up propagating - // the last events through the event system, in which case we might strand - // the write lock. Unlock - // it here so when we do to tear down the process we don't get an error - // destroying the lock. + // If we have been interrupted (to kill us) in the middle of running, we + // may not end up propagating the last events through the event system, in + // which case we might strand the write lock. Unlock it here so when we do + // to tear down the process we don't get an error destroying the lock. m_public_run_lock.SetStopped(); } @@ -3612,11 +3580,11 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) { break; case eStateRunning: case eStateStepping: - // If we've started the target running, we handle the cases where we - // are already running and where there is a transition from stopped to - // running differently. - // running -> running: Automatically suppress extra running events - // stopped -> running: Report except when there is one or more no votes + // If we've started the target running, we handle the cases where we are + // already running and where there is a transition from stopped to running + // differently. running -> running: Automatically suppress extra running + // events stopped -> running: Report except when there is one or more no + // votes // and no yes votes. SynchronouslyNotifyStateChanged(state); if (m_force_next_event_delivery) @@ -3630,11 +3598,10 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) { break; default: // TODO: make this work correctly. For now always report - // run if we aren't running so we don't miss any running - // events. If I run the lldb/test/thread/a.out file and - // break at main.cpp:58, run and hit the breakpoints on - // multiple threads, then somehow during the stepping over - // of all breakpoints no run gets reported. + // run if we aren't running so we don't miss any running events. If I + // run the lldb/test/thread/a.out file and break at main.cpp:58, run + // and hit the breakpoints on multiple threads, then somehow during the + // stepping over of all breakpoints no run gets reported. // This is a transition from stop to run. switch (m_thread_list.ShouldReportRun(event_ptr)) { @@ -3653,11 +3620,10 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) { case eStateStopped: case eStateCrashed: case eStateSuspended: - // We've stopped. First see if we're going to restart the target. - // If we are going to stop, then we always broadcast the event. - // If we aren't going to stop, let the thread plans decide if we're going to - // report this event. - // If no thread has an opinion, we don't report it. + // We've stopped. First see if we're going to restart the target. If we + // are going to stop, then we always broadcast the event. If we aren't + // going to stop, let the thread plans decide if we're going to report this + // event. If no thread has an opinion, we don't report it. m_stdio_communication.SynchronizeWithReadThread(); RefreshStateAfterStop(); @@ -3667,8 +3633,7 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) { "interrupt, state: %s", static_cast<void *>(event_ptr), StateAsCString(state)); // Even though we know we are going to stop, we should let the threads - // have a look at the stop, - // so they can properly set their state. + // have a look at the stop, so they can properly set their state. m_thread_list.ShouldStop(event_ptr); return_value = true; } else { @@ -3676,10 +3641,8 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) { bool should_resume = false; // It makes no sense to ask "ShouldStop" if we've already been - // restarted... - // Asking the thread list is also not likely to go well, since we are - // running again. - // So in that case just report the event. + // restarted... Asking the thread list is also not likely to go well, + // since we are running again. So in that case just report the event. if (!was_restarted) should_resume = !m_thread_list.ShouldStop(event_ptr); @@ -3722,16 +3685,13 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) { m_force_next_event_delivery = false; // We do some coalescing of events (for instance two consecutive running - // events get coalesced.) - // But we only coalesce against events we actually broadcast. So we use - // m_last_broadcast_state - // to track that. NB - you can't use "m_public_state.GetValue()" for that - // purpose, as was originally done, - // because the PublicState reflects the last event pulled off the queue, and - // there may be several - // events stacked up on the queue unserviced. So the PublicState may not - // reflect the last broadcasted event - // yet. m_last_broadcast_state gets updated here. + // events get coalesced.) But we only coalesce against events we actually + // broadcast. So we use m_last_broadcast_state to track that. NB - you + // can't use "m_public_state.GetValue()" for that purpose, as was originally + // done, because the PublicState reflects the last event pulled off the + // queue, and there may be several events stacked up on the queue unserviced. + // So the PublicState may not reflect the last broadcasted event yet. + // m_last_broadcast_state gets updated here. if (return_value) m_last_broadcast_state = state; @@ -3757,8 +3717,8 @@ bool Process::StartPrivateStateThread(bool is_secondary_thread) { if (!is_secondary_thread && already_running) return true; - // Create a thread that watches our internal state and controls which - // events make it to clients (into the DCProcess event queue). + // Create a thread that watches our internal state and controls which events + // make it to clients (into the DCProcess event queue). char thread_name[1024]; uint32_t max_len = llvm::get_max_thread_name_length(); if (max_len > 0 && max_len <= 30) { @@ -3823,10 +3783,10 @@ void Process::ControlPrivateStateThread(uint32_t signal) { // Signal the private state thread if (m_private_state_thread.IsJoinable()) { // Broadcast the event. - // It is important to do this outside of the if below, because - // it's possible that the thread state is invalid but that the - // thread is waiting on a control event instead of simply being - // on its way out (this should not happen, but it apparently can). + // It is important to do this outside of the if below, because it's + // possible that the thread state is invalid but that the thread is waiting + // on a control event instead of simply being on its way out (this should + // not happen, but it apparently can). if (log) log->Printf("Sending control event of type: %d.", signal); std::shared_ptr<EventDataReceipt> event_receipt_sp(new EventDataReceipt()); @@ -3838,13 +3798,13 @@ void Process::ControlPrivateStateThread(uint32_t signal) { if (PrivateStateThreadIsValid()) { while (!receipt_received) { bool timed_out = false; - // Check for a receipt for 2 seconds and then check if the private state - // thread is still around. + // Check for a receipt for 2 seconds and then check if the private + // state thread is still around. receipt_received = event_receipt_sp->WaitForEventReceived( std::chrono::seconds(2), &timed_out); if (!receipt_received) { - // Check if the private state thread is still around. If it isn't then - // we are done waiting + // Check if the private state thread is still around. If it isn't + // then we are done waiting if (!PrivateStateThreadIsValid()) break; // Private state thread exited or is exiting, we are done } @@ -3894,9 +3854,9 @@ void Process::HandlePrivateEvent(EventSP &event_sp) { break; case NextEventAction::eEventActionExit: - // Handle Exiting Here. If we already got an exited event, - // we should just propagate it. Otherwise, swallow this event, - // and set our state to exit so the next event will kill us. + // Handle Exiting Here. If we already got an exited event, we should + // just propagate it. Otherwise, swallow this event, and set our state + // to exit so the next event will kill us. if (new_state != eStateExited) { // FIXME: should cons up an exited event, and discard this one. SetExitStatus(0, m_next_event_action_ap->GetExitString()); @@ -3922,9 +3882,9 @@ void Process::HandlePrivateEvent(EventSP &event_sp) { } Process::ProcessEventData::SetUpdateStateOnRemoval(event_sp.get()); if (StateIsRunningState(new_state)) { - // Only push the input handler if we aren't fowarding events, - // as this means the curses GUI is in use... - // Or don't push it if we are launching since it will come up stopped. + // Only push the input handler if we aren't fowarding events, as this + // means the curses GUI is in use... Or don't push it if we are launching + // since it will come up stopped. if (!GetTarget().GetDebugger().IsForwardingEvents() && new_state != eStateLaunching && new_state != eStateAttaching) { PushProcessIOHandler(); @@ -3936,27 +3896,26 @@ void Process::HandlePrivateEvent(EventSP &event_sp) { } } else if (StateIsStoppedState(new_state, false)) { if (!Process::ProcessEventData::GetRestartedFromEvent(event_sp.get())) { - // If the lldb_private::Debugger is handling the events, we don't - // want to pop the process IOHandler here, we want to do it when - // we receive the stopped event so we can carefully control when - // the process IOHandler is popped because when we stop we want to - // display some text stating how and why we stopped, then maybe some - // process/thread/frame info, and then we want the "(lldb) " prompt - // to show up. If we pop the process IOHandler here, then we will - // cause the command interpreter to become the top IOHandler after - // the process pops off and it will update its prompt right away... - // See the Debugger.cpp file where it calls the function as + // If the lldb_private::Debugger is handling the events, we don't want + // to pop the process IOHandler here, we want to do it when we receive + // the stopped event so we can carefully control when the process + // IOHandler is popped because when we stop we want to display some + // text stating how and why we stopped, then maybe some + // process/thread/frame info, and then we want the "(lldb) " prompt to + // show up. If we pop the process IOHandler here, then we will cause + // the command interpreter to become the top IOHandler after the + // process pops off and it will update its prompt right away... See the + // Debugger.cpp file where it calls the function as // "process_sp->PopProcessIOHandler()" to see where I am talking about. // Otherwise we end up getting overlapping "(lldb) " prompts and // garbled output. // // If we aren't handling the events in the debugger (which is indicated - // by "m_target.GetDebugger().IsHandlingEvents()" returning false) or we - // are hijacked, then we always pop the process IO handler manually. + // by "m_target.GetDebugger().IsHandlingEvents()" returning false) or + // we are hijacked, then we always pop the process IO handler manually. // Hijacking happens when the internal process state thread is running - // thread plans, or when commands want to run in synchronous mode - // and they call "process->WaitForProcessToStop()". An example of - // something + // thread plans, or when commands want to run in synchronous mode and + // they call "process->WaitForProcessToStop()". An example of something // that will hijack the events is a simple expression: // // (lldb) expr (int)puts("hello") @@ -4059,23 +4018,19 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { ") failed to halt the process: %s", __FUNCTION__, static_cast<void *>(this), GetID(), error.AsCString()); - // Halt should generate a stopped event. Make a note of the fact that we - // were - // doing the interrupt, so we can set the interrupted flag after we - // receive the - // event. We deliberately set this to true even if HaltPrivate failed, - // so that we - // can interrupt on the next natural stop. + // Halt should generate a stopped event. Make a note of the fact that + // we were doing the interrupt, so we can set the interrupted flag + // after we receive the event. We deliberately set this to true even if + // HaltPrivate failed, so that we can interrupt on the next natural + // stop. interrupt_requested = true; } else { // This can happen when someone (e.g. Process::Halt) sees that we are - // running and - // sends an interrupt request, but the process actually stops before we - // receive - // it. In that case, we can just ignore the request. We use - // m_last_broadcast_state, because the Stopped event may not have been - // popped of - // the event queue yet, which is when the public state gets updated. + // running and sends an interrupt request, but the process actually + // stops before we receive it. In that case, we can just ignore the + // request. We use m_last_broadcast_state, because the Stopped event + // may not have been popped of the event queue yet, which is when the + // public state gets updated. if (log) log->Printf( "Process::%s ignoring interrupt as we have already stopped.", @@ -4097,8 +4052,7 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { if (interrupt_requested) { if (StateIsStoppedState(internal_state, true)) { // We requested the interrupt, so mark this as such in the stop event - // so - // clients can tell an interrupted process from a natural stop + // so clients can tell an interrupted process from a natural stop ProcessEventData::SetInterruptedInEvent(event_sp.get(), true); interrupt_requested = false; } else if (log) { @@ -4129,10 +4083,8 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { __FUNCTION__, static_cast<void *>(this), GetID()); // If we are a secondary thread, then the primary thread we are working for - // will have already - // acquired the public_run_lock, and isn't done with what it was doing yet, so - // don't - // try to change it on the way out. + // will have already acquired the public_run_lock, and isn't done with what + // it was doing yet, so don't try to change it on the way out. if (!is_secondary_thread) m_public_run_lock.SetStopped(); return NULL; @@ -4172,14 +4124,12 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { return; // This function gets called twice for each event, once when the event gets - // pulled - // off of the private process event queue, and then any number of times, first - // when it gets pulled off of - // the public event queue, then other times when we're pretending that this is - // where we stopped at the - // end of expression evaluation. m_update_state is used to distinguish these - // three cases; it is 0 when we're just pulling it off for private handling, - // and > 1 for expression evaluation, and we don't want to do the breakpoint + // pulled off of the private process event queue, and then any number of + // times, first when it gets pulled off of the public event queue, then other + // times when we're pretending that this is where we stopped at the end of + // expression evaluation. m_update_state is used to distinguish these three + // cases; it is 0 when we're just pulling it off for private handling, and > + // 1 for expression evaluation, and we don't want to do the breakpoint // command handling then. if (m_update_state != 1) return; @@ -4188,16 +4138,15 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { m_state, Process::ProcessEventData::GetRestartedFromEvent(event_ptr)); if (m_state == eStateStopped && !m_restarted) { - // Let process subclasses know we are about to do a public stop and - // do anything they might need to in order to speed up register and - // memory accesses. + // Let process subclasses know we are about to do a public stop and do + // anything they might need to in order to speed up register and memory + // accesses. process_sp->WillPublicStop(); } // If this is a halt event, even if the halt stopped with some reason other - // than a plain interrupt (e.g. we had - // already stopped for a breakpoint when the halt request came through) don't - // do the StopInfo actions, as they may + // than a plain interrupt (e.g. we had already stopped for a breakpoint when + // the halt request came through) don't do the StopInfo actions, as they may // end up restarting the process. if (m_interrupted) return; @@ -4209,37 +4158,32 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { uint32_t idx; // The actions might change one of the thread's stop_info's opinions about - // whether we should - // stop the process, so we need to query that as we go. + // whether we should stop the process, so we need to query that as we go. // One other complication here, is that we try to catch any case where the - // target has run (except for expressions) - // and immediately exit, but if we get that wrong (which is possible) then - // the thread list might have changed, and - // that would cause our iteration here to crash. We could make a copy of - // the thread list, but we'd really like - // to also know if it has changed at all, so we make up a vector of the - // thread ID's and check what we get back - // against this list & bag out if anything differs. + // target has run (except for expressions) and immediately exit, but if we + // get that wrong (which is possible) then the thread list might have + // changed, and that would cause our iteration here to crash. We could + // make a copy of the thread list, but we'd really like to also know if it + // has changed at all, so we make up a vector of the thread ID's and check + // what we get back against this list & bag out if anything differs. std::vector<uint32_t> thread_index_array(num_threads); for (idx = 0; idx < num_threads; ++idx) thread_index_array[idx] = curr_thread_list.GetThreadAtIndex(idx)->GetIndexID(); // Use this to track whether we should continue from here. We will only - // continue the target running if - // no thread says we should stop. Of course if some thread's PerformAction - // actually sets the target running, - // then it doesn't matter what the other threads say... + // continue the target running if no thread says we should stop. Of course + // if some thread's PerformAction actually sets the target running, then it + // doesn't matter what the other threads say... bool still_should_stop = false; // Sometimes - for instance if we have a bug in the stub we are talking to, - // we stop but no thread has a - // valid stop reason. In that case we should just stop, because we have no - // way of telling what the right - // thing to do is, and it's better to let the user decide than continue - // behind their backs. + // we stop but no thread has a valid stop reason. In that case we should + // just stop, because we have no way of telling what the right thing to do + // is, and it's better to let the user decide than continue behind their + // backs. bool does_anybody_have_an_opinion = false; @@ -4276,13 +4220,11 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { stop_info_sp->GetOverriddenShouldStopValue(); } else { stop_info_sp->PerformAction(event_ptr); - // The stop action might restart the target. If it does, then we want - // to mark that in the - // event so that whoever is receiving it will know to wait for the - // running event and reflect - // that state appropriately. - // We also need to stop processing actions, since they aren't - // expecting the target to be running. + // The stop action might restart the target. If it does, then we + // want to mark that in the event so that whoever is receiving it + // will know to wait for the running event and reflect that state + // appropriately. We also need to stop processing actions, since they + // aren't expecting the target to be running. // FIXME: we might have run. if (stop_info_sp->HasTargetRunSinceMe()) { @@ -4302,12 +4244,12 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { if (!still_should_stop && does_anybody_have_an_opinion) { // We've been asked to continue, so do that here. SetRestarted(true); - // Use the public resume method here, since this is just - // extending a public resume. + // Use the public resume method here, since this is just extending a + // public resume. process_sp->PrivateResume(); } else { - // If we didn't restart, run the Stop Hooks here: - // They might also restart the target, so watch for that. + // If we didn't restart, run the Stop Hooks here: They might also + // restart the target, so watch for that. process_sp->GetTarget().RunStopHooks(); if (process_sp->GetPrivateState() == eStateRunning) SetRestarted(true); @@ -4579,9 +4521,8 @@ public: ~IOHandlerProcessSTDIO() override = default; - // Each IOHandler gets to run until it is done. It should read data - // from the "in" and place output into "out" and "err and return - // when done. + // Each IOHandler gets to run until it is done. It should read data from the + // "in" and place output into "out" and "err and return when done. void Run() override { if (!m_read_file.IsValid() || !m_write_file.IsValid() || !m_pipe.CanRead() || !m_pipe.CanWrite()) { @@ -4645,9 +4586,8 @@ public: void Cancel() override { SetIsDone(true); // Only write to our pipe to cancel if we are in - // IOHandlerProcessSTDIO::Run(). - // We can end up with a python command that is being run from the command - // interpreter: + // IOHandlerProcessSTDIO::Run(). We can end up with a python command that + // is being run from the command interpreter: // // (lldb) step_process_thousands_of_times // @@ -4664,12 +4604,11 @@ public: } bool Interrupt() override { - // Do only things that are safe to do in an interrupt context (like in - // a SIGINT handler), like write 1 byte to a file descriptor. This will + // Do only things that are safe to do in an interrupt context (like in a + // SIGINT handler), like write 1 byte to a file descriptor. This will // interrupt the IOHandlerProcessSTDIO::Run() and we can look at the byte // that was written to the pipe and then call - // m_process->SendAsyncInterrupt() - // from a much safer location in code. + // m_process->SendAsyncInterrupt() from a much safer location in code. if (m_active) { char ch = 'i'; // Send 'i' for interrupt size_t bytes_written = 0; @@ -4677,16 +4616,13 @@ public: return result.Success(); } else { // This IOHandler might be pushed on the stack, but not being run - // currently - // so do the right thing if we aren't actively watching for STDIN by - // sending - // the interrupt to the process. Otherwise the write to the pipe above - // would - // do nothing. This can happen when the command interpreter is running and - // gets a "expression ...". It will be on the IOHandler thread and sending - // the input is complete to the delegate which will cause the expression - // to - // run, which will push the process IO handler, but not run it. + // currently so do the right thing if we aren't actively watching for + // STDIN by sending the interrupt to the process. Otherwise the write to + // the pipe above would do nothing. This can happen when the command + // interpreter is running and gets a "expression ...". It will be on the + // IOHandler thread and sending the input is complete to the delegate + // which will cause the expression to run, which will push the process IO + // handler, but not run it. if (StateIsRunningState(m_process->GetState())) { m_process->SendAsyncInterrupt(); @@ -4761,12 +4697,10 @@ void Process::SettingsInitialize() { Thread::SettingsInitialize(); } void Process::SettingsTerminate() { Thread::SettingsTerminate(); } namespace { -// RestorePlanState is used to record the "is private", "is master" and "okay to -// discard" fields of -// the plan we are running, and reset it on Clean or on destruction. -// It will only reset the state once, so you can call Clean and then monkey with -// the state and it -// won't get reset on you again. +// RestorePlanState is used to record the "is private", "is master" and "okay +// to discard" fields of the plan we are running, and reset it on Clean or on +// destruction. It will only reset the state once, so you can call Clean and +// then monkey with the state and it won't get reset on you again. class RestorePlanState { public: @@ -4822,8 +4756,8 @@ GetOneThreadExpressionTimeout(const EvaluateExpressionOptions &options) { static Timeout<std::micro> GetExpressionTimeout(const EvaluateExpressionOptions &options, bool before_first_timeout) { - // If we are going to run all threads the whole time, or if we are only - // going to run one thread, we can just return the overall timeout. + // If we are going to run all threads the whole time, or if we are only going + // to run one thread, we can just return the overall timeout. if (!options.GetStopOthers() || !options.GetTryAllThreads()) return options.GetTimeout(); @@ -4860,7 +4794,8 @@ HandleStoppedEvent(Thread &thread, const ThreadPlanSP &thread_plan_sp, if (!options.DoesIgnoreBreakpoints()) { // Restore the plan state and then force Private to false. We are going // to stop because of this plan so we need it to become a public plan or - // it won't report correctly when we continue to its termination later on. + // it won't report correctly when we continue to its termination later + // on. restorer.Clean(); thread_plan_sp->SetPrivate(false); event_to_broadcast_sp = event_sp; @@ -4915,23 +4850,20 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } // We need to change some of the thread plan attributes for the thread plan - // runner. This will restore them - // when we are done: + // runner. This will restore them when we are done: RestorePlanState thread_plan_restorer(thread_plan_sp); - // We rely on the thread plan we are running returning "PlanCompleted" if when - // it successfully completes. - // For that to be true the plan can't be private - since private plans - // suppress themselves in the - // GetCompletedPlan call. + // We rely on the thread plan we are running returning "PlanCompleted" if + // when it successfully completes. For that to be true the plan can't be + // private - since private plans suppress themselves in the GetCompletedPlan + // call. thread_plan_sp->SetPrivate(false); // The plans run with RunThreadPlan also need to be terminal master plans or - // when they are done we will end - // up asking the plan above us whether we should stop, which may give the - // wrong answer. + // when they are done we will end up asking the plan above us whether we + // should stop, which may give the wrong answer. thread_plan_sp->SetIsMasterPlan(true); thread_plan_sp->SetOkayToDiscard(false); @@ -4958,8 +4890,8 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } } - // Make sure the timeout values make sense. The one thread timeout needs to be - // smaller than the overall timeout. + // Make sure the timeout values make sense. The one thread timeout needs to + // be smaller than the overall timeout. if (options.GetOneThreadTimeout() && options.GetTimeout() && *options.GetTimeout() < *options.GetOneThreadTimeout()) { diagnostic_manager.PutString(eDiagnosticSeverityError, @@ -4971,8 +4903,8 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, StackID ctx_frame_id = selected_frame_sp->GetStackID(); // N.B. Running the target may unset the currently selected thread and frame. - // We don't want to do that either, - // so we should arrange to reset them as well. + // We don't want to do that either, so we should arrange to reset them as + // well. lldb::ThreadSP selected_thread_sp = GetThreadList().GetSelectedThread(); @@ -4993,11 +4925,10 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, LIBLLDB_LOG_PROCESS)); if (m_private_state_thread.EqualsThread(Host::GetCurrentThread())) { // Yikes, we are running on the private state thread! So we can't wait for - // public events on this thread, since - // we are the thread that is generating public events. - // The simplest thing to do is to spin up a temporary thread to handle - // private state thread events while - // we are fielding public events here. + // public events on this thread, since we are the thread that is generating + // public events. The simplest thing to do is to spin up a temporary thread + // to handle private state thread events while we are fielding public + // events here. if (log) log->Printf("Running thread plan on private state thread, spinning up " "another state thread to handle the events."); @@ -5005,15 +4936,12 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, backup_private_state_thread = m_private_state_thread; // One other bit of business: we want to run just this thread plan and - // anything it pushes, and then stop, - // returning control here. - // But in the normal course of things, the plan above us on the stack would - // be given a shot at the stop - // event before deciding to stop, and we don't want that. So we insert a - // "stopper" base plan on the stack - // before the plan we want to run. Since base plans always stop and return - // control to the user, that will - // do just what we want. + // anything it pushes, and then stop, returning control here. But in the + // normal course of things, the plan above us on the stack would be given a + // shot at the stop event before deciding to stop, and we don't want that. + // So we insert a "stopper" base plan on the stack before the plan we want + // to run. Since base plans always stop and return control to the user, + // that will do just what we want. stopper_base_plan_sp.reset(new ThreadPlanBase(*thread)); thread->QueueThreadPlan(stopper_base_plan_sp, false); // Have to make sure our public state is stopped, since otherwise the @@ -5029,15 +4957,13 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, thread_plan_sp, false); // This used to pass "true" does that make sense? if (options.GetDebug()) { - // In this case, we aren't actually going to run, we just want to stop right - // away. - // Flush this thread so we will refetch the stacks and show the correct - // backtrace. + // In this case, we aren't actually going to run, we just want to stop + // right away. Flush this thread so we will refetch the stacks and show the + // correct backtrace. // FIXME: To make this prettier we should invent some stop reason for this, // but that // is only cosmetic, and this functionality is only of use to lldb - // developers who can - // live with not pretty... + // developers who can live with not pretty... thread->Flush(); return eExpressionStoppedForDebug; } @@ -5049,12 +4975,11 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, { // This process event hijacker Hijacks the Public events and its destructor - // makes sure that the process events get - // restored on exit to the function. + // makes sure that the process events get restored on exit to the function. // // If the event needs to propagate beyond the hijacker (e.g., the process - // exits during execution), then the event - // is put into event_to_broadcast_sp for rebroadcasting. + // exits during execution), then the event is put into + // event_to_broadcast_sp for rebroadcasting. ProcessEventHijacker run_thread_plan_hijacker(*this, listener_sp); @@ -5089,10 +5014,10 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, options.GetStopOthers(), options.GetTryAllThreads(), before_first_timeout); - // This isn't going to work if there are unfetched events on the queue. - // Are there cases where we might want to run the remaining events here, and - // then try to - // call the function? That's probably being too tricky for our own good. + // This isn't going to work if there are unfetched events on the queue. Are + // there cases where we might want to run the remaining events here, and + // then try to call the function? That's probably being too tricky for our + // own good. Event *other_events = listener_sp->PeekAtNextEvent(); if (other_events != nullptr) { @@ -5103,32 +5028,29 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } // We also need to make sure that the next event is delivered. We might be - // calling a function as part of - // a thread plan, in which case the last delivered event could be the - // running event, and we don't want - // event coalescing to cause us to lose OUR running event... + // calling a function as part of a thread plan, in which case the last + // delivered event could be the running event, and we don't want event + // coalescing to cause us to lose OUR running event... ForceNextEventDelivery(); // This while loop must exit out the bottom, there's cleanup that we need to do -// when we are done. -// So don't call return anywhere within it. +// when we are done. So don't call return anywhere within it. #ifdef LLDB_RUN_THREAD_HALT_WITH_EVENT - // It's pretty much impossible to write test cases for things like: - // One thread timeout expires, I go to halt, but the process already stopped - // on the function call stop breakpoint. Turning on this define will make - // us not - // fetch the first event till after the halt. So if you run a quick - // function, it will have - // completed, and the completion event will be waiting, when you interrupt - // for halt. - // The expression evaluation should still succeed. + // It's pretty much impossible to write test cases for things like: One + // thread timeout expires, I go to halt, but the process already stopped on + // the function call stop breakpoint. Turning on this define will make us + // not fetch the first event till after the halt. So if you run a quick + // function, it will have completed, and the completion event will be + // waiting, when you interrupt for halt. The expression evaluation should + // still succeed. bool miss_first_event = true; #endif while (true) { - // We usually want to resume the process if we get to the top of the loop. - // The only exception is if we get two running events with no intervening - // stop, which can happen, we will just wait for then next stop event. + // We usually want to resume the process if we get to the top of the + // loop. The only exception is if we get two running events with no + // intervening stop, which can happen, we will just wait for then next + // stop event. if (log) log->Printf("Top of while loop: do_resume: %i handle_running_event: %i " "before_first_timeout: %i.", @@ -5187,9 +5109,8 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, if (restarted) { // This is probably an overabundance of caution, I don't think I - // should ever get a stopped & restarted - // event here. But if I do, the best thing is to Halt and then get - // out of here. + // should ever get a stopped & restarted event here. But if I do, + // the best thing is to Halt and then get out of here. const bool clear_thread_plans = false; const bool use_run_lock = false; Halt(clear_thread_plans, use_run_lock); @@ -5205,14 +5126,11 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, if (log) log->PutCString("Process::RunThreadPlan(): resuming succeeded."); - // We need to call the function synchronously, so spin waiting for it to - // return. - // If we get interrupted while executing, we're going to lose our - // context, and - // won't be able to gather the result at this point. - // We set the timeout AFTER the resume, since the resume takes some time - // and we - // don't want to charge that to the timeout. + // We need to call the function synchronously, so spin waiting for it + // to return. If we get interrupted while executing, we're going to + // lose our context, and won't be able to gather the result at this + // point. We set the timeout AFTER the resume, since the resume takes + // some time and we don't want to charge that to the timeout. } else { if (log) log->PutCString("Process::RunThreadPlan(): waiting for next event."); @@ -5304,9 +5222,8 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, case lldb::eStateRunning: // This shouldn't really happen, but sometimes we do get two - // running events without an - // intervening stop, and in that case we should just go back to - // waiting for the stop. + // running events without an intervening stop, and in that case + // we should just go back to waiting for the stop. do_resume = false; keep_going = true; handle_running_event = false; @@ -5341,10 +5258,10 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, break; } } else { - // If we didn't get an event that means we've timed out... - // We will interrupt the process here. Depending on what we were asked - // to do we will - // either exit, or try with all threads running for the same timeout. + // If we didn't get an event that means we've timed out... We will + // interrupt the process here. Depending on what we were asked to do + // we will either exit, or try with all threads running for the same + // timeout. if (log) { if (options.GetTryAllThreads()) { @@ -5362,14 +5279,13 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } // It is possible that between the time we issued the Halt, and we get - // around to calling Halt the target - // could have stopped. That's fine, Halt will figure that out and send - // the appropriate Stopped event. + // around to calling Halt the target could have stopped. That's fine, + // Halt will figure that out and send the appropriate Stopped event. // BUT it is also possible that we stopped & restarted (e.g. hit a // signal with "stop" set to false.) In // that case, we'll get the stopped & restarted event, and we should go - // back to waiting for the Halt's - // stopped event. That's what this while loop does. + // back to waiting for the Halt's stopped event. That's what this + // while loop does. bool back_to_top = true; uint32_t try_halt_again = 0; @@ -5418,8 +5334,8 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } // Between the time we initiated the Halt and the time we - // delivered it, the process could have - // already finished its job. Check that here: + // delivered it, the process could have already finished its + // job. Check that here: const bool handle_interrupts = false; if (auto result = HandleStoppedEvent( *thread, thread_plan_sp, thread_plan_restorer, event_sp, @@ -5482,8 +5398,8 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } } // END WAIT LOOP - // If we had to start up a temporary private state thread to run this thread - // plan, shut it down now. + // If we had to start up a temporary private state thread to run this + // thread plan, shut it down now. if (backup_private_state_thread.IsJoinable()) { StopPrivateStateThread(); Status error; @@ -5503,11 +5419,9 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, log->PutString(s.GetString()); } // Restore the thread state if we are going to discard the plan execution. - // There are three cases where this - // could happen: - // 1) The execution successfully completed - // 2) We hit a breakpoint, and ignore_breakpoints was true - // 3) We got some other error, and discard_on_error was true + // There are three cases where this could happen: 1) The execution + // successfully completed 2) We hit a breakpoint, and ignore_breakpoints + // was true 3) We got some other error, and discard_on_error was true bool should_unwind = (return_value == eExpressionInterrupted && options.DoesUnwindOnError()) || (return_value == eExpressionHitBreakpoint && @@ -5647,18 +5561,15 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, } // Thread we ran the function in may have gone away because we ran the - // target - // Check that it's still there, and if it is put it back in the context. - // Also restore the - // frame in the context if it is still present. + // target Check that it's still there, and if it is put it back in the + // context. Also restore the frame in the context if it is still present. thread = GetThreadList().FindThreadByIndexID(thread_idx_id, true).get(); if (thread) { exe_ctx.SetFrameSP(thread->GetFrameWithStackID(ctx_frame_id)); } // Also restore the current process'es selected frame & thread, since this - // function calling may - // be done behind the user's back. + // function calling may be done behind the user's back. if (selected_tid != LLDB_INVALID_THREAD_ID) { if (GetThreadList().SetSelectedThreadByIndexID(selected_tid) && @@ -5749,10 +5660,9 @@ size_t Process::GetThreadStatus(Stream &strm, size_t num_thread_infos_dumped = 0; // You can't hold the thread list lock while calling Thread::GetStatus. That - // very well might run code (e.g. if we need it - // to get return values or arguments.) For that to work the process has to be - // able to acquire it. So instead copy the thread - // ID's, and look them up one by one: + // very well might run code (e.g. if we need it to get return values or + // arguments.) For that to work the process has to be able to acquire it. + // So instead copy the thread ID's, and look them up one by one: uint32_t num_threads; std::vector<lldb::tid_t> thread_id_array; @@ -5865,12 +5775,12 @@ void Process::DidExec() { DoDidExec(); CompleteAttach(); // Flush the process (threads and all stack frames) after running - // CompleteAttach() - // in case the dynamic loader loaded things in new locations. + // CompleteAttach() in case the dynamic loader loaded things in new + // locations. Flush(); - // After we figure out what was loaded/unloaded in CompleteAttach, - // we need to let the target know so it can do any cleanup it needs to. + // After we figure out what was loaded/unloaded in CompleteAttach, we need to + // let the target know so it can do any cleanup it needs to. target.DidExec(); } @@ -5921,17 +5831,17 @@ void Process::ModulesDidLoad(ModuleList &module_list) { runtime->ModulesDidLoad(module_list); } - // Let any language runtimes we have already created know - // about the modules that loaded. + // Let any language runtimes we have already created know about the modules + // that loaded. - // Iterate over a copy of this language runtime list in case - // the language runtime ModulesDidLoad somehow causes the language - // riuntime to be unloaded. + // Iterate over a copy of this language runtime list in case the language + // runtime ModulesDidLoad somehow causes the language riuntime to be + // unloaded. LanguageRuntimeCollection language_runtimes(m_language_runtimes); for (const auto &pair : language_runtimes) { - // We must check language_runtime_sp to make sure it is not - // nullptr as we might cache the fact that we didn't have a - // language runtime for a language. + // We must check language_runtime_sp to make sure it is not nullptr as we + // might cache the fact that we didn't have a language runtime for a + // language. LanguageRuntimeSP language_runtime_sp = pair.second; if (language_runtime_sp) language_runtime_sp->ModulesDidLoad(module_list); @@ -6131,9 +6041,9 @@ Process::GetMemoryRegions(std::vector<lldb::MemoryRegionInfoSP> ®ion_list) { Status Process::ConfigureStructuredData(const ConstString &type_name, const StructuredData::ObjectSP &config_sp) { - // If you get this, the Process-derived class needs to implement a method - // to enable an already-reported asynchronous structured data feature. - // See ProcessGDBRemote for an example implementation over gdb-remote. + // If you get this, the Process-derived class needs to implement a method to + // enable an already-reported asynchronous structured data feature. See + // ProcessGDBRemote for an example implementation over gdb-remote. return Status("unimplemented"); } @@ -6175,9 +6085,9 @@ void Process::MapSupportedStructuredDataPlugins( return true; }); - // For each StructuredDataPlugin, if the plugin handles any of the - // types in the supported_type_names, map that type name to that plugin. - // Stop when we've consumed all the type names. + // For each StructuredDataPlugin, if the plugin handles any of the types in + // the supported_type_names, map that type name to that plugin. Stop when + // we've consumed all the type names. // FIXME: should we return an error if there are type names nobody // supports? for (uint32_t plugin_index = 0; !const_type_names.empty(); plugin_index++) { @@ -6190,13 +6100,12 @@ void Process::MapSupportedStructuredDataPlugins( // Create the plugin. StructuredDataPluginSP plugin_sp = (*create_instance)(*this); if (!plugin_sp) { - // This plugin doesn't think it can work with the process. - // Move on to the next. + // This plugin doesn't think it can work with the process. Move on to the + // next. continue; } - // For any of the remaining type names, map any that this plugin - // supports. + // For any of the remaining type names, map any that this plugin supports. std::vector<ConstString> names_to_remove; for (auto &type_name : const_type_names) { if (plugin_sp->SupportsStructuredDataType(type_name)) { @@ -6223,8 +6132,8 @@ bool Process::RouteAsyncStructuredData( if (!object_sp) return false; - // The contract is this must be a dictionary, so we can look up the - // routing key via the top-level 'type' string value within the dictionary. + // The contract is this must be a dictionary, so we can look up the routing + // key via the top-level 'type' string value within the dictionary. StructuredData::Dictionary *dictionary = object_sp->GetAsDictionary(); if (!dictionary) return false; diff --git a/lldb/source/Target/ProcessInfo.cpp b/lldb/source/Target/ProcessInfo.cpp index 594a748d248..3b4fdf69618 100644 --- a/lldb/source/Target/ProcessInfo.cpp +++ b/lldb/source/Target/ProcessInfo.cpp @@ -93,9 +93,9 @@ void ProcessInfo::SetArguments(char const **argv, if (first_arg_is_executable) { const char *first_arg = m_arguments.GetArgumentAtIndex(0); if (first_arg) { - // Yes the first argument is an executable, set it as the executable - // in the launch options. Don't resolve the file path as the path - // could be a remote platform path + // Yes the first argument is an executable, set it as the executable in + // the launch options. Don't resolve the file path as the path could be a + // remote platform path const bool resolve = false; m_executable.SetFile(first_arg, resolve); } @@ -110,9 +110,9 @@ void ProcessInfo::SetArguments(const Args &args, bool first_arg_is_executable) { if (first_arg_is_executable) { const char *first_arg = m_arguments.GetArgumentAtIndex(0); if (first_arg) { - // Yes the first argument is an executable, set it as the executable - // in the launch options. Don't resolve the file path as the path - // could be a remote platform path + // Yes the first argument is an executable, set it as the executable in + // the launch options. Don't resolve the file path as the path could be a + // remote platform path const bool resolve = false; m_executable.SetFile(first_arg, resolve); } diff --git a/lldb/source/Target/ProcessLaunchInfo.cpp b/lldb/source/Target/ProcessLaunchInfo.cpp index 284df9fd8b5..60fe26bb60d 100644 --- a/lldb/source/Target/ProcessLaunchInfo.cpp +++ b/lldb/source/Target/ProcessLaunchInfo.cpp @@ -210,8 +210,7 @@ void ProcessLaunchInfo::FinalizeFileActions(Target *target, Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS)); // If nothing for stdin or stdout or stderr was specified, then check the - // process for any default - // settings that were set with "settings set" + // process for any default settings that were set with "settings set" if (GetFileActionForFD(STDIN_FILENO) == nullptr || GetFileActionForFD(STDOUT_FILENO) == nullptr || GetFileActionForFD(STDERR_FILENO) == nullptr) { @@ -221,8 +220,8 @@ void ProcessLaunchInfo::FinalizeFileActions(Target *target, __FUNCTION__); if (m_flags.Test(eLaunchFlagLaunchInTTY)) { - // Do nothing, if we are launching in a remote terminal - // no file actions should be done at all. + // Do nothing, if we are launching in a remote terminal no file actions + // should be done at all. return; } @@ -235,16 +234,15 @@ void ProcessLaunchInfo::FinalizeFileActions(Target *target, AppendSuppressFileAction(STDOUT_FILENO, false, true); AppendSuppressFileAction(STDERR_FILENO, false, true); } else { - // Check for any values that might have gotten set with any of: - // (lldb) settings set target.input-path - // (lldb) settings set target.output-path + // Check for any values that might have gotten set with any of: (lldb) + // settings set target.input-path (lldb) settings set target.output-path // (lldb) settings set target.error-path FileSpec in_file_spec; FileSpec out_file_spec; FileSpec err_file_spec; if (target) { - // Only override with the target settings if we don't already have - // an action for in, out or error + // Only override with the target settings if we don't already have an + // action for in, out or error if (GetFileActionForFD(STDIN_FILENO) == nullptr) in_file_spec = target->GetStandardInputPath(); if (GetFileActionForFD(STDOUT_FILENO) == nullptr) @@ -295,9 +293,9 @@ void ProcessLaunchInfo::FinalizeFileActions(Target *target, int open_flags = O_RDWR | O_NOCTTY; #if !defined(_WIN32) - // We really shouldn't be specifying platform specific flags - // that are intended for a system call in generic code. But - // this will have to do for now. + // We really shouldn't be specifying platform specific flags that are + // intended for a system call in generic code. But this will have to + // do for now. open_flags |= O_CLOEXEC; #endif if (m_pty->OpenFirstAvailableMaster(open_flags, nullptr, 0)) { @@ -351,14 +349,13 @@ bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell( StreamString shell_command; if (will_debug) { - // Add a modified PATH environment variable in case argv[0] - // is a relative path. + // Add a modified PATH environment variable in case argv[0] is a + // relative path. const char *argv0 = argv[0]; FileSpec arg_spec(argv0, false); if (arg_spec.IsRelative()) { - // We have a relative path to our executable which may not work if - // we just try to run "a.out" (without it being converted to - // "./a.out") + // We have a relative path to our executable which may not work if we + // just try to run "a.out" (without it being converted to "./a.out") FileSpec working_dir = GetWorkingDirectory(); // Be sure to put quotes around PATH's value in case any paths have // spaces... @@ -410,8 +407,8 @@ bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell( } if (first_arg_is_full_shell_command) { - // There should only be one argument that is the shell command itself to - // be used as is + // There should only be one argument that is the shell command itself + // to be used as is if (argv[0] && !argv[1]) shell_command.Printf("%s", argv[0]); else diff --git a/lldb/source/Target/RegisterContext.cpp b/lldb/source/Target/RegisterContext.cpp index 28beb7bcb5e..2d45c8be0d7 100644 --- a/lldb/source/Target/RegisterContext.cpp +++ b/lldb/source/Target/RegisterContext.cpp @@ -76,8 +76,7 @@ RegisterContext::UpdateDynamicRegisterSize(const lldb_private::ArchSpec &arch, ExecutionContext exe_ctx(CalculateThread()); // In MIPS, the floating point registers size is depends on FR bit of SR - // register. - // if SR.FR == 1 then all floating point registers are 64 bits. + // register. if SR.FR == 1 then all floating point registers are 64 bits. // else they are all 32 bits. int expr_result; @@ -262,8 +261,7 @@ bool RegisterContext::CopyFromRegisterContext(lldb::RegisterContextSP context) { RegisterValue reg_value; // If we can reconstruct the register from the frame we are copying from, - // then do so, otherwise - // use the value from frame 0. + // then do so, otherwise use the value from frame 0. if (context->ReadRegister(reg_info, reg_value)) { WriteRegister(reg_info, reg_value); } else if (frame_zero_context->ReadRegister(reg_info, reg_value)) { @@ -355,12 +353,11 @@ Status RegisterContext::ReadRegisterValueFromMemory( return error; } - // We now have a memory buffer that contains the part or all of the register - // value. Set the register value using this memory data. + // We now have a memory buffer that contains the part or all of the + // register value. Set the register value using this memory data. // TODO: we might need to add a parameter to this function in case the byte // order of the memory data doesn't match the process. For now we are - // assuming - // they are the same. + // assuming they are the same. reg_value.SetFromMemoryData(reg_info, src, src_len, process_sp->GetByteOrder(), error); } else @@ -381,8 +378,7 @@ Status RegisterContext::WriteRegisterValueToMemory( // TODO: we might need to add a parameter to this function in case the byte // order of the memory data doesn't match the process. For now we are - // assuming - // they are the same. + // assuming they are the same. const uint32_t bytes_copied = reg_value.GetAsMemoryData( reg_info, dst, dst_len, process_sp->GetByteOrder(), error); @@ -431,9 +427,9 @@ ThreadSP RegisterContext::CalculateThread() { } StackFrameSP RegisterContext::CalculateStackFrame() { - // Register contexts might belong to many frames if we have inlined - // functions inside a frame since all inlined functions share the - // same registers, so we can't definitively say which frame we come from... + // Register contexts might belong to many frames if we have inlined functions + // inside a frame since all inlined functions share the same registers, so we + // can't definitively say which frame we come from... return StackFrameSP(); } diff --git a/lldb/source/Target/SectionLoadHistory.cpp b/lldb/source/Target/SectionLoadHistory.cpp index 740f48bc829..5844da5d41e 100644 --- a/lldb/source/Target/SectionLoadHistory.cpp +++ b/lldb/source/Target/SectionLoadHistory.cpp @@ -43,13 +43,12 @@ SectionLoadHistory::GetSectionLoadListForStopID(uint32_t stop_id, if (!m_stop_id_to_section_load_list.empty()) { if (read_only) { // The section load list is for reading data only so we don't need to - // create - // a new SectionLoadList for the current stop ID, just return the section - // load list for the stop ID that is equal to or less than the current - // stop ID + // create a new SectionLoadList for the current stop ID, just return the + // section load list for the stop ID that is equal to or less than the + // current stop ID if (stop_id == eStopIDNow) { - // If we are asking for the latest and greatest value, it is always - // at the end of our list because that will be the highest stop ID. + // If we are asking for the latest and greatest value, it is always at + // the end of our list because that will be the highest stop ID. StopIDToSectionLoadList::reverse_iterator rpos = m_stop_id_to_section_load_list.rbegin(); return rpos->second.get(); @@ -70,10 +69,8 @@ SectionLoadHistory::GetSectionLoadListForStopID(uint32_t stop_id, assert(stop_id != eStopIDNow); // We are updating the section load list (not read only), so if the stop - // ID - // passed in isn't the same as the last stop ID in our collection, then - // create - // a new node using the current stop ID + // ID passed in isn't the same as the last stop ID in our collection, + // then create a new node using the current stop ID StopIDToSectionLoadList::iterator pos = m_stop_id_to_section_load_list.lower_bound(stop_id); if (pos != m_stop_id_to_section_load_list.end() && diff --git a/lldb/source/Target/SectionLoadList.cpp b/lldb/source/Target/SectionLoadList.cpp index 31ccf17369d..6839aaccaa5 100644 --- a/lldb/source/Target/SectionLoadList.cpp +++ b/lldb/source/Target/SectionLoadList.cpp @@ -97,12 +97,12 @@ bool SectionLoadList::SetSectionLoadAddress(const lldb::SectionSP §ion, // we have multiple load addresses that correspond to a section, we will // always attribute the section to the be last section that claims it // exists at that address. Sometimes it is ok for more that one section - // to be loaded at a specific load address, and other times it isn't. - // The "warn_multiple" parameter tells us if we should warn in this case - // or not. The DynamicLoader plug-in subclasses should know which - // sections should warn and which shouldn't (darwin shared cache modules - // all shared the same "__LINKEDIT" sections, so the dynamic loader can - // pass false for "warn_multiple"). + // to be loaded at a specific load address, and other times it isn't. The + // "warn_multiple" parameter tells us if we should warn in this case or + // not. The DynamicLoader plug-in subclasses should know which sections + // should warn and which shouldn't (darwin shared cache modules all + // shared the same "__LINKEDIT" sections, so the dynamic loader can pass + // false for "warn_multiple"). if (warn_multiple && section != ats_pos->second) { ModuleSP module_sp(section->GetModule()); if (module_sp) { @@ -228,8 +228,8 @@ bool SectionLoadList::ResolveLoadAddress(addr_t load_addr, Address &so_addr, } } } else { - // There are no entries that have an address that is >= load_addr, - // so we need to check the last entry on our collection. + // There are no entries that have an address that is >= load_addr, so we + // need to check the last entry on our collection. addr_to_sect_collection::const_reverse_iterator rpos = m_addr_to_sect.rbegin(); if (load_addr >= rpos->first) { diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index dd44eac8e50..d6fafffbee0 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -60,8 +60,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, m_is_history_frame(is_history_frame), m_variable_list_sp(), m_variable_list_value_objects(), m_disassembly(), m_mutex() { // If we don't have a CFA value, use the frame index for our StackID so that - // recursive - // functions properly aren't confused with one another on a history stack. + // recursive functions properly aren't confused with one another on a history + // stack. if (m_is_history_frame && !m_cfa_is_valid) { m_id.SetCFA(m_frame_index); } @@ -136,17 +136,17 @@ StackFrame::~StackFrame() = default; StackID &StackFrame::GetStackID() { std::lock_guard<std::recursive_mutex> guard(m_mutex); - // Make sure we have resolved the StackID object's symbol context scope if - // we already haven't looked it up. + // Make sure we have resolved the StackID object's symbol context scope if we + // already haven't looked it up. if (m_flags.IsClear(RESOLVED_FRAME_ID_SYMBOL_SCOPE)) { if (m_id.GetSymbolContextScope()) { - // We already have a symbol context scope, we just don't have our - // flag bit set. + // We already have a symbol context scope, we just don't have our flag + // bit set. m_flags.Set(RESOLVED_FRAME_ID_SYMBOL_SCOPE); } else { - // Calculate the frame block and use this for the stack ID symbol - // context scope if we have one. + // Calculate the frame block and use this for the stack ID symbol context + // scope if we have one. SymbolContextScope *scope = GetFrameBlock(); if (scope == nullptr) { // We don't have a block, so use the symbol @@ -247,13 +247,13 @@ Block *StackFrame::GetFrameBlock() { if (m_sc.block) { Block *inline_block = m_sc.block->GetContainingInlinedBlock(); if (inline_block) { - // Use the block with the inlined function info - // as the frame block we want this frame to have only the variables - // for the inlined function and its non-inlined block child blocks. + // Use the block with the inlined function info as the frame block we + // want this frame to have only the variables for the inlined function + // and its non-inlined block child blocks. return inline_block; } else { - // This block is not contained within any inlined function blocks - // with so we want to use the top most function block. + // This block is not contained within any inlined function blocks with so + // we want to use the top most function block. return &m_sc.function->GetBlock(false); } } @@ -263,8 +263,8 @@ Block *StackFrame::GetFrameBlock() { //---------------------------------------------------------------------- // Get the symbol context if we already haven't done so by resolving the // PC address as much as possible. This way when we pass around a -// StackFrame object, everyone will have as much information as -// possible and no one will ever have to look things up manually. +// StackFrame object, everyone will have as much information as possible and no +// one will ever have to look things up manually. //---------------------------------------------------------------------- const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { std::lock_guard<std::recursive_mutex> guard(m_mutex); @@ -279,15 +279,15 @@ const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { resolved |= eSymbolContextTarget; } - // Resolve our PC to section offset if we haven't already done so - // and if we don't have a module. The resolved address section will - // contain the module to which it belongs + // Resolve our PC to section offset if we haven't already done so and if we + // don't have a module. The resolved address section will contain the + // module to which it belongs if (!m_sc.module_sp && m_flags.IsClear(RESOLVED_FRAME_CODE_ADDR)) GetFrameCodeAddress(); - // If this is not frame zero, then we need to subtract 1 from the PC - // value when doing address lookups since the PC will be on the - // instruction following the function call instruction... + // If this is not frame zero, then we need to subtract 1 from the PC value + // when doing address lookups since the PC will be on the instruction + // following the function call instruction... Address lookup_addr(GetFrameCodeAddress()); if (m_frame_index > 0 && lookup_addr.IsValid()) { @@ -296,10 +296,9 @@ const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { lookup_addr.SetOffset(offset - 1); } else { - // lookup_addr is the start of a section. We need - // do the math on the actual load address and re-compute - // the section. We're working with a 'noreturn' function - // at the end of a section. + // lookup_addr is the start of a section. We need do the math on the + // actual load address and re-compute the section. We're working with + // a 'noreturn' function at the end of a section. ThreadSP thread_sp(GetThread()); if (thread_sp) { TargetSP target_sp(thread_sp->CalculateTarget()); @@ -315,9 +314,9 @@ const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { } if (m_sc.module_sp) { - // We have something in our stack frame symbol context, lets check - // if we haven't already tried to lookup one of those things. If we - // haven't then we will do the query. + // We have something in our stack frame symbol context, lets check if we + // haven't already tried to lookup one of those things. If we haven't + // then we will do the query. uint32_t actual_resolve_scope = 0; @@ -367,17 +366,16 @@ const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { } if (actual_resolve_scope) { - // We might be resolving less information than what is already - // in our current symbol context so resolve into a temporary - // symbol context "sc" so we don't clear out data we have - // already found in "m_sc" + // We might be resolving less information than what is already in our + // current symbol context so resolve into a temporary symbol context + // "sc" so we don't clear out data we have already found in "m_sc" SymbolContext sc; // Set flags that indicate what we have tried to resolve resolved |= m_sc.module_sp->ResolveSymbolContextForAddress( lookup_addr, actual_resolve_scope, sc); - // Only replace what we didn't already have as we may have - // information for an inlined function scope that won't match - // what a standard lookup by address would match + // Only replace what we didn't already have as we may have information + // for an inlined function scope that won't match what a standard + // lookup by address would match if ((resolved & eSymbolContextCompUnit) && m_sc.comp_unit == nullptr) m_sc.comp_unit = sc.comp_unit; if ((resolved & eSymbolContextFunction) && m_sc.function == nullptr) @@ -404,9 +402,9 @@ const SymbolContext &StackFrame::GetSymbolContext(uint32_t resolve_scope) { // Update our internal flags so we remember what we have tried to locate so // we don't have to keep trying when more calls to this function are made. - // We might have dug up more information that was requested (for example - // if we were asked to only get the block, we will have gotten the - // compile unit, and function) so set any additional bits that we resolved + // We might have dug up more information that was requested (for example if + // we were asked to only get the block, we will have gotten the compile + // unit, and function) so set any additional bits that we resolved m_flags.Set(resolve_scope | resolved); } @@ -546,8 +544,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( } if (!var_sp && (options & eExpressionPathOptionsAllowDirectIVarAccess)) { - // Check for direct ivars access which helps us with implicit - // access to ivars with the "this->" or "self->" + // Check for direct ivars access which helps us with implicit access to + // ivars with the "this->" or "self->" GetSymbolContext(eSymbolContextFunction | eSymbolContextBlock); lldb::LanguageType method_language = eLanguageTypeUnknown; bool is_instance_method = false; @@ -622,9 +620,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( valobj_sp->GetCompilerType().GetTypeInfo(nullptr); if ((pointer_type_flags & eTypeIsObjC) && (pointer_type_flags & eTypeIsPointer)) { - // This was an objective C object pointer and - // it was requested we skip any fragile ivars - // so return nothing here + // This was an objective C object pointer and it was requested we + // skip any fragile ivars so return nothing here return ValueObjectSP(); } } @@ -659,15 +656,14 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-["))); if (check_ptr_vs_member) { - // We either have a pointer type and need to verify - // valobj_sp is a pointer, or we have a member of a - // class/union/struct being accessed with the . syntax - // and need to verify we don't have a pointer. + // We either have a pointer type and need to verify valobj_sp is a + // pointer, or we have a member of a class/union/struct being accessed + // with the . syntax and need to verify we don't have a pointer. const bool actual_is_ptr = valobj_sp->IsPointerType(); if (actual_is_ptr != expr_is_ptr) { - // Incorrect use of "." with a pointer, or "->" with - // a class/union/struct instance or reference. + // Incorrect use of "." with a pointer, or "->" with a + // class/union/struct instance or reference. valobj_sp->GetExpressionPath(var_expr_path_strm, false); if (actual_is_ptr) error.SetErrorStringWithFormat( @@ -697,10 +693,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( // No child member with name "child_name" if (synthetically_added_instance_object) { // We added a "this->" or "self->" to the beginning of the - // expression - // and this is the first pointer ivar access, so just return - // the normal - // error + // expression and this is the first pointer ivar access, so just + // return the normal error error.SetErrorStringWithFormat( "no variable or instance variable named '%s' found in " "this frame", @@ -735,8 +729,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( } break; case '[': { - // Array member access, or treating pointer as an array - // Need at least two brackets and a number + // Array member access, or treating pointer as an array Need at least two + // brackets and a number if (var_expr.size() <= 2) { error.SetErrorStringWithFormat( "invalid square bracket encountered after \"%s\" in \"%s\"", @@ -790,11 +784,10 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( deref = false; } else if (valobj_sp->GetCompilerType().IsArrayOfScalarType() && deref) { - // what we have is *arr[low]. the most similar C++ syntax is - // to get arr[0] - // (an operation that is equivalent to deref-ing arr) - // and extract bit low out of it. reading array item low - // would be done by saying arr[low], without a deref * sign + // what we have is *arr[low]. the most similar C++ syntax is to get + // arr[0] (an operation that is equivalent to deref-ing arr) and + // extract bit low out of it. reading array item low would be done by + // saying arr[low], without a deref * sign Status error; ValueObjectSP temp(valobj_sp->GetChildAtIndex(0, true)); if (error.Fail()) { @@ -828,8 +821,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( return ValueObjectSP(); } else if (is_objc_pointer) { - // dereferencing ObjC variables is not valid.. so let's try - // and recur to synthetic children + // dereferencing ObjC variables is not valid.. so let's try and + // recur to synthetic children ValueObjectSP synthetic = valobj_sp->GetSyntheticValue(); if (!synthetic /* no synthetic */ || synthetic == valobj_sp) /* synthetic is the same as @@ -874,9 +867,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( } } else if (valobj_sp->GetCompilerType().IsArrayType( nullptr, nullptr, &is_incomplete_array)) { - // Pass false to dynamic_value here so we can tell the - // difference between - // no dynamic value and no member of this type... + // Pass false to dynamic_value here so we can tell the difference + // between no dynamic value and no member of this type... child_valobj_sp = valobj_sp->GetChildAtIndex(child_index, true); if (!child_valobj_sp && (is_incomplete_array || !no_synth_child)) child_valobj_sp = @@ -976,8 +968,8 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( if (valobj_sp->GetCompilerType().IsPointerToScalarType() && deref) { // what we have is *ptr[low-high]. the most similar C++ syntax is to // deref ptr and extract bits low thru high out of it. reading array - // items low thru high would be done by saying ptr[low-high], without - // a deref * sign + // items low thru high would be done by saying ptr[low-high], without a + // deref * sign Status error; ValueObjectSP temp(valobj_sp->Dereference(error)); if (error.Fail()) { @@ -991,10 +983,10 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath( valobj_sp = temp; deref = false; } else if (valobj_sp->GetCompilerType().IsArrayOfScalarType() && deref) { - // what we have is *arr[low-high]. the most similar C++ syntax is to get - // arr[0] (an operation that is equivalent to deref-ing arr) and extract - // bits low thru high out of it. reading array items low thru high would - // be done by saying arr[low-high], without a deref * sign + // what we have is *arr[low-high]. the most similar C++ syntax is to + // get arr[0] (an operation that is equivalent to deref-ing arr) and + // extract bits low thru high out of it. reading array items low thru + // high would be done by saying arr[low-high], without a deref * sign Status error; ValueObjectSP temp(valobj_sp->GetChildAtIndex(0, true)); if (error.Fail()) { @@ -1091,8 +1083,8 @@ bool StackFrame::GetFrameBaseValue(Scalar &frame_base, Status *error_ptr) { if (m_sc.function->GetFrameBaseExpression().Evaluate( &exe_ctx, nullptr, loclist_base_addr, nullptr, nullptr, expr_value, &m_frame_base_error) == false) { - // We should really have an error if evaluate returns, but in case - // we don't, lets set the error to something at least. + // We should really have an error if evaluate returns, but in case we + // don't, lets set the error to something at least. if (m_frame_base_error.Success()) m_frame_base_error.SetErrorString( "Evaluation of the frame base expression failed."); @@ -1420,8 +1412,8 @@ ValueObjectSP GetValueForDereferincingOffset(StackFrame &frame, ValueObjectSP &base, int64_t offset) { // base is a pointer to something - // offset is the thing to add to the pointer - // We return the most sensible ValueObject for the result of *(base+offset) + // offset is the thing to add to the pointer We return the most sensible + // ValueObject for the result of *(base+offset) if (!base->IsPointerOrReferenceType()) { return ValueObjectSP(); @@ -1486,8 +1478,8 @@ lldb::ValueObjectSP DoGuessValueAt(StackFrame &frame, ConstString reg, // // f, a pointer to a struct, is known to be at -0x8(%rbp). // - // DoGuessValueAt(frame, rdi, 4, dis, vars, 0x22) finds the instruction at +18 - // that assigns to rdi, and calls itself recursively for that dereference + // DoGuessValueAt(frame, rdi, 4, dis, vars, 0x22) finds the instruction at + // +18 that assigns to rdi, and calls itself recursively for that dereference // DoGuessValueAt(frame, rdi, 8, dis, vars, 0x18) finds the instruction at // +14 that assigns to rdi, and calls itself recursively for that // derefernece @@ -1533,9 +1525,9 @@ lldb::ValueObjectSP DoGuessValueAt(StackFrame &frame, ConstString reg, for (uint32_t ii = current_inst - 1; ii != (uint32_t)-1; --ii) { // This is not an exact algorithm, and it sacrifices accuracy for - // generality. Recognizing "mov" and "ld" instructions –– and which are - // their source and destination operands -- is something the disassembler - // should do for us. + // generality. Recognizing "mov" and "ld" instructions –– and which + // are their source and destination operands -- is something the + // disassembler should do for us. InstructionSP instruction_sp = disassembler.GetInstructionList().GetInstructionAtIndex(ii); diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index be7fa800121..e2e7ba76d7f 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -51,8 +51,8 @@ StackFrameList::StackFrameList(Thread &thread, } StackFrameList::~StackFrameList() { - // Call clear since this takes a lock and clears the stack frame list - // in case another thread is currently using this stack frame list + // Call clear since this takes a lock and clears the stack frame list in case + // another thread is currently using this stack frame list Clear(); } @@ -95,13 +95,12 @@ void StackFrameList::ResetCurrentInlinedDepth() { log->Printf( "ResetCurrentInlinedDepth: Invalidating current inlined depth.\n"); } else { - // We only need to do something special about inlined blocks when we - // are at the beginning of an inlined function: + // We only need to do something special about inlined blocks when we are + // at the beginning of an inlined function: // FIXME: We probably also have to do something special if the PC is at // the END // of an inlined function, which coincides with the end of either its - // containing - // function or another inlined function. + // containing function or another inlined function. lldb::addr_t curr_pc = m_thread.GetRegisterContext()->GetPC(); Block *block_ptr = m_frames[0]->GetFrameBlock(); @@ -114,19 +113,15 @@ void StackFrameList::ResetCurrentInlinedDepth() { containing_range)) { if (pc_as_address == containing_range.GetBaseAddress()) { // If we got here because of a breakpoint hit, then set the inlined - // depth depending on where - // the breakpoint was set. - // If we got here because of a crash, then set the inlined depth to - // the deepest most block. - // Otherwise, we stopped here naturally as the result of a step, so - // set ourselves in the - // containing frame of the whole set of nested inlines, so the user - // can then "virtually" - // step into the frames one by one, or next over the whole mess. - // Note: We don't have to handle being somewhere in the middle of - // the stack here, since - // ResetCurrentInlinedDepth doesn't get called if there is a valid - // inlined depth set. + // depth depending on where the breakpoint was set. If we got here + // because of a crash, then set the inlined depth to the deepest + // most block. Otherwise, we stopped here naturally as the result + // of a step, so set ourselves in the containing frame of the whole + // set of nested inlines, so the user can then "virtually" step + // into the frames one by one, or next over the whole mess. Note: + // We don't have to handle being somewhere in the middle of the + // stack here, since ResetCurrentInlinedDepth doesn't get called if + // there is a valid inlined depth set. StopInfoSP stop_info_sp = m_thread.GetStopInfo(); if (stop_info_sp) { switch (stop_info_sp->GetStopReason()) { @@ -134,7 +129,8 @@ void StackFrameList::ResetCurrentInlinedDepth() { case eStopReasonException: case eStopReasonExec: case eStopReasonSignal: - // In all these cases we want to stop in the deepest most frame. + // In all these cases we want to stop in the deepest most + // frame. m_current_inlined_pc = curr_pc; m_current_inlined_depth = 0; break; @@ -142,17 +138,14 @@ void StackFrameList::ResetCurrentInlinedDepth() { // FIXME: Figure out what this break point is doing, and set the // inline depth // appropriately. Be careful to take into account breakpoints - // that implement - // step over prologue, since that should do the default - // calculation. - // For now, if the breakpoints corresponding to this hit are all - // internal, + // that implement step over prologue, since that should do the + // default calculation. For now, if the breakpoints + // corresponding to this hit are all internal, // I set the stop location to the top of the inlined stack, // since that will make - // things like stepping over prologues work right. But if there - // are any non-internal - // breakpoints I do to the bottom of the stack, since that was - // the old behavior. + // things like stepping over prologues work right. But if + // there are any non-internal breakpoints I do to the bottom of + // the stack, since that was the old behavior. uint32_t bp_site_id = stop_info_sp->GetValue(); BreakpointSiteSP bp_site_sp( m_thread.GetProcess()->GetBreakpointSiteList().FindByID( @@ -177,10 +170,9 @@ void StackFrameList::ResetCurrentInlinedDepth() { LLVM_FALLTHROUGH; default: { // Otherwise, we should set ourselves at the container of the - // inlining, so that the - // user can descend into them. - // So first we check whether we have more than one inlined block - // sharing this PC: + // inlining, so that the user can descend into them. So first + // we check whether we have more than one inlined block sharing + // this PC: int num_inlined_functions = 0; for (Block *container_ptr = block_ptr->GetInlinedParent(); @@ -250,13 +242,11 @@ void StackFrameList::GetFramesUpTo(uint32_t end_idx) { #if defined(DEBUG_STACK_FRAMES) StreamFile s(stdout, false); #endif - // If we are hiding some frames from the outside world, we need to add those - // onto the total count of - // frames to fetch. However, we don't need to do that if end_idx is 0 since - // in that case we always - // get the first concrete frame and all the inlined frames below it... And - // of course, if end_idx is - // UINT32_MAX that means get all, so just do that... + // If we are hiding some frames from the outside world, we need to add + // those onto the total count of frames to fetch. However, we don't need + // to do that if end_idx is 0 since in that case we always get the first + // concrete frame and all the inlined frames below it... And of course, if + // end_idx is UINT32_MAX that means get all, so just do that... uint32_t inlined_depth = 0; if (end_idx > 0 && end_idx != UINT32_MAX) { @@ -273,17 +263,17 @@ void StackFrameList::GetFramesUpTo(uint32_t end_idx) { lldb::addr_t pc = LLDB_INVALID_ADDRESS; lldb::addr_t cfa = LLDB_INVALID_ADDRESS; if (idx == 0) { - // We might have already created frame zero, only create it - // if we need to + // We might have already created frame zero, only create it if we need + // to if (m_frames.empty()) { RegisterContextSP reg_ctx_sp(m_thread.GetRegisterContext()); if (reg_ctx_sp) { const bool success = unwinder && unwinder->GetFrameInfoAtIndex(idx, cfa, pc); - // There shouldn't be any way not to get the frame info for frame 0. - // But if the unwinder can't make one, lets make one by hand with - // the + // There shouldn't be any way not to get the frame info for frame + // 0. But if the unwinder can't make one, lets make one by hand + // with the // SP as the CFA and see if that gets any further. if (!success) { cfa = reg_ctx_sp->GetSP(); @@ -323,18 +313,16 @@ void StackFrameList::GetFramesUpTo(uint32_t end_idx) { if (unwind_block) { Address curr_frame_address(unwind_frame_sp->GetFrameCodeAddress()); TargetSP target_sp = m_thread.CalculateTarget(); - // Be sure to adjust the frame address to match the address - // that was used to lookup the symbol context above. If we are - // in the first concrete frame, then we lookup using the current - // address, else we decrement the address by one to get the correct - // location. + // Be sure to adjust the frame address to match the address that was + // used to lookup the symbol context above. If we are in the first + // concrete frame, then we lookup using the current address, else we + // decrement the address by one to get the correct location. if (idx > 0) { if (curr_frame_address.GetOffset() == 0) { // If curr_frame_address points to the first address in a section - // then after - // adjustment it will point to an other section. In that case - // resolve the - // address again to the correct section plus offset form. + // then after adjustment it will point to an other section. In that + // case resolve the address again to the correct section plus + // offset form. addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress( target_sp.get(), eAddressClassCode); curr_frame_address.SetOpcodeLoadAddress( @@ -414,8 +402,8 @@ void StackFrameList::GetFramesUpTo(uint32_t end_idx) { break; prev_frame->UpdatePreviousFrameFromCurrentFrame(*curr_frame); - // Now copy the fixed up previous frame into the current frames - // so the pointer doesn't change + // Now copy the fixed up previous frame into the current frames so the + // pointer doesn't change m_frames[curr_frame_idx] = prev_frame_sp; // curr_frame->UpdateCurrentFrameFromPreviousFrame (*prev_frame); @@ -495,9 +483,8 @@ StackFrameSP StackFrameList::GetFrameAtIndex(uint32_t idx) { if (frame_sp) return frame_sp; - // GetFramesUpTo will fill m_frames with as many frames as you asked for, - // if there are that many. If there weren't then you asked for too many - // frames. + // GetFramesUpTo will fill m_frames with as many frames as you asked for, if + // there are that many. If there weren't then you asked for too many frames. GetFramesUpTo(idx); if (idx < m_frames.size()) { if (m_show_inlined_frames) { @@ -519,8 +506,8 @@ StackFrameSP StackFrameList::GetFrameAtIndex(uint32_t idx) { Function *function = frame_sp->GetSymbolContext(eSymbolContextFunction).function; if (function) { - // When we aren't showing inline functions we always use - // the top most function block as the scope. + // When we aren't showing inline functions we always use the top + // most function block as the scope. frame_sp->SetSymbolContextScope(&function->GetBlock(false)); } else { // Set the symbol scope from the symbol regardless if it is nullptr @@ -534,9 +521,8 @@ StackFrameSP StackFrameList::GetFrameAtIndex(uint32_t idx) { } } else if (original_idx == 0) { // There should ALWAYS be a frame at index 0. If something went wrong with - // the CurrentInlinedDepth such that - // there weren't as many frames as we thought taking that into account, then - // reset the current inlined depth + // the CurrentInlinedDepth such that there weren't as many frames as we + // thought taking that into account, then reset the current inlined depth // and return the real zeroth frame. if (m_frames.empty()) { // Why do we have a thread with zero frames, that should not ever @@ -554,11 +540,11 @@ StackFrameSP StackFrameList::GetFrameAtIndex(uint32_t idx) { StackFrameSP StackFrameList::GetFrameWithConcreteFrameIndex(uint32_t unwind_idx) { // First try assuming the unwind index is the same as the frame index. The - // unwind index is always greater than or equal to the frame index, so it - // is a good place to start. If we have inlined frames we might have 5 - // concrete frames (frame unwind indexes go from 0-4), but we might have 15 - // frames after we make all the inlined frames. Most of the time the unwind - // frame index (or the concrete frame index) is the same as the frame index. + // unwind index is always greater than or equal to the frame index, so it is + // a good place to start. If we have inlined frames we might have 5 concrete + // frames (frame unwind indexes go from 0-4), but we might have 15 frames + // after we make all the inlined frames. Most of the time the unwind frame + // index (or the concrete frame index) is the same as the frame index. uint32_t frame_idx = unwind_idx; StackFrameSP frame_sp(GetFrameAtIndex(frame_idx)); while (frame_sp) { @@ -719,9 +705,9 @@ void StackFrameList::Merge(std::unique_ptr<StackFrameList> &curr_ap, #if defined(DEBUG_STACK_FRAMES) s.PutCString("No previous frames, so use current frames...\n"); #endif - // We either don't have any previous frames, or since we have more than - // one current frames it means we have all the frames and can safely - // replace our previous frames. + // We either don't have any previous frames, or since we have more than one + // current frames it means we have all the frames and can safely replace + // our previous frames. prev_sp.reset(curr_ap.release()); return; } @@ -733,8 +719,8 @@ void StackFrameList::Merge(std::unique_ptr<StackFrameList> &curr_ap, s.PutCString( "We have more than one current frame, so use current frames...\n"); #endif - // We have more than one current frames it means we have all the frames - // and can safely replace our previous frames. + // We have more than one current frames it means we have all the frames and + // can safely replace our previous frames. prev_sp.reset(curr_ap.release()); #if defined(DEBUG_STACK_FRAMES) diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index 889cf89b9be..341c902af99 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -69,12 +69,11 @@ bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) { // FIXME: We are assuming that the stacks grow downward in memory. That's not // necessary, but true on // all the machines we care about at present. If this changes, we'll have to - // deal with that. The ABI is the - // agent who knows this ordering, but the StackID has no access to the ABI. - // The most straightforward way - // to handle this is to add a "m_grows_downward" bool to the StackID, and set - // it in the constructor. - // But I'm not going to waste a bool per StackID on this till we need it. + // deal with that. The ABI is the agent who knows this ordering, but the + // StackID has no access to the ABI. The most straightforward way to handle + // this is to add a "m_grows_downward" bool to the StackID, and set it in the + // constructor. But I'm not going to waste a bool per StackID on this till we + // need it. if (lhs_cfa != rhs_cfa) return lhs_cfa < rhs_cfa; diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 652ad8a0544..f8b17dc10ec 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -63,12 +63,10 @@ bool StopInfo::HasTargetRunSinceMe() { return true; } else if (ret_type == eStateStopped) { // This is a little tricky. We want to count "run and stopped again - // before you could - // ask this question as a "TRUE" answer to HasTargetRunSinceMe. But we - // don't want to - // include any running of the target done for expressions. So we track - // both resumes, - // and resumes caused by expressions, and check if there are any resumes + // before you could ask this question as a "TRUE" answer to + // HasTargetRunSinceMe. But we don't want to include any running of the + // target done for expressions. So we track both resumes, and resumes + // caused by expressions, and check if there are any resumes // NOT caused // by expressions. @@ -199,8 +197,7 @@ public: if (bp_site_sp) { StreamString strm; // If we have just hit an internal breakpoint, and it has a kind - // description, print that instead of the - // full breakpoint printing: + // description, print that instead of the full breakpoint printing: if (bp_site_sp->IsInternal()) { size_t num_owners = bp_site_sp->GetNumberOfOwners(); for (size_t idx = 0; idx < num_owners; idx++) { @@ -258,9 +255,9 @@ public: protected: bool ShouldStop(Event *event_ptr) override { - // This just reports the work done by PerformAction or the synchronous stop. - // It should - // only ever get called after they have had a chance to run. + // This just reports the work done by PerformAction or the synchronous + // stop. It should only ever get called after they have had a chance to + // run. assert(m_should_stop_is_valid); return m_should_stop; } @@ -293,54 +290,47 @@ protected: if (bp_site_sp) { // Let's copy the owners list out of the site and store them in a local - // list. That way if - // one of the breakpoint actions changes the site, then we won't be - // operating on a bad list. + // list. That way if one of the breakpoint actions changes the site, + // then we won't be operating on a bad list. BreakpointLocationCollection site_locations; size_t num_owners = bp_site_sp->CopyOwnersList(site_locations); if (num_owners == 0) { m_should_stop = true; } else { - // We go through each location, and test first its precondition - this - // overrides everything. Note, - // we only do this once per breakpoint - not once per location... - // Then check the condition. If the condition says to stop, - // then we run the callback for that location. If that callback says - // to stop as well, then - // we set m_should_stop to true; we are going to stop. - // But we still want to give all the breakpoints whose conditions say - // we are going to stop a - // chance to run their callbacks. - // Of course if any callback restarts the target by putting "continue" - // in the callback, then + // We go through each location, and test first its precondition - + // this overrides everything. Note, we only do this once per + // breakpoint - not once per location... Then check the condition. + // If the condition says to stop, then we run the callback for that + // location. If that callback says to stop as well, then we set + // m_should_stop to true; we are going to stop. But we still want to + // give all the breakpoints whose conditions say we are going to stop + // a chance to run their callbacks. Of course if any callback + // restarts the target by putting "continue" in the callback, then // we're going to restart, without running the rest of the callbacks. - // And in this case we will - // end up not stopping even if another location said we should stop. - // But that's better than not - // running all the callbacks. + // And in this case we will end up not stopping even if another + // location said we should stop. But that's better than not running + // all the callbacks. m_should_stop = false; // We don't select threads as we go through them testing breakpoint - // conditions and running commands. - // So we need to set the thread for expression evaluation here: + // conditions and running commands. So we need to set the thread for + // expression evaluation here: ThreadList::ExpressionExecutionThreadPusher thread_pusher(thread_sp); ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0)); Process *process = exe_ctx.GetProcessPtr(); if (process->GetModIDRef().IsLastResumeForUserExpression()) { // If we are in the middle of evaluating an expression, don't run - // asynchronous breakpoint commands or - // expressions. That could lead to infinite recursion if the - // command or condition re-calls the function - // with this breakpoint. + // asynchronous breakpoint commands or expressions. That could + // lead to infinite recursion if the command or condition re-calls + // the function with this breakpoint. // TODO: We can keep a list of the breakpoints we've seen while // running expressions in the nested // PerformAction calls that can arise when the action runs a - // function that hits another breakpoint, - // and only stop running commands when we see the same breakpoint - // hit a second time. + // function that hits another breakpoint, and only stop running + // commands when we see the same breakpoint hit a second time. m_should_stop_is_valid = true; if (log) @@ -377,12 +367,11 @@ protected: StoppointCallbackContext context(event_ptr, exe_ctx, false); // For safety's sake let's also grab an extra reference to the - // breakpoint owners of the locations we're - // going to examine, since the locations are going to have to get back - // to their breakpoints, and the - // locations don't keep their owners alive. I'm just sticking the - // BreakpointSP's in a vector since - // I'm only using it to locally increment their retain counts. + // breakpoint owners of the locations we're going to examine, since + // the locations are going to have to get back to their breakpoints, + // and the locations don't keep their owners alive. I'm just + // sticking the BreakpointSP's in a vector since I'm only using it to + // locally increment their retain counts. std::vector<lldb::BreakpointSP> location_owners; @@ -404,8 +393,8 @@ protected: continue; // The breakpoint site may have many locations associated with it, - // not all of them valid for - // this thread. Skip the ones that aren't: + // not all of them valid for this thread. Skip the ones that + // aren't: if (!bp_loc_sp->ValidForThisThread(thread_sp.get())) { if (log) { log->Printf("Breakpoint %s hit on thread 0x%llx but it was not " @@ -419,8 +408,7 @@ protected: internal_breakpoint = bp_loc_sp->GetBreakpoint().IsInternal(); // First run the precondition, but since the precondition is per - // breakpoint, only run it once - // per breakpoint. + // breakpoint, only run it once per breakpoint. std::pair<std::unordered_set<break_id_t>::iterator, bool> result = precondition_breakpoints.insert( bp_loc_sp->GetBreakpoint().GetID()); @@ -433,9 +421,8 @@ protected: continue; // Next run the condition for the breakpoint. If that says we - // should stop, then we'll run - // the callback for the breakpoint. If the callback says we - // shouldn't stop that will win. + // should stop, then we'll run the callback for the breakpoint. If + // the callback says we shouldn't stop that will win. if (bp_loc_sp->GetConditionText() != nullptr) { Status condition_error; @@ -470,9 +457,8 @@ protected: } if (!condition_says_stop) { // We don't want to increment the hit count of breakpoints if - // the condition fails. - // We've already bumped it by the time we get here, so undo - // the bump: + // the condition fails. We've already bumped it by the time + // we get here, so undo the bump: bp_loc_sp->UndoBumpHitCount(); continue; } @@ -524,8 +510,8 @@ protected: thread_sp->GetProcess()->GetTarget().RemoveBreakpointByID( bp_loc_sp->GetBreakpoint().GetID()); } - // Also make sure that the callback hasn't continued the target. - // If it did, when we'll set m_should_start to false and get out of + // Also make sure that the callback hasn't continued the target. If + // it did, when we'll set m_should_start to false and get out of // here. if (HasTargetRunSinceMe()) { m_should_stop = false; @@ -551,13 +537,13 @@ protected: if ((m_should_stop == false || internal_breakpoint) && thread_sp->CompletedPlanOverridesBreakpoint()) { - // Override should_stop decision when we have - // completed step plan additionally to the breakpoint + // Override should_stop decision when we have completed step plan + // additionally to the breakpoint m_should_stop = true; - // Here we clean the preset stop info so the next - // GetStopInfo call will find the appropriate stop info, - // which should be the stop info related to the completed plan + // Here we clean the preset stop info so the next GetStopInfo call will + // find the appropriate stop info, which should be the stop info + // related to the completed plan thread_sp->ResetStopInfo(); } @@ -652,8 +638,8 @@ public: protected: bool ShouldStopSynchronous(Event *event_ptr) override { - // ShouldStop() method is idempotent and should not affect hit count. - // See Process::RunPrivateStateThread()->Process()->HandlePrivateEvent() + // ShouldStop() method is idempotent and should not affect hit count. See + // Process::RunPrivateStateThread()->Process()->HandlePrivateEvent() // -->Process()::ShouldBroadcastEvent()->ThreadList::ShouldStop()-> // Thread::ShouldStop()->ThreadPlanBase::ShouldStop()-> // StopInfoWatchpoint::ShouldStop() and @@ -689,9 +675,9 @@ protected: } bool ShouldStop(Event *event_ptr) override { - // This just reports the work done by PerformAction or the synchronous stop. - // It should - // only ever get called after they have had a chance to run. + // This just reports the work done by PerformAction or the synchronous + // stop. It should only ever get called after they have had a chance to + // run. assert(m_should_stop_is_valid); return m_should_stop; } @@ -699,8 +685,8 @@ protected: void PerformAction(Event *event_ptr) override { Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS); // We're going to calculate if we should stop or not in some way during the - // course of - // this code. Also by default we're going to stop, so set that here. + // course of this code. Also by default we're going to stop, so set that + // here. m_should_stop = true; @@ -716,10 +702,8 @@ protected: { // check if this process is running on an architecture where - // watchpoints trigger - // before the associated instruction runs. if so, disable the WP, - // single-step and then - // re-enable the watchpoint + // watchpoints trigger before the associated instruction runs. if so, + // disable the WP, single-step and then re-enable the watchpoint if (process_sp) { uint32_t num; bool wp_triggers_after; @@ -727,10 +711,9 @@ protected: if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after) .Success()) { if (!wp_triggers_after) { - // We need to preserve the watch_index before watchpoint - // is disable. Since Watchpoint::SetEnabled will clear the - // watch index. - // This will fix TestWatchpointIter failure + // We need to preserve the watch_index before watchpoint is + // disable. Since Watchpoint::SetEnabled will clear the watch + // index. This will fix TestWatchpointIter failure Watchpoint *wp = wp_sp.get(); uint32_t watch_index = wp->GetHardwareIndex(); process_sp->DisableWatchpoint(wp, false); @@ -759,8 +742,8 @@ protected: } // This sentry object makes sure the current watchpoint is disabled - // while performing watchpoint actions, - // and it is then enabled after we are finished. + // while performing watchpoint actions, and it is then enabled after we + // are finished. WatchpointSentry sentry(process_sp, wp_sp); /* @@ -789,16 +772,13 @@ protected: // TODO: This condition should be checked in the synchronous part of the // watchpoint code // (Watchpoint::ShouldStop), so that we avoid pulling an event even if - // the watchpoint fails - // the ignore count condition. It is moved here temporarily, because for - // archs with - // watchpoint_exceptions_received=before, the code in the previous lines - // takes care of moving - // the inferior to next PC. We have to check the ignore count condition - // after this is done, - // otherwise we will hit same watchpoint multiple times until we pass - // ignore condition, but we - // won't actually be ignoring them. + // the watchpoint fails the ignore count condition. It is moved here + // temporarily, because for archs with + // watchpoint_exceptions_received=before, the code in the previous + // lines takes care of moving the inferior to next PC. We have to check + // the ignore count condition after this is done, otherwise we will hit + // same watchpoint multiple times until we pass ignore condition, but + // we won't actually be ignoring them. if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) m_should_stop = false; @@ -824,9 +804,9 @@ protected: if (result_value_sp->ResolveValue(scalar_value)) { if (scalar_value.ULongLong(1) == 0) { // We have been vetoed. This takes precedence over querying - // the watchpoint whether it should stop (aka ignore count and - // friends). See also StopInfoWatchpoint::ShouldStop() as - // well as Process::ProcessEventData::DoOnRemoval(). + // the watchpoint whether it should stop (aka ignore count + // and friends). See also StopInfoWatchpoint::ShouldStop() + // as well as Process::ProcessEventData::DoOnRemoval(). m_should_stop = false; } else m_should_stop = true; @@ -877,9 +857,8 @@ protected: debugger.SetAsyncExecution(old_async); - // Also make sure that the callback hasn't continued the target. - // If it did, when we'll set m_should_stop to false and get out of - // here. + // Also make sure that the callback hasn't continued the target. If + // it did, when we'll set m_should_stop to false and get out of here. if (HasTargetRunSinceMe()) m_should_stop = false; diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 69d62d93787..6db30f7c8f6 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -382,8 +382,8 @@ BreakpointSP Target::CreateBreakpoint(lldb::addr_t addr, bool internal, // Check for any reason we want to move this breakpoint to other address. addr = GetBreakableLoadAddress(addr); - // Attempt to resolve our load address if possible, though it is ok if - // it doesn't resolve to section/offset. + // Attempt to resolve our load address if possible, though it is ok if it + // doesn't resolve to section/offset. // Try and resolve as a load address if possible GetSectionLoadList().ResolveLoadAddress(addr, so_addr); @@ -537,8 +537,7 @@ SearchFilterSP Target::GetSearchFilterForModuleAndCUList( SearchFilterSP filter_sp; if (containingModules == nullptr) { // We could make a special "CU List only SearchFilter". Better yet was if - // these could be composable, - // but that will take a little reworking. + // these could be composable, but that will take a little reworking. filter_sp.reset(new SearchFilterByModuleListAndCU( shared_from_this(), FileSpecList(), *containingSourceFiles)); @@ -744,8 +743,8 @@ static bool CheckIfWatchpointsExhausted(Target *target, Status &error) { return true; } -// See also Watchpoint::SetWatchpointType(uint32_t type) and -// the OptionGroupWatchpoint::WatchType enum type. +// See also Watchpoint::SetWatchpointType(uint32_t type) and the +// OptionGroupWatchpoint::WatchType enum type. WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size, const CompilerType *type, uint32_t kind, Status &error) { @@ -776,8 +775,8 @@ WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size, if (!CheckIfWatchpointsExhausted(this, error)) return wp_sp; - // Currently we only support one watchpoint per address, with total number - // of watchpoints limited by the hardware which the inferior is running on. + // Currently we only support one watchpoint per address, with total number of + // watchpoints limited by the hardware which the inferior is running on. // Grab the list mutex while doing operations. const bool notify = false; // Don't notify about all the state changes we do @@ -814,9 +813,8 @@ WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size, wp_sp->GetID()); if (error.Fail()) { - // Enabling the watchpoint on the device side failed. - // Remove the said watchpoint from the list maintained by the target - // instance. + // Enabling the watchpoint on the device side failed. Remove the said + // watchpoint from the list maintained by the target instance. m_watchpoint_list.Remove(wp_sp->GetID(), true); // See if we could provide more helpful error message. if (!OptionGroupWatchpoint::IsWatchSizeSupported(size)) @@ -1028,8 +1026,7 @@ Status Target::SerializeBreakpointsToFile(const FileSpec &file, Breakpoint *bp = GetBreakpointByID(bp_id).get(); StructuredData::ObjectSP bkpt_save_sp = bp->SerializeToStructuredData(); // If the user explicitly asked to serialize a breakpoint, and we - // can't, then - // raise an error: + // can't, then raise an error: if (!bkpt_save_sp) { error.SetErrorStringWithFormat("Unable to serialize breakpoint %d", bp_id); @@ -1142,8 +1139,8 @@ bool Target::RemoveAllWatchpoints(bool end_to_end) { return true; // Success! } -// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end to -// end operations. +// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end +// to end operations. bool Target::DisableAllWatchpoints(bool end_to_end) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS)); if (log) @@ -1172,8 +1169,8 @@ bool Target::DisableAllWatchpoints(bool end_to_end) { return true; // Success! } -// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end to -// end operations. +// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end +// to end operations. bool Target::EnableAllWatchpoints(bool end_to_end) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS)); if (log) @@ -1236,8 +1233,8 @@ bool Target::ClearAllWatchpointHistoricValues() { return true; // Success! } -// Assumption: Caller holds the list mutex lock for m_watchpoint_list -// during these operations. +// Assumption: Caller holds the list mutex lock for m_watchpoint_list during +// these operations. bool Target::IgnoreAllWatchpoints(uint32_t ignore_count) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS)); if (log) @@ -1460,7 +1457,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec) { if (compatible_local_arch || missing_local_arch) { // If we haven't got a valid arch spec, or the architectures are compatible - // update the architecture, unless the one we already have is more specified + // update the architecture, unless the one we already have is more + // specified if (replace_local_arch) m_arch = other; LLDB_LOG(log, "set architecture to {0} ({1})", @@ -1503,8 +1501,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec) { bool Target::MergeArchitecture(const ArchSpec &arch_spec) { if (arch_spec.IsValid()) { if (m_arch.GetSpec().IsCompatibleMatch(arch_spec)) { - // The current target arch is compatible with "arch_spec", see if we - // can improve our current architecture using bits from "arch_spec" + // The current target arch is compatible with "arch_spec", see if we can + // improve our current architecture using bits from "arch_spec" // Merge bits from arch_spec into "merged_arch" and set our architecture ArchSpec merged_arch(m_arch.GetSpec()); @@ -1663,8 +1661,8 @@ size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache, lldb::addr_t *load_addr_ptr) { error.Clear(); - // if we end up reading this from process memory, we will fill this - // with the actual load address + // if we end up reading this from process memory, we will fill this with the + // actual load address if (load_addr_ptr) *load_addr_ptr = LLDB_INVALID_ADDRESS; @@ -1676,16 +1674,16 @@ size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache, if (!addr.IsSectionOffset()) { SectionLoadList §ion_load_list = GetSectionLoadList(); if (section_load_list.IsEmpty()) { - // No sections are loaded, so we must assume we are not running - // yet and anything we are given is a file address. + // No sections are loaded, so we must assume we are not running yet and + // anything we are given is a file address. file_addr = addr.GetOffset(); // "addr" doesn't have a section, so its // offset is the file address m_images.ResolveFileAddress(file_addr, resolved_addr); } else { - // We have at least one section loaded. This can be because - // we have manually loaded some sections with "target modules load ..." - // or because we have have a live process that has sections loaded - // through the dynamic loader + // We have at least one section loaded. This can be because we have + // manually loaded some sections with "target modules load ..." or + // because we have have a live process that has sections loaded through + // the dynamic loader load_addr = addr.GetOffset(); // "addr" doesn't have a section, so its // offset is the load address section_load_list.ResolveLoadAddress(load_addr, resolved_addr); @@ -1732,19 +1730,19 @@ size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache, *load_addr_ptr = load_addr; return bytes_read; } - // If the address is not section offset we have an address that - // doesn't resolve to any address in any currently loaded shared - // libraries and we failed to read memory so there isn't anything - // more we can do. If it is section offset, we might be able to - // read cached memory from the object file. + // If the address is not section offset we have an address that doesn't + // resolve to any address in any currently loaded shared libraries and we + // failed to read memory so there isn't anything more we can do. If it is + // section offset, we might be able to read cached memory from the object + // file. if (!resolved_addr.IsSectionOffset()) return 0; } } if (!prefer_file_cache && resolved_addr.IsSectionOffset()) { - // If we didn't already try and read from the object file cache, then - // try it after failing to read from the process. + // If we didn't already try and read from the object file cache, then try + // it after failing to read from the process. return ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error); } return 0; @@ -1761,8 +1759,8 @@ size_t Target::ReadCStringFromMemory(const Address &addr, std::string &out_str, if (length == 0) break; out_str.append(buf, length); - // If we got "length - 1" bytes, we didn't get the whole C string, we - // need to read some more characters + // If we got "length - 1" bytes, we didn't get the whole C string, we need + // to read some more characters if (length == sizeof(buf) - 1) curr_addr += length; else @@ -1783,9 +1781,9 @@ size_t Target::ReadCStringFromMemory(const Address &addr, char *dst, addr_t curr_addr = addr.GetLoadAddress(this); Address address(addr); - // We could call m_process_sp->GetMemoryCacheLineSize() but I don't - // think this really needs to be tied to the memory cache subsystem's - // cache line size, so leave this as a fixed constant. + // We could call m_process_sp->GetMemoryCacheLineSize() but I don't think + // this really needs to be tied to the memory cache subsystem's cache line + // size, so leave this as a fixed constant. const size_t cache_line_size = 512; size_t bytes_left = dst_max_len - 1; @@ -1876,18 +1874,18 @@ bool Target::ReadPointerFromMemory(const Address &addr, bool prefer_file_cache, if (pointer_vm_addr != LLDB_INVALID_ADDRESS) { SectionLoadList §ion_load_list = GetSectionLoadList(); if (section_load_list.IsEmpty()) { - // No sections are loaded, so we must assume we are not running - // yet and anything we are given is a file address. + // No sections are loaded, so we must assume we are not running yet and + // anything we are given is a file address. m_images.ResolveFileAddress(pointer_vm_addr, pointer_addr); } else { - // We have at least one section loaded. This can be because - // we have manually loaded some sections with "target modules load ..." - // or because we have have a live process that has sections loaded - // through the dynamic loader + // We have at least one section loaded. This can be because we have + // manually loaded some sections with "target modules load ..." or + // because we have have a live process that has sections loaded through + // the dynamic loader section_load_list.ResolveLoadAddress(pointer_vm_addr, pointer_addr); } - // We weren't able to resolve the pointer value, so just return - // an address with no section + // We weren't able to resolve the pointer value, so just return an + // address with no section if (!pointer_addr.IsValid()) pointer_addr.SetOffset(pointer_vm_addr); return true; @@ -1903,9 +1901,8 @@ ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec, Status error; // First see if we already have this module in our module list. If we do, - // then we're done, we don't need - // to consult the shared modules list. But only do this if we are passed a - // UUID. + // then we're done, we don't need to consult the shared modules list. But + // only do this if we are passed a UUID. if (module_spec.GetUUID().IsValid()) module_sp = m_images.FindFirstModule(module_spec); @@ -1960,8 +1957,8 @@ ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec, } // We found a module that wasn't in our target list. Let's make sure that - // there wasn't an equivalent - // module in the list already, and if there was, let's remove it. + // there wasn't an equivalent module in the list already, and if there was, + // let's remove it. if (module_sp) { ObjectFile *objfile = module_sp->GetObjectFile(); if (objfile) { @@ -1995,18 +1992,14 @@ ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec, return ModuleSP(); } // GetSharedModule is not guaranteed to find the old shared module, for - // instance - // in the common case where you pass in the UUID, it is only going to - // find the one - // module matching the UUID. In fact, it has no good way to know what - // the "old module" - // relevant to this target is, since there might be many copies of a - // module with this file spec - // in various running debug sessions, but only one of them will belong - // to this target. - // So let's remove the UUID from the module list, and look in the - // target's module list. - // Only do this if there is SOMETHING else in the module spec... + // instance in the common case where you pass in the UUID, it is only + // going to find the one module matching the UUID. In fact, it has no + // good way to know what the "old module" relevant to this target is, + // since there might be many copies of a module with this file spec in + // various running debug sessions, but only one of them will belong to + // this target. So let's remove the UUID from the module list, and look + // in the target's module list. Only do this if there is SOMETHING else + // in the module spec... if (!old_module_sp) { if (module_spec.GetUUID().IsValid() && !module_spec.GetFileSpec().GetFilename().IsEmpty() && @@ -2286,8 +2279,8 @@ ExpressionResults Target::EvaluateExpression( if (expr.empty()) return execution_results; - // We shouldn't run stop hooks in expressions. - // Be sure to reset this if you return anywhere within this function. + // We shouldn't run stop hooks in expressions. Be sure to reset this if you + // return anywhere within this function. bool old_suppress_value = m_suppress_stop_hooks; m_suppress_stop_hooks = true; @@ -2301,8 +2294,8 @@ ExpressionResults Target::EvaluateExpression( CalculateExecutionContext(exe_ctx); } - // Make sure we aren't just trying to see the value of a persistent - // variable (something like "$0") + // Make sure we aren't just trying to see the value of a persistent variable + // (something like "$0") lldb::ExpressionVariableSP persistent_var_sp; // Only check for persistent variables the expression starts with a '$' if (expr[0] == '$') @@ -2404,8 +2397,7 @@ lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr, code_addr |= 1ull; } else if (addr_class == eAddressClassCodeAlternateISA) { // We checked the address and the address claims to be the alternate - // ISA - // which means thumb, so set bit zero. + // ISA which means thumb, so set bit zero. code_addr |= 1ull; } } @@ -2471,8 +2463,8 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) { SectionLoadList §ion_load_list = GetSectionLoadList(); if (section_load_list.IsEmpty()) - // No sections are loaded, so we must assume we are not running yet - // and need to operate only on file address. + // No sections are loaded, so we must assume we are not running yet and + // need to operate only on file address. m_images.ResolveFileAddress(addr, resolved_addr); else section_load_list.ResolveLoadAddress(addr, resolved_addr); @@ -2547,30 +2539,29 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) { } else if (i == 2) { // Here we may get one 4-byte instruction or two 2-byte instructions. if (num_insns == 2) { - // Looks like there are two 2-byte instructions above our breakpoint - // target address. - // Now the upper 2-byte instruction is either a valid 2-byte - // instruction or could be a part of it's upper 4-byte instruction. - // In both cases we don't care because in this case lower 2-byte - // instruction is definitely a valid instruction - // and whatever i=1 iteration has found out is true. + // Looks like there are two 2-byte instructions above our + // breakpoint target address. Now the upper 2-byte instruction is + // either a valid 2-byte instruction or could be a part of it's + // upper 4-byte instruction. In both cases we don't care because in + // this case lower 2-byte instruction is definitely a valid + // instruction and whatever i=1 iteration has found out is true. inst_to_choose = 1; break; } else if (insn_size == 4) { // This instruction claims its a valid 4-byte instruction. But it - // could be a part of it's upper 4-byte instruction. - // Lets try scanning upper 2 bytes to verify this. + // could be a part of it's upper 4-byte instruction. Lets try + // scanning upper 2 bytes to verify this. instruction_list.Append(prev_insn); inst_to_choose = 2; } } else if (i == 3) { if (insn_size == 4) // FIXME: We reached here that means instruction at [target - 4] has - // already claimed to be a 4-byte instruction, - // and now instruction at [target - 6] is also claiming that it's a - // 4-byte instruction. This can not be true. - // In this case we can not decide the valid previous instruction so - // we let lldb set the breakpoint at the address given by user. + // already claimed to be a 4-byte instruction, and now instruction + // at [target - 6] is also claiming that it's a 4-byte instruction. + // This can not be true. In this case we can not decide the valid + // previous instruction so we let lldb set the breakpoint at the + // address given by user. inst_to_choose = 0; else // This is straight-forward @@ -2686,8 +2677,8 @@ void Target::RunStopHooks() { return; // <rdar://problem/12027563> make sure we check that we are not stopped - // because of us running a user expression - // since in that case we do not want to run the stop-hooks + // because of us running a user expression since in that case we do not want + // to run the stop-hooks if (m_process_sp->GetModIDRef().IsLastResumeForUserExpression()) return; @@ -2976,18 +2967,16 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) { launch_info.GetFlags().Set(eLaunchFlagDebug); // Get the value of synchronous execution here. If you wait till after you - // have started to - // run, then you could have hit a breakpoint, whose command might switch the - // value, and - // then you'll pick up that incorrect value. + // have started to run, then you could have hit a breakpoint, whose command + // might switch the value, and then you'll pick up that incorrect value. Debugger &debugger = GetDebugger(); const bool synchronous_execution = debugger.GetCommandInterpreter().GetSynchronous(); PlatformSP platform_sp(GetPlatform()); - // Finalize the file actions, and if none were given, default to opening - // up a pseudo terminal + // Finalize the file actions, and if none were given, default to opening up a + // pseudo terminal const bool default_to_use_pty = platform_sp ? platform_sp->IsHost() : false; if (log) log->Printf("Target::%s have platform=%s, platform_sp->IsHost()=%s, " @@ -3026,10 +3015,10 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) { GetPlatform()->DebugProcess(launch_info, debugger, this, error); // Cleanup the old process since someone might still have a strong - // reference to this process and we would like to allow it to cleanup - // as much as it can without the object being destroyed. We try to - // lock the shared pointer and if that works, then someone else still - // has a strong reference to the process. + // reference to this process and we would like to allow it to cleanup as + // much as it can without the object being destroyed. We try to lock the + // shared pointer and if that works, then someone else still has a strong + // reference to the process. ProcessSP old_process_sp(process_wp.lock()); if (old_process_sp) @@ -3153,8 +3142,8 @@ Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) { const ModuleSP old_exec_module_sp = GetExecutableModule(); - // If no process info was specified, then use the target executable - // name as the process to attach to by default + // If no process info was specified, then use the target executable name as + // the process to attach to by default if (!attach_info.ProcessInfoSpecified()) { if (old_exec_module_sp) attach_info.GetExecutableFile().GetFilename() = @@ -3581,9 +3570,8 @@ public: bool will_modify, uint32_t idx) const override { // When getting the value for a key from the target options, we will always - // try and grab the setting from the current target if there is one. Else we - // just - // use the one from this instance. + // try and grab the setting from the current target if there is one. Else + // we just use the one from this instance. if (idx == ePropertyEnvVars) GetHostEnvironmentIfNeeded(); @@ -3674,8 +3662,8 @@ TargetProperties::TargetProperties(Target *target) m_collection_sp.reset( new TargetOptionValueProperties(target, Target::GetGlobalProperties())); - // Set callbacks to update launch_info whenever "settins set" updated any of - // these properties + // Set callbacks to update launch_info whenever "settins set" updated any + // of these properties m_collection_sp->SetValueChangedCallback( ePropertyArg0, TargetProperties::Arg0ValueChangedCallback, this); m_collection_sp->SetValueChangedCallback( diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp index 5c652aced39..b245a6d167f 100644 --- a/lldb/source/Target/TargetList.cpp +++ b/lldb/source/Target/TargetList.cpp @@ -84,9 +84,9 @@ Status TargetList::CreateTargetInternal( Status error; PlatformSP platform_sp; - // This is purposely left empty unless it is specified by triple_cstr. - // If not initialized via triple_cstr, then the currently selected platform - // will set the architecture correctly. + // This is purposely left empty unless it is specified by triple_cstr. If not + // initialized via triple_cstr, then the currently selected platform will set + // the architecture correctly. const ArchSpec arch(triple_str); if (!triple_str.empty()) { if (!arch.IsValid()) { @@ -123,8 +123,7 @@ Status TargetList::CreateTargetInternal( module_spec.GetFileSpec().SetFile(user_exe_path, true); // Resolve the executable in case we are given a path to a application - // bundle - // like a .app bundle on MacOSX + // bundle like a .app bundle on MacOSX Host::ResolveExecutableInBundle(module_spec.GetFileSpec()); lldb::offset_t file_offset = 0; @@ -140,7 +139,8 @@ Status TargetList::CreateTargetInternal( if (platform_arch.IsCompatibleMatch( matching_module_spec.GetArchitecture())) { // If the OS or vendor weren't specified, then adopt the module's - // architecture so that the platform matching can be more accurate + // architecture so that the platform matching can be more + // accurate if (!platform_arch.TripleOSWasSpecified() || !platform_arch.TripleVendorWasSpecified()) { prefer_platform_arch = true; @@ -194,7 +194,8 @@ Status TargetList::CreateTargetInternal( } } - // Next check the host platform it if wasn't already checked above + // Next check the host platform it if wasn't already checked + // above if (host_platform_sp && (!platform_sp || host_platform_sp->GetName() != platform_sp->GetName())) { @@ -275,8 +276,7 @@ Status TargetList::CreateTargetInternal( } } else if (platform_arch.IsValid()) { // if "arch" isn't valid, yet "platform_arch" is, it means we have an - // executable file with - // a single architecture which should be used + // executable file with a single architecture which should be used ArchSpec fixed_platform_arch; if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, &fixed_platform_arch)) { @@ -410,8 +410,8 @@ Status TargetList::CreateTargetInternal(Debugger &debugger, sizeof(resolved_bundle_exe_path)); } } else { - // No file was specified, just create an empty target with any arch - // if a valid arch was specified + // No file was specified, just create an empty target with any arch if a + // valid arch was specified target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target)); } diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index 217cbccedf6..6bf6d42a87a 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -105,9 +105,8 @@ public: bool will_modify, uint32_t idx) const override { // When getting the value for a key from the thread options, we will always - // try and grab the setting from the current thread if there is one. Else we - // just - // use the one from this instance. + // try and grab the setting from the current thread if there is one. Else + // we just use the one from this instance. if (exe_ctx) { Thread *thread = exe_ctx->GetThreadPtr(); if (thread) { @@ -275,8 +274,8 @@ Thread::~Thread() { void Thread::DestroyThread() { // Tell any plans on the plan stacks that the thread is being destroyed since - // any plans that have a thread go away in the middle of might need - // to do cleanup, or in some cases NOT do cleanup... + // any plans that have a thread go away in the middle of might need to do + // cleanup, or in some cases NOT do cleanup... for (auto plan : m_plan_stack) plan->ThreadDestroyed(); @@ -291,11 +290,10 @@ void Thread::DestroyThread() { m_discarded_plan_stack.clear(); m_completed_plan_stack.clear(); - // Push a ThreadPlanNull on the plan stack. That way we can continue assuming - // that the - // plan stack is never empty, but if somebody errantly asks questions of a - // destroyed thread - // without checking first whether it is destroyed, they won't crash. + // Push a ThreadPlanNull on the plan stack. That way we can continue + // assuming that the plan stack is never empty, but if somebody errantly asks + // questions of a destroyed thread without checking first whether it is + // destroyed, they won't crash. ThreadPlanSP null_plan_sp(new ThreadPlanNull(*this)); m_plan_stack.push_back(null_plan_sp); @@ -385,11 +383,10 @@ lldb::StopInfoSP Thread::GetStopInfo() { ProcessSP process_sp(GetProcess()); const uint32_t stop_id = process_sp ? process_sp->GetStopID() : UINT32_MAX; - // Here we select the stop info according to priorirty: - // - m_stop_info_sp (if not trace) - preset value - // - completed plan stop info - new value with plan from completed plan stack - // - m_stop_info_sp (trace stop reason is OK now) - // - ask GetPrivateStopInfo to set stop info + // Here we select the stop info according to priorirty: - m_stop_info_sp (if + // not trace) - preset value - completed plan stop info - new value with plan + // from completed plan stack - m_stop_info_sp (trace stop reason is OK now) - + // ask GetPrivateStopInfo to set stop info bool have_valid_stop_info = m_stop_info_sp && m_stop_info_sp ->IsValid() && @@ -432,13 +429,13 @@ lldb::StopInfoSP Thread::GetPrivateStopInfo() { } } - // The stop info can be manually set by calling Thread::SetStopInfo() - // prior to this function ever getting called, so we can't rely on - // "m_stop_info_stop_id != process_stop_id" as the condition for - // the if statement below, we must also check the stop info to see - // if we need to override it. See the header documentation in - // Process::GetStopInfoOverrideCallback() for more information on - // the stop info override callback. + // The stop info can be manually set by calling Thread::SetStopInfo() prior + // to this function ever getting called, so we can't rely on + // "m_stop_info_stop_id != process_stop_id" as the condition for the if + // statement below, we must also check the stop info to see if we need to + // override it. See the header documentation in + // Process::GetStopInfoOverrideCallback() for more information on the stop + // info override callback. if (m_stop_info_override_stop_id != process_stop_id) { m_stop_info_override_stop_id = process_stop_id; if (m_stop_info_sp) { @@ -509,9 +506,8 @@ void Thread::SetShouldReportStop(Vote vote) { void Thread::SetStopInfoToNothing() { // Note, we can't just NULL out the private reason, or the native thread - // implementation will try to - // go calculate it again. For now, just set it to a Unix Signal with an - // invalid signal number. + // implementation will try to go calculate it again. For now, just set it to + // a Unix Signal with an invalid signal number. SetStopInfo( StopInfo::CreateStopReasonWithSignal(*this, LLDB_INVALID_SIGNAL_NUMBER)); } @@ -604,10 +600,8 @@ void Thread::WillStop() { void Thread::SetupForResume() { if (GetResumeState() != eStateSuspended) { // If we're at a breakpoint push the step-over breakpoint plan. Do this - // before - // telling the current plan it will resume, since we might change what the - // current - // plan is. + // before telling the current plan it will resume, since we might change + // what the current plan is. lldb::RegisterContextSP reg_ctx_sp(GetRegisterContext()); if (reg_ctx_sp) { @@ -615,9 +609,8 @@ void Thread::SetupForResume() { BreakpointSiteSP bp_site_sp = GetProcess()->GetBreakpointSiteList().FindByAddress(thread_pc); if (bp_site_sp) { - // Note, don't assume there's a ThreadPlanStepOverBreakpoint, the target - // may not require anything - // special to step over a breakpoint. + // Note, don't assume there's a ThreadPlanStepOverBreakpoint, the + // target may not require anything special to step over a breakpoint. ThreadPlan *cur_plan = GetCurrentPlan(); @@ -669,11 +662,9 @@ bool Thread::ShouldResume(StateType resume_state) { GetPrivateStopInfo(); // This is a little dubious, but we are trying to limit how often we actually - // fetch stop info from - // the target, 'cause that slows down single stepping. So assume that if we - // got to the point where - // we're about to resume, and we haven't yet had to fetch the stop reason, - // then it doesn't need to know + // fetch stop info from the target, 'cause that slows down single stepping. + // So assume that if we got to the point where we're about to resume, and we + // haven't yet had to fetch the stop reason, then it doesn't need to know // about the fact that we are resuming... const uint32_t process_stop_id = GetProcess()->GetStopID(); if (m_stop_info_stop_id == process_stop_id && @@ -684,9 +675,9 @@ bool Thread::ShouldResume(StateType resume_state) { } // Tell all the plans that we are about to resume in case they need to clear - // any state. - // We distinguish between the plan on the top of the stack and the lower - // plans in case a plan needs to do any special business before it runs. + // any state. We distinguish between the plan on the top of the stack and the + // lower plans in case a plan needs to do any special business before it + // runs. bool need_to_resume = false; ThreadPlan *plan_ptr = GetCurrentPlan(); @@ -698,8 +689,7 @@ bool Thread::ShouldResume(StateType resume_state) { } // If the WillResume for the plan says we are faking a resume, then it will - // have set an appropriate stop info. - // In that case, don't reset it here. + // have set an appropriate stop info. In that case, don't reset it here. if (need_to_resume && resume_state != eStateSuspended) { m_stop_info_sp.reset(); @@ -743,9 +733,8 @@ bool Thread::ShouldStop(Event *event_ptr) { } // Based on the current thread plan and process stop info, check if this - // thread caused the process to stop. NOTE: this must take place before - // the plan is moved from the current plan stack to the completed plan - // stack. + // thread caused the process to stop. NOTE: this must take place before the + // plan is moved from the current plan stack to the completed plan stack. if (!ThreadStoppedForAReason()) { if (log) log->Printf("Thread::%s for tid = 0x%4.4" PRIx64 " 0x%4.4" PRIx64 @@ -775,10 +764,9 @@ bool Thread::ShouldStop(Event *event_ptr) { current_plan->DoTraceLog(); // First query the stop info's ShouldStopSynchronous. This handles - // "synchronous" stop reasons, for example the breakpoint - // command on internal breakpoints. If a synchronous stop reason says we - // should not stop, then we don't have to - // do any more work on this stop. + // "synchronous" stop reasons, for example the breakpoint command on internal + // breakpoints. If a synchronous stop reason says we should not stop, then + // we don't have to do any more work on this stop. StopInfoSP private_stop_info(GetPrivateStopInfo()); if (private_stop_info && !private_stop_info->ShouldStopSynchronous(event_ptr)) { @@ -788,8 +776,8 @@ bool Thread::ShouldStop(Event *event_ptr) { return false; } - // If we've already been restarted, don't query the plans since the state they - // would examine is not current. + // If we've already been restarted, don't query the plans since the state + // they would examine is not current. if (Process::ProcessEventData::GetRestartedFromEvent(event_ptr)) return false; @@ -798,12 +786,10 @@ bool Thread::ShouldStop(Event *event_ptr) { GetStackFrameList()->CalculateCurrentInlinedDepth(); // If the base plan doesn't understand why we stopped, then we have to find a - // plan that does. - // If that plan is still working, then we don't need to do any more work. If - // the plan that explains - // the stop is done, then we should pop all the plans below it, and pop it, - // and then let the plans above it decide - // whether they still need to do more work. + // plan that does. If that plan is still working, then we don't need to do + // any more work. If the plan that explains the stop is done, then we should + // pop all the plans below it, and pop it, and then let the plans above it + // decide whether they still need to do more work. bool done_processing_current_plan = false; @@ -812,16 +798,16 @@ bool Thread::ShouldStop(Event *event_ptr) { done_processing_current_plan = true; should_stop = false; } else { - // If the current plan doesn't explain the stop, then find one that - // does and let it handle the situation. + // If the current plan doesn't explain the stop, then find one that does + // and let it handle the situation. ThreadPlan *plan_ptr = current_plan; while ((plan_ptr = GetPreviousPlan(plan_ptr)) != nullptr) { if (plan_ptr->PlanExplainsStop(event_ptr)) { should_stop = plan_ptr->ShouldStop(event_ptr); - // plan_ptr explains the stop, next check whether plan_ptr is done, if - // so, then we should take it - // and all the plans below it off the stack. + // plan_ptr explains the stop, next check whether plan_ptr is done, + // if so, then we should take it and all the plans below it off the + // stack. if (plan_ptr->MischiefManaged()) { // We're going to pop the plans up to and including the plan that @@ -833,9 +819,9 @@ bool Thread::ShouldStop(Event *event_ptr) { current_plan->WillStop(); PopPlan(); } while ((current_plan = GetCurrentPlan()) != prev_plan_ptr); - // Now, if the responsible plan was not "Okay to discard" then we're - // done, - // otherwise we forward this to the next plan in the stack below. + // Now, if the responsible plan was not "Okay to discard" then + // we're done, otherwise we forward this to the next plan in the + // stack below. done_processing_current_plan = (plan_ptr->IsMasterPlan() && !plan_ptr->OkayToDiscard()); } else @@ -860,9 +846,9 @@ bool Thread::ShouldStop(Event *event_ptr) { if (log) log->Printf("Base plan says should stop: %i.", should_stop); } else { - // Otherwise, don't let the base plan override what the other plans say to - // do, since - // presumably if there were other plans they would know what to do... + // Otherwise, don't let the base plan override what the other plans say + // to do, since presumably if there were other plans they would know what + // to do... while (1) { if (PlanIsBasePlan(current_plan)) break; @@ -875,9 +861,8 @@ bool Thread::ShouldStop(Event *event_ptr) { if (should_stop) current_plan->WillStop(); - // If a Master Plan wants to stop, and wants to stick on the stack, we - // let it. - // Otherwise, see if the plan's parent wants to stop. + // If a Master Plan wants to stop, and wants to stick on the stack, + // we let it. Otherwise, see if the plan's parent wants to stop. if (should_stop && current_plan->IsMasterPlan() && !current_plan->OkayToDiscard()) { @@ -902,18 +887,16 @@ bool Thread::ShouldStop(Event *event_ptr) { } // One other potential problem is that we set up a master plan, then stop in - // before it is complete - for instance - // by hitting a breakpoint during a step-over - then do some step/finish/etc - // operations that wind up - // past the end point condition of the initial plan. We don't want to strand - // the original plan on the stack, - // This code clears stale plans off the stack. + // before it is complete - for instance by hitting a breakpoint during a + // step-over - then do some step/finish/etc operations that wind up past the + // end point condition of the initial plan. We don't want to strand the + // original plan on the stack, This code clears stale plans off the stack. if (should_stop) { ThreadPlan *plan_ptr = GetCurrentPlan(); - // Discard the stale plans and all plans below them in the stack, - // plus move the completed plans to the completed plan stack + // Discard the stale plans and all plans below them in the stack, plus move + // the completed plans to the completed plan stack while (!PlanIsBasePlan(plan_ptr)) { bool stale = plan_ptr->IsPlanStale(); ThreadPlan *examined_plan = plan_ptr; @@ -928,8 +911,9 @@ bool Thread::ShouldStop(Event *event_ptr) { DiscardPlan(); } if (examined_plan->IsPlanComplete()) { - // plan is complete but does not explain the stop (example: step to a line - // with breakpoint), let us move the plan to completed_plan_stack anyway + // plan is complete but does not explain the stop (example: step to a + // line with breakpoint), let us move the plan to + // completed_plan_stack anyway PopPlan(); } else DiscardPlan(); @@ -1097,8 +1081,8 @@ void Thread::DiscardPlan() { } ThreadPlan *Thread::GetCurrentPlan() { - // There will always be at least the base plan. If somebody is mucking with a - // thread with an empty plan stack, we should assert right away. + // There will always be at least the base plan. If somebody is mucking with + // a thread with an empty plan stack, we should assert right away. return m_plan_stack.empty() ? nullptr : m_plan_stack.back().get(); } @@ -1212,8 +1196,7 @@ void Thread::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) { bool Thread::DiscardUserThreadPlansUpToIndex(uint32_t thread_index) { // Count the user thread plans from the back end to get the number of the one - // we want - // to discard: + // we want to discard: uint32_t idx = 0; ThreadPlan *up_to_plan_ptr = nullptr; @@ -1249,8 +1232,7 @@ void Thread::DiscardThreadPlansUpToPlan(ThreadPlan *up_to_plan_ptr) { int stack_size = m_plan_stack.size(); // If the input plan is nullptr, discard all plans. Otherwise make sure this - // plan is in the - // stack, and if so discard up to and including it. + // plan is in the stack, and if so discard up to and including it. if (up_to_plan_ptr == nullptr) { for (int i = stack_size - 1; i > 0; i--) @@ -1314,8 +1296,7 @@ void Thread::DiscardThreadPlans(bool force) { // Now discard the master plan itself. // The bottom-most plan never gets discarded. "OkayToDiscard" for it - // means - // discard it's dependent plans, but not it... + // means discard it's dependent plans, but not it... if (master_plan_idx > 0) { DiscardPlan(); } @@ -1340,8 +1321,7 @@ Status Thread::UnwindInnermostExpression() { int stack_size = m_plan_stack.size(); // If the input plan is nullptr, discard all plans. Otherwise make sure this - // plan is in the - // stack, and if so discard up to and including it. + // plan is in the stack, and if so discard up to and including it. for (int i = stack_size - 1; i > 0; i--) { if (m_plan_stack[i]->GetKind() == ThreadPlan::eKindCallFunction) { @@ -1380,7 +1360,8 @@ ThreadPlanSP Thread::QueueThreadPlanForStepOverRange( return thread_plan_sp; } -// Call the QueueThreadPlanForStepOverRange method which takes an address range. +// Call the QueueThreadPlanForStepOverRange method which takes an address +// range. ThreadPlanSP Thread::QueueThreadPlanForStepOverRange( bool abort_other_plans, const LineEntry &line_entry, const SymbolContext &addr_context, lldb::RunMode stop_other_threads, @@ -1500,11 +1481,9 @@ lldb::ThreadPlanSP Thread::QueueThreadPlanForStepScripted( ThreadPlanSP thread_plan_sp(new ThreadPlanPython(*this, class_name)); QueueThreadPlan(thread_plan_sp, abort_other_plans); // This seems a little funny, but I don't want to have to split up the - // constructor and the - // DidPush in the scripted plan, that seems annoying. - // That means the constructor has to be in DidPush. - // So I have to validate the plan AFTER pushing it, and then take it off - // again... + // constructor and the DidPush in the scripted plan, that seems annoying. + // That means the constructor has to be in DidPush. So I have to validate the + // plan AFTER pushing it, and then take it off again... if (!thread_plan_sp->ValidatePlan(nullptr)) { DiscardThreadPlansUpToPlan(thread_plan_sp); return ThreadPlanSP(); @@ -1701,10 +1680,9 @@ Status Thread::ReturnFromFrame(lldb::StackFrameSP frame_sp, return return_error; } - // Now write the return registers for the chosen frame: - // Note, we can't use ReadAllRegisterValues->WriteAllRegisterValues, since the - // read & write - // cook their data + // Now write the return registers for the chosen frame: Note, we can't use + // ReadAllRegisterValues->WriteAllRegisterValues, since the read & write cook + // their data StackFrameSP youngest_frame_sp = thread->GetStackFrameAtIndex(0); if (youngest_frame_sp) { @@ -1754,13 +1732,11 @@ Status Thread::JumpToLine(const FileSpec &file, uint32_t line, target->GetImages().FindAddressesForLine(target_sp, file, line, sc.function, within_function, outside_function); - // If possible, we try and stay within the current function. - // Within a function, we accept multiple locations (optimized code may do - // this, - // there's no solution here so we do the best we can). - // However if we're trying to leave the function, we don't know how to pick - // the - // right location, so if there's more than one then we bail. + // If possible, we try and stay within the current function. Within a + // function, we accept multiple locations (optimized code may do this, + // there's no solution here so we do the best we can). However if we're + // trying to leave the function, we don't know how to pick the right + // location, so if there's more than one then we bail. if (!within_function.empty()) candidates = within_function; else if (outside_function.size() == 1 && can_leave_function) @@ -1836,8 +1812,8 @@ lldb::addr_t Thread::GetThreadPointer() { return LLDB_INVALID_ADDRESS; } addr_t Thread::GetThreadLocalData(const ModuleSP module, lldb::addr_t tls_file_addr) { - // The default implementation is to ask the dynamic loader for it. - // This can be overridden for specific platforms. + // The default implementation is to ask the dynamic loader for it. This can + // be overridden for specific platforms. DynamicLoader *loader = GetProcess()->GetDynamicLoader(); if (loader) return loader->GetThreadLocalData(module, shared_from_this(), @@ -2096,10 +2072,8 @@ void Thread::Flush() { bool Thread::IsStillAtLastBreakpointHit() { // If we are currently stopped at a breakpoint, always return that stopinfo - // and don't reset it. - // This allows threads to maintain their breakpoint stopinfo, such as when - // thread-stepping in - // multithreaded programs. + // and don't reset it. This allows threads to maintain their breakpoint + // stopinfo, such as when thread-stepping in multithreaded programs. if (m_stop_info_sp) { StopReason stop_reason = m_stop_info_sp->GetStopReason(); if (stop_reason == lldb::eStopReasonBreakpoint) { diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 61483914644..ee57a401f74 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -40,8 +40,8 @@ ThreadList::ThreadList(const ThreadList &rhs) const ThreadList &ThreadList::operator=(const ThreadList &rhs) { if (this != &rhs) { - // Lock both mutexes to make sure neither side changes anyone on us - // while the assignment occurs + // Lock both mutexes to make sure neither side changes anyone on us while + // the assignment occurs std::lock_guard<std::recursive_mutex> guard(GetMutex()); std::lock_guard<std::recursive_mutex> rhs_guard(rhs.GetMutex()); @@ -54,9 +54,8 @@ const ThreadList &ThreadList::operator=(const ThreadList &rhs) { } ThreadList::~ThreadList() { - // Clear the thread list. Clear will take the mutex lock - // which will ensure that if anyone is using the list - // they won't get it removed while using it. + // Clear the thread list. Clear will take the mutex lock which will ensure + // that if anyone is using the list they won't get it removed while using it. Clear(); } @@ -231,13 +230,13 @@ bool ThreadList::ShouldStop(Event *event_ptr) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); - // The ShouldStop method of the threads can do a whole lot of work, - // figuring out whether the thread plan conditions are met. So we don't want - // to keep the ThreadList locked the whole time we are doing this. + // The ShouldStop method of the threads can do a whole lot of work, figuring + // out whether the thread plan conditions are met. So we don't want to keep + // the ThreadList locked the whole time we are doing this. // FIXME: It is possible that running code could cause new threads - // to be created. If that happens, we will miss asking them whether - // they should stop. This is not a big deal since we haven't had - // a chance to hang any interesting operations on those threads yet. + // to be created. If that happens, we will miss asking them whether they + // should stop. This is not a big deal since we haven't had a chance to hang + // any interesting operations on those threads yet. collection threads_copy; { @@ -247,25 +246,21 @@ bool ThreadList::ShouldStop(Event *event_ptr) { m_process->UpdateThreadListIfNeeded(); for (lldb::ThreadSP thread_sp : m_threads) { // This is an optimization... If we didn't let a thread run in between - // the previous stop and this - // one, we shouldn't have to consult it for ShouldStop. So just leave it - // off the list we are going to - // inspect. - // On Linux, if a thread-specific conditional breakpoint was hit, it won't - // necessarily be the thread - // that hit the breakpoint itself that evaluates the conditional - // expression, so the thread that hit - // the breakpoint could still be asked to stop, even though it hasn't been - // allowed to run since the - // previous stop. + // the previous stop and this one, we shouldn't have to consult it for + // ShouldStop. So just leave it off the list we are going to inspect. On + // Linux, if a thread-specific conditional breakpoint was hit, it won't + // necessarily be the thread that hit the breakpoint itself that + // evaluates the conditional expression, so the thread that hit the + // breakpoint could still be asked to stop, even though it hasn't been + // allowed to run since the previous stop. if (thread_sp->GetTemporaryResumeState() != eStateSuspended || thread_sp->IsStillAtLastBreakpointHit()) threads_copy.push_back(thread_sp); } // It is possible the threads we were allowing to run all exited and then - // maybe the user interrupted - // or something, then fall back on looking at all threads: + // maybe the user interrupted or something, then fall back on looking at + // all threads: if (threads_copy.size() == 0) threads_copy = m_threads; @@ -296,12 +291,11 @@ bool ThreadList::ShouldStop(Event *event_ptr) { } // Now we run through all the threads and get their stop info's. We want to - // make sure to do this first before - // we start running the ShouldStop, because one thread's ShouldStop could - // destroy information (like deleting a - // thread specific breakpoint another thread had stopped at) which could lead - // us to compute the StopInfo incorrectly. - // We don't need to use it here, we just want to make sure it gets computed. + // make sure to do this first before we start running the ShouldStop, because + // one thread's ShouldStop could destroy information (like deleting a thread + // specific breakpoint another thread had stopped at) which could lead us to + // compute the StopInfo incorrectly. We don't need to use it here, we just + // want to make sure it gets computed. for (pos = threads_copy.begin(); pos != end; ++pos) { ThreadSP thread_sp(*pos); @@ -312,27 +306,24 @@ bool ThreadList::ShouldStop(Event *event_ptr) { ThreadSP thread_sp(*pos); // We should never get a stop for which no thread had a stop reason, but - // sometimes we do see this - - // for instance when we first connect to a remote stub. In that case we - // should stop, since we can't figure out - // the right thing to do and stopping gives the user control over what to do - // in this instance. + // sometimes we do see this - for instance when we first connect to a + // remote stub. In that case we should stop, since we can't figure out the + // right thing to do and stopping gives the user control over what to do in + // this instance. // // Note, this causes a problem when you have a thread specific breakpoint, - // and a bunch of threads hit the breakpoint, - // but not the thread which we are waiting for. All the threads that are - // not "supposed" to hit the breakpoint - // are marked as having no stop reason, which is right, they should not show - // a stop reason. But that triggers this - // code and causes us to stop seemingly for no reason. + // and a bunch of threads hit the breakpoint, but not the thread which we + // are waiting for. All the threads that are not "supposed" to hit the + // breakpoint are marked as having no stop reason, which is right, they + // should not show a stop reason. But that triggers this code and causes + // us to stop seemingly for no reason. // // Since the only way we ever saw this error was on first attach, I'm only - // going to trigger set did_anybody_stop_for_a_reason - // to true unless this is the first stop. + // going to trigger set did_anybody_stop_for_a_reason to true unless this + // is the first stop. // // If this becomes a problem, we'll have to have another StopReason like - // "StopInfoHidden" which will look invalid - // everywhere but at this check. + // "StopInfoHidden" which will look invalid everywhere but at this check. if (thread_sp->GetProcess()->GetStopID() > 1) did_anybody_stop_for_a_reason = true; @@ -379,8 +370,8 @@ Vote ThreadList::ShouldReportStop(Event *event_ptr) { log->Printf("ThreadList::%s %" PRIu64 " threads", __FUNCTION__, (uint64_t)m_threads.size()); - // Run through the threads and ask whether we should report this event. - // For stopping, a YES vote wins over everything. A NO vote wins over NO + // Run through the threads and ask whether we should report this event. For + // stopping, a YES vote wins over everything. A NO vote wins over NO // opinion. for (pos = m_threads.begin(); pos != end; ++pos) { ThreadSP thread_sp(*pos); @@ -427,8 +418,8 @@ Vote ThreadList::ShouldReportRun(Event *event_ptr) { m_process->UpdateThreadListIfNeeded(); collection::iterator pos, end = m_threads.end(); - // Run through the threads and ask whether we should report this event. - // The rule is NO vote wins over everything, a YES vote wins over no opinion. + // Run through the threads and ask whether we should report this event. The + // rule is NO vote wins over everything, a YES vote wins over no opinion. Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); @@ -485,8 +476,8 @@ void ThreadList::RefreshStateAfterStop() { } void ThreadList::DiscardThreadPlans() { - // You don't need to update the thread list here, because only threads - // that you currently know about have any thread plans. + // You don't need to update the thread list here, because only threads that + // you currently know about have any thread plans. std::lock_guard<std::recursive_mutex> guard(GetMutex()); collection::iterator pos, end = m_threads.end(); @@ -495,9 +486,9 @@ void ThreadList::DiscardThreadPlans() { } bool ThreadList::WillResume() { - // Run through the threads and perform their momentary actions. - // But we only do this for threads that are running, user suspended - // threads stay where they are. + // Run through the threads and perform their momentary actions. But we only + // do this for threads that are running, user suspended threads stay where + // they are. std::lock_guard<std::recursive_mutex> guard(GetMutex()); m_process->UpdateThreadListIfNeeded(); @@ -505,14 +496,11 @@ bool ThreadList::WillResume() { collection::iterator pos, end = m_threads.end(); // See if any thread wants to run stopping others. If it does, then we won't - // setup the other threads for resume, since they aren't going to get a chance - // to run. This is necessary because the SetupForResume might add - // "StopOthers" - // plans which would then get to be part of the who-gets-to-run negotiation, - // but - // they're coming in after the fact, and the threads that are already set up - // should - // take priority. + // setup the other threads for resume, since they aren't going to get a + // chance to run. This is necessary because the SetupForResume might add + // "StopOthers" plans which would then get to be part of the who-gets-to-run + // negotiation, but they're coming in after the fact, and the threads that + // are already set up should take priority. bool wants_solo_run = false; @@ -544,11 +532,9 @@ bool ThreadList::WillResume() { } // Give all the threads that are likely to run a last chance to set up their - // state before we - // negotiate who is actually going to get a chance to run... + // state before we negotiate who is actually going to get a chance to run... // Don't set to resume suspended threads, and if any thread wanted to stop - // others, only - // call setup on the threads that request StopOthers... + // others, only call setup on the threads that request StopOthers... for (pos = m_threads.begin(); pos != end; ++pos) { if ((*pos)->GetResumeState() != eStateSuspended && @@ -650,13 +636,12 @@ void ThreadList::DidStop() { std::lock_guard<std::recursive_mutex> guard(GetMutex()); collection::iterator pos, end = m_threads.end(); for (pos = m_threads.begin(); pos != end; ++pos) { - // Notify threads that the process just stopped. - // Note, this currently assumes that all threads in the list - // stop when the process stops. In the future we will want to support - // a debugging model where some threads continue to run while others - // are stopped. We either need to handle that somehow here or - // create a special thread list containing only threads which will - // stop in the code that calls this method (currently + // Notify threads that the process just stopped. Note, this currently + // assumes that all threads in the list stop when the process stops. In + // the future we will want to support a debugging model where some threads + // continue to run while others are stopped. We either need to handle that + // somehow here or create a special thread list containing only threads + // which will stop in the code that calls this method (currently // Process::SetPrivateState). ThreadSP thread_sp(*pos); if (StateIsRunningState(thread_sp->GetState())) @@ -717,8 +702,8 @@ void ThreadList::NotifySelectedThreadChanged(lldb::tid_t tid) { void ThreadList::Update(ThreadList &rhs) { if (this != &rhs) { - // Lock both mutexes to make sure neither side changes anyone on us - // while the assignment occurs + // Lock both mutexes to make sure neither side changes anyone on us while + // the assignment occurs std::lock_guard<std::recursive_mutex> guard(GetMutex()); m_process = rhs.m_process; @@ -726,13 +711,12 @@ void ThreadList::Update(ThreadList &rhs) { m_threads.swap(rhs.m_threads); m_selected_tid = rhs.m_selected_tid; - // Now we look for threads that we are done with and - // make sure to clear them up as much as possible so - // anyone with a shared pointer will still have a reference, - // but the thread won't be of much use. Using std::weak_ptr - // for all backward references (such as a thread to a process) - // will eventually solve this issue for us, but for now, we - // need to work around the issue + // Now we look for threads that we are done with and make sure to clear + // them up as much as possible so anyone with a shared pointer will still + // have a reference, but the thread won't be of much use. Using + // std::weak_ptr for all backward references (such as a thread to a + // process) will eventually solve this issue for us, but for now, we need + // to work around the issue collection::iterator rhs_pos, rhs_end = rhs.m_threads.end(); for (rhs_pos = rhs.m_threads.begin(); rhs_pos != rhs_end; ++rhs_pos) { const lldb::tid_t tid = (*rhs_pos)->GetID(); diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp index 27c9e2aa9c0..31ee1e92249 100644 --- a/lldb/source/Target/ThreadPlan.cpp +++ b/lldb/source/Target/ThreadPlan.cpp @@ -100,8 +100,8 @@ bool ThreadPlan::StopOthers() { } void ThreadPlan::SetStopOthers(bool new_value) { - // SetStopOthers doesn't work up the hierarchy. You have to set the - // explicit ThreadPlan you want to affect. + // SetStopOthers doesn't work up the hierarchy. You have to set the explicit + // ThreadPlan you want to affect. } bool ThreadPlan::WillResume(StateType resume_state, bool current_plan) { diff --git a/lldb/source/Target/ThreadPlanBase.cpp b/lldb/source/Target/ThreadPlanBase.cpp index d1c2f6dd120..65b11458417 100644 --- a/lldb/source/Target/ThreadPlanBase.cpp +++ b/lldb/source/Target/ThreadPlanBase.cpp @@ -28,8 +28,8 @@ using namespace lldb; using namespace lldb_private; //---------------------------------------------------------------------- -// ThreadPlanBase: This one always stops, and never has anything particular -// to do. +// ThreadPlanBase: This one always stops, and never has anything particular to +// do. // FIXME: The "signal handling" policies should probably go here. //---------------------------------------------------------------------- @@ -59,8 +59,8 @@ void ThreadPlanBase::GetDescription(Stream *s, lldb::DescriptionLevel level) { bool ThreadPlanBase::ValidatePlan(Stream *error) { return true; } bool ThreadPlanBase::DoPlanExplainsStop(Event *event_ptr) { - // The base plan should defer to its tracer, since by default it - // always handles the stop. + // The base plan should defer to its tracer, since by default it always + // handles the stop. if (TracerExplainsStop()) return false; else @@ -99,10 +99,9 @@ bool ThreadPlanBase::ShouldStop(Event *event_ptr) { case eStopReasonBreakpoint: case eStopReasonWatchpoint: if (stop_info_sp->ShouldStopSynchronous(event_ptr)) { - // If we are going to stop for a breakpoint, then unship the other plans - // at this point. Don't force the discard, however, so Master plans can - // stay - // in place if they want to. + // If we are going to stop for a breakpoint, then unship the other + // plans at this point. Don't force the discard, however, so Master + // plans can stay in place if they want to. if (log) log->Printf( "Base plan discarding thread plans for thread tid = 0x%4.4" PRIx64 @@ -112,9 +111,8 @@ bool ThreadPlanBase::ShouldStop(Event *event_ptr) { return true; } // If we aren't going to stop at this breakpoint, and it is internal, - // don't report this stop or the subsequent running event. - // Otherwise we will post the stopped & running, but the stopped event - // will get marked + // don't report this stop or the subsequent running event. Otherwise we + // will post the stopped & running, but the stopped event will get marked // with "restarted" so the UI will know to wait and expect the consequent // "running". if (stop_info_sp->ShouldNotify(event_ptr)) { @@ -131,10 +129,9 @@ bool ThreadPlanBase::ShouldStop(Event *event_ptr) { break; case eStopReasonException: - // If we crashed, discard thread plans and stop. Don't force the discard, - // however, - // since on rerun the target may clean up this exception and continue - // normally from there. + // If we crashed, discard thread plans and stop. Don't force the + // discard, however, since on rerun the target may clean up this + // exception and continue normally from there. if (log) log->Printf( "Base plan discarding thread plans for thread tid = 0x%4.4" PRIx64 @@ -144,10 +141,9 @@ bool ThreadPlanBase::ShouldStop(Event *event_ptr) { return true; case eStopReasonExec: - // If we crashed, discard thread plans and stop. Don't force the discard, - // however, - // since on rerun the target may clean up this exception and continue - // normally from there. + // If we crashed, discard thread plans and stop. Don't force the + // discard, however, since on rerun the target may clean up this + // exception and continue normally from there. if (log) log->Printf( "Base plan discarding thread plans for thread tid = 0x%4.4" PRIx64 @@ -198,8 +194,7 @@ bool ThreadPlanBase::WillStop() { return true; } bool ThreadPlanBase::DoWillResume(lldb::StateType resume_state, bool current_plan) { // Reset these to the default values so we don't set them wrong, then not get - // asked - // for a while, then return the wrong answer. + // asked for a while, then return the wrong answer. m_run_vote = eVoteNoOpinion; m_stop_vote = eVoteNo; return true; diff --git a/lldb/source/Target/ThreadPlanCallFunction.cpp b/lldb/source/Target/ThreadPlanCallFunction.cpp index e3b9ae15dc9..a00087fee27 100644 --- a/lldb/source/Target/ThreadPlanCallFunction.cpp +++ b/lldb/source/Target/ThreadPlanCallFunction.cpp @@ -56,8 +56,7 @@ bool ThreadPlanCallFunction::ConstructorSetup( m_function_sp = thread.GetRegisterContext()->GetSP() - abi->GetRedZoneSize(); // If we can't read memory at the point of the process where we are planning - // to put our function, we're - // not going to get any further... + // to put our function, we're not going to get any further... Status error; process_sp->ReadUnsignedIntegerFromMemory(m_function_sp, 4, 0, error); if (!error.Success()) { @@ -278,9 +277,8 @@ bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) { LIBLLDB_LOG_PROCESS)); m_real_stop_info_sp = GetPrivateStopInfo(); - // If our subplan knows why we stopped, even if it's done (which would forward - // the question to us) - // we answer yes. + // If our subplan knows why we stopped, even if it's done (which would + // forward the question to us) we answer yes. if (m_subplan_sp && m_subplan_sp->PlanExplainsStop(event_ptr)) { SetPlanComplete(); return true; @@ -302,8 +300,8 @@ bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) { return true; // One more quirk here. If this event was from Halt interrupting the target, - // then we should not consider - // ourselves complete. Return true to acknowledge the stop. + // then we should not consider ourselves complete. Return true to + // acknowledge the stop. if (Process::ProcessEventData::GetInterruptedFromEvent(event_ptr)) { if (log) log->Printf("ThreadPlanCallFunction::PlanExplainsStop: The event is an " @@ -312,8 +310,8 @@ bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) { } // We control breakpoints separately from other "stop reasons." So first, // check the case where we stopped for an internal breakpoint, in that case, - // continue on. - // If it is not an internal breakpoint, consult m_ignore_breakpoints. + // continue on. If it is not an internal breakpoint, consult + // m_ignore_breakpoints. if (stop_reason == eStopReasonBreakpoint) { ProcessSP process_sp(m_thread.CalculateProcess()); @@ -364,15 +362,13 @@ bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) { // should be propagated up the stack. return false; } else { - // If the subplan is running, any crashes are attributable to us. - // If we want to discard the plan, then we say we explain the stop - // but if we are going to be discarded, let whoever is above us - // explain the stop. - // But don't discard the plan if the stop would restart itself (for instance - // if it is a - // signal that is set not to stop. Check that here first. We just say we - // explain the stop - // but aren't done and everything will continue on from there. + // If the subplan is running, any crashes are attributable to us. If we + // want to discard the plan, then we say we explain the stop but if we are + // going to be discarded, let whoever is above us explain the stop. But + // don't discard the plan if the stop would restart itself (for instance if + // it is a signal that is set not to stop. Check that here first. We just + // say we explain the stop but aren't done and everything will continue on + // from there. if (m_real_stop_info_sp && m_real_stop_info_sp->ShouldStopSynchronous(event_ptr)) { @@ -385,8 +381,8 @@ bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) { bool ThreadPlanCallFunction::ShouldStop(Event *event_ptr) { // We do some computation in DoPlanExplainsStop that may or may not set the - // plan as complete. - // We need to do that here to make sure our state is correct. + // plan as complete. We need to do that here to make sure our state is + // correct. DoPlanExplainsStop(event_ptr); if (IsPlanComplete()) { @@ -405,9 +401,8 @@ void ThreadPlanCallFunction::DidPush() { //#define SINGLE_STEP_EXPRESSIONS // Now set the thread state to "no reason" so we don't run with whatever - // signal was outstanding... - // Wait till the plan is pushed so we aren't changing the stop info till we're - // about to run. + // signal was outstanding... Wait till the plan is pushed so we aren't + // changing the stop info till we're about to run. GetThread().SetStopInfoToNothing(); @@ -483,10 +478,9 @@ bool ThreadPlanCallFunction::BreakpointsExplainStop() { SetPlanComplete(false); - // If the user has set the ObjC language breakpoint, it would normally get - // priority over our internal - // catcher breakpoint, but in this case we can't let that happen, so force - // the ShouldStop here. + // If the user has set the ObjC language breakpoint, it would normally + // get priority over our internal catcher breakpoint, but in this case we + // can't let that happen, so force the ShouldStop here. stop_info_sp->OverrideShouldStop(true); return true; } diff --git a/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp b/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp index 0dae98f03ef..e8ea73f3c6a 100644 --- a/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp +++ b/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp @@ -23,8 +23,8 @@ ThreadPlanCallOnFunctionExit::ThreadPlanCallOnFunctionExit( } void ThreadPlanCallOnFunctionExit::DidPush() { - // We now want to queue the "step out" thread plan so it executes - // and completes. + // We now want to queue the "step out" thread plan so it executes and + // completes. // Set stop vote to eVoteNo. m_step_out_threadplan_sp = GetThread().QueueThreadPlanForStepOut( @@ -59,9 +59,9 @@ bool ThreadPlanCallOnFunctionExit::ValidatePlan(Stream *error) { } bool ThreadPlanCallOnFunctionExit::ShouldStop(Event *event_ptr) { - // If this is where we find out that an internal stop came in, then: - // Check if the step-out plan completed. If it did, then we want to - // run the callback here (our reason for living...) + // If this is where we find out that an internal stop came in, then: Check if + // the step-out plan completed. If it did, then we want to run the callback + // here (our reason for living...) if (m_step_out_threadplan_sp && m_step_out_threadplan_sp->IsPlanComplete()) { m_callback(); @@ -71,8 +71,8 @@ bool ThreadPlanCallOnFunctionExit::ShouldStop(Event *event_ptr) { // Indicate that this plan is done and can be discarded. SetPlanComplete(); - // We're done now, but we want to return false so that we - // don't cause the thread to really stop. + // We're done now, but we want to return false so that we don't cause the + // thread to really stop. } return false; @@ -80,21 +80,20 @@ bool ThreadPlanCallOnFunctionExit::ShouldStop(Event *event_ptr) { bool ThreadPlanCallOnFunctionExit::WillStop() { // The code looks like the return value is ignored via ThreadList:: - // ShouldStop(). - // This is called when we really are going to stop. We don't care - // and don't need to do anything here. + // ShouldStop(). This is called when we really are going to stop. We don't + // care and don't need to do anything here. return false; } bool ThreadPlanCallOnFunctionExit::DoPlanExplainsStop(Event *event_ptr) { - // We don't ever explain a stop. The only stop that is relevant - // to us directly is the step_out plan we added to do the heavy lifting - // of getting us past the current method. + // We don't ever explain a stop. The only stop that is relevant to us + // directly is the step_out plan we added to do the heavy lifting of getting + // us past the current method. return false; } lldb::StateType ThreadPlanCallOnFunctionExit::GetPlanRunState() { - // This value doesn't matter - we'll never be the top thread plan, so - // nobody will ask us this question. + // This value doesn't matter - we'll never be the top thread plan, so nobody + // will ask us this question. return eStateRunning; } diff --git a/lldb/source/Target/ThreadPlanCallUserExpression.cpp b/lldb/source/Target/ThreadPlanCallUserExpression.cpp index 15cbd0baa9a..7bf0dd39993 100644 --- a/lldb/source/Target/ThreadPlanCallUserExpression.cpp +++ b/lldb/source/Target/ThreadPlanCallUserExpression.cpp @@ -44,8 +44,8 @@ ThreadPlanCallUserExpression::ThreadPlanCallUserExpression( lldb::UserExpressionSP &user_expression_sp) : ThreadPlanCallFunction(thread, function, CompilerType(), args, options), m_user_expression_sp(user_expression_sp) { - // User expressions are generally "User generated" so we should set them up to - // stop when done. + // User expressions are generally "User generated" so we should set them up + // to stop when done. SetIsMasterPlan(true); SetOkayToDiscard(false); } diff --git a/lldb/source/Target/ThreadPlanPython.cpp b/lldb/source/Target/ThreadPlanPython.cpp index af5c7651838..7796b8a0ab2 100644 --- a/lldb/source/Target/ThreadPlanPython.cpp +++ b/lldb/source/Target/ThreadPlanPython.cpp @@ -59,8 +59,7 @@ bool ThreadPlanPython::ValidatePlan(Stream *error) { void ThreadPlanPython::DidPush() { // We set up the script side in DidPush, so that it can push other plans in - // the constructor, - // and doesn't have to care about the details of DidPush. + // the constructor, and doesn't have to care about the details of DidPush. if (!m_class_name.empty()) { ScriptInterpreter *script_interp = m_thread.GetProcess() diff --git a/lldb/source/Target/ThreadPlanRunToAddress.cpp b/lldb/source/Target/ThreadPlanRunToAddress.cpp index 44de724b608..6d1a8b5c27f 100644 --- a/lldb/source/Target/ThreadPlanRunToAddress.cpp +++ b/lldb/source/Target/ThreadPlanRunToAddress.cpp @@ -133,8 +133,7 @@ void ThreadPlanRunToAddress::GetDescription(Stream *s, } bool ThreadPlanRunToAddress::ValidatePlan(Stream *error) { - // If we couldn't set the breakpoint for some reason, then this won't - // work. + // If we couldn't set the breakpoint for some reason, then this won't work. bool all_bps_good = true; size_t num_break_ids = m_break_ids.size(); diff --git a/lldb/source/Target/ThreadPlanShouldStopHere.cpp b/lldb/source/Target/ThreadPlanShouldStopHere.cpp index 42aaa4227cf..c7afe0d9a8a 100644 --- a/lldb/source/Target/ThreadPlanShouldStopHere.cpp +++ b/lldb/source/Target/ThreadPlanShouldStopHere.cpp @@ -86,8 +86,8 @@ bool ThreadPlanShouldStopHere::DefaultShouldStopHereCallback( // Always avoid code with line number 0. // FIXME: At present the ShouldStop and the StepFromHere calculate this // independently. If this ever - // becomes expensive (this one isn't) we can try to have this set a state that - // the StepFromHere can use. + // becomes expensive (this one isn't) we can try to have this set a state + // that the StepFromHere can use. if (frame) { SymbolContext sc; sc = frame->GetSymbolContext(eSymbolContextLineEntry); @@ -104,9 +104,8 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback( const bool stop_others = false; const size_t frame_index = 0; ThreadPlanSP return_plan_sp; - // If we are stepping through code at line number 0, then we need to step over - // this range. Otherwise - // we will step out. + // If we are stepping through code at line number 0, then we need to step + // over this range. Otherwise we will step out. Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); StackFrame *frame = current_plan->GetThread().GetStackFrameAtIndex(0).get(); @@ -119,8 +118,7 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback( AddressRange range = sc.line_entry.range; // If the whole function is marked line 0 just step out, that's easier & - // faster than continuing - // to step through it. + // faster than continuing to step through it. bool just_step_out = false; if (sc.symbol && sc.symbol->ValueIsAddress()) { Address symbol_end = sc.symbol->GetAddress(); diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp index 015c784ff4b..5810e9d9181 100644 --- a/lldb/source/Target/ThreadPlanStepInRange.cpp +++ b/lldb/source/Target/ThreadPlanStepInRange.cpp @@ -35,8 +35,7 @@ uint32_t ThreadPlanStepInRange::s_default_flag_values = //---------------------------------------------------------------------- // ThreadPlanStepInRange: Step through a stack range, either stepping over or -// into -// based on the value of \a type. +// into based on the value of \a type. //---------------------------------------------------------------------- ThreadPlanStepInRange::ThreadPlanStepInRange( @@ -164,15 +163,14 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { if (m_virtual_step) { // If we've just completed a virtual step, all we need to do is check for a - // ShouldStopHere plan, and otherwise - // we're done. + // ShouldStopHere plan, and otherwise we're done. // FIXME - This can be both a step in and a step out. Probably should // record which in the m_virtual_step. m_sub_plan_sp = CheckShouldStopHereAndQueueStepOut(eFrameCompareYounger); } else { // Stepping through should be done running other threads in general, since - // we're setting a breakpoint and - // continuing. So only stop others if we are explicitly told to do so. + // we're setting a breakpoint and continuing. So only stop others if we + // are explicitly told to do so. bool stop_others = (m_stop_others == lldb::eOnlyThisThread); @@ -185,9 +183,8 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { // A caveat to this is if we think the frame is older but we're actually // in a trampoline. // I'm going to make the assumption that you wouldn't RETURN to a - // trampoline. So if we are - // in a trampoline we think the frame is older because the trampoline - // confused the backtracer. + // trampoline. So if we are in a trampoline we think the frame is older + // because the trampoline confused the backtracer. m_sub_plan_sp = m_thread.QueueThreadPlanForStepThrough(m_stack_id, false, stop_others); if (!m_sub_plan_sp) { @@ -204,19 +201,15 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { "Thought I stepped out, but in fact arrived at a trampoline."); } } else if (frame_order == eFrameCompareEqual && InSymbol()) { - // If we are not in a place we should step through, we're done. - // One tricky bit here is that some stubs don't push a frame, so we have - // to check - // both the case of a frame that is younger, or the same as this frame. - // However, if the frame is the same, and we are still in the symbol we - // started - // in, the we don't need to do this. This first check isn't strictly - // necessary, - // but it is more efficient. + // If we are not in a place we should step through, we're done. One + // tricky bit here is that some stubs don't push a frame, so we have to + // check both the case of a frame that is younger, or the same as this + // frame. However, if the frame is the same, and we are still in the + // symbol we started in, the we don't need to do this. This first check + // isn't strictly necessary, but it is more efficient. // If we're still in the range, keep going, either by running to the next - // branch breakpoint, or by - // stepping. + // branch breakpoint, or by stepping. if (InRange()) { SetNextBranchBreakpoint(); return false; @@ -244,15 +237,13 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { log->Printf("No step through plan found."); } - // If not, give the "should_stop" callback a chance to push a plan to get us - // out of here. - // But only do that if we actually have stepped in. + // If not, give the "should_stop" callback a chance to push a plan to get + // us out of here. But only do that if we actually have stepped in. if (!m_sub_plan_sp && frame_order == eFrameCompareYounger) m_sub_plan_sp = CheckShouldStopHereAndQueueStepOut(frame_order); // If we've stepped in and we are going to stop here, check to see if we - // were asked to - // run past the prologue, and if so do that. + // were asked to run past the prologue, and if so do that. if (!m_sub_plan_sp && frame_order == eFrameCompareYounger && m_step_past_prologue) { @@ -413,8 +404,8 @@ bool ThreadPlanStepInRange::DefaultShouldStopHereCallback( SymbolContext sc = frame->GetSymbolContext( eSymbolContextFunction | eSymbolContextBlock | eSymbolContextSymbol); if (sc.symbol != nullptr) { - // First try an exact match, since that's cheap with ConstStrings. Then - // do a strstr compare. + // First try an exact match, since that's cheap with ConstStrings. + // Then do a strstr compare. if (step_in_range_plan->m_step_into_target == sc.GetFunctionName()) { should_stop_here = true; } else { @@ -449,25 +440,19 @@ bool ThreadPlanStepInRange::DefaultShouldStopHereCallback( bool ThreadPlanStepInRange::DoPlanExplainsStop(Event *event_ptr) { // We always explain a stop. Either we've just done a single step, in which - // case we'll do our ordinary processing, or we stopped for some - // reason that isn't handled by our sub-plans, in which case we want to just - // stop right - // away. - // In general, we don't want to mark the plan as complete for unexplained - // stops. - // For instance, if you step in to some code with no debug info, so you step - // out - // and in the course of that hit a breakpoint, then you want to stop & show - // the user - // the breakpoint, but not unship the step in plan, since you still may want - // to complete that - // plan when you continue. This is particularly true when doing "step in to - // target function." + // case we'll do our ordinary processing, or we stopped for some reason that + // isn't handled by our sub-plans, in which case we want to just stop right + // away. In general, we don't want to mark the plan as complete for + // unexplained stops. For instance, if you step in to some code with no debug + // info, so you step out and in the course of that hit a breakpoint, then you + // want to stop & show the user the breakpoint, but not unship the step in + // plan, since you still may want to complete that plan when you continue. + // This is particularly true when doing "step in to target function." // stepping. // - // The only variation is that if we are doing "step by running to next branch" - // in which case - // if we hit our branch breakpoint we don't set the plan to complete. + // The only variation is that if we are doing "step by running to next + // branch" in which case if we hit our branch breakpoint we don't set the + // plan to complete. bool return_value = false; diff --git a/lldb/source/Target/ThreadPlanStepInstruction.cpp b/lldb/source/Target/ThreadPlanStepInstruction.cpp index ca45960e1ed..dd8f129a935 100644 --- a/lldb/source/Target/ThreadPlanStepInstruction.cpp +++ b/lldb/source/Target/ThreadPlanStepInstruction.cpp @@ -76,8 +76,8 @@ void ThreadPlanStepInstruction::GetDescription(Stream *s, } bool ThreadPlanStepInstruction::ValidatePlan(Stream *error) { - // Since we read the instruction we're stepping over from the thread, - // this plan will always work. + // Since we read the instruction we're stepping over from the thread, this + // plan will always work. return true; } @@ -106,8 +106,8 @@ bool ThreadPlanStepInstruction::IsPlanStale() { return (m_thread.GetRegisterContext()->GetPC(0) != m_instruction_addr); } else if (cur_frame_id < m_stack_id) { // If the current frame is younger than the start frame and we are stepping - // over, then we need to continue, - // but if we are doing just one step, we're done. + // over, then we need to continue, but if we are doing just one step, we're + // done. return !m_step_over; } else { if (log) { @@ -140,8 +140,7 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { return true; } else { // We are still stepping, reset the start pc, and in case we've - // stepped out, - // reset the current stack id. + // stepped out, reset the current stack id. SetUpState(); return false; } @@ -154,9 +153,8 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { if (return_frame->GetStackID() != m_parent_frame_id || m_start_has_symbol) { // next-instruction shouldn't step out of inlined functions. But we - // may have stepped into a - // real function that starts with an inlined function, and we do want - // to step out of that... + // may have stepped into a real function that starts with an inlined + // function, and we do want to step out of that... if (cur_frame_sp->IsInlined()) { StackFrameSP parent_frame_sp = @@ -190,9 +188,8 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { log->Printf("%s.", s.GetData()); } - // StepInstruction should probably have the tri-state RunMode, but for - // now it is safer to - // run others. + // StepInstruction should probably have the tri-state RunMode, but + // for now it is safer to run others. const bool stop_others = false; m_thread.QueueThreadPlanForStepOutNoShouldStop( false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0); @@ -222,8 +219,7 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { return true; } else { // We are still stepping, reset the start pc, and in case we've stepped - // in or out, - // reset the current stack id. + // in or out, reset the current stack id. SetUpState(); return false; } diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index 6b6ed06e9b3..f2db6e4b6f4 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -63,17 +63,14 @@ ThreadPlanStepOut::ThreadPlanStepOut( m_step_out_to_id = return_frame_sp->GetStackID(); m_immediate_step_from_id = immediate_return_from_sp->GetStackID(); - // If the frame directly below the one we are returning to is inlined, we have - // to be - // a little more careful. It is non-trivial to determine the real "return - // code address" for - // an inlined frame, so we have to work our way to that frame and then step - // out. + // If the frame directly below the one we are returning to is inlined, we + // have to be a little more careful. It is non-trivial to determine the real + // "return code address" for an inlined frame, so we have to work our way to + // that frame and then step out. if (immediate_return_from_sp && immediate_return_from_sp->IsInlined()) { if (frame_idx > 0) { // First queue a plan that gets us to this inlined frame, and when we get - // there we'll queue a second - // plan that walks us out of this frame. + // there we'll queue a second plan that walks us out of this frame. m_step_out_to_inline_plan_sp.reset(new ThreadPlanStepOut( m_thread, nullptr, false, stop_others, eVoteNoOpinion, eVoteNoOpinion, frame_idx - 1, eLazyBoolNo, continue_to_next_branch)); @@ -81,8 +78,8 @@ ThreadPlanStepOut::ThreadPlanStepOut( ->SetShouldStopHereCallbacks(nullptr, nullptr); m_step_out_to_inline_plan_sp->SetPrivate(true); } else { - // If we're already at the inlined frame we're stepping through, then just - // do that now. + // If we're already at the inlined frame we're stepping through, then + // just do that now. QueueInlinedStepPlan(false); } } else if (return_frame_sp) { @@ -217,8 +214,8 @@ bool ThreadPlanStepOut::ValidatePlan(Stream *error) { } bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) { - // If the step out plan is done, then we just need to step through the inlined - // frame. + // If the step out plan is done, then we just need to step through the + // inlined frame. if (m_step_out_to_inline_plan_sp) { return m_step_out_to_inline_plan_sp->MischiefManaged(); } else if (m_step_through_inline_plan_sp) { @@ -233,15 +230,14 @@ bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) { } // We don't explain signals or breakpoints (breakpoints that handle stepping - // in or - // out will be handled by a child plan. + // in or out will be handled by a child plan. StopInfoSP stop_info_sp = GetPrivateStopInfo(); if (stop_info_sp) { StopReason reason = stop_info_sp->GetStopReason(); if (reason == eStopReasonBreakpoint) { - // If this is OUR breakpoint, we're fine, otherwise we don't know why this - // happened... + // If this is OUR breakpoint, we're fine, otherwise we don't know why + // this happened... BreakpointSiteSP site_sp( m_thread.GetProcess()->GetBreakpointSiteList().FindByID( stop_info_sp->GetValue())); @@ -268,11 +264,10 @@ bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) { } // If there was only one owner, then we're done. But if we also hit - // some - // user breakpoint on our way out, we should mark ourselves as done, but - // also not claim to explain the stop, since it is more important to - // report - // the user breakpoint than the step out completion. + // some user breakpoint on our way out, we should mark ourselves as + // done, but also not claim to explain the stop, since it is more + // important to report the user breakpoint than the step out + // completion. if (site_sp->GetNumberOfOwners() == 1) return true; @@ -320,9 +315,8 @@ bool ThreadPlanStepOut::ShouldStop(Event *event_ptr) { done = !(frame_zero_id < m_step_out_to_id); } - // The normal step out computations think we are done, so all we need to do is - // consult the ShouldStopHere, - // and we are done. + // The normal step out computations think we are done, so all we need to do + // is consult the ShouldStopHere, and we are done. if (done) { if (InvokeShouldStopHereCallback(eFrameCompareOlder)) { @@ -377,8 +371,7 @@ bool ThreadPlanStepOut::MischiefManaged() { // I also check the stack depth, since if we've blown past the breakpoint // for some // reason and we're now stopping for some other reason altogether, then - // we're done - // with this step out operation. + // we're done with this step out operation. Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); if (log) @@ -397,10 +390,8 @@ bool ThreadPlanStepOut::MischiefManaged() { bool ThreadPlanStepOut::QueueInlinedStepPlan(bool queue_now) { // Now figure out the range of this inlined block, and set up a "step through - // range" - // plan for that. If we've been provided with a context, then use the block - // in that - // context. + // range" plan for that. If we've been provided with a context, then use the + // block in that context. StackFrameSP immediate_return_from_sp(m_thread.GetStackFrameAtIndex(0)); if (!immediate_return_from_sp) return false; @@ -477,8 +468,8 @@ void ThreadPlanStepOut::CalculateReturnValue() { } bool ThreadPlanStepOut::IsPlanStale() { - // If we are still lower on the stack than the frame we are returning to, then - // there's something for us to do. Otherwise, we're stale. + // If we are still lower on the stack than the frame we are returning to, + // then there's something for us to do. Otherwise, we're stale. StackID frame_zero_id = m_thread.GetStackFrameAtIndex(0)->GetStackID(); return !(frame_zero_id < m_step_out_to_id); diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp index 3896a0b2471..43b2d9f4c69 100644 --- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp +++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp @@ -31,8 +31,8 @@ ThreadPlanStepOverBreakpoint::ThreadPlanStepOverBreakpoint(Thread &thread) ThreadPlan::eKindStepOverBreakpoint, "Step over breakpoint trap", thread, eVoteNo, eVoteNoOpinion), // We need to report the run since this happens - // first in the thread plan stack when stepping - // over a breakpoint + // first in the thread plan stack when stepping over + // a breakpoint m_breakpoint_addr(LLDB_INVALID_ADDRESS), m_auto_continue(false), m_reenabled_breakpoint_site(false) @@ -57,16 +57,15 @@ bool ThreadPlanStepOverBreakpoint::DoPlanExplainsStop(Event *event_ptr) { StopInfoSP stop_info_sp = GetPrivateStopInfo(); if (stop_info_sp) { // It's a little surprising that we stop here for a breakpoint hit. - // However, when you single step ONTO a breakpoint - // we still want to call that a breakpoint hit, and trigger the actions, - // etc. Otherwise you would see the + // However, when you single step ONTO a breakpoint we still want to call + // that a breakpoint hit, and trigger the actions, etc. Otherwise you + // would see the // PC at the breakpoint without having triggered the actions, then you'd // continue, the PC wouldn't change, - // and you'd see the breakpoint hit, which would be odd. - // So the lower levels fake "step onto breakpoint address" and return that - // as a breakpoint. So our trace - // step COULD appear as a breakpoint hit if the next instruction also - // contained a breakpoint. + // and you'd see the breakpoint hit, which would be odd. So the lower + // levels fake "step onto breakpoint address" and return that as a + // breakpoint. So our trace step COULD appear as a breakpoint hit if the + // next instruction also contained a breakpoint. StopReason reason = stop_info_sp->GetStopReason(); switch (reason) { @@ -75,20 +74,17 @@ bool ThreadPlanStepOverBreakpoint::DoPlanExplainsStop(Event *event_ptr) { return true; case eStopReasonBreakpoint: // It's a little surprising that we stop here for a breakpoint hit. - // However, when you single step ONTO a - // breakpoint we still want to call that a breakpoint hit, and trigger the - // actions, etc. Otherwise you + // However, when you single step ONTO a breakpoint we still want to call + // that a breakpoint hit, and trigger the actions, etc. Otherwise you // would see the PC at the breakpoint without having triggered the - // actions, then you'd continue, the PC - // wouldn't change, and you'd see the breakpoint hit, which would be odd. - // So the lower levels fake "step onto breakpoint address" and return that - // as a breakpoint hit. So our trace - // step COULD appear as a breakpoint hit if the next instruction also - // contained a breakpoint. We don't want - // to handle that, since we really don't know what to do with breakpoint - // hits. But make sure we don't set - // ourselves to auto-continue or we'll wrench control away from the plans - // that can deal with this. + // actions, then you'd continue, the PC wouldn't change, and you'd see + // the breakpoint hit, which would be odd. So the lower levels fake "step + // onto breakpoint address" and return that as a breakpoint hit. So our + // trace step COULD appear as a breakpoint hit if the next instruction + // also contained a breakpoint. We don't want to handle that, since we + // really don't know what to do with breakpoint hits. But make sure we + // don't set ourselves to auto-continue or we'll wrench control away from + // the plans that can deal with this. SetAutoContinue(false); return false; default: @@ -130,8 +126,7 @@ bool ThreadPlanStepOverBreakpoint::MischiefManaged() { if (pc_addr == m_breakpoint_addr) { // If we are still at the PC of our breakpoint, then for some reason we - // didn't - // get a chance to run. + // didn't get a chance to run. return false; } else { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index d5778fb5e8d..b88d25b707e 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -32,8 +32,7 @@ uint32_t ThreadPlanStepOverRange::s_default_flag_values = 0; //---------------------------------------------------------------------- // ThreadPlanStepOverRange: Step through a stack range, either stepping over or -// into -// based on the value of \a type. +// into based on the value of \a type. //---------------------------------------------------------------------- ThreadPlanStepOverRange::ThreadPlanStepOverRange( @@ -91,21 +90,17 @@ void ThreadPlanStepOverRange::SetupAvoidNoDebug( else GetFlags().Clear(ThreadPlanShouldStopHere::eStepOutAvoidNoDebug); // Step Over plans should always avoid no-debug on step in. Seems like you - // shouldn't - // have to say this, but a tail call looks more like a step in that a step - // out, so - // we want to catch this case. + // shouldn't have to say this, but a tail call looks more like a step in that + // a step out, so we want to catch this case. GetFlags().Set(ThreadPlanShouldStopHere::eStepInAvoidNoDebug); } bool ThreadPlanStepOverRange::IsEquivalentContext( const SymbolContext &context) { - // Match as much as is specified in the m_addr_context: - // This is a fairly loose sanity check. Note, sometimes the target doesn't - // get filled - // in so I left out the target check. And sometimes the module comes in as - // the .o file from the - // inlined range, so I left that out too... + // Match as much as is specified in the m_addr_context: This is a fairly + // loose sanity check. Note, sometimes the target doesn't get filled in so I + // left out the target check. And sometimes the module comes in as the .o + // file from the inlined range, so I left that out too... if (m_addr_context.comp_unit) { if (m_addr_context.comp_unit != context.comp_unit) return false; @@ -113,8 +108,8 @@ bool ThreadPlanStepOverRange::IsEquivalentContext( if (m_addr_context.function != context.function) return false; // It is okay to return to a different block of a straight function, we - // only have to - // be more careful if returning from one inlined block to another. + // only have to be more careful if returning from one inlined block to + // another. if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr && context.block->GetInlinedFunctionInfo() == nullptr) return true; @@ -140,8 +135,8 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { } // If we're out of the range but in the same frame or in our caller's frame - // then we should stop. - // When stepping out we only stop others if we are forcing running one thread. + // then we should stop. When stepping out we only stop others if we are + // forcing running one thread. bool stop_others = (m_stop_others == lldb::eOnlyThisThread); ThreadPlanSP new_plan_sp; FrameComparison frame_order = CompareCurrentFrameToStartFrame(); @@ -152,11 +147,9 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { // A caveat to this is if we think the frame is older but we're actually in // a trampoline. // I'm going to make the assumption that you wouldn't RETURN to a - // trampoline. So if we are - // in a trampoline we think the frame is older because the trampoline - // confused the backtracer. - // As below, we step through first, and then try to figure out how to get - // back out again. + // trampoline. So if we are in a trampoline we think the frame is older + // because the trampoline confused the backtracer. As below, we step + // through first, and then try to figure out how to get back out again. new_plan_sp = m_thread.QueueThreadPlanForStepThrough(m_stack_id, false, stop_others); @@ -166,8 +159,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { "Thought I stepped out, but in fact arrived at a trampoline."); } else if (frame_order == eFrameCompareYounger) { // Make sure we really are in a new frame. Do that by unwinding and seeing - // if the - // start function really is our start function... + // if the start function really is our start function... for (uint32_t i = 1;; ++i) { StackFrameSP older_frame_sp = m_thread.GetStackFrameAtIndex(i); if (!older_frame_sp) { @@ -200,28 +192,23 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { if (!InSymbol()) { // This one is a little tricky. Sometimes we may be in a stub or - // something similar, - // in which case we need to get out of there. But if we are in a stub - // then it's - // likely going to be hard to get out from here. It is probably easiest - // to step into the - // stub, and then it will be straight-forward to step out. + // something similar, in which case we need to get out of there. But if + // we are in a stub then it's likely going to be hard to get out from + // here. It is probably easiest to step into the stub, and then it will + // be straight-forward to step out. new_plan_sp = m_thread.QueueThreadPlanForStepThrough(m_stack_id, false, stop_others); } else { - // The current clang (at least through 424) doesn't always get the address - // range for the - // DW_TAG_inlined_subroutines right, so that when you leave the inlined - // range the line table says - // you are still in the source file of the inlining function. This is - // bad, because now you are missing - // the stack frame for the function containing the inlining, and if you - // sensibly do "finish" to get - // out of this function you will instead exit the containing function. - // To work around this, we check whether we are still in the source file - // we started in, and if not assume - // it is an error, and push a plan to get us out of this line and back to - // the containing file. + // The current clang (at least through 424) doesn't always get the + // address range for the DW_TAG_inlined_subroutines right, so that when + // you leave the inlined range the line table says you are still in the + // source file of the inlining function. This is bad, because now you + // are missing the stack frame for the function containing the inlining, + // and if you sensibly do "finish" to get out of this function you will + // instead exit the containing function. To work around this, we check + // whether we are still in the source file we started in, and if not + // assume it is an error, and push a plan to get us out of this line and + // back to the containing file. if (m_addr_context.line_entry.IsValid()) { SymbolContext sc; @@ -244,14 +231,11 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { bool step_past_remaining_inline = false; if (entry_idx > 0) { // We require the previous line entry and the current line - // entry come - // from the same file. - // The other requirement is that the previous line table entry - // be part of an - // inlined block, we don't want to step past cases where - // people have inlined - // some code fragment by using #include <source-fragment.c> - // directly. + // entry come from the same file. The other requirement is + // that the previous line table entry be part of an inlined + // block, we don't want to step past cases where people have + // inlined some code fragment by using #include <source- + // fragment.c> directly. LineEntry prev_line_entry; if (line_table->GetLineEntryAtIndex(entry_idx - 1, prev_line_entry) && @@ -338,8 +322,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { if (!new_plan_sp) { // For efficiencies sake, we know we're done here so we don't have to do - // this - // calculation again in MischiefManaged. + // this calculation again in MischiefManaged. SetPlanComplete(); return true; } else @@ -347,16 +330,12 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { } bool ThreadPlanStepOverRange::DoPlanExplainsStop(Event *event_ptr) { - // For crashes, breakpoint hits, signals, etc, let the base plan (or some plan - // above us) - // handle the stop. That way the user can see the stop, step around, and then - // when they - // are done, continue and have their step complete. The exception is if we've - // hit our - // "run to next branch" breakpoint. - // Note, unlike the step in range plan, we don't mark ourselves complete if we - // hit an - // unexplained breakpoint/crash. + // For crashes, breakpoint hits, signals, etc, let the base plan (or some + // plan above us) handle the stop. That way the user can see the stop, step + // around, and then when they are done, continue and have their step + // complete. The exception is if we've hit our "run to next branch" + // breakpoint. Note, unlike the step in range plan, we don't mark ourselves + // complete if we hit an unexplained breakpoint/crash. Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); StopInfoSP stop_info_sp = GetPrivateStopInfo(); @@ -387,8 +366,8 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state, m_first_resume = false; if (resume_state == eStateStepping && current_plan) { // See if we are about to step over an inlined call in the middle of the - // inlined stack, if so figure - // out its extents and reset our range to step over that. + // inlined stack, if so figure out its extents and reset our range to + // step over that. bool in_inlined_stack = m_thread.DecrementCurrentInlinedDepth(); if (in_inlined_stack) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index 09e606f490a..5a71119015e 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -31,8 +31,8 @@ using namespace lldb; using namespace lldb_private; //---------------------------------------------------------------------- -// ThreadPlanStepRange: Step through a stack range, either stepping over or into -// based on the value of \a type. +// ThreadPlanStepRange: Step through a stack range, either stepping over or +// into based on the value of \a type. //---------------------------------------------------------------------- ThreadPlanStepRange::ThreadPlanStepRange(ThreadPlanKind kind, const char *name, @@ -74,15 +74,14 @@ Vote ThreadPlanStepRange::ShouldReportStop(Event *event_ptr) { } void ThreadPlanStepRange::AddRange(const AddressRange &new_range) { - // For now I'm just adding the ranges. At some point we may want to - // condense the ranges if they overlap, though I don't think it is likely - // to be very important. + // For now I'm just adding the ranges. At some point we may want to condense + // the ranges if they overlap, though I don't think it is likely to be very + // important. m_address_ranges.push_back(new_range); // Fill the slot for this address range with an empty DisassemblerSP in the - // instruction ranges. I want the - // indices to match, but I don't want to do the work to disassemble this range - // if I don't step into it. + // instruction ranges. I want the indices to match, but I don't want to do + // the work to disassemble this range if I don't step into it. m_instruction_ranges.push_back(DisassemblerSP()); } @@ -158,12 +157,11 @@ bool ThreadPlanStepRange::InRange() { } else if (new_context.line_entry.range.GetBaseAddress().GetLoadAddress( m_thread.CalculateTarget().get()) != pc_load_addr) { // Another thing that sometimes happens here is that we step out of - // one line into the MIDDLE of another - // line. So far I mostly see this due to bugs in the debug - // information. - // But we probably don't want to be in the middle of a line range, so - // in that case reset the stepping - // range to the line we've stepped into the middle of and continue. + // one line into the MIDDLE of another line. So far I mostly see + // this due to bugs in the debug information. But we probably don't + // want to be in the middle of a line range, so in that case reset + // the stepping range to the line we've stepped into the middle of + // and continue. m_addr_context = new_context; m_address_ranges.clear(); AddRange(m_addr_context.line_entry.range); @@ -260,9 +258,8 @@ InstructionList *ThreadPlanStepRange::GetInstructionsForAddress( return nullptr; else { // Find where we are in the instruction list as well. If we aren't at - // an instruction, - // return nullptr. In this case, we're probably lost, and shouldn't try - // to do anything fancy. + // an instruction, return nullptr. In this case, we're probably lost, + // and shouldn't try to do anything fancy. insn_offset = m_instruction_ranges[i] @@ -297,8 +294,7 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); // Stepping through ranges using breakpoints doesn't work yet, but with this - // off we fall back to instruction - // single stepping. + // off we fall back to instruction single stepping. if (!m_use_fast_step) return false; @@ -383,9 +379,8 @@ bool ThreadPlanStepRange::NextRangeBreakpointExplainsStop( size_t num_owners = bp_site_sp->GetNumberOfOwners(); bool explains_stop = true; // If all the owners are internal, then we are probably just stepping over - // this range from multiple threads, - // or multiple frames, so we want to continue. If one is not internal, then - // we should not explain the stop, + // this range from multiple threads, or multiple frames, so we want to + // continue. If one is not internal, then we should not explain the stop, // and let the user breakpoint handle the stop. for (size_t i = 0; i < num_owners; i++) { if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) { @@ -418,8 +413,8 @@ bool ThreadPlanStepRange::MischiefManaged() { // I do this check first because we might have stepped somewhere that will // fool InRange into // thinking it needs to step past the end of that line. This happens, for - // instance, when stepping - // over inlined code that is in the middle of the current line. + // instance, when stepping over inlined code that is in the middle of the + // current line. if (!m_no_more_plans) return false; @@ -457,8 +452,8 @@ bool ThreadPlanStepRange::IsPlanStale() { } return true; } else if (frame_order == eFrameCompareEqual && InSymbol()) { - // If we are not in a place we should step through, we've gotten stale. - // One tricky bit here is that some stubs don't push a frame, so we should. + // If we are not in a place we should step through, we've gotten stale. One + // tricky bit here is that some stubs don't push a frame, so we should. // check that we are in the same symbol. if (!InRange()) { // Set plan Complete when we reach next instruction just after the range diff --git a/lldb/source/Target/ThreadPlanStepThrough.cpp b/lldb/source/Target/ThreadPlanStepThrough.cpp index 46aadb00f2a..c039a32f551 100644 --- a/lldb/source/Target/ThreadPlanStepThrough.cpp +++ b/lldb/source/Target/ThreadPlanStepThrough.cpp @@ -26,9 +26,8 @@ using namespace lldb_private; //---------------------------------------------------------------------- // ThreadPlanStepThrough: If the current instruction is a trampoline, step -// through it -// If it is the beginning of the prologue of a function, step through that as -// well. +// through it If it is the beginning of the prologue of a function, step +// through that as well. // FIXME: At present only handles DYLD trampolines. //---------------------------------------------------------------------- @@ -49,9 +48,8 @@ ThreadPlanStepThrough::ThreadPlanStepThrough(Thread &thread, m_start_address = GetThread().GetRegisterContext()->GetPC(0); // We are going to return back to the concrete frame 1, we might pass by - // some inlined code that we're in - // the middle of by doing this, but it's easier than trying to figure out - // where the inlined code might return to. + // some inlined code that we're in the middle of by doing this, but it's + // easier than trying to figure out where the inlined code might return to. StackFrameSP return_frame_sp = m_thread.GetFrameWithStackID(m_stack_id); @@ -136,10 +134,8 @@ bool ThreadPlanStepThrough::ValidatePlan(Stream *error) { bool ThreadPlanStepThrough::DoPlanExplainsStop(Event *event_ptr) { // If we have a sub-plan, it will have been asked first if we explain the - // stop, and - // we won't get asked. The only time we would be the one directly asked this - // question - // is if we hit our backstop breakpoint. + // stop, and we won't get asked. The only time we would be the one directly + // asked this question is if we hit our backstop breakpoint. return HitOurBackstopBreakpoint(); } @@ -156,8 +152,7 @@ bool ThreadPlanStepThrough::ShouldStop(Event *event_ptr) { } // If we don't have a sub-plan, then we're also done (can't see how we would - // ever get here - // without a plan, but just in case. + // ever get here without a plan, but just in case. if (!m_sub_plan_sp) { SetPlanComplete(); @@ -165,15 +160,13 @@ bool ThreadPlanStepThrough::ShouldStop(Event *event_ptr) { } // If the current sub plan is not done, we don't want to stop. Actually, we - // probably won't - // ever get here in this state, since we generally won't get asked any - // questions if out - // current sub-plan is not done... + // probably won't ever get here in this state, since we generally won't get + // asked any questions if out current sub-plan is not done... if (!m_sub_plan_sp->IsPlanComplete()) return false; - // If our current sub plan failed, then let's just run to our backstop. If we - // can't do that then just stop. + // If our current sub plan failed, then let's just run to our backstop. If + // we can't do that then just stop. if (!m_sub_plan_sp->PlanSucceeded()) { if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID) { m_sub_plan_sp.reset(); @@ -185,8 +178,7 @@ bool ThreadPlanStepThrough::ShouldStop(Event *event_ptr) { } // Next see if there is a specific step through plan at our current pc (these - // might - // chain, for instance stepping through a dylib trampoline to the objc + // might chain, for instance stepping through a dylib trampoline to the objc // dispatch function...) LookForPlanToStepThroughFromCurrentPC(); if (m_sub_plan_sp) { diff --git a/lldb/source/Target/ThreadPlanStepUntil.cpp b/lldb/source/Target/ThreadPlanStepUntil.cpp index 01f5f948a28..9984ee925c8 100644 --- a/lldb/source/Target/ThreadPlanStepUntil.cpp +++ b/lldb/source/Target/ThreadPlanStepUntil.cpp @@ -151,8 +151,8 @@ void ThreadPlanStepUntil::AnalyzeStop() { StopReason reason = stop_info_sp->GetStopReason(); if (reason == eStopReasonBreakpoint) { - // If this is OUR breakpoint, we're fine, otherwise we don't know why this - // happened... + // If this is OUR breakpoint, we're fine, otherwise we don't know why + // this happened... BreakpointSiteSP this_site = m_thread.GetProcess()->GetBreakpointSiteList().FindByID( stop_info_sp->GetValue()); @@ -163,18 +163,13 @@ void ThreadPlanStepUntil::AnalyzeStop() { if (this_site->IsBreakpointAtThisSite(m_return_bp_id)) { // If we are at our "step out" breakpoint, and the stack depth has - // shrunk, then - // this is indeed our stop. - // If the stack depth has grown, then we've hit our step out breakpoint - // recursively. - // If we are the only breakpoint at that location, then we do explain - // the stop, and - // we'll just continue. - // If there was another breakpoint here, then we don't explain the stop, - // but we won't - // mark ourselves Completed, because maybe that breakpoint will - // continue, and then - // we'll finish the "until". + // shrunk, then this is indeed our stop. If the stack depth has grown, + // then we've hit our step out breakpoint recursively. If we are the + // only breakpoint at that location, then we do explain the stop, and + // we'll just continue. If there was another breakpoint here, then we + // don't explain the stop, but we won't mark ourselves Completed, + // because maybe that breakpoint will continue, and then we'll finish + // the "until". bool done; StackID cur_frame_zero_id; @@ -209,8 +204,8 @@ void ThreadPlanStepUntil::AnalyzeStop() { else { StackFrameSP older_frame_sp = m_thread.GetStackFrameAtIndex(1); - // But if we can't even unwind one frame we should just get out of - // here & stop... + // But if we can't even unwind one frame we should just get out + // of here & stop... if (older_frame_sp) { const SymbolContext &older_context = older_frame_sp->GetSymbolContext(eSymbolContextEverything); @@ -230,8 +225,8 @@ void ThreadPlanStepUntil::AnalyzeStop() { // Otherwise we've hit this breakpoint recursively. If we're the // only breakpoint here, then we do explain the stop, and we'll - // continue. - // If not then we should let higher plans handle this stop. + // continue. If not then we should let higher plans handle this + // stop. if (this_site->GetNumberOfOwners() == 1) m_explains_stop = true; else { @@ -242,8 +237,8 @@ void ThreadPlanStepUntil::AnalyzeStop() { } } } - // If we get here we haven't hit any of our breakpoints, so let the higher - // plans take care of the stop. + // If we get here we haven't hit any of our breakpoints, so let the + // higher plans take care of the stop. m_explains_stop = false; return; } else if (IsUsuallyUnexplainedStopReason(reason)) { @@ -256,16 +251,15 @@ void ThreadPlanStepUntil::AnalyzeStop() { bool ThreadPlanStepUntil::DoPlanExplainsStop(Event *event_ptr) { // We don't explain signals or breakpoints (breakpoints that handle stepping - // in or - // out will be handled by a child plan. + // in or out will be handled by a child plan. AnalyzeStop(); return m_explains_stop; } bool ThreadPlanStepUntil::ShouldStop(Event *event_ptr) { - // If we've told our self in ExplainsStop that we plan to continue, then - // do so here. Otherwise, as long as this thread has stopped for a reason, - // we will stop. + // If we've told our self in ExplainsStop that we plan to continue, then do + // so here. Otherwise, as long as this thread has stopped for a reason, we + // will stop. StopInfoSP stop_info_sp = GetPrivateStopInfo(); if (!stop_info_sp || stop_info_sp->GetStopReason() == eStopReasonNone) diff --git a/lldb/source/Target/UnixSignals.cpp b/lldb/source/Target/UnixSignals.cpp index 150c6619859..9cfb1b74ade 100644 --- a/lldb/source/Target/UnixSignals.cpp +++ b/lldb/source/Target/UnixSignals.cpp @@ -66,9 +66,8 @@ UnixSignals::~UnixSignals() = default; void UnixSignals::Reset() { // This builds one standard set of Unix Signals. If yours aren't quite in - // this - // order, you can either subclass this class, and use Add & Remove to change - // them + // this order, you can either subclass this class, and use Add & Remove to + // change them // or you can subclass and build them afresh in your constructor; // // Note: the signals below are the Darwin signals. Do not change these! @@ -306,8 +305,8 @@ UnixSignals::GetFilteredSignals(llvm::Optional<bool> should_suppress, bool signal_notify = false; GetSignalInfo(signo, signal_suppress, signal_stop, signal_notify); - // If any of filtering conditions are not met, - // we move on to the next signal. + // If any of filtering conditions are not met, we move on to the next + // signal. if (should_suppress.hasValue() && signal_suppress != should_suppress.getValue()) continue; |
