This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 11/15/2017 01:28 AM, Richard Biener wrote:
On Tue, 14 Nov 2017, Jeff Law wrote:

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.

I'd argue that this is simply a programming error and I doubt the
libstdc++ variant works by design/specification.

It's by design.  You can find the discussion of this very issue
in C++ standard library issue 526:

  http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-closed.html#526

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]