Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 101717 Mismatched new/delete
Starred by 2 users Project Member Reported by timurrrr@chromium.org, Oct 26 2011 Back to list
Status: Fixed
Owner:
Closed: Sep 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
I'm seeing this when running full Chrome under Dr. Memory
INVALID HEAP ARGUMENT: wrong free/delete/delete[] called 0x19ae6ef0
# 0 scoped_ptr<_TOKEN_DEFAULT_DACL>::~scoped_ptr<_TOKEN_DEFAULT_DACL>                    [base\memory\scoped_ptr.h:75]
# 1 sandbox::AddSidToDefaultDacl                                                         [sandbox\src\acl.cc:76]
# 2 sandbox::RestrictedToken::GetRestrictedTokenHandle                                   [sandbox\src\restricted_token.cc:126]
# 3 sandbox::CreateRestrictedToken                                                       [sandbox\src\restricted_token_utils.cc:125]
# 4 sandbox::PolicyBase::MakeTokens                                                      [sandbox\src\sandbox_policy_base.cc:132]
# 5 sandbox::BrokerServicesBase::SpawnTarget                                             [sandbox\src\broker_services.cc:246]
# 6 sandbox::StartProcessWithAccess                                                      [content\common\sandbox_policy.cc:529]
# 7 ChildProcessLauncher::Context::LaunchInternal                                        [content\browser\child_process_launcher.cc:121]

Derek,
I'm not sure if this is a real issue or a bug in DrM - can you please take a look?

xref  issue 101537 

 
One more:
INVALID HEAP ARGUMENT: wrong free/delete/delete[] called 0x0f65e938
# 0 sandbox::PolicyBase::~PolicyBase                                [sandbox\src\sandbox_policy_base.cc:113]
# 1 sandbox::PolicyBase::`scalar deleting destructor'
# 2 sandbox::PolicyBase::Release                                    [sandbox\src\sandbox_policy_base.h:43]
# 3 sandbox::BrokerServicesBase::FreeResources                      [sandbox\src\broker_services.cc:128]
# 4 sandbox::BrokerServicesBase::TargetEventsThread                 [sandbox\src\broker_services.cc:173]

This one is definitely an error:
PolicyBase::~PolicyBase() {
  ...
  delete policy_;
}

vs
ResultCode PolicyBase::AddRule(SubSystem subsystem, Semantics semantics,
                               const wchar_t* pattern) {
  if (NULL == policy_) {
    policy_ = MakeBrokerPolicyMemory();
...

sandbox::PolicyGlobal* MakeBrokerPolicyMemory() {
  const size_t kTotalPolicySz = kPolMemSize;
  char* mem = new char[kTotalPolicySz];
  DCHECK(mem);
  memset(mem, 0, kTotalPolicySz);
  sandbox::PolicyGlobal* policy = reinterpret_cast<sandbox::PolicyGlobal*>(mem);
  policy->data_size = kTotalPolicySz - sizeof(sandbox::PolicyGlobal);
  return policy;
}
Owner: timurrrr@chromium.org
comment 0 is a real issue:

AddSidToDefaultDacl():
  scoped_ptr<TOKEN_DEFAULT_DACL> default_dacl;
  if (!GetDefaultDacl(token, &default_dacl))

GetDefaultDacl():
  TOKEN_DEFAULT_DACL* acl =
      reinterpret_cast<TOKEN_DEFAULT_DACL*>(new char[length]);
  default_dacl->reset(acl);

so we have an array stored in a scoped_ptr instead of a scoped_array.  like  issue 101537  there's no real harm done b/c char has no destructor so delete[]==delete but it's still bad form and seems worth fixing.
Cc: drmemory-team@google.com
Owner: jsc...@chromium.org
Status: Assigned
The blamelist mentions initial.commit so assigning to the most active commiter per the log of sandbox/src :)

Can you please take a look?
Comment 4 by jsc...@chromium.org, Oct 26 2011
You're probably going to find a bunch more of these (that's what I meant when I called it a "typical Windows pattern" in  bug 101537 ). They should all be harmless, but you can keep assigning the bugs to me and I'll go through and clean them up when I get a chance.
Sure, thanks!
Project Member Comment 6 by bugdroid1@chromium.org, Nov 3 2011
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=108514

