New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 814446 link

Starred by 1 user
Status: Fixed
Owner:
primiano@chromium.org
Closed: Feb 2018
Cc:
█ lalitm@chromium.org
ssid@chromium.org
erikc...@chromium.org
█ picksi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 2
Type: Bug
Hotlist-MemoryInfra

Blocking:
issue 482593



Sign in to add a comment

CoordinatorImplTest test fails under MSAN with use-of-uninitialized-value

Project Member Reported by thestig@chromium.org, Feb 21 2018 
I tried enabling services_unittests on MSAN bots and the test suite failed. [1] The CoordinatorImplTest.DumpByPidSuccess failure [2] [3] looks most actionable,

WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x722995c in __find_equal<int> ./../../buildtools/third_party/libc++/trunk/include/__tree:2007:17
    #1 0x722995c in std::__1::pair<std::__1::__tree_iterator<std::__1::__value_type<int, memory_instrumentation::mojom::ProcessType>, std::__1::__tree_node<std::__1::__value_type<int, memory_instrumentation::mojom::ProcessType>, void*>*, long>, bool> std::__1::__tree<std::__1::__value_type<int, memory_instrumentation::mojom::ProcessType>, std::__1::__map_value_compare<int, std::__1::__value_type<int, memory_instrumentation::mojom::ProcessType>, std::__1::less<int>, true>, std::__1::allocator<std::__1::__value_type<int, memory_instrumentation::mojom::ProcessType> > >::__emplace_unique_key_args<int, std::__1::piecewise_construct_t const&, std::__1::tuple<int const&>, std::__1::tuple<> >(int const&, std::__1::piecewise_construct_t const&, std::__1::tuple<int const&>&&, std::__1::tuple<>&&) ./../../buildtools/third_party/libc++/trunk/include/__tree:2131:0
    #2 0x721a292 in operator[] ./../../buildtools/third_party/libc++/trunk/include/map:1319:20
    #3 0x721a292 in memory_instrumentation::QueuedRequestDispatcher::Finalize(memory_instrumentation::QueuedRequest*, memory_instrumentation::TracingObserver*) ./../../services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc:408:0
    #4 0x71e3607 in memory_instrumentation::CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() ./../../services/resource_coordinator/memory_instrumentation/coordinator_impl.cc:455:3
    #5 0x71e668e in memory_instrumentation::CoordinatorImpl::OnOSMemoryDumpResponse(unsigned long, memory_instrumentation::mojom::ClientProcess*, bool, std::__1::unordered_map<int, mojo::StructPtr<memory_instrumentation::mojom::RawOSMemDump>, std::__1::hash<int>, std::__1::equal_to<int>, std::__1::allocator<std::__1::pair<int const, mojo::StructPtr<memory_instrumentation::mojom::RawOSMemDump> > > >) ./../../services/resource_coordinator/memory_instrumentation/coordinator_impl.cc:395:3
