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

Issue 709273 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

base::StackString broken on Windows

Project Member Reported by sunn...@chromium.org, Apr 6 2017

Issue description

I was trying to use base::StackString to put some state on the stack so that it'd show up in minidumps.

The code looks something like this:

  std::unique_ptr<base::trace_event::ConvertableToTraceFormat> scheduler_state =
      scheduler_->AsValue();
  base::StackString<4000> stack_string;
  std::string state_string = scheduler_state->ToString();
  stack_string->append(state_string.begin(), state_string.end());

Here state_string.size() is less than the size of the stack string (4000).

This does not work because string::reserve(n) (called in StackString's ctor) always allocates n + 16 bytes with the given stack allocator. This means that we hit the StackAllocator heap fallback, since n + 16 > n. Therefore, the string is not on the stack nor the minidump. The only other use of StackString is in tools/gn/escape.cc so I've put this in the Build component. Stack trace:

n = 4016

> cc.dll!base::StackAllocator<char,4000>::allocate(unsigned int n, void * hint) Line 108  C++
  cc.dll!std::_Wrap_alloc<base::StackAllocator<char,4000> >::allocate(unsigned int _Count) Line 977 C++
  cc.dll!std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >::_Copy(unsigned int _Newsize, unsigned int _Oldlen) Line 2196  C++
  cc.dll!std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >::_Grow(unsigned int _Newsize, bool _Trim) Line 2228  C++
  cc.dll!std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >::reserve(unsigned int _Newcap) Line 1797 C++
  cc.dll!base::StackContainer<std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >,4000>::StackContainer<std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >,4000>() Line 151  C++
  cc.dll!base::StackString<4000>::StackString<4000>() Line 199  C++

Also, string specializes the allocator we give it for the type std::_Container_proxy and allocates 1 slot with it. This allocator doesn't have a reference to the stack storage so it always allocates on the heap. I don't really understand the purpose of this proxy object. Stack trace:

n = 1, this->source_ = NULL

> cc.dll!base::StackAllocator<std::_Container_proxy,4000>::allocate(unsigned int n, void * hint) Line 114 C++
  cc.dll!std::_Wrap_alloc<base::StackAllocator<std::_Container_proxy,4000> >::allocate(unsigned int _Count) Line 977  C++
  cc.dll!std::_String_alloc<std::_String_base_types<char,base::StackAllocator<char,4000> > >::_Alloc_proxy() Line 649 C++
  cc.dll!std::_String_alloc<std::_String_base_types<char,base::StackAllocator<char,4000> > >::_String_alloc<std::_String_base_types<char,base::StackAllocator<char,4000> > ><base::StackAllocator<char,4000> const &,void>(const base::StackAllocator<char,4000> & _Al) Line 624  C++
  cc.dll!std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >(const base::StackAllocator<char,4000> & _Al) Line 801  C++
  cc.dll!base::StackContainer<std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >,4000>::StackContainer<std::basic_string<char,std::char_traits<char>,base::StackAllocator<char,4000> >,4000>() Line 150  C++
  cc.dll!base::StackString<4000>::StackString<4000>() Line 199  C++
 
I think we should just delete StackString.

Comment 2 by brettw@chromium.org, Apr 14 2017

Owner: brettw@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 14 2017

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

commit eacc100de55e5b286aa8548b22c99c815cd69486
Author: brettw <brettw@chromium.org>
Date: Fri Apr 14 18:54:32 2017

Remove base::StackString.

This was only used in one place and it turns out that StackString was
broken on Windows anyway due to the way that STL reserves buffers.

BUG= 709273 

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

[modify] https://crrev.com/eacc100de55e5b286aa8548b22c99c815cd69486/base/containers/stack_container.h
[modify] https://crrev.com/eacc100de55e5b286aa8548b22c99c815cd69486/tools/gn/escape.cc

Comment 4 by brettw@chromium.org, Apr 14 2017

Status: Fixed (was: Started)
StackString was removed.

Sign in to add a comment