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

Issue 608053 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug



Sign in to add a comment

UNADDRESSABLE ACCESS in SafeBrowsingV4GetHashProtocolManagerTest_TestGetHashErrorHandlingNetwork

Project Member Reported by osh...@chromium.org, Apr 29 2016

Issue description

It started on https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/4639

I suspect one of these changes is culprit:

https://codereview.chromium.org/1843383002
https://codereview.chromium.org/1881353002

kcarattini@, can you look into this?


UNADDRESSABLE ACCESS: reading 0x0000000c-0x00000010 4 byte(s)
# 0 safe_browsing::V4GetHashProtocolManager::Create                            [components\safe_browsing_db\v4_get_hash_protocol_manager.cc:99]
# 1 safe_browsing::SafeBrowsingV4GetHashProtocolManagerTest_TestGetHashErrorHandlingNetwork_Test::TestBody [components\safe_browsing_db\v4_get_hash_protocol_manager_unittest.cc:88]
# 2 testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
Note: @0:04:09.850 in thread 4228
Note: instruction: call   0x04(%eax) %esp -> %esp 0xfffffffc(%esp)
Suppression (error hash=#25B5D62ECF719C5E#):
For more info on using suppressions see http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
{
UNADDRESSABLE ACCESS
name=<insert_a_suppression_name_here>
*!safe_browsing::V4GetHashProtocolManager::Create
*!safe_browsing::SafeBrowsingV4GetHashProtocolManagerTest_TestGetHashErrorHandlingNetwork_Test::TestBody
*!testing::internal::HandleExceptionsInMethodIfSupported<>
}
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 29 2016

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

commit ba239c184e5505b8a87770df746bb1ee7f1ce339
Author: oshima <oshima@chromium.org>
Date: Fri Apr 29 22:22:16 2016

Excluding the failing test that is also causing memory error on DrMemory bot

BUG= 608053 
TBR=kcarattini@chromium.org

Review-Url: https://codereview.chromium.org/1934783002
Cr-Commit-Position: refs/heads/master@{#390783}

[modify] https://crrev.com/ba239c184e5505b8a87770df746bb1ee7f1ce339/tools/valgrind/gtest_exclude/components_unittests.gtest-drmemory_win32.txt

Comment 2 by osh...@chromium.org, Apr 30 2016

Cc: pasko@chromium.org nparker@chromium.org
crbug.com/607973 may be related.


Project Member

Comment 3 by bugdroid1@chromium.org, Apr 30 2016

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

commit fc260975054dcacc3d5c0ac36cecec6bc6fba9a1
Author: oshima <oshima@chromium.org>
Date: Sat Apr 30 07:41:45 2016

Exclude more tests in SafeBrowsingV4GetHashProtocolManagerTest

BUG= 608053 
TBR=kcarattini@chromium.o

Review-Url: https://codereview.chromium.org/1938763002
Cr-Commit-Position: refs/heads/master@{#390849}

[modify] https://crrev.com/fc260975054dcacc3d5c0ac36cecec6bc6fba9a1/tools/valgrind/gtest_exclude/components_unittests.gtest-drmemory_win32.txt

Cc: vakh@chromium.org
Cc: reillyg@chromium.org
+todya's sheriff.

SafeBrowsingV4GetHashProtocolManagerTest.TestParseHashResponse is now failing


https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/4755

We probably should exclude all of SafeBrowsingVGGetHashProtocolManagerTest, or revert culprit.
Cc: benwells@chromium.org
+benwells@
Cc: kcaratt...@chromium.org
I find V4GetHashProtocolManager::RegisterFactory suspicious. kcarattini@chromium.org recently added a test that registers a stack variable as the factory instance but doesn't clear it afterwards. Not only will this leak an existing factory if it exists but is a use-after-free for any later tests (if they share the process):

https://codereview.chromium.org/1843383002
Project Member

Comment 8 by bugdroid1@chromium.org, May 2 2016

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

commit 177293e05b9ca513af66dd2b2c22df19b94638a5
Author: reillyg <reillyg@chromium.org>
Date: Mon May 02 21:22:48 2016

Add suppressions for issues  608053  and  608064 .

This should be more effective than disabling individual tests as these
errors have been seen across many tests.

BUG= 608053 , 608064 
TBR=oshima@chromium.org
NOTRY=True

Review-Url: https://codereview.chromium.org/1937403002
Cr-Commit-Position: refs/heads/master@{#391053}

[modify] https://crrev.com/177293e05b9ca513af66dd2b2c22df19b94638a5/tools/valgrind/drmemory/suppressions.txt

Owner: reillyg@chromium.org
Status: Started (was: Assigned)
I'm about to send out a review for a fix.
Labels: Stability-Memory-MemorySanitizer Stability-Memory-DrMemory OS-Linux OS-Windows
Status: Fixed (was: Started)

Sign in to add a comment