...

  Uninitialized value was created by a heap allocation
    #0 0x1040d99 in operator new(unsigned long) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/msan_new_delete.cc:45:35
    #1 0x71ee386 in __construct_node<const std::__1::piecewise_construct_t &, std::__1::tuple<memory_instrumentation::mojom::ClientProcess *const &>, std::__1::tuple<> > ./../../buildtools/third_party/libc++/trunk/include/new:228:10
    #2 0x71ee386 in std::__1::pair<std::__1::__tree_iterator<std::__1::__value_type<memory_instrumentation::mojom::ClientProcess*, memory_instrumentation::QueuedRequest::Response>, std::__1::__tree_node<std::__1::__value_type<memory_instrumentation::mojom::ClientProcess*, memory_instrumentation::QueuedRequest::Response>, void*>*, long>, bool> std::__1::__tree<std::__1::__value_type<memory_instrumentation::mojom::ClientProcess*, memory_instrumentation::QueuedRequest::Response>, std::__1::__map_value_compare<memory_instrumentation::mojom::ClientProcess*, std::__1::__value_type<memory_instrumentation::mojom::ClientProcess*, memory_instrumentation::QueuedRequest::Response>, std::__1::less<memory_instrumentation::mojom::ClientProcess*>, true>, std::__1::allocator<std::__1::__value_type<memory_instrumentation::mojom::ClientProcess*, memory_instrumentation::QueuedRequest::Response> > >::__emplace_unique_key_args<memory_instrumentation::mojom::ClientProcess*, std::__1::piecewise_construct_t const&, std::__1::tuple<memory_instrumentation::mojom::ClientProcess* const&>, std::__1::tuple<> >(memory_instrumentation::mojom::ClientProcess* const&, std::__1::piecewise_construct_t const&, std::__1::tuple<memory_instrumentation::mojom::ClientProcess* const&>&&, std::__1::tuple<>&&) ./../../buildtools/third_party/libc++/trunk/include/__tree:2137:0
    #3 0x71e651d in operator[] ./../../buildtools/third_party/libc++/trunk/include/map:1319:20
    #4 0x71e651d in memory_instrumentation::CoordinatorImpl::OnOSMemoryDumpResponse(unsigned long, memory_instrumentation::mojom::ClientProcess*, bool, std::__1::unordered_map<int, mojo::StructPtr<memory_instrumentation::mojom::RawOSMemDump>, std::__1::hash<int>, std::__1::equal_to<int>, std::__1::allocator<std::__1::pair<int const, mojo::StructPtr<memory_instrumentation::mojom::RawOSMemDump> > > >) ./../../services/resource_coordinator/memory_instrumentation/coordinator_impl.cc:388:0
...


[1] https://chromium-review.googlesource.com/c/chromium/src/+/928324
[2] https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_chromeos_msan_rel_ng%2F626%2F%2B%2Frecipes%2Fsteps%2Fservices_unittests__with_patch_%2F0%2Flogs%2FCoordinatorImplTest.DumpByPidSuccess%2F0
[3] https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_msan_rel_ng%2F924%2F%2B%2Frecipes%2Fsteps%2Fservices_unittests__with_patch_%2F0%2Flogs%2FCoordinatorImplTest.DumpByPidSuccess%2F0

Comment 1 by primiano@chromium.org, Feb 21 2018

Cc: erikc...@chromium.org ssid@chromium.org
+ssid, erikchen FYI

I am leaving the office right now, but my feeling by super quickly glancing at the code is that 

struct Response {
    Response();
    ~Response();

    base::ProcessId process_id;
    mojom::ProcessType process_type;
}

is missing = 0 to |process_id| and |process_type|
They are both PODs and the default ctor doesn't seem to initialize them.

Comment 2 by primiano@chromium.org, Feb 21 2018 

Speculative fix in https://chromium-review.googlesource.com/#/c/chromium/src/+/929323
but couldn't test as I have to run

Comment 3 by thestig@chromium.org, Feb 21 2018

Cc: lalitm@chromium.org
Owner: primiano@chromium.org
Status: Started (was: Untriaged)
Speculative fix is right fix.
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 22 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c426cf30b94ed4aca8764fbea7ce877c12c04635

commit c426cf30b94ed4aca8764fbea7ce877c12c04635
Author: Primiano Tucci <primiano@chromium.org>
Date: Thu Feb 22 01:22:26 2018

Default initialize fields in memory_instrumentation/queued_request.h

Speculative fix for  crbug.com/814446 

Bug:  814446 
Change-Id: I18a720dd3c50f129c4e4de3fbb8348d238cb2555
Reviewed-on: https://chromium-review.googlesource.com/929323
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Commit-Queue: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538297}
[modify] https://crrev.com/c426cf30b94ed4aca8764fbea7ce877c12c04635/services/resource_coordinator/memory_instrumentation/queued_request.h

Comment 5 by thestig@chromium.org, Feb 22 2018

Status: Fixed (was: Started)

Comment 6 by thestig@chromium.org, Feb 23 2018

Blocking: 482593
►

Sign in to add a comment