Re: [PATCH] Fix use-after-free in the strlen pass (PR tree-optimization/82977)

[Jeff Law <law at redhat dot com>]

On 11/14/2017 02:30 PM, Jakub Jelinek wrote:
> On Tue, Nov 14, 2017 at 02:24:28PM -0700, Martin Sebor wrote:
>> On 11/14/2017 02:04 PM, Jakub Jelinek wrote:
>>> Hi!
>>>
>>> strlen_to_stridx.get (rhs1) returns an address into the hash_map, and
>>> strlen_to_stridx.put (lhs, *ps); (in order to be efficient) doesn't make a
>>> copy of the argument just in case, first inserts the slot into it which
>>> may cause reallocation, and only afterwards runs the copy ctor to assign
>>> the value into the new slot.  So, passing it a reference to something
>>> in the hash_map is wrong.  Fixed thusly, bootstrapped/regtested on
>>> x86_64-linux and i686-linux, ok for trunk?
>>
>> This seems like an unnecessary gotcha that should be possible to
>> avoid in the hash_map.  The corresponding standard containers
>> require it to work and so it's surprising when it doesn't in GCC.
>>
>> I've been looking at how this is implemented and it seems to me
>> that a fix should be doable by having the hash_map check to see if
>> the underlying table needs to expand and if so, create a temporary
>> copy of the element before reallocating it.
> 
> That would IMHO just slow down and enlarge the hash_map for all users,
> even when most of them don't really need it.
> While it is reasonable for STL containers to make sure it works, we
> aren't using STL containers and can pose additional restrictions.
But when we make our containers behave differently than the STL it makes
it much easier for someone to make a mistake such as this one.

IMHO this kind of difference in behavior is silly and long term just
makes our jobs harder.

I'd vote for fixing our containers.

jeff