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

Issue 731809 link

Starred by 1 user
Status: Archived
Owner:
hashimoto@chromium.org
Closed: Jun 2017
Cc:
kinaba@chromium.org
█ levarum@chromium.org
hashimoto@chromium.org
hidehiko@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

cryptohome_testrunner leaks detected on amd64-generic-tot-asan-informational

Project Member Reported by lpique@chromium.org, Jun 9 2017 
https://luci-milo.appspot.com/buildbot/chromiumos.chromium/amd64-generic-tot-asan-informational/13327

cryptohome-0.0.1-r1814:  * SUMMARY: AddressSanitizer: 738 byte(s) leaked in 10 allocation(s).

The stacktraces all appear to point at a series of allocations made by the code here

cryptohome-0.0.1-r1814:  *     #7 0x7f859b1f6474 in cryptohome::dircrypto_data_migrator::MigrationHelper::RecordFileErrorWithCurrentErrno(cryptohome::DircryptoMigrationFailedOperationType, base::FilePath const&) /build/amd64-generic/tmp/portage/chromeos-base/cryptohome-0.0.1-r1814/work/build/out/Default/../../../cryptohome-0.0.1/platform2/cryptohome/dircrypto_data_migrator/migration_helper.cc:822:37

It looks like this code was added in https://chromium-review.googlesource.com/c/500238/, but that landed a month ago. I'm not sure why it just failed today. Perhaps some other code change was made that leaked the objects that were constructed here? I'll take a quick look at where they are passed ....

Comment 1 by lpique@chromium.org, Jun 9 2017

Cc: hashimoto@chromium.org
Browsing the recent changes, this one caught my eye:

https://chromium-review.googlesource.com/c/527782/

I expect it will turn out that the new Abort() call that was added does not clean up anything/everything.

I'll go ahead and confirm with a local build that the new test is the culprit.

Comment 2 by lpique@chromium.org, Jun 9 2017

Status: Started (was: Untriaged)
It is that CL. I'll try landing a revert now.

Comment 4 by hashimoto@chromium.org, Jun 12 2017

Cc: hidehiko@chromium.org
Owner: hashimoto@chromium.org
There are two issues:
 1. The test is calling RecordFileErrorWithCurrentErrno() with errno=0 which is unexpected.
 2. Unexpected errno is reported with UMA_HISTOGRAM_SPARSE_SLOWLY, and it results in leaking base::SparseHistogram.

I made a CL to fix #1 https://chromium-review.googlesource.com/c/530870/.

In Chrome, #2 is addressed by annotating pointers with ANNOTATE_LEAKING_OBJECT_PTR in base/metrics/statistics_recorder.cc, but currently it's not working in Chrome OS unit tests because libchrome is not built with -DLEAK_SANITIZER when building with USE=asan.
Fixing #2 would be nice, but may require a bit of hassle.
Let me close this issue once #1 is fixed.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 12 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/334cf4e102a0e60c78592da32f304012fb5f150c

commit 334cf4e102a0e60c78592da32f304012fb5f150c
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Mon Jun 12 15:47:56 2017

cryptohome: Set errno to suppress lsan

When unexpected errno is given, base::File::OSErrorToFileError calls
UMA_HISTOGRAM_SPARSE_SLOWLY, and for some reason it results in leaks
reported by lsan.

BUG= chromium:731809 
TEST=USE=asan cros_workon_make --board=amd64-generic cryptohome

Change-Id: Idffb71732bf203536abb27dcd880c92089dce940
Reviewed-on: https://chromium-review.googlesource.com/530870
Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/334cf4e102a0e60c78592da32f304012fb5f150c/cryptohome/dircrypto_data_migrator/migration_helper_unittest.cc

Comment 6 by hashimoto@chromium.org, Jun 13 2017

Status: Fixed (was: Started)

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
►

Sign in to add a comment