diff --git a/Makefile.am b/Makefile.am index 53b50831..b6270ad8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -33,6 +33,16 @@ # This allows #includes to be relative to src/ AM_CPPFLAGS = -I$(top_srcdir)/src +if GCC +# These are good warnings to be treated as errors +AM_CXXFLAGS = \ + -Werror=non-virtual-dtor \ + -Werror=vla \ + -Werror=unused-variable \ + -Werror=missing-braces \ + -Werror=overloaded-virtual +endif + # Specify include paths for ac macros ACLOCAL_AMFLAGS = -I m4 @@ -294,6 +304,7 @@ src_client_linux_linux_client_unittest_SOURCES = \ src/client/linux/minidump_writer/linux_core_dumper_unittest.cc \ src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc \ src/client/linux/minidump_writer/minidump_writer_unittest.cc \ + src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc \ src/common/linux/linux_libc_support_unittest.cc \ src/common/linux/tests/crash_generator.cc \ src/common/memory_unittest.cc \ diff --git a/Makefile.in b/Makefile.in index 9c65ee50..a4318fe9 100644 --- a/Makefile.in +++ b/Makefile.in @@ -395,6 +395,7 @@ am__src_client_linux_linux_client_unittest_SOURCES_DIST = \ src/client/linux/minidump_writer/linux_core_dumper_unittest.cc \ src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc \ src/client/linux/minidump_writer/minidump_writer_unittest.cc \ + src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc \ src/common/linux/linux_libc_support_unittest.cc \ src/common/linux/tests/crash_generator.cc \ src/common/memory_unittest.cc src/common/tests/file_utils.cc \ @@ -409,6 +410,7 @@ am__src_client_linux_linux_client_unittest_SOURCES_DIST = \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_core_dumper_unittest.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_ptrace_dumper_unittest.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.$(OBJEXT) \ +@LINUX_HOST_TRUE@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/common/linux/tests/src_client_linux_linux_client_unittest-crash_generator.$(OBJEXT) \ @LINUX_HOST_TRUE@ src/common/src_client_linux_linux_client_unittest-memory_unittest.$(OBJEXT) \ @@ -1203,6 +1205,15 @@ top_srcdir = @top_srcdir@ # This allows #includes to be relative to src/ AM_CPPFLAGS = -I$(top_srcdir)/src +# These are good warnings to be treated as errors +@GCC_TRUE@AM_CXXFLAGS = \ +@GCC_TRUE@ -Werror=non-virtual-dtor \ +@GCC_TRUE@ -Werror=vla \ +@GCC_TRUE@ -Werror=unused-variable \ +@GCC_TRUE@ -Werror=missing-braces \ +@GCC_TRUE@ -Werror=overloaded-virtual + + # Specify include paths for ac macros ACLOCAL_AMFLAGS = -I m4 dist_doc_DATA = \ @@ -1373,6 +1384,7 @@ TESTS_ENVIRONMENT = @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_core_dumper_unittest.cc \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc \ @LINUX_HOST_TRUE@ src/client/linux/minidump_writer/minidump_writer_unittest.cc \ +@LINUX_HOST_TRUE@ src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc \ @LINUX_HOST_TRUE@ src/common/linux/linux_libc_support_unittest.cc \ @LINUX_HOST_TRUE@ src/common/linux/tests/crash_generator.cc \ @LINUX_HOST_TRUE@ src/common/memory_unittest.cc \ @@ -2466,6 +2478,9 @@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_pt src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.$(OBJEXT): \ src/client/linux/minidump_writer/$(am__dirstamp) \ src/client/linux/minidump_writer/$(DEPDIR)/$(am__dirstamp) +src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.$(OBJEXT): \ + src/client/linux/minidump_writer/$(am__dirstamp) \ + src/client/linux/minidump_writer/$(DEPDIR)/$(am__dirstamp) src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.$(OBJEXT): \ src/common/linux/$(am__dirstamp) \ src/common/linux/$(DEPDIR)/$(am__dirstamp) @@ -3107,6 +3122,7 @@ mostlyclean-compile: -rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_core_dumper_unittest.$(OBJEXT) -rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_ptrace_dumper_unittest.$(OBJEXT) -rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.$(OBJEXT) + -rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.$(OBJEXT) -rm -f src/client/linux/minidump_writer/src_client_linux_linux_dumper_unittest_helper-linux_dumper_unittest_helper.$(OBJEXT) -rm -f src/client/minidump_file_writer.$(OBJEXT) -rm -f src/common/convert_UTF.$(OBJEXT) @@ -3329,6 +3345,7 @@ distclean-compile: @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_core_dumper_unittest.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_ptrace_dumper_unittest.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest.Po@am__quote@ +@AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_dumper_unittest_helper-linux_dumper_unittest_helper.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/convert_UTF.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/dwarf_cfi_to_module.Po@am__quote@ @@ -3649,6 +3666,20 @@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump @AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ @am__fastdepCXX_FALSE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.obj `if test -f 'src/client/linux/minidump_writer/minidump_writer_unittest.cc'; then $(CYGPATH_W) 'src/client/linux/minidump_writer/minidump_writer_unittest.cc'; else $(CYGPATH_W) '$(srcdir)/src/client/linux/minidump_writer/minidump_writer_unittest.cc'; fi` +src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o: src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc +@am__fastdepCXX_TRUE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o -MD -MP -MF src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o `test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' || echo '$(srcdir)/'`src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc +@am__fastdepCXX_TRUE@ $(am__mv) src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Po +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ source='src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' object='src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o' libtool=no @AMDEPBACKSLASH@ +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ +@am__fastdepCXX_FALSE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o `test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' || echo '$(srcdir)/'`src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc + +src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj: src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc +@am__fastdepCXX_TRUE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj -MD -MP -MF src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj `if test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; then $(CYGPATH_W) 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; else $(CYGPATH_W) '$(srcdir)/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; fi` +@am__fastdepCXX_TRUE@ $(am__mv) src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Po +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ source='src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' object='src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj' libtool=no @AMDEPBACKSLASH@ +@AMDEP_TRUE@@am__fastdepCXX_FALSE@ DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@ +@am__fastdepCXX_FALSE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj `if test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; then $(CYGPATH_W) 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; else $(CYGPATH_W) '$(srcdir)/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; fi` + src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.o: src/common/linux/linux_libc_support_unittest.cc @am__fastdepCXX_TRUE@ $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.o -MD -MP -MF src/common/linux/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_libc_support_unittest.Tpo -c -o src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.o `test -f 'src/common/linux/linux_libc_support_unittest.cc' || echo '$(srcdir)/'`src/common/linux/linux_libc_support_unittest.cc @am__fastdepCXX_TRUE@ $(am__mv) src/common/linux/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_libc_support_unittest.Tpo src/common/linux/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_libc_support_unittest.Po diff --git a/configure b/configure index 8678406d..b2308814 100755 --- a/configure +++ b/configure @@ -612,6 +612,8 @@ PTHREAD_CC ax_pthread_config EGREP GREP +GCC_FALSE +GCC_TRUE RANLIB am__fastdepCXX_FALSE am__fastdepCXX_TRUE @@ -4498,6 +4500,14 @@ else RANLIB="$ac_cv_prog_RANLIB" fi + if test "$GCC" = yes; then + GCC_TRUE= + GCC_FALSE='#' +else + GCC_TRUE='#' + GCC_FALSE= +fi + # let the Makefile know if we're gcc @@ -5463,6 +5473,10 @@ if test -z "${am__fastdepCXX_TRUE}" && test -z "${am__fastdepCXX_FALSE}"; then as_fn_error "conditional \"am__fastdepCXX\" was never defined. Usually this means the macro was only invoked conditionally." "$LINENO" 5 fi +if test -z "${GCC_TRUE}" && test -z "${GCC_FALSE}"; then + as_fn_error "conditional \"GCC\" was never defined. +Usually this means the macro was only invoked conditionally." "$LINENO" 5 +fi if test -z "${LINUX_HOST_TRUE}" && test -z "${LINUX_HOST_FALSE}"; then as_fn_error "conditional \"LINUX_HOST\" was never defined. Usually this means the macro was only invoked conditionally." "$LINENO" 5 diff --git a/configure.ac b/configure.ac index 2e08169a..c2fa749d 100644 --- a/configure.ac +++ b/configure.ac @@ -45,6 +45,7 @@ AM_PROG_CC_C_O AC_PROG_CPP AC_PROG_CXX AC_PROG_RANLIB +AM_CONDITIONAL(GCC, test "$GCC" = yes) # let the Makefile know if we're gcc AC_HEADER_STDC m4_include(m4/ax_pthread.m4) diff --git a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc index e7af11d4..d04ef3e1 100644 --- a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc @@ -83,11 +83,10 @@ TEST(LinuxCoreDumperTest, VerifyDumpWithMultipleThreads) { return; } - pid_t pid = getpid(); const string core_file = crash_generator.GetCoreFilePath(); const string procfs_path = crash_generator.GetDirectoryOfProcFilesCopy(); LinuxCoreDumper dumper(child_pid, core_file.c_str(), procfs_path.c_str()); - dumper.Init(); + EXPECT_TRUE(dumper.Init()); EXPECT_TRUE(dumper.IsPostMortem()); diff --git a/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc b/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc index 418e7e67..df4ecece 100644 --- a/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc +++ b/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc @@ -38,6 +38,7 @@ #include #include +#include "processor/scoped_ptr.h" #include "third_party/lss/linux_syscall_support.h" #if defined(__ARM_EABI__) @@ -77,7 +78,7 @@ int main(int argc, char *argv[]) { fprintf(stderr, "ERROR: number of threads is 0"); return 1; } - pthread_t threads[num_threads]; + google_breakpad::scoped_array threads(new pthread_t[num_threads]); pthread_attr_t thread_attributes; pthread_attr_init(&thread_attributes); pthread_attr_setdetachstate(&thread_attributes, PTHREAD_CREATE_DETACHED); diff --git a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc index 5e2b431c..6fabfd84 100644 --- a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc @@ -33,11 +33,13 @@ // This file was renamed from linux_dumper_unittest.cc and modified due // to LinuxDumper being splitted into two classes. +#include #include #include #include #include #include +#include #include #include #include @@ -47,6 +49,7 @@ #include "breakpad_googletest_includes.h" #include "client/linux/minidump_writer/linux_ptrace_dumper.h" +#include "client/linux/minidump_writer/minidump_writer_unittest_utils.h" #include "common/linux/eintr_wrapper.h" #include "common/linux/file_id.h" #include "common/linux/safe_readlink.h" @@ -59,23 +62,6 @@ namespace { typedef testing::Test LinuxPtraceDumperTest; -string GetHelperBinary() { - // Locate helper binary next to the current binary. - char self_path[PATH_MAX]; - if (!SafeReadLink("/proc/self/exe", self_path)) { - return ""; - } - string helper_path(self_path); - size_t pos = helper_path.rfind('/'); - if (pos == string::npos) { - return ""; - } - helper_path.erase(pos + 1); - helper_path += "linux_dumper_unittest_helper"; - - return helper_path; -} - } // namespace TEST(LinuxPtraceDumperTest, Setup) { @@ -134,7 +120,8 @@ TEST(LinuxPtraceDumperTest, MergedMappings) { const size_t kPageSize = sysconf(_SC_PAGESIZE); const size_t kMappingSize = 3 * kPageSize; int fd = open(helper_path.c_str(), O_RDONLY); - ASSERT_NE(-1, fd); + ASSERT_NE(-1, fd) << "Failed to open file: " << helper_path + << ", Error: " << strerror(errno); char* mapping = reinterpret_cast(mmap(NULL, kMappingSize, diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc index b751f50a..4dbbbe86 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -40,12 +40,14 @@ #include "client/linux/handler/exception_handler.h" #include "client/linux/minidump_writer/linux_dumper.h" #include "client/linux/minidump_writer/minidump_writer.h" +#include "client/linux/minidump_writer/minidump_writer_unittest_utils.h" #include "common/linux/eintr_wrapper.h" #include "common/linux/file_id.h" #include "common/linux/safe_readlink.h" #include "common/tests/auto_tempdir.h" #include "common/using_std_string.h" #include "google_breakpad/processor/minidump.h" +#include "processor/scoped_ptr.h" using namespace google_breakpad; @@ -94,7 +96,7 @@ TEST(MinidumpWriterTest, MappingInfo) { // These are defined here so the parent can use them to check the // data from the minidump afterwards. - const u_int32_t kMemorySize = sysconf(_SC_PAGESIZE); + const u_int32_t memory_size = sysconf(_SC_PAGESIZE); const char* kMemoryName = "a fake module"; const u_int8_t kModuleGUID[sizeof(MDGUID)] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, @@ -117,7 +119,7 @@ TEST(MinidumpWriterTest, MappingInfo) { // Get some memory. char* memory = reinterpret_cast(mmap(NULL, - kMemorySize, + memory_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, @@ -145,7 +147,7 @@ TEST(MinidumpWriterTest, MappingInfo) { // Add information about the mapped memory. MappingInfo info; info.start_addr = kMemoryAddress; - info.size = kMemorySize; + info.size = memory_size; info.offset = 0; strcpy(info.name, kMemoryName); @@ -170,7 +172,7 @@ TEST(MinidumpWriterTest, MappingInfo) { ASSERT_TRUE(module); EXPECT_EQ(kMemoryAddress, module->base_address()); - EXPECT_EQ(kMemorySize, module->size()); + EXPECT_EQ(memory_size, module->size()); EXPECT_EQ(kMemoryName, module->code_file()); EXPECT_EQ(module_identifier, module->debug_identifier()); @@ -186,7 +188,7 @@ TEST(MinidumpWriterTest, MappingInfoContained) { // These are defined here so the parent can use them to check the // data from the minidump afterwards. - const u_int32_t kMemorySize = sysconf(_SC_PAGESIZE); + const u_int32_t memory_size = sysconf(_SC_PAGESIZE); const char* kMemoryName = "a fake module"; const u_int8_t kModuleGUID[sizeof(MDGUID)] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, @@ -213,14 +215,14 @@ TEST(MinidumpWriterTest, MappingInfoContained) { ASSERT_NE(-1, fd); unlink(tempfile.c_str()); // fill with zeros - char buffer[kMemorySize]; - memset(buffer, 0, kMemorySize); - ASSERT_EQ(kMemorySize, write(fd, buffer, kMemorySize)); + google_breakpad::scoped_array buffer(new char[memory_size]); + memset(buffer.get(), 0, memory_size); + ASSERT_EQ(memory_size, write(fd, buffer.get(), memory_size)); lseek(fd, 0, SEEK_SET); char* memory = reinterpret_cast(mmap(NULL, - kMemorySize, + memory_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, @@ -248,8 +250,8 @@ TEST(MinidumpWriterTest, MappingInfoContained) { // Add information about the mapped memory. Report it as being larger than // it actually is. MappingInfo info; - info.start_addr = kMemoryAddress - kMemorySize; - info.size = kMemorySize * 3; + info.start_addr = kMemoryAddress - memory_size; + info.size = memory_size * 3; info.offset = 0; strcpy(info.name, kMemoryName); @@ -287,20 +289,11 @@ TEST(MinidumpWriterTest, DeletedBinary) { char kNumberOfThreadsArgument[2]; sprintf(kNumberOfThreadsArgument, "%d", kNumberOfThreadsInHelperProgram); - // Locate helper binary next to the current binary. - char self_path[PATH_MAX]; - if (!SafeReadLink("/proc/self/exe", self_path)) { - FAIL() << "readlink failed"; + string helper_path(GetHelperBinary()); + if (helper_path.empty()) { + FAIL() << "Couldn't find helper binary"; exit(1); } - string helper_path(self_path); - size_t pos = helper_path.rfind('/'); - if (pos == string::npos) { - FAIL() << "no trailing slash in path: " << helper_path; - exit(1); - } - helper_path.erase(pos + 1); - helper_path += "linux_dumper_unittest_helper"; // Copy binary to a temp file. AutoTempDir temp_dir; @@ -308,7 +301,7 @@ TEST(MinidumpWriterTest, DeletedBinary) { char cmdline[2 * PATH_MAX]; sprintf(cmdline, "/bin/cp \"%s\" \"%s\"", helper_path.c_str(), binpath.c_str()); - ASSERT_EQ(0, system(cmdline)); + ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline; ASSERT_EQ(0, chmod(binpath.c_str(), 0755)); int fds[2]; @@ -381,6 +374,7 @@ TEST(MinidumpWriterTest, DeletedBinary) { kGUIDStringSize); string module_identifier(identifier_string); // Strip out dashes + size_t pos; while ((pos = module_identifier.find('-')) != string::npos) { module_identifier.erase(pos, 1); } diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc b/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc new file mode 100644 index 00000000..9f46fa65 --- /dev/null +++ b/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc @@ -0,0 +1,66 @@ +// Copyright (c) 2011 Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// minidump_writer_unittest_utils.cc: +// Shared routines used by unittests under client/linux/minidump_writer. + +#include +#include + +#include "client/linux/minidump_writer/minidump_writer_unittest_utils.h" +#include "common/linux/safe_readlink.h" +#include "common/using_std_string.h" + +namespace google_breakpad { + +string GetHelperBinary() { + string helper_path; + char *bindir = getenv("bindir"); + if (bindir) { + helper_path = string(bindir) + "/"; + } else { + // Locate helper binary next to the current binary. + char self_path[PATH_MAX]; + if (!SafeReadLink("/proc/self/exe", self_path)) { + return ""; + } + helper_path = string(self_path); + size_t pos = helper_path.rfind('/'); + if (pos == string::npos) { + return ""; + } + helper_path.erase(pos + 1); + } + + helper_path += "linux_dumper_unittest_helper"; + + return helper_path; +} + +} // namespace google_breakpad diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest_utils.h b/src/client/linux/minidump_writer/minidump_writer_unittest_utils.h new file mode 100644 index 00000000..f16cc086 --- /dev/null +++ b/src/client/linux/minidump_writer/minidump_writer_unittest_utils.h @@ -0,0 +1,49 @@ +// Copyright (c) 2012, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// minidump_writer_unittest_utils.h: +// Shared routines used by unittests under client/linux/minidump_writer. + +#ifndef CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_UNITTEST_UTILS_H_ +#define CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_UNITTEST_UTILS_H_ + +#include + +#include "common/using_std_string.h" + +namespace google_breakpad { + +// Returns the full path to linux_dumper_unittest_helper. The full path is +// discovered either by using the environment variable "bindir" or by using +// the location of the main module of the currently running process. +string GetHelperBinary(); + +} // namespace google_breakpad + +#endif // CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_UNITTEST_UTILS_H_ diff --git a/src/common/basictypes.h b/src/common/basictypes.h index 694f7022..84668b79 100644 --- a/src/common/basictypes.h +++ b/src/common/basictypes.h @@ -32,8 +32,10 @@ // A macro to disallow the copy constructor and operator= functions // This should be used in the private: declarations for a class +#ifndef DISALLOW_COPY_AND_ASSIGN #define DISALLOW_COPY_AND_ASSIGN(TypeName) \ TypeName(const TypeName&); \ void operator=(const TypeName&) +#endif // DISALLOW_COPY_AND_ASSIGN #endif // COMMON_BASICTYPES_H_ diff --git a/src/common/language.h b/src/common/language.h index 4811e7f0..bbe30334 100644 --- a/src/common/language.h +++ b/src/common/language.h @@ -50,6 +50,10 @@ namespace google_breakpad { // language. class Language { public: + // A base class destructor should be either public and virtual, + // or protected and nonvirtual. + virtual ~Language() {} + // Return true if this language has functions to which we can assign // line numbers. (Debugging info for assembly language, for example, // can have source location information, but does not have functions diff --git a/src/common/linux/elf_core_dump_unittest.cc b/src/common/linux/elf_core_dump_unittest.cc index b799d3f0..e872a4fd 100644 --- a/src/common/linux/elf_core_dump_unittest.cc +++ b/src/common/linux/elf_core_dump_unittest.cc @@ -183,7 +183,9 @@ TEST(ElfCoreDumpTest, ValidCoreFile) { size_t num_nt_prpsinfo = 0; size_t num_nt_prstatus = 0; size_t num_nt_fpregset = 0; +#if defined(__i386__) size_t num_nt_prxfpreg = 0; +#endif set actual_thread_ids; ElfCoreDump::Note note = core.GetFirstNote(); while (note.IsValid()) { @@ -211,7 +213,7 @@ TEST(ElfCoreDumpTest, ValidCoreFile) { ++num_nt_prstatus; break; } -#if defined(__i386) || defined(__x86_64) +#if defined(__i386__) || defined(__x86_64__) case NT_FPREGSET: { EXPECT_TRUE(description.data() != NULL); EXPECT_EQ(sizeof(user_fpregs_struct), description.length()); @@ -219,7 +221,7 @@ TEST(ElfCoreDumpTest, ValidCoreFile) { break; } #endif -#if defined(__i386) +#if defined(__i386__) case NT_PRXFPREG: { EXPECT_TRUE(description.data() != NULL); EXPECT_EQ(sizeof(user_fpxregs_struct), description.length()); @@ -236,10 +238,10 @@ TEST(ElfCoreDumpTest, ValidCoreFile) { EXPECT_TRUE(expected_thread_ids == actual_thread_ids); EXPECT_EQ(1, num_nt_prpsinfo); EXPECT_EQ(kNumOfThreads, num_nt_prstatus); -#if defined(__i386) || defined(__x86_64) +#if defined(__i386__) || defined(__x86_64__) EXPECT_EQ(kNumOfThreads, num_nt_fpregset); #endif -#if defined(__i386) +#if defined(__i386__) EXPECT_EQ(kNumOfThreads, num_nt_prxfpreg); #endif } diff --git a/src/common/linux/file_id_unittest.cc b/src/common/linux/file_id_unittest.cc index 4c803152..daef1e3f 100644 --- a/src/common/linux/file_id_unittest.cc +++ b/src/common/linux/file_id_unittest.cc @@ -73,9 +73,11 @@ TEST(FileIDStripTest, StripSelf) { string templ = temp_dir.path() + "/file-id-unittest"; char cmdline[4096]; sprintf(cmdline, "cp \"%s\" \"%s\"", exe_name, templ.c_str()); - ASSERT_EQ(system(cmdline), 0); + ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline; + sprintf(cmdline, "chmod u+w \"%s\"", templ.c_str()); + ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline; sprintf(cmdline, "strip \"%s\"", templ.c_str()); - ASSERT_EQ(system(cmdline), 0); + ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline; uint8_t identifier1[sizeof(MDGUID)]; uint8_t identifier2[sizeof(MDGUID)]; diff --git a/src/common/linux/linux_libc_support_unittest.cc b/src/common/linux/linux_libc_support_unittest.cc index a7c5a26a..6f4b2da6 100644 --- a/src/common/linux/linux_libc_support_unittest.cc +++ b/src/common/linux/linux_libc_support_unittest.cc @@ -27,8 +27,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#include "breakpad_googletest_includes.h" #include "common/linux/linux_libc_support.h" -#include "testing/gtest/include/gtest/gtest.h" namespace { typedef testing::Test LinuxLibcSupportTest; diff --git a/src/common/memory_unittest.cc b/src/common/memory_unittest.cc index d580c1fe..69d9f8ab 100644 --- a/src/common/memory_unittest.cc +++ b/src/common/memory_unittest.cc @@ -27,8 +27,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#include "breakpad_googletest_includes.h" #include "common/memory.h" -#include "testing/gtest/include/gtest/gtest.h" using namespace google_breakpad; diff --git a/src/common/test_assembler.h b/src/common/test_assembler.h index 401e8528..891cf677 100644 --- a/src/common/test_assembler.h +++ b/src/common/test_assembler.h @@ -271,7 +271,10 @@ class Section { public: Section(Endianness endianness = kUnsetEndian) : endianness_(endianness) { }; - ~Section() { }; + + // A base class destructor should be either public and virtual, + // or protected and nonvirtual. + virtual ~Section() { }; // Set the default endianness of this section to ENDIANNESS. This // sets the behavior of the D appending functions. If the diff --git a/src/processor/synth_minidump_unittest.cc b/src/processor/synth_minidump_unittest.cc index 3b905f4b..4376933b 100644 --- a/src/processor/synth_minidump_unittest.cc +++ b/src/processor/synth_minidump_unittest.cc @@ -51,7 +51,6 @@ using google_breakpad::SynthMinidump::Section; using google_breakpad::SynthMinidump::Stream; using google_breakpad::SynthMinidump::String; using google_breakpad::SynthMinidump::SystemInfo; -using google_breakpad::SynthMinidump::Thread; using google_breakpad::test_assembler::kBigEndian; using google_breakpad::test_assembler::kLittleEndian; using google_breakpad::test_assembler::Label; @@ -168,11 +167,12 @@ TEST(Thread, Simple) { Memory stack(dump, 0xaad55a93cc3c0efcULL); stack.Append("stack contents"); stack.Finish(0xe08cdbd1); - Thread thread(dump, 0x3d7ec360, stack, context, - 0x3593f44d, // suspend count - 0xab352b82, // priority class - 0x2753d838, // priority - 0xeb2de4be3f29e3e9ULL); // thread environment block + google_breakpad::SynthMinidump::Thread thread( + dump, 0x3d7ec360, stack, context, + 0x3593f44d, // suspend count + 0xab352b82, // priority class + 0x2753d838, // priority + 0xeb2de4be3f29e3e9ULL); // thread environment block string contents; ASSERT_TRUE(thread.GetContents(&contents)); static const u_int8_t expected_bytes[] = { diff --git a/src/processor/synth_minidump_unittest_data.h b/src/processor/synth_minidump_unittest_data.h index 81995d21..be3bd701 100644 --- a/src/processor/synth_minidump_unittest_data.h +++ b/src/processor/synth_minidump_unittest_data.h @@ -247,22 +247,24 @@ static const MDRawContextARM arm_raw_context = { // context_flags 0x591b9e6a, // iregs - 0xa21594de, - 0x820d8a25, - 0xc4e133b2, - 0x173a1c02, - 0x105fb175, - 0xe871793f, - 0x5def70b3, - 0xcee3a623, - 0x7b3aa9b8, - 0x52518537, - 0x627012c5, - 0x22723dcc, - 0x16fcc971, - 0x20988bcb, - 0xf1ab806b, - 0x99d5fc03, + { + 0xa21594de, + 0x820d8a25, + 0xc4e133b2, + 0x173a1c02, + 0x105fb175, + 0xe871793f, + 0x5def70b3, + 0xcee3a623, + 0x7b3aa9b8, + 0x52518537, + 0x627012c5, + 0x22723dcc, + 0x16fcc971, + 0x20988bcb, + 0xf1ab806b, + 0x99d5fc03, + }, // cpsr 0xb70df511, // float_save @@ -270,47 +272,51 @@ static const MDRawContextARM arm_raw_context = { // fpscr 0xa1e1f7ce1077e6b5ULL, // regs - 0xbcb8d002eed7fbdeULL, - 0x4dd26a43b96ae97fULL, - 0x8eec22db8b31741cULL, - 0xfd634bd7c5ad66a0ULL, - 0x1681da0daeb3debeULL, - 0x474a32bdf72d0b71ULL, - 0xcaf464f8b1044834ULL, - 0xcaa6592ae5c7582aULL, - 0x4ee46889d877c3dbULL, - 0xf8930cf301645cf5ULL, - 0x4da7e9ebba27f7c7ULL, - 0x69a7b02761944da3ULL, - 0x2cda2b2e78195c06ULL, - 0x66b227ab9b460a42ULL, - 0x7e77e49e52ee0849ULL, - 0xd62cd9663e76f255ULL, - 0xe9370f082451514bULL, - 0x50a1c674dd1b6029ULL, - 0x405db4575829eac4ULL, - 0x67b948764649eee7ULL, - 0x93731885419229d4ULL, - 0xdb0338bad72a4ce7ULL, - 0xa0a451f996fca4c8ULL, - 0xb4508ea668400a45ULL, - 0xbff28c5c7a142423ULL, - 0x4f31b42b96f3a431ULL, - 0x2ce6789d4ea1ff37ULL, - 0xfa150b52e4f82a3cULL, - 0xe9ec40449e6ed4f3ULL, - 0x5ceca87836fe2251ULL, - 0x66f50de463ee238cULL, - 0x42823efcd59ab511ULL, + { + 0xbcb8d002eed7fbdeULL, + 0x4dd26a43b96ae97fULL, + 0x8eec22db8b31741cULL, + 0xfd634bd7c5ad66a0ULL, + 0x1681da0daeb3debeULL, + 0x474a32bdf72d0b71ULL, + 0xcaf464f8b1044834ULL, + 0xcaa6592ae5c7582aULL, + 0x4ee46889d877c3dbULL, + 0xf8930cf301645cf5ULL, + 0x4da7e9ebba27f7c7ULL, + 0x69a7b02761944da3ULL, + 0x2cda2b2e78195c06ULL, + 0x66b227ab9b460a42ULL, + 0x7e77e49e52ee0849ULL, + 0xd62cd9663e76f255ULL, + 0xe9370f082451514bULL, + 0x50a1c674dd1b6029ULL, + 0x405db4575829eac4ULL, + 0x67b948764649eee7ULL, + 0x93731885419229d4ULL, + 0xdb0338bad72a4ce7ULL, + 0xa0a451f996fca4c8ULL, + 0xb4508ea668400a45ULL, + 0xbff28c5c7a142423ULL, + 0x4f31b42b96f3a431ULL, + 0x2ce6789d4ea1ff37ULL, + 0xfa150b52e4f82a3cULL, + 0xe9ec40449e6ed4f3ULL, + 0x5ceca87836fe2251ULL, + 0x66f50de463ee238cULL, + 0x42823efcd59ab511ULL, + }, // extra - 0xe9e14cd2, - 0x865bb640, - 0x9f3f0b3e, - 0x94a71c52, - 0x3c012f19, - 0x6436637c, - 0x46ccedcb, - 0x7b341be7 + { + 0xe9e14cd2, + 0x865bb640, + 0x9f3f0b3e, + 0x94a71c52, + 0x3c012f19, + 0x6436637c, + 0x46ccedcb, + 0x7b341be7, + } } }; diff --git a/src/tools/linux/md2core/minidump-2-core.cc b/src/tools/linux/md2core/minidump-2-core.cc index 05357aba..5b20175b 100644 --- a/src/tools/linux/md2core/minidump-2-core.cc +++ b/src/tools/linux/md2core/minidump-2-core.cc @@ -49,10 +49,10 @@ #include "client/linux/minidump_writer/minidump_extension_linux.h" #include "common/linux/memory_mapped_file.h" #include "google_breakpad/common/minidump_format.h" +#include "processor/scoped_ptr.h" #include "third_party/lss/linux_syscall_support.h" #include "tools/linux/md2core/minidump_memory_range.h" - #if __WORDSIZE == 64 #define ELF_CLASS ELFCLASS64 #else @@ -1100,9 +1100,9 @@ main(int argc, char** argv) { } if (note_align) { - char scratch[note_align]; - memset(scratch, 0, sizeof(scratch)); - if (!writea(1, scratch, sizeof(scratch))) + google_breakpad::scoped_array scratch(new char[note_align]); + memset(scratch.get(), 0, note_align); + if (!writea(1, scratch.get(), note_align)) return 1; }