Treat warnings as error and fix most level 4 warnings in the breakpad windows client projects.

Some of the lint errors in the files touched by this change were also fixed.

BUG=533
Review URL: https://breakpad.appspot.com/601002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1189 4c0a9323-5329-0410-9bdc-e9ce6186880e
This commit is contained in:
ivan.penkov@gmail.com 2013-06-04 16:51:01 +00:00
parent 5bdee65f7a
commit 6b46d4e872
12 changed files with 173 additions and 124 deletions

View file

@ -507,7 +507,7 @@
'msvs_disabled_warnings': [4800], 'msvs_disabled_warnings': [4800],
'msvs_settings': { 'msvs_settings': {
'VCCLCompilerTool': { 'VCCLCompilerTool': {
'WarnAsError': 'false', 'WarnAsError': 'true',
'Detect64BitPortabilityProblems': 'false', 'Detect64BitPortabilityProblems': 'false',
}, },
}, },
@ -1174,7 +1174,7 @@
'$(VSInstallDir)/VC/atlmfc/include', '$(VSInstallDir)/VC/atlmfc/include',
], ],
'msvs_cygwin_dirs': ['<(DEPTH)/third_party/cygwin'], 'msvs_cygwin_dirs': ['<(DEPTH)/third_party/cygwin'],
'msvs_disabled_warnings': [4396, 4503, 4819], 'msvs_disabled_warnings': [4100, 4127, 4396, 4503, 4512, 4819, 4995],
'msvs_settings': { 'msvs_settings': {
'VCCLCompilerTool': { 'VCCLCompilerTool': {
'MinimalRebuild': 'false', 'MinimalRebuild': 'false',
@ -1182,7 +1182,7 @@
'BufferSecurityCheck': 'true', 'BufferSecurityCheck': 'true',
'EnableFunctionLevelLinking': 'true', 'EnableFunctionLevelLinking': 'true',
'RuntimeTypeInfo': 'false', 'RuntimeTypeInfo': 'false',
'WarningLevel': '3', 'WarningLevel': '4',
'WarnAsError': 'true', 'WarnAsError': 'true',
'DebugInformationFormat': '3', 'DebugInformationFormat': '3',
'conditions': [ 'conditions': [

View file

@ -96,7 +96,7 @@ ExceptionHandler::ExceptionHandler(const wstring& dump_path,
pipe_handle, pipe_handle,
NULL, // crash_generation_client NULL, // crash_generation_client
custom_info); custom_info);
} }
ExceptionHandler::ExceptionHandler( ExceptionHandler::ExceptionHandler(
const wstring& dump_path, const wstring& dump_path,
@ -104,19 +104,19 @@ ExceptionHandler::ExceptionHandler(
MinidumpCallback callback, MinidumpCallback callback,
void* callback_context, void* callback_context,
int handler_types, int handler_types,
MINIDUMP_TYPE dump_type, CrashGenerationClient* crash_generation_client) {
CrashGenerationClient* crash_generation_client, // The dump_type, pipe_name and custom_info that are passed in to Initialize()
const CustomClientInfo* custom_info) { // are not used. The ones set in crash_generation_client are used instead.
Initialize(dump_path, Initialize(dump_path,
filter, filter,
callback, callback,
callback_context, callback_context,
handler_types, handler_types,
MiniDumpNormal, MiniDumpNormal, // dump_type - not used
NULL, // pipe_name NULL, // pipe_name - not used
NULL, // pipe_handle NULL, // pipe_handle
crash_generation_client, crash_generation_client,
custom_info); NULL); // custom_info - not used
} }
ExceptionHandler::ExceptionHandler(const wstring &dump_path, ExceptionHandler::ExceptionHandler(const wstring &dump_path,
@ -875,7 +875,7 @@ BOOL CALLBACK ExceptionHandler::MinidumpWriteDumpCallback(
callback_context->iter++; callback_context->iter++;
return TRUE; return TRUE;
} }
// Include all modules. // Include all modules.
case IncludeModuleCallback: case IncludeModuleCallback:
case ModuleCallback: case ModuleCallback:

View file

@ -61,9 +61,9 @@
#include <DbgHelp.h> #include <DbgHelp.h>
#include <rpc.h> #include <rpc.h>
#pragma warning( push ) #pragma warning(push)
// Disable exception handler warnings. // Disable exception handler warnings.
#pragma warning( disable : 4530 ) #pragma warning(disable:4530)
#include <list> #include <list>
#include <string> #include <string>
@ -212,9 +212,7 @@ class ExceptionHandler {
MinidumpCallback callback, MinidumpCallback callback,
void* callback_context, void* callback_context,
int handler_types, int handler_types,
MINIDUMP_TYPE dump_type, CrashGenerationClient* crash_generation_client);
CrashGenerationClient* crash_generation_client,
const CustomClientInfo* custom_info);
~ExceptionHandler(); ~ExceptionHandler();
@ -497,7 +495,7 @@ class ExceptionHandler {
static CRITICAL_SECTION handler_stack_critical_section_; static CRITICAL_SECTION handler_stack_critical_section_;
// The number of instances of this class. // The number of instances of this class.
volatile static LONG instance_count_; static volatile LONG instance_count_;
// disallow copy ctor and operator= // disallow copy ctor and operator=
explicit ExceptionHandler(const ExceptionHandler &); explicit ExceptionHandler(const ExceptionHandler &);
@ -506,6 +504,6 @@ class ExceptionHandler {
} // namespace google_breakpad } // namespace google_breakpad
#pragma warning( pop ) #pragma warning(pop)
#endif // CLIENT_WINDOWS_HANDLER_EXCEPTION_HANDLER_H__ #endif // CLIENT_WINDOWS_HANDLER_EXCEPTION_HANDLER_H__

View file

@ -345,9 +345,9 @@ void CleanUp() {
// Processes messages for the main window. // Processes messages for the main window.
// //
// WM_COMMAND - process the application menu. // WM_COMMAND - process the application menu.
// WM_PAINT - Paint the main window. // WM_PAINT - Paint the main window.
// WM_DESTROY - post a quit message and return. // WM_DESTROY - post a quit message and return.
LRESULT CALLBACK WndProc(HWND wnd, LRESULT CALLBACK WndProc(HWND wnd,
UINT message, UINT message,
WPARAM w_param, WPARAM w_param,
@ -359,7 +359,7 @@ LRESULT CALLBACK WndProc(HWND wnd,
#pragma warning(push) #pragma warning(push)
#pragma warning(disable:4312) #pragma warning(disable:4312)
// Disable warning C4312: 'type cast' : conversion from 'LONG' to // Disable warning C4312: 'type cast' : conversion from 'LONG' to
// 'HINSTANCE' of greater size. // 'HINSTANCE' of greater size.
// The value returned by GetwindowLong in the case below returns unsigned. // The value returned by GetwindowLong in the case below returns unsigned.
HINSTANCE instance = (HINSTANCE)GetWindowLong(wnd, GWL_HINSTANCE); HINSTANCE instance = (HINSTANCE)GetWindowLong(wnd, GWL_HINSTANCE);
@ -415,16 +415,16 @@ LRESULT CALLBACK WndProc(HWND wnd,
instance, instance,
NULL); NULL);
break; break;
case WM_SIZE: case WM_SIZE:
// Make the edit control the size of the window's client area. // Make the edit control the size of the window's client area.
MoveWindow(client_status_edit_box, MoveWindow(client_status_edit_box,
0, 0,
0, 0,
LOWORD(l_param), // width of client area. LOWORD(l_param), // width of client area.
HIWORD(l_param), // height of client area. HIWORD(l_param), // height of client area.
TRUE); // repaint window. TRUE); // repaint window.
break; break;
case WM_SETFOCUS: case WM_SETFOCUS:
SetFocus(client_status_edit_box); SetFocus(client_status_edit_box);
break; break;
case WM_PAINT: case WM_PAINT:
@ -501,7 +501,7 @@ int APIENTRY _tWinMain(HINSTANCE instance,
MyRegisterClass(instance); MyRegisterClass(instance);
// Perform application initialization. // Perform application initialization.
if (!InitInstance (instance, command_show)) { if (!InitInstance(instance, command_show)) {
return FALSE; return FALSE;
} }
@ -518,5 +518,5 @@ int APIENTRY _tWinMain(HINSTANCE instance,
} }
} }
return (int)msg.wParam; return static_cast<int>(msg.wParam);
} }

View file

@ -124,17 +124,19 @@ TEST_F(ExceptionHandlerDeathTest, InProcTest) {
// the semantics of the exception handler being inherited/not // the semantics of the exception handler being inherited/not
// inherited across CreateProcess(). // inherited across CreateProcess().
ASSERT_TRUE(DoesPathExist(temp_path_)); ASSERT_TRUE(DoesPathExist(temp_path_));
google_breakpad::ExceptionHandler *exc = scoped_ptr<google_breakpad::ExceptionHandler> exc(
new google_breakpad::ExceptionHandler( new google_breakpad::ExceptionHandler(
temp_path_, NULL, &MinidumpWrittenCallback, NULL, temp_path_,
google_breakpad::ExceptionHandler::HANDLER_ALL); NULL,
&MinidumpWrittenCallback,
NULL,
google_breakpad::ExceptionHandler::HANDLER_ALL));
// Disable GTest SEH handler // Disable GTest SEH handler
testing::DisableExceptionHandlerInScope disable_exception_handler; testing::DisableExceptionHandlerInScope disable_exception_handler;
int *i = NULL; int *i = NULL;
ASSERT_DEATH((*i)++, kSuccessIndicator); ASSERT_DEATH((*i)++, kSuccessIndicator);
delete exc;
} }
static bool gDumpCallbackCalled = false; static bool gDumpCallbackCalled = false;
@ -147,26 +149,24 @@ void clientDumpCallback(void *dump_context,
void ExceptionHandlerDeathTest::DoCrashAccessViolation( void ExceptionHandlerDeathTest::DoCrashAccessViolation(
const OutOfProcGuarantee out_of_proc_guarantee) { const OutOfProcGuarantee out_of_proc_guarantee) {
google_breakpad::ExceptionHandler *exc = NULL; scoped_ptr<google_breakpad::ExceptionHandler> exc;
if (out_of_proc_guarantee == OUT_OF_PROC_GUARANTEED) { if (out_of_proc_guarantee == OUT_OF_PROC_GUARANTEED) {
google_breakpad::CrashGenerationClient *client = google_breakpad::CrashGenerationClient *client =
new google_breakpad::CrashGenerationClient(kPipeName, new google_breakpad::CrashGenerationClient(kPipeName,
MiniDumpNormal, MiniDumpNormal,
NULL); // custom_info NULL); // custom_info
ASSERT_TRUE(client->Register()); ASSERT_TRUE(client->Register());
exc = new google_breakpad::ExceptionHandler( exc.reset(new google_breakpad::ExceptionHandler(
temp_path_, temp_path_,
NULL, // filter NULL, // filter
NULL, // callback NULL, // callback
NULL, // callback_context NULL, // callback_context
google_breakpad::ExceptionHandler::HANDLER_ALL, google_breakpad::ExceptionHandler::HANDLER_ALL,
MiniDumpNormal, client));
client,
NULL); // custom_info
} else { } else {
ASSERT_TRUE(out_of_proc_guarantee == OUT_OF_PROC_BEST_EFFORT); ASSERT_TRUE(out_of_proc_guarantee == OUT_OF_PROC_BEST_EFFORT);
exc = new google_breakpad::ExceptionHandler( exc.reset(new google_breakpad::ExceptionHandler(
temp_path_, temp_path_,
NULL, // filter NULL, // filter
NULL, // callback NULL, // callback
@ -174,7 +174,7 @@ void ExceptionHandlerDeathTest::DoCrashAccessViolation(
google_breakpad::ExceptionHandler::HANDLER_ALL, google_breakpad::ExceptionHandler::HANDLER_ALL,
MiniDumpNormal, MiniDumpNormal,
kPipeName, kPipeName,
NULL); // custom_info NULL)); // custom_info
} }
// Disable GTest SEH handler // Disable GTest SEH handler
@ -302,17 +302,20 @@ wstring find_minidump_in_directory(const wstring &directory) {
filename = directory + L"\\" + find_data.cFileName; filename = directory + L"\\" + find_data.cFileName;
break; break;
} }
} while(FindNextFile(find_handle, &find_data)); } while (FindNextFile(find_handle, &find_data));
FindClose(find_handle); FindClose(find_handle);
return filename; return filename;
} }
TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) { TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
ASSERT_TRUE(DoesPathExist(temp_path_)); ASSERT_TRUE(DoesPathExist(temp_path_));
google_breakpad::ExceptionHandler *exc = scoped_ptr<google_breakpad::ExceptionHandler> exc(
new google_breakpad::ExceptionHandler( new google_breakpad::ExceptionHandler(
temp_path_, NULL, NULL, NULL, temp_path_,
google_breakpad::ExceptionHandler::HANDLER_ALL); NULL,
NULL,
NULL,
google_breakpad::ExceptionHandler::HANDLER_ALL));
// Disable GTest SEH handler // Disable GTest SEH handler
testing::DisableExceptionHandlerInScope disable_exception_handler; testing::DisableExceptionHandlerInScope disable_exception_handler;
@ -333,7 +336,7 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
// minidump should contain 128 bytes on either side of the // minidump should contain 128 bytes on either side of the
// instruction pointer. // instruction pointer.
memcpy(memory + kOffset, instructions, sizeof(instructions)); memcpy(memory + kOffset, instructions, sizeof(instructions));
// Now execute the instructions, which should crash. // Now execute the instructions, which should crash.
typedef void (*void_function)(void); typedef void (*void_function)(void);
void_function memory_function = void_function memory_function =
@ -382,11 +385,10 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
uint8_t suffix_bytes[kMemorySize - kOffset - sizeof(instructions)]; uint8_t suffix_bytes[kMemorySize - kOffset - sizeof(instructions)];
memset(prefix_bytes, 0, sizeof(prefix_bytes)); memset(prefix_bytes, 0, sizeof(prefix_bytes));
memset(suffix_bytes, 0, sizeof(suffix_bytes)); memset(suffix_bytes, 0, sizeof(suffix_bytes));
EXPECT_TRUE(memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)) == 0); EXPECT_EQ(0, memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)));
EXPECT_TRUE(memcmp(bytes + kOffset, instructions, EXPECT_EQ(0, memcmp(bytes + kOffset, instructions, sizeof(instructions)));
sizeof(instructions)) == 0); EXPECT_EQ(0, memcmp(bytes + kOffset + sizeof(instructions),
EXPECT_TRUE(memcmp(bytes + kOffset + sizeof(instructions), suffix_bytes, sizeof(suffix_bytes)));
suffix_bytes, sizeof(suffix_bytes)) == 0);
} }
DeleteFileW(minidump_filename_wide.c_str()); DeleteFileW(minidump_filename_wide.c_str());
@ -394,10 +396,13 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemory) {
TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) { TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) {
ASSERT_TRUE(DoesPathExist(temp_path_)); ASSERT_TRUE(DoesPathExist(temp_path_));
google_breakpad::ExceptionHandler *exc = scoped_ptr<google_breakpad::ExceptionHandler> exc(
new google_breakpad::ExceptionHandler( new google_breakpad::ExceptionHandler(
temp_path_, NULL, NULL, NULL, temp_path_,
google_breakpad::ExceptionHandler::HANDLER_ALL); NULL,
NULL,
NULL,
google_breakpad::ExceptionHandler::HANDLER_ALL));
// Disable GTest SEH handler // Disable GTest SEH handler
testing::DisableExceptionHandlerInScope disable_exception_handler; testing::DisableExceptionHandlerInScope disable_exception_handler;
@ -426,7 +431,7 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) {
// minidump should contain 128 bytes on either side of the // minidump should contain 128 bytes on either side of the
// instruction pointer. // instruction pointer.
memcpy(memory + kOffset, instructions, sizeof(instructions)); memcpy(memory + kOffset, instructions, sizeof(instructions));
// Now execute the instructions, which should crash. // Now execute the instructions, which should crash.
typedef void (*void_function)(void); typedef void (*void_function)(void);
void_function memory_function = void_function memory_function =
@ -484,10 +489,13 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMinBound) {
TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) { TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) {
ASSERT_TRUE(DoesPathExist(temp_path_)); ASSERT_TRUE(DoesPathExist(temp_path_));
google_breakpad::ExceptionHandler *exc = scoped_ptr<google_breakpad::ExceptionHandler> exc(
new google_breakpad::ExceptionHandler( new google_breakpad::ExceptionHandler(
temp_path_, NULL, NULL, NULL, temp_path_,
google_breakpad::ExceptionHandler::HANDLER_ALL); NULL,
NULL,
NULL,
google_breakpad::ExceptionHandler::HANDLER_ALL));
// Disable GTest SEH handler // Disable GTest SEH handler
testing::DisableExceptionHandlerInScope disable_exception_handler; testing::DisableExceptionHandlerInScope disable_exception_handler;
@ -511,7 +519,7 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) {
// Write some instructions that will crash. // Write some instructions that will crash.
memcpy(memory + kOffset, instructions, sizeof(instructions)); memcpy(memory + kOffset, instructions, sizeof(instructions));
// Now execute the instructions, which should crash. // Now execute the instructions, which should crash.
typedef void (*void_function)(void); typedef void (*void_function)(void);
void_function memory_function = void_function memory_function =
@ -559,9 +567,9 @@ TEST_F(ExceptionHandlerDeathTest, InstructionPointerMemoryMaxBound) {
uint8_t prefix_bytes[kPrefixSize]; uint8_t prefix_bytes[kPrefixSize];
memset(prefix_bytes, 0, sizeof(prefix_bytes)); memset(prefix_bytes, 0, sizeof(prefix_bytes));
EXPECT_TRUE(memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)) == 0); EXPECT_EQ(0, memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)));
EXPECT_TRUE(memcmp(bytes + kPrefixSize, EXPECT_EQ(0, memcmp(bytes + kPrefixSize,
instructions, sizeof(instructions)) == 0); instructions, sizeof(instructions)));
} }
DeleteFileW(minidump_filename_wide.c_str()); DeleteFileW(minidump_filename_wide.c_str());