------------------------------------------------------------------------
r108514 | timurrrr@chromium.org | Thu Nov 03 12:19:18 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/tools/valgrind/drmemory_analyze.py?r1=108514&r2=108513&pathrev=108514
 M http://src.chromium.org/viewvc/chrome/trunk/src/tools/valgrind/drmemory/suppressions.txt?r1=108514&r2=108513&pathrev=108514

Add name= fields to sanity suppressions, suppress a couple of new[]/delete mismatches
Also fix the printed suppression count regexp so it works with leaks again
BUG= 101537 , 101717 
TBR=bruening
Review URL: http://codereview.chromium.org/8437089
------------------------------------------------------------------------
One more:
http://build.chromium.org/p/chromium.fyi/builders/Windows%20Tests%20%28DrMemory%20UI%29/builds/84/steps/memory%20test%3A%20ui/logs/stdio
INVALID HEAP ARGUMENT: allocated with operator new[], freed with operator delete
# 0 scoped_ptr<_UNICODE_STRING>::~scoped_ptr<_UNICODE_STRING>               [base\memory\scoped_ptr.h:75]
# 1 sandbox::GetHandleName                                                  [sandbox\src\handle_closer.cc:198]
# 2 sandbox::HandleCloserAgent::CloseHandles                                [sandbox\src\handle_closer_agent.cc:111]
# 3 `anonymous namespace'::CloseOpenHandles                                 [sandbox\src\target_services.cc:50]
# 4 sandbox::TargetServicesBase::LowerToken                                 [sandbox\src\target_services.cc:86]
# 5 RendererMainPlatformDelegate::EnableSandbox                             [content\renderer\renderer_main_platform_delegate_win.cc:128]
# 6 RendererMain                                                            [content\renderer\renderer_main.cc:218]
# 7 `anonymous namespace'::RunNamedProcessTypeMain                          [content\app\content_main.cc:261]
# 8 content::ContentMain                                                    [content\app\content_main.cc:446]
# 9 ChromeMain                                                              [chrome\app\chrome_main.cc:28]
And one more:
http://build.chromium.org/p/chromium.fyi/builders/Windows%20Tests%20%28DrMemory%20UI%29/builds/84/steps/memory%20test%3A%20ui/logs/stdio
INVALID HEAP ARGUMENT: allocated with operator new[], freed with operator delete
# 0 scoped_ptr<_OBJECT_NAME_INFORMATION>::~scoped_ptr<_OBJECT_NAME_INFORMATION>          [base\memory\scoped_ptr.h:75]
# 1 sandbox::GetPathFromHandle                                                           [sandbox\src\win_utils.cc:265]
# 2 sandbox::SameObject                                                                  [sandbox\src\win_utils.cc:134]
# 3 `anonymous namespace'::NtCreateFileInTarget                                          [sandbox\src\filesystem_policy.cc:43]
# 4 sandbox::FileSystemPolicy::CreateFileAction                                          [sandbox\src\filesystem_policy.cc:256]
# 5 sandbox::FilesystemDispatcher::NtCreateFile                                          [sandbox\src\filesystem_dispatcher.cc:117]
# 6 sandbox::SharedMemIPCServer::InvokeCallback                                          [sandbox\src\sharedmem_ipc_server.cc:307]
# 7 sandbox::SharedMemIPCServer::ThreadPingEventReady                                    [sandbox\src\sharedmem_ipc_server.cc:380]
Project Member Comment 9 by bugdroid1@chromium.org, Nov 8 2011
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=109018

------------------------------------------------------------------------
r109018 | timurrrr@chromium.org | Tue Nov 08 05:35:00 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/tools/valgrind/drmemory/suppressions.txt?r1=109018&r2=109017&pathrev=109018

Suppress some Dr. Memory INVALID HEAP reports on UI tests
TBR=bruening
BUG=103365, 101717 
Review URL: http://codereview.chromium.org/8496013
------------------------------------------------------------------------
Project Member Comment 10 by bugdroid1@chromium.org, Jan 12 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=117476

------------------------------------------------------------------------
r117476 | cpu@chromium.org | Thu Jan 12 11:39:55 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/src/acl.cc?r1=117476&r2=117475&pathrev=117476
 M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/src/acl.h?r1=117476&r2=117475&pathrev=117476

