Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(62)

Side by Side Diff: snapshot/win/process_reader_win.cc

Issue 1326443007: win: Fix incorrect thread suspend count due to ScopedProcessSuspend (Closed) Base URL: https://chromium.googlesource.com/crashpad/crashpad@master
Patch Set: assert some threads captured Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « snapshot/win/process_reader_win.h ('k') | snapshot/win/process_reader_win_test.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2015 The Crashpad Authors. All rights reserved. 1 // Copyright 2015 The Crashpad Authors. All rights reserved.
2 // 2 //
3 // Licensed under the Apache License, Version 2.0 (the "License"); 3 // Licensed under the Apache License, Version 2.0 (the "License");
4 // you may not use this file except in compliance with the License. 4 // you may not use this file except in compliance with the License.
5 // You may obtain a copy of the License at 5 // You may obtain a copy of the License at
6 // 6 //
7 // http://www.apache.org/licenses/LICENSE-2.0 7 // http://www.apache.org/licenses/LICENSE-2.0
8 // 8 //
9 // Unless required by applicable law or agreed to in writing, software 9 // Unless required by applicable law or agreed to in writing, software
10 // distributed under the License is distributed on an "AS IS" BASIS, 10 // distributed under the License is distributed on an "AS IS" BASIS,
(...skipping 99 matching lines...) Expand 10 before | Expand all | Expand 10 after
110 return handle; 110 return handle;
111 } 111 }
112 112
113 // It's necessary to suspend the thread to grab CONTEXT. SuspendThread has a 113 // It's necessary to suspend the thread to grab CONTEXT. SuspendThread has a
114 // side-effect of returning the SuspendCount of the thread on success, so we 114 // side-effect of returning the SuspendCount of the thread on success, so we
115 // fill out these two pieces of semi-unrelated data in the same function. 115 // fill out these two pieces of semi-unrelated data in the same function.
116 template <class Traits> 116 template <class Traits>
117 void FillThreadContextAndSuspendCount( 117 void FillThreadContextAndSuspendCount(
118 const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<Traits>& 118 const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<Traits>&
119 thread_info, 119 thread_info,
120 ProcessReaderWin::Thread* thread) { 120 ProcessReaderWin::Thread* thread,
121 121 ProcessSuspensionState suspension_state) {
122 // Don't suspend the thread if it's this thread. This is really only for test 122 // Don't suspend the thread if it's this thread. This is really only for test
123 // binaries, as we won't be walking ourselves, in general. 123 // binaries, as we won't be walking ourselves, in general.
124 bool is_current_thread = thread_info.ClientId.UniqueThread == 124 bool is_current_thread = thread_info.ClientId.UniqueThread ==
125 reinterpret_cast<process_types::TEB<Traits>*>( 125 reinterpret_cast<process_types::TEB<Traits>*>(
126 NtCurrentTeb())->ClientId.UniqueThread; 126 NtCurrentTeb())->ClientId.UniqueThread;
127 127
128 ScopedKernelHANDLE thread_handle(OpenThread(thread_info)); 128 ScopedKernelHANDLE thread_handle(OpenThread(thread_info));
129 129
130 // TODO(scottmg): Handle cross-bitness in this function. 130 // TODO(scottmg): Handle cross-bitness in this function.
131 131
132 if (is_current_thread) { 132 if (is_current_thread) {
133 DCHECK(suspension_state == ProcessSuspensionState::kRunning);
133 thread->suspend_count = 0; 134 thread->suspend_count = 0;
134 RtlCaptureContext(&thread->context); 135 RtlCaptureContext(&thread->context);
135 } else { 136 } else {
136 DWORD previous_suspend_count = SuspendThread(thread_handle.get()); 137 DWORD previous_suspend_count = SuspendThread(thread_handle.get());
137 if (previous_suspend_count == -1) { 138 if (previous_suspend_count == -1) {
138 PLOG(ERROR) << "SuspendThread failed"; 139 PLOG(ERROR) << "SuspendThread failed";
139 return; 140 return;
140 } 141 }
141 thread->suspend_count = previous_suspend_count; 142 DCHECK(previous_suspend_count > 0 ||
143 suspension_state == ProcessSuspensionState::kRunning);
144 thread->suspend_count =
145 previous_suspend_count -
146 (suspension_state == ProcessSuspensionState::kSuspended ? 1 : 0);
142 147
143 memset(&thread->context, 0, sizeof(thread->context)); 148 memset(&thread->context, 0, sizeof(thread->context));
144 thread->context.ContextFlags = CONTEXT_ALL; 149 thread->context.ContextFlags = CONTEXT_ALL;
145 if (!GetThreadContext(thread_handle.get(), &thread->context)) { 150 if (!GetThreadContext(thread_handle.get(), &thread->context)) {
146 PLOG(ERROR) << "GetThreadContext failed"; 151 PLOG(ERROR) << "GetThreadContext failed";
147 return; 152 return;
148 } 153 }
149 154
150 if (!ResumeThread(thread_handle.get())) { 155 if (!ResumeThread(thread_handle.get())) {
151 PLOG(ERROR) << "ResumeThread failed"; 156 PLOG(ERROR) << "ResumeThread failed";
(...skipping 12 matching lines...) Expand all
164 suspend_count(0), 169 suspend_count(0),
165 priority_class(0), 170 priority_class(0),
166 priority(0) { 171 priority(0) {
167 } 172 }
168 173
169 ProcessReaderWin::ProcessReaderWin() 174 ProcessReaderWin::ProcessReaderWin()
170 : process_(INVALID_HANDLE_VALUE), 175 : process_(INVALID_HANDLE_VALUE),
171 process_info_(), 176 process_info_(),
172 threads_(), 177 threads_(),
173 modules_(), 178 modules_(),
179 suspension_state_(),
174 initialized_threads_(false), 180 initialized_threads_(false),
175 initialized_() { 181 initialized_() {
176 } 182 }
177 183
178 ProcessReaderWin::~ProcessReaderWin() { 184 ProcessReaderWin::~ProcessReaderWin() {
179 } 185 }
180 186
181 bool ProcessReaderWin::Initialize(HANDLE process) { 187 bool ProcessReaderWin::Initialize(HANDLE process,
188 ProcessSuspensionState suspension_state) {
182 INITIALIZATION_STATE_SET_INITIALIZING(initialized_); 189 INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
183 190
184 process_ = process; 191 process_ = process;
192 suspension_state_ = suspension_state;
185 process_info_.Initialize(process); 193 process_info_.Initialize(process);
186 194
187 INITIALIZATION_STATE_SET_VALID(initialized_); 195 INITIALIZATION_STATE_SET_VALID(initialized_);
188 return true; 196 return true;
189 } 197 }
190 198
191 bool ProcessReaderWin::ReadMemory(WinVMAddress at, 199 bool ProcessReaderWin::ReadMemory(WinVMAddress at,
192 WinVMSize num_bytes, 200 WinVMSize num_bytes,
193 void* into) { 201 void* into) {
194 SIZE_T bytes_read; 202 SIZE_T bytes_read;
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
247 GetProcessInformation<SizeTraits>(process_, &buffer); 255 GetProcessInformation<SizeTraits>(process_, &buffer);
248 if (!process_information) 256 if (!process_information)
249 return threads_; 257 return threads_;
250 258
251 for (unsigned long i = 0; i < process_information->NumberOfThreads; ++i) { 259 for (unsigned long i = 0; i < process_information->NumberOfThreads; ++i) {
252 const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<SizeTraits>& 260 const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION<SizeTraits>&
253 thread_info = process_information->Threads[i]; 261 thread_info = process_information->Threads[i];
254 Thread thread; 262 Thread thread;
255 thread.id = thread_info.ClientId.UniqueThread; 263 thread.id = thread_info.ClientId.UniqueThread;
256 264
257 FillThreadContextAndSuspendCount(thread_info, &thread); 265 FillThreadContextAndSuspendCount(thread_info, &thread, suspension_state_);
258 266
259 // TODO(scottmg): I believe we could reverse engineer the PriorityClass from 267 // TODO(scottmg): I believe we could reverse engineer the PriorityClass from
260 // the Priority, BasePriority, and 268 // the Priority, BasePriority, and
261 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100 . 269 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100 .
262 // MinidumpThreadWriter doesn't handle it yet in any case, so investigate 270 // MinidumpThreadWriter doesn't handle it yet in any case, so investigate
263 // both of those at the same time if it's useful. 271 // both of those at the same time if it's useful.
264 thread.priority_class = NORMAL_PRIORITY_CLASS; 272 thread.priority_class = NORMAL_PRIORITY_CLASS;
265 273
266 thread.priority = thread_info.Priority; 274 thread.priority = thread_info.Priority;
267 thread.teb = thread_info.TebBase; 275 thread.teb = thread_info.TebBase;
(...skipping 19 matching lines...) Expand all
287 INITIALIZATION_STATE_DCHECK_VALID(initialized_); 295 INITIALIZATION_STATE_DCHECK_VALID(initialized_);
288 296
289 if (!process_info_.Modules(&modules_)) { 297 if (!process_info_.Modules(&modules_)) {
290 LOG(ERROR) << "couldn't retrieve modules"; 298 LOG(ERROR) << "couldn't retrieve modules";
291 } 299 }
292 300
293 return modules_; 301 return modules_;
294 } 302 }
295 303
296 } // namespace crashpad 304 } // namespace crashpad
OLDNEW
« no previous file with comments | « snapshot/win/process_reader_win.h ('k') | snapshot/win/process_reader_win_test.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698