View file

@ -149,7 +149,7 @@ void InstallExceptionHandlerAndCrash(bool install_filter,
ASSERT_TRUE(DoesPathExist(temp_path)); ASSERT_TRUE(DoesPathExist(temp_path));
google_breakpad::ExceptionHandler exc( google_breakpad::ExceptionHandler exc(
temp_path, temp_path,
install_filter ? install_filter ?
(filter_return_value ? (filter_return_value ?
&CrashHandlerFilter<true> : &CrashHandlerFilter<true> :
&CrashHandlerFilter<false>) : &CrashHandlerFilter<false>) :
@ -178,7 +178,7 @@ TEST(AssertDeathSanity, Regex) {
std::string(kFoo) + std::string(kFoo) +
std::string(kEndOfLine)); std::string(kEndOfLine));
ASSERT_DEATH(DoCrash(kBar), ASSERT_DEATH(DoCrash(kBar),
std::string(kStartOfLine) + std::string(kStartOfLine) +
std::string(kBar) + std::string(kBar) +
std::string(kEndOfLine)); std::string(kEndOfLine));

View file

@ -390,7 +390,7 @@ TEST_F(ExceptionHandlerTest, WriteMinidumpTest) {
// Read the minidump and verify some info. // Read the minidump and verify some info.
Minidump minidump(minidump_filename); Minidump minidump(minidump_filename);
ASSERT_TRUE(minidump.Read()); ASSERT_TRUE(minidump.Read());
//TODO(ted): more comprehensive tests... // TODO(ted): more comprehensive tests...
} }
// Test that an additional memory region can be included in the minidump. // Test that an additional memory region can be included in the minidump.

View file

@ -161,7 +161,8 @@ bool HasFileInfo(const std::wstring& file_path) {
} }
TEST_F(MinidumpTest, Version) { TEST_F(MinidumpTest, Version) {
API_VERSION* version = ::ImagehlpApiVersion(); // Loads DbgHelp.dll in process
ImagehlpApiVersion();
HMODULE dbg_help = ::GetModuleHandle(L"dbghelp.dll"); HMODULE dbg_help = ::GetModuleHandle(L"dbghelp.dll");
ASSERT_TRUE(dbg_help != NULL); ASSERT_TRUE(dbg_help != NULL);

View file

@ -140,7 +140,7 @@ string UTF16ToUTF8(const vector<uint16_t> &in, bool swap) {
scoped_array<UTF8> target_buffer(new UTF8[target_capacity]); scoped_array<UTF8> target_buffer(new UTF8[target_capacity]);
UTF8 *target_ptr = target_buffer.get(); UTF8 *target_ptr = target_buffer.get();
UTF8 *target_end_ptr = target_ptr + target_capacity; UTF8 *target_end_ptr = target_ptr + target_capacity;
ConversionResult result = ConvertUTF16toUTF8(&source_ptr, source_end_ptr, ConversionResult result = ConvertUTF16toUTF8(&source_ptr, source_end_ptr,
&target_ptr, target_end_ptr, &target_ptr, target_end_ptr,
strictConversion); strictConversion);

View file

@ -30,7 +30,7 @@
#include <assert.h> #include <assert.h>
// Disable exception handler warnings. // Disable exception handler warnings.
#pragma warning( disable : 4530 ) #pragma warning(disable:4530)
#include <fstream> #include <fstream>
@ -163,7 +163,7 @@ bool HTTPUpload::SendRequest(const wstring &url,
fwprintf(stderr, L"Could not unset receive timeout, continuing...\n"); fwprintf(stderr, L"Could not unset receive timeout, continuing...\n");
} }
} }
if (!HttpSendRequest(request.get(), NULL, 0, if (!HttpSendRequest(request.get(), NULL, 0,
const_cast<char *>(request_body.data()), const_cast<char *>(request_body.data()),
static_cast<DWORD>(request_body.size()))) { static_cast<DWORD>(request_body.size()))) {
@ -215,8 +215,7 @@ bool HTTPUpload::ReadResponse(HINTERNET request, wstring *response) {
BOOL return_code; BOOL return_code;
while (((return_code = InternetQueryDataAvailable(request, &bytes_available, while (((return_code = InternetQueryDataAvailable(request, &bytes_available,
0, 0)) != 0) && bytes_available > 0) { 0, 0)) != 0) && bytes_available > 0) {
vector<char> response_buffer(bytes_available); vector<char> response_buffer(bytes_available);
DWORD size_read; DWORD size_read;
@ -323,6 +322,7 @@ bool HTTPUpload::GenerateRequestBody(const map<wstring, wstring> &parameters,
// static // static
bool HTTPUpload::GetFileContents(const wstring &filename, bool HTTPUpload::GetFileContents(const wstring &filename,
vector<char> *contents) { vector<char> *contents) {
bool rv = false;
// The "open" method on pre-MSVC8 ifstream implementations doesn't accept a // The "open" method on pre-MSVC8 ifstream implementations doesn't accept a
// wchar_t* filename, so use _wfopen directly in that case. For VC8 and // wchar_t* filename, so use _wfopen directly in that case. For VC8 and
// later, _wfopen has been deprecated in favor of _wfopen_s, which does // later, _wfopen has been deprecated in favor of _wfopen_s, which does
@ -336,15 +336,21 @@ bool HTTPUpload::GetFileContents(const wstring &filename,
if (file.is_open()) { if (file.is_open()) {
file.seekg(0, ios::end); file.seekg(0, ios::end);
std::streamoff length = file.tellg(); std::streamoff length = file.tellg();
contents->resize(length); // Check for loss of data when converting lenght from std::streamoff into
if (length != 0) { // std::vector<char>::size_type
file.seekg(0, ios::beg); std::vector<char>::size_type vector_size =
file.read(&((*contents)[0]), length); static_cast<std::vector<char>::size_type>(length);
if (static_cast<std::streamoff>(vector_size) == length) {
contents->resize(vector_size);
if (length != 0) {
file.seekg(0, ios::beg);
file.read(&((*contents)[0]), length);
}
rv = true;
} }
file.close(); file.close();
return true;
} }
return false; return rv;
} }
// static // static

View file

@ -477,7 +477,7 @@ class MinidumpModule : public MinidumpObject,
// True after a successful Read. This is different from valid_, which is // True after a successful Read. This is different from valid_, which is
// not set true until ReadAuxiliaryData also completes successfully. // not set true until ReadAuxiliaryData also completes successfully.
// module_valid_ is only used by ReadAuxiliaryData and the functions it // module_valid_ is only used by ReadAuxiliaryData and the functions it
// calls to determine whether the object is ready for auxiliary data to // calls to determine whether the object is ready for auxiliary data to
// be read. // be read.
bool module_valid_; bool module_valid_;
@ -821,7 +821,7 @@ class MinidumpMemoryInfo : public MinidumpObject {
uint64_t GetBase() const { return valid_ ? memory_info_.base_address : 0; } uint64_t GetBase() const { return valid_ ? memory_info_.base_address : 0; }
// The size, in bytes, of the memory region. // The size, in bytes, of the memory region.
uint32_t GetSize() const { return valid_ ? memory_info_.region_size : 0; } uint64_t GetSize() const { return valid_ ? memory_info_.region_size : 0; }
// Return true if the memory protection allows execution. // Return true if the memory protection allows execution.
bool IsExecutable() const; bool IsExecutable() const;

View file

@ -65,8 +65,6 @@
#include "processor/basic_code_modules.h" #include "processor/basic_code_modules.h"
#include "processor/logging.h" #include "processor/logging.h"
namespace google_breakpad { namespace google_breakpad {
@ -226,19 +224,19 @@ static string* UTF16ToUTF8(const vector<uint16_t>& in,
// Convert the Unicode code point (unichar) into its UTF-8 representation, // Convert the Unicode code point (unichar) into its UTF-8 representation,
// appending it to the out string. // appending it to the out string.
if (unichar < 0x80) { if (unichar < 0x80) {
(*out) += unichar; (*out) += static_cast<char>(unichar);
} else if (unichar < 0x800) { } else if (unichar < 0x800) {
(*out) += 0xc0 | (unichar >> 6); (*out) += 0xc0 | static_cast<char>(unichar >> 6);
(*out) += 0x80 | (unichar & 0x3f); (*out) += 0x80 | static_cast<char>(unichar & 0x3f);
} else if (unichar < 0x10000) { } else if (unichar < 0x10000) {
(*out) += 0xe0 | (unichar >> 12); (*out) += 0xe0 | static_cast<char>(unichar >> 12);
(*out) += 0x80 | ((unichar >> 6) & 0x3f); (*out) += 0x80 | static_cast<char>((unichar >> 6) & 0x3f);
(*out) += 0x80 | (unichar & 0x3f); (*out) += 0x80 | static_cast<char>(unichar & 0x3f);
} else if (unichar < 0x200000) { } else if (unichar < 0x200000) {
(*out) += 0xf0 | (unichar >> 18); (*out) += 0xf0 | static_cast<char>(unichar >> 18);
(*out) += 0x80 | ((unichar >> 12) & 0x3f); (*out) += 0x80 | static_cast<char>((unichar >> 12) & 0x3f);
(*out) += 0x80 | ((unichar >> 6) & 0x3f); (*out) += 0x80 | static_cast<char>((unichar >> 6) & 0x3f);
(*out) += 0x80 | (unichar & 0x3f); (*out) += 0x80 | static_cast<char>(unichar & 0x3f);
} else { } else {
BPLOG(ERROR) << "UTF16ToUTF8 cannot represent high value " << BPLOG(ERROR) << "UTF16ToUTF8 cannot represent high value " <<
HexString(unichar) << " in UTF-8"; HexString(unichar) << " in UTF-8";
@ -329,7 +327,7 @@ bool MinidumpContext::Read(uint32_t expected_size) {
} }
if (cpu_type != MD_CONTEXT_AMD64) { if (cpu_type != MD_CONTEXT_AMD64) {
//TODO: fall through to switch below? // TODO: fall through to switch below?
// need a Tell method to be able to SeekSet back to beginning // need a Tell method to be able to SeekSet back to beginning
// http://code.google.com/p/google-breakpad/issues/detail?id=224 // http://code.google.com/p/google-breakpad/issues/detail?id=224
BPLOG(ERROR) << "MinidumpContext not actually amd64 context"; BPLOG(ERROR) << "MinidumpContext not actually amd64 context";
@ -388,7 +386,7 @@ bool MinidumpContext::Read(uint32_t expected_size) {
Swap(&context_amd64->r14); Swap(&context_amd64->r14);
Swap(&context_amd64->r15); Swap(&context_amd64->r15);
Swap(&context_amd64->rip); Swap(&context_amd64->rip);
//FIXME: I'm not sure what actually determines // FIXME: I'm not sure what actually determines
// which member of the union {flt_save, sse_registers} // which member of the union {flt_save, sse_registers}
// is valid. We're not currently using either, // is valid. We're not currently using either,
// but it would be good to have them swapped properly. // but it would be good to have them swapped properly.
@ -408,10 +406,9 @@ bool MinidumpContext::Read(uint32_t expected_size) {
context_flags_ = context_amd64->context_flags; context_flags_ = context_amd64->context_flags;
context_.amd64 = context_amd64.release(); context_.amd64 = context_amd64.release();
} } else if (expected_size == sizeof(MDRawContextPPC64)) {
// |context_flags| of MDRawContextPPC64 is 64 bits, but other MDRawContext // |context_flags| of MDRawContextPPC64 is 64 bits, but other MDRawContext
// in the else case have 32 bits |context_flags|, so special case it here. // in the else case have 32 bits |context_flags|, so special case it here.
else if (expected_size == sizeof(MDRawContextPPC64)) {
uint64_t context_flags; uint64_t context_flags;
if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) { if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
BPLOG(ERROR) << "MinidumpContext could not read context flags"; BPLOG(ERROR) << "MinidumpContext could not read context flags";
@ -421,9 +418,9 @@ bool MinidumpContext::Read(uint32_t expected_size) {
Swap(&context_flags); Swap(&context_flags);
uint32_t cpu_type = context_flags & MD_CONTEXT_CPU_MASK; uint32_t cpu_type = context_flags & MD_CONTEXT_CPU_MASK;
scoped_ptr<MDRawContextPPC64> context_ppc64(new MDRawContextPPC64()); scoped_ptr<MDRawContextPPC64> context_ppc64(new MDRawContextPPC64());
// Set the context_flags member, which has already been read, and // Set the context_flags member, which has already been read, and
// read the rest of the structure beginning with the first member // read the rest of the structure beginning with the first member
// after context_flags. // after context_flags.
@ -477,11 +474,18 @@ bool MinidumpContext::Read(uint32_t expected_size) {
Swap(&context_ppc64->vector_save.save_vrvalid); Swap(&context_ppc64->vector_save.save_vrvalid);
} }
context_flags_ = context_ppc64->context_flags; context_flags_ = static_cast<uint32_t>(context_ppc64->context_flags);
context_.ppc64 = context_ppc64.release();
}
else { // Check for data loss when converting context flags from uint64_t into
// uint32_t
if (static_cast<uint64_t>(context_flags_) !=
context_ppc64->context_flags) {
BPLOG(ERROR) << "Data loss detected when converting PPC64 context_flags";
return false;
}
context_.ppc64 = context_ppc64.release();
} else {
uint32_t context_flags; uint32_t context_flags;
if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) { if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
BPLOG(ERROR) << "MinidumpContext could not read context flags"; BPLOG(ERROR) << "MinidumpContext could not read context flags";
@ -1205,7 +1209,7 @@ void MinidumpContext::Print() {
printf(" r14 = 0x%" PRIx64 "\n", context_amd64->r14); printf(" r14 = 0x%" PRIx64 "\n", context_amd64->r14);
printf(" r15 = 0x%" PRIx64 "\n", context_amd64->r15); printf(" r15 = 0x%" PRIx64 "\n", context_amd64->r15);
printf(" rip = 0x%" PRIx64 "\n", context_amd64->rip); printf(" rip = 0x%" PRIx64 "\n", context_amd64->rip);
//TODO: print xmm, vector, debug registers // TODO: print xmm, vector, debug registers
printf("\n"); printf("\n");
break; break;
} }
@ -1673,7 +1677,8 @@ bool MinidumpThreadList::Read(uint32_t expected_size) {
thread_count * sizeof(MDRawThread)) { thread_count * sizeof(MDRawThread)) {
uint32_t useless; uint32_t useless;
if (!minidump_->ReadBytes(&useless, 4)) { if (!minidump_->ReadBytes(&useless, 4)) {
BPLOG(ERROR) << "MinidumpThreadList cannot read threadlist padded bytes"; BPLOG(ERROR) << "MinidumpThreadList cannot read threadlist padded "
"bytes";
return false; return false;
} }
} else { } else {
@ -1950,7 +1955,7 @@ string MinidumpModule::code_identifier() const {
case MD_OS_IOS: case MD_OS_IOS:
case MD_OS_SOLARIS: case MD_OS_SOLARIS:
case MD_OS_ANDROID: case MD_OS_ANDROID:
case MD_OS_LINUX: case MD_OS_LINUX:
case MD_OS_PS3: { case MD_OS_PS3: {
// TODO(mmentovai): support uuid extension if present, otherwise fall // TODO(mmentovai): support uuid extension if present, otherwise fall
// back to version (from LC_ID_DYLIB?), otherwise fall back to something // back to version (from LC_ID_DYLIB?), otherwise fall back to something
@ -2560,7 +2565,8 @@ bool MinidumpModuleList::Read(uint32_t expected_size) {
module_count * MD_MODULE_SIZE) { module_count * MD_MODULE_SIZE) {
uint32_t useless; uint32_t useless;
if (!minidump_->ReadBytes(&useless, 4)) { if (!minidump_->ReadBytes(&useless, 4)) {
BPLOG(ERROR) << "MinidumpModuleList cannot read modulelist padded bytes"; BPLOG(ERROR) << "MinidumpModuleList cannot read modulelist padded "
"bytes";
return false; return false;
} }
} else { } else {
@ -2806,12 +2812,13 @@ bool MinidumpMemoryList::Read(uint32_t expected_size) {
region_count * sizeof(MDMemoryDescriptor)) { region_count * sizeof(MDMemoryDescriptor)) {
uint32_t useless; uint32_t useless;
if (!minidump_->ReadBytes(&useless, 4)) { if (!minidump_->ReadBytes(&useless, 4)) {
BPLOG(ERROR) << "MinidumpMemoryList cannot read memorylist padded bytes"; BPLOG(ERROR) << "MinidumpMemoryList cannot read memorylist padded "
"bytes";
return false; return false;
} }
} else { } else {
BPLOG(ERROR) << "MinidumpMemoryList size mismatch, " << expected_size << BPLOG(ERROR) << "MinidumpMemoryList size mismatch, " << expected_size <<
" != " << sizeof(region_count) + " != " << sizeof(region_count) +
region_count * sizeof(MDMemoryDescriptor); region_count * sizeof(MDMemoryDescriptor);
return false; return false;
} }
@ -3142,7 +3149,7 @@ bool MinidumpAssertion::Read(uint32_t expected_size) {
if (new_expression.get()) if (new_expression.get())
expression_ = *new_expression; expression_ = *new_expression;
} }
// assertion // assertion
word_length = UTF16codeunits(assertion_.function, word_length = UTF16codeunits(assertion_.function,
sizeof(assertion_.function)); sizeof(assertion_.function));
@ -3791,7 +3798,7 @@ bool MinidumpMemoryInfoList::Read(uint32_t expected_size) {
} }
// Sanity check that the header is the expected size. // Sanity check that the header is the expected size.
//TODO(ted): could possibly handle this more gracefully, assuming // TODO(ted): could possibly handle this more gracefully, assuming
// that future versions of the structs would be backwards-compatible. // that future versions of the structs would be backwards-compatible.
if (header.size_of_header != sizeof(MDRawMemoryInfoList)) { if (header.size_of_header != sizeof(MDRawMemoryInfoList)) {
BPLOG(ERROR) << "MinidumpMemoryInfoList header size mismatch, " << BPLOG(ERROR) << "MinidumpMemoryInfoList header size mismatch, " <<
@ -3824,9 +3831,20 @@ bool MinidumpMemoryInfoList::Read(uint32_t expected_size) {
return false; return false;
} }
// Check for data loss when converting header.number_of_entries from
// uint64_t into MinidumpMemoryInfos::size_type (uint32_t)
MinidumpMemoryInfos::size_type header_number_of_entries =
static_cast<unsigned int>(header.number_of_entries);
if (static_cast<uint64_t>(header_number_of_entries) !=
header.number_of_entries) {
BPLOG(ERROR) << "Data loss detected when converting "
"the header's number_of_entries";
return false;
}
if (header.number_of_entries != 0) { if (header.number_of_entries != 0) {
scoped_ptr<MinidumpMemoryInfos> infos( scoped_ptr<MinidumpMemoryInfos> infos(
new MinidumpMemoryInfos(header.number_of_entries, new MinidumpMemoryInfos(header_number_of_entries,
MinidumpMemoryInfo(minidump_))); MinidumpMemoryInfo(minidump_)));
for (unsigned int index = 0; for (unsigned int index = 0;
@ -3842,7 +3860,7 @@ bool MinidumpMemoryInfoList::Read(uint32_t expected_size) {
} }
uint64_t base_address = info->GetBase(); uint64_t base_address = info->GetBase();
uint32_t region_size = info->GetSize(); uint64_t region_size = info->GetSize();
if (!range_map_->StoreRange(base_address, region_size, index)) { if (!range_map_->StoreRange(base_address, region_size, index)) {
BPLOG(ERROR) << "MinidumpMemoryInfoList could not store" BPLOG(ERROR) << "MinidumpMemoryInfoList could not store"
@ -3857,7 +3875,7 @@ bool MinidumpMemoryInfoList::Read(uint32_t expected_size) {
infos_ = infos.release(); infos_ = infos.release();
} }
info_count_ = header.number_of_entries; info_count_ = header_number_of_entries;
valid_ = true; valid_ = true;
return true; return true;
@ -4312,17 +4330,27 @@ bool Minidump::ReadBytes(void* bytes, size_t count) {
return false; return false;
} }
stream_->read(static_cast<char*>(bytes), count); stream_->read(static_cast<char*>(bytes), count);
size_t bytes_read = stream_->gcount(); std::streamsize bytes_read = stream_->gcount();
if (bytes_read != count) { if (bytes_read == -1) {
if (bytes_read == size_t(-1)) { string error_string;
string error_string; int error_code = ErrnoString(&error_string);
int error_code = ErrnoString(&error_string); BPLOG(ERROR) << "ReadBytes: error " << error_code << ": " << error_string;
BPLOG(ERROR) << "ReadBytes: error " << error_code << ": " << error_string;
} else {
BPLOG(ERROR) << "ReadBytes: read " << bytes_read << "/" << count;
}
return false; return false;
} }
// Convert to size_t and check for data loss
size_t bytes_read_converted = static_cast<size_t>(bytes_read);
if (static_cast<std::streamsize>(bytes_read_converted) != bytes_read) {
BPLOG(ERROR) << "ReadBytes: conversion data loss detected when converting "
<< bytes_read << " to " << bytes_read_converted;
return false;
}
if (bytes_read_converted != count) {
BPLOG(ERROR) << "ReadBytes: read " << bytes_read_converted << "/" << count;
return false;
}
return true; return true;
} }
@ -4348,7 +4376,15 @@ off_t Minidump::Tell() {
return (off_t)-1; return (off_t)-1;
} }
return stream_->tellg(); // Check for conversion data loss
std::streamoff std_streamoff = stream_->tellg();
off_t rv = static_cast<off_t>(std_streamoff);
if (static_cast<std::streamoff>(rv) == std_streamoff) {
return rv;
} else {
BPLOG(ERROR) << "Data loss detected";
return (off_t)-1;
}
} }