Fix memory dealocatiom mismatch by using scoped_ptr_malloc

- Flagged by almost all tools

BUG= 101717 
TEST= sandbox tests in the waterfall are green.

Review URL: http://codereview.chromium.org/9107029
------------------------------------------------------------------------
 Issue 119291  has been merged into this issue.
Similar error reported in browser_tests running with Dr.Memory 
$ ~/Workspace/DrMemory/builds/build_x86_rel.svn/bin/drmemory.exe -no_check_gdi -dr_ops "-privlib_privheap" -suppress ../../tools/valgrind/drmemory/suppressions_full.txt -no_count_leaks -callstack_max_frames 40 -- ./browser_tests.exe --ui-test-action-timeout=60000000 --ui-test-action-max-timeout=60000000 --gtest_filter=WebUIAssertionsTestFail.*

~~9064~~ Error #2: INVALID HEAP ARGUMENT: allocated with operator new[], freed with operator delete
~~9064~~ # 0 sandbox::PolicyBase::~PolicyBase                                [d:\src\chrome-int\src\sandbox\win\src\sandbox_policy_base.cc:126]
~~9064~~ # 1 sandbox::PolicyBase::`scalar deleting destructor'
~~9064~~ # 2 sandbox::PolicyBase::Release                                    [d:\src\chrome-int\src\sandbox\win\src\sandbox_policy_base.cc:136]
~~9064~~ # 3 sandbox::BrokerServicesBase::FreeResources                      [d:\src\chrome-int\src\sandbox\win\src\broker_services.cc:174]
~~9064~~ # 4 sandbox::BrokerServicesBase::TargetEventsThread                 [d:\src\chrome-int\src\sandbox\win\src\broker_services.cc:219]
~~9064~~ # 5 KERNEL32.dll!BaseThreadInitThunk
~~9064~~ # 6 ntdll.dll!__RtlUserThreadStart
~~9064~~ Note: @0:02:38.760 in thread 9064
[6624:12632:1120/212652:100667304:ERROR:bluetooth_adapter_win.cc(27)] NOT IMPLEMENTED
[       OK ] WebUIAssertionsTestFail.testAssertFailFails (151022 ms)
[----------] 1 test from WebUIAssertionsTestFail (151240 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (151359 ms total)
[  PASSED  ] 1 test.
[6624:12632:1120/212656:100671189:ERROR:window_impl.cc(55)] Failed to unregister class Chrome_WidgetWin_0. Error = 1412
~~Dr.M~~
~~Dr.M~~ Error #3: INVALID HEAP ARGUMENT: allocated with operator new[], freed with operator delete
~~Dr.M~~ # 0 sandbox::PolicyBase::~PolicyBase                                          [d:\src\chrome-int\src\sandbox\win\src\sandbox_policy_base.cc:126]
~~Dr.M~~ # 1 sandbox::PolicyBase::`scalar deleting destructor'
~~Dr.M~~ # 2 sandbox::PolicyBase::Release                                              [d:\src\chrome-int\src\sandbox\win\src\sandbox_policy_base.cc:136]
~~Dr.M~~ # 3 sandbox::BrokerServicesBase::FreeResources                                [d:\src\chrome-int\src\sandbox\win\src\broker_services.cc:174]
~~Dr.M~~ # 4 sandbox::BrokerServicesBase::~BrokerServicesBase                          [d:\src\chrome-int\src\sandbox\win\src\broker_services.cc:140]
~~Dr.M~~ # 5 sandbox::BrokerServicesBase::`scalar deleting destructor'
~~Dr.M~~ # 6 sandbox::SingletonBase<sandbox::BrokerServicesBase>::OnExit               [d:\src\chrome-int\src\sandbox\win\src\win_utils.h:60]
~~Dr.M~~ # 7 MSVCR100D.dll!__freeCrtMemory
~~Dr.M~~ # 8 MSVCR100D.dll!exit
~~Dr.M~~ # 9 __tmainCRTStartup                                                         [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c:566]
~~Dr.M~~ #10 mainCRTStartup                                                            [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c:370]
~~Dr.M~~ #11 KERNEL32.dll!BaseThreadInitThunk
~~Dr.M~~ #12 ntdll.dll!__RtlUserThreadStart

Project Member Comment 13 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Area-Internals -Stability-DrMemory Cr-Internals Performance-Memory-DrMemory
Project Member Comment 14 by bugdroid1@chromium.org, Apr 1 2013
Labels: -Performance-Memory-DrMemory Stability-Memory-DrMemory
It looks like you guys went through and cleaned all of these up at some point?
Cc: jsc...@chromium.org
Owner: ----
Status: Available
Actually, either way I should remove myself since I'm obviously not going to be the one cleaning these up.
Cc: zhaoqin@chromium.org bruening@chromium.org
Is this fixed? What's left to do? r117476 and r205053 fixed 2 out of 3 errors. Not sure about the win_utils.cc error.
Cc: -drmemory-team@google.com
If the 3 suppressions labeled with 101717 are not being hit then this can be closed.
Comment 20 by wfh@chromium.org, Apr 1 2015
Owner: wfh@chromium.org
Status: Started
Project Member Comment 21 by bugdroid1@chromium.org, Apr 7 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7804b679be3af7e24578394af0a85ba1249344a9

commit 7804b679be3af7e24578394af0a85ba1249344a9
Author: wfh <wfh@chromium.org>
Date: Tue Apr 07 04:05:19 2015

Fix scoped_ptr free to use delete [] instead of delete.

BUG= 101717 

Review URL: https://codereview.chromium.org/1055433003

Cr-Commit-Position: refs/heads/master@{#324013}

[modify] http://crrev.com/7804b679be3af7e24578394af0a85ba1249344a9/sandbox/win/src/win_utils.cc

Comment 22 by wfh@chromium.org, Apr 7 2015
Status: Fixed
These are all fixed now.
Project Member Comment 23 by bugdroid1@chromium.org, Apr 7 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a02e87219ffae9473c3bb2fcdc5c31adc1dfc43

commit 2a02e87219ffae9473c3bb2fcdc5c31adc1dfc43
Author: kcarattini <kcarattini@chromium.org>
Date: Tue Apr 07 07:00:14 2015

Revert of Fix scoped_ptr free to use delete [] instead of delete. (patchset #1 id:1 of https://codereview.chromium.org/1055433003/)

Reason for revert:
It looks like this broke the following test on XP Tests (1):
sbox_unittests WinUtils.SameObject

Example output:
WinUtils.SameObject (run #1):
[ RUN      ] WinUtils.SameObject
c:\b\build\slave\win_builder\build\src\sandbox\win\src\win_utils_unittest.cc(78): error: Value of: SameObject(file.Get(), file_name_nt1.c_str())
  Actual: false
Expected: true
c:\b\build\slave\win_builder\build\src\sandbox\win\src\win_utils_unittest.cc(79): error: Value of: SameObject(file.Get(), file_name_nt2.c_str())
  Actual: false
Expected: true
[  FAILED  ] WinUtils.SameObject (16 ms)

Original issue's description:
> Fix scoped_ptr free to use delete [] instead of delete.
>
> BUG= 101717 
>
> Committed: https://crrev.com/7804b679be3af7e24578394af0a85ba1249344a9
> Cr-Commit-Position: refs/heads/master@{#324013}

TBR=cpu@chromium.org,wfh@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 101717 

Review URL: https://codereview.chromium.org/1065833002

Cr-Commit-Position: refs/heads/master@{#324016}

[modify] http://crrev.com/2a02e87219ffae9473c3bb2fcdc5c31adc1dfc43/sandbox/win/src/win_utils.cc

Status: Started
Project Member Comment 25 by bugdroid1@chromium.org, Apr 9 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8be51d2c404486d434bbd0496950024995661bb3

commit 8be51d2c404486d434bbd0496950024995661bb3
Author: wfh <wfh@chromium.org>
Date: Thu Apr 09 03:47:13 2015

Fix scoped_ptr free to use delete [] instead of delete.

BUG= 101717 

Review URL: https://codereview.chromium.org/1066203003

Cr-Commit-Position: refs/heads/master@{#324355}

[modify] http://crrev.com/8be51d2c404486d434bbd0496950024995661bb3/sandbox/win/src/win_utils.cc

Comment 26 by wfh@chromium.org, Sep 26 2015
Status: Fixed
Sign in to add a comment