1. 21 Jun, 2016 1 commit
  2. 20 Jun, 2016 1 commit
    • Ivan Penkov's avatar
      Server-side workaround to handle overlapping modules. · 24f5931c
      Ivan Penkov authored
      This change is resolving an issue that was caused by the combination of:
       - Android system libraries being relro packed in N+.
       - Breakpad dealing with relro packed libraries in a hack way.
      
      This is a fix for http://crbug/611824.
      
      I also found an use-after-free issue (bug in Minidump::SeekToStreamType).  I disallowed the MinidumpStreamInfo copy and assign constructors and the compiler detected another similar issue in Minidump::Print.  Then I disabled the copy and assign constructors for most classes in minidump.h (just in case).  There are a couple of classes where I couldn't disallow them (since assign is used).  This will require a small refactor so I left it out of this CL.
      
      R=mark@chromium.org
      
      Review URL: https://codereview.chromium.org/2060663002 .
      24f5931c
  3. 14 Jun, 2016 1 commit
  4. 10 Jun, 2016 3 commits
  5. 08 Jun, 2016 1 commit
  6. 06 Jun, 2016 3 commits
  7. 03 Jun, 2016 1 commit
  8. 02 Jun, 2016 1 commit
  9. 26 May, 2016 1 commit
    • Mike Frysinger's avatar
      fix signed warning errors in unittests · bad9e55e
      Mike Frysinger authored
      A bunch of gtest assert statements fail due to signed warnings as
      unadorned constants are treated as signed integers.  Mark them all
      unsigned to avoid that.
      
      One example (focus on the "[with ...]" blocks that show the types):
      In file included from src/breakpad_googletest_includes.h:33:0,
                       from src/common/memory_unittest.cc:30:
      src/testing/gtest/include/gtest/gtest.h: In instantiation of 'testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = long unsigned int]':
      src/testing/gtest/include/gtest/gtest.h:1524:23: required from 'static testing::AssertionResult testing::internal::EqHelper<true>::Compare(const char*, const char*, const T1&, const T2&, typename testing::internal::EnableIf<(! testing::internal::is_pointer<T2>::value)>::type*) [with T1 = int; T2 = long unsigned int; typename testing::internal::EnableIf<(! testing::internal::is_pointer<T2>::value)>::type = void]'
      src/common/memory_unittest.cc:41:246: required from here
      src/testing/gtest/include/gtest/gtest.h:1448:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
         if (expected == actual) {
                      ^
      cc1plus: some warnings being treated as errors
      Makefile:5180: recipe for target 'src/common/src_client_linux_linux_client_unittest_shlib-memory_unittest.o' failed
      make[2]: *** [src/common/src_client_linux_linux_client_unittest_shlib-memory_unittest.o] Error 1
      
      R=ted.mielczarek@gmail.com
      
      Review URL: https://codereview.chromium.org/2013893003 .
      bad9e55e
  10. 25 May, 2016 3 commits
  11. 24 May, 2016 2 commits
  12. 23 May, 2016 2 commits
  13. 18 May, 2016 2 commits
  14. 16 May, 2016 1 commit
  15. 13 May, 2016 1 commit
  16. 12 May, 2016 2 commits
  17. 11 May, 2016 1 commit
  18. 04 May, 2016 3 commits
  19. 03 May, 2016 2 commits
  20. 28 Apr, 2016 1 commit
    • Primiano Tucci's avatar
      Revert of Extend mapping merge to include reserved but unused mappings.... · 17ad0c18
      Primiano Tucci authored
      Revert of Extend mapping merge to include reserved but unused mappings. (https://breakpad.appspot.com/7714003)
      
      Reason for revert:
      It is causing breakpad crash reports to be invalid (see the associated
      bug).
      
      Merging empty holes in r-x mappings was originally introduced in
      https://breakpad.appspot.com/7714003 to deal with the first generation
      of relro packing, which could introduce holes within a .so mapping:
      
        [libchrome.so]
        [guard region]
        [libchrome.so]
      
      However, the logic is broken for the case of two *different* adjacent
      .so mappings with a guard region in the middle:
      
        [libfoo.so]
        [guard region]
        [libchrome.so]
      
      In this case the guard region is mistakenly associated with libfoo.so,
      but that is not the right thing to do. In fact, the second generation of
      rerlo packing added the guard region to prevent mmaps from overlapping
      and to give room for the non-zero vaddr of relro-packed libraries, which
      require an anticipated load bias.
      
      As the first generation of relro packing is not used anymore, there is
      no reason to keep this buggy code, which causes failures in decoding
      crashes where an arbitrary library is mapped immediately before a rerlo
      packed library.
      
      Original issue's description:
      > Extend mapping merge to include reserved but unused mappings.
      >
      > When parsing /proc/pid/maps, current code merges adjacent entries that
      > refer to the same library and where the start of the second is equal to
      > the end of the first, for example:
      >
      >   40022000-40025000 r-xp 00000000 b3:11 827        /system/lib/liblog.so
      >   40025000-40026000 r--p 00002000 b3:11 827        /system/lib/liblog.so
      >   40026000-40027000 rw-p 00003000 b3:11 827        /system/lib/liblog.so
      >
      > When the system linker loads a library it first reserves all the address
      > space required, from the smallest start to the largest end address, using
      > an anonymous mapping, and then maps loaded segments inside that reservation.
      > If the loaded segments do not fully occupy the reservation this leaves
      > gaps, and these gaps prevent merges that should occur from occurring:
      >
      >   40417000-4044a000 r-xp 00000000 b3:11 820        /system/lib/libjpeg.so
      > > 4044a000-4044b000 ---p 00000000 00:00 0
      >   4044b000-4044c000 r--p 00033000 b3:11 820        /system/lib/libjpeg.so
      >   4044c000-4044d000 rw-p 00034000 b3:11 820        /system/lib/libjpeg.so
      >
      > Where the segments that follow this gap do not contain executable code
      > the failure to merge does not affect breakpad operation.  However, where
      > they do then the merge needs to occur.  Packing relocations in a large
      > library splits the executable segment into two, resulting in:
      >
      >   73b0c000-73b21000 r-xp 00000000 b3:19 786460
      > /data/.../libchrome.2160.0.so
      > > 73b21000-73d12000 ---p 00000000 00:00 0
      >   73d12000-75a90000 r-xp 00014000 b3:19 786460
      > /data/.../libchrome.2160.0.so
      >   75a90000-75c0d000 rw-p 01d91000 b3:19 786460
      > /data/.../libchrome.2160.0.so
      >
      > Here the mapping at 73d12000-75a90000 must be merged into 73b0c000-73b21000
      > so that breakpad correctly calculates the base address for text.
      >
      > This change enables the full merge by also merging anonymous maps which
      > result from unused reservation, identified as '---p' with offset 0, and
      > which follow on from an executable mapping, into that executable mapping.
      >
      > BUG=chromium:394703
      
      BUG=chromium:499747
      R=primiano@chromium.org, rmcilroy@chromium.org
      
      Review URL: https://codereview.chromium.org/1923383002 .
      17ad0c18
  21. 21 Apr, 2016 1 commit
  22. 19 Apr, 2016 2 commits
  23. 15 Apr, 2016 1 commit
  24. 14 Apr, 2016 3 commits
    • Ted Mielczarek's avatar
    • Sebastien Marchand's avatar
      Add a missing const to an accessor. · d986b9d3
      Sebastien Marchand authored
      R=ivanpe@chromium.org
      
      Review URL: https://codereview.chromium.org/1882833004 .
      d986b9d3
    • Ted Mielczarek's avatar
      Fix DWARF handling of inlined functions in namespaces · 2e266396
      Ted Mielczarek authored
      Currently an inlined function in a namespace in DWARF will
      be given a name comprised of just `namespace::`. This is due
      to a logic error in ComputeQualifiedName, where it doesn't
      handle an empty `unqualified_name` properly.
      
      We apparently have a fair number of these in our Mac builds,
      an example of the DWARF that's being mishandled looks like:
      0x117eda40:     TAG_namespace [5] *
                       AT_name( "js" )
                       AT_decl_file( "../../dist/include/js/Utility.h" )
                       AT_decl_line( 35 )
      
      0x11808500:         TAG_subprogram [251] *
                           AT_low_pc( 0x0000000002f12110 )
                           AT_high_pc( 0x0000000002f1216b )
                           AT_APPLE_omit_frame_ptr( 0x01 )
                           AT_frame_base( rsp )
                           AT_abstract_origin( {0x0000000011800a4f}"_ZN2js40TraceManuallyBarrieredGenericPointerEdgeEP8JSTracerPPNS_2gc4CellEPKc" )
                            AT_MIPS_linkage_name( "_ZN2js40TraceManuallyBarrieredGenericPointerEdgeEP8JSTracerPPNS_2gc4CellEPKc" )
                            AT_name( "TraceManuallyBarrieredGenericPointerEdge" )
                            AT_decl_file( "/builds/slave/rel-m-rel-m64_bld-000000000000/build/js/src/gc/Marking.cpp" )
                            AT_decl_line( 547 )
                            AT_external( 0x01 )
                            AT_APPLE_optimized( 0x01 )
                            AT_inline( DW_INL_inlined )
      
      This turned a few instances of this in the file I was testing on into
      `<name omitted>`, which seems to just be a symptom of the
      "DW_AT_abstract_origin comes later in the file" issue. (Which is probably
      also worth fixing given that it occurs some 29k times when dumping
      symbols from Firefox's XUL binary, but it's a separate issue.)
      
      R=mark@chromium.org
      BUG=
      
      Review URL: https://codereview.chromium.org/1887033002 .
      2e266396