https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109442
--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> (In reply to Jonathan Wakely from comment #5)
> > (In reply to Richard Biener from comment #4)
> > > (In reply to Jonathan Wakely from comment #3)
> > > > Ah, maybe the problem is that the library code manually elides
> > > > destroying
> > > > the elements, precisely because it's a no-op. So we don't actually
> > > > destroy
> > > > the elements, which means the compiler might think they're still
> > > > initialized
> > > > and so could be inspected.
> > > >
> > > > If the library explicitly does vec[i].~T() for every i then would that
> > > > help?
> > > > The compiler would know there are no valid elements in the storage, and
> > > > so
> > > > nothing operator delete could inspect.
> > > >
> > > > We could continue to elide destroying the elements when !defined
> > > > __OPTIMIZE__ so that we don't run a loop that does nothing, but with
> > > > optimization enabled rely on the compiler to remove that loop.
> > >
> > > I don't think that would help. The issue is the compiler thinks that
> > >
> > > operator delete (_37, _49);
> > >
> > > uses the memory at _37 and thus the stores
> > >
> > > *_37 = _24;
> > >
> > > and
> > >
> > > __builtin_memmove (_37, pretmp_63, _23);
> > >
> > > are not dead.
> >
> > But if the library did _37->~_Tp() to destroy the element at _37 then it
> > would be dead, and accessing the element outside its lifetime would be
> > undefined. The bytes can only be accessed as char, unsigned char or
> > std::byte after that.
> >
> > > IIRC 'operator delete' (_ZdlPvm in this case), can be
> > > overridden by the user and can inspect the memory state before "releasing"
> > > the storage?
> >
> > That would be insane for operator delete to do that. Maybe possible, but
> > insane.
>
> Well, the standard doesn't prohibit "insanity" so we have to generate
> correct code even in those cases, no?
Indeed.
> After all this is what PR101480
> was about ("insane" stuff) done even to new/delete _expressions_.
>
> Would it be correct (by the standards wording) to extend the current
> handling of new/delete expressions - which is that 'new' returns a pointer
> not aliasing to any other pointer and that 'delete' does not read from
> the memory pointed to by the first argument and this pointer also does not
> escape to direct calls of operator new/delete?
No, I don't think so. The [expr.new] p13 permission to elide calls to operator
new seems to only apply to new expressions and not direct calls to operator
new.
> Basically in gimple.cc:gimple_call_fnspec we currenty do
>
> /* If the call is to a replaceable operator delete and results
> from a delete expression as opposed to a direct call to
> such operator, then we can treat it as free. */
> if (fndecl
> && DECL_IS_OPERATOR_DELETE_P (fndecl)
> && DECL_IS_REPLACEABLE_OPERATOR (fndecl)
> && gimple_call_from_new_or_delete (stmt))
> return ". o ";
> /* Similarly operator new can be treated as malloc. */
> if (fndecl
> && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
> && gimple_call_from_new_or_delete (stmt))
> return "m ";
>
> where the comments do not exactly match the behavior (we also allow
> arbitrary side-effects on global memory). That we model 'delete'
> as possibly writing to the released storage is so it serves as barrier for
> earlier loads/stores to avoid sinking them across.
>
> > In any case, the stores to _37 would be dead now, right? So even if operator
> > delete inspects the memory as raw bytes using memcmp or similar, it's
> > reading uninitialized storage, so any value is acceptable. So the stores to
> > _37 should be DSE-able now.
>
> There's noting in the IL indicating that though (but I haven't updated
> libstd++ a while to create a updated preprocessed source - will do so now).
I'm talking about a potential change to libstdc++ to add _37->~_Tp() so you
won't see it in the IL now.
What I meant in comment 3 is that libstdc++ uses template metaprogramming to
manually elide destroying the elements, because trivial types like 'int' have
no destructor. See libstdc++-v3/include/bits/stl_construct.h lines 155-197:
template<bool>
struct _Destroy_aux
{
template<typename _ForwardIterator>
static _GLIBCXX20_CONSTEXPR void
__destroy(_ForwardIterator __first, _ForwardIterator __last)
{
for (; __first != __last; ++__first)
std::_Destroy(std::__addressof(*__first));
}
};
template<>
struct _Destroy_aux<true>
{
template<typename _ForwardIterator>
static void
__destroy(_ForwardIterator, _ForwardIterator) { }
};
For vector<int> we use the _Destroy_aux<true> specialization, which is a no-op.
So the compiler thinks that the elements of the std::vector<int> are still
alive when we free the storage.
I'm suggesting that if we actually ended the lifetime of the int elements, then
the compiler would see that any stores to _37 are now dead, because we clobber
them before calling operator delete.
e.g. something like:
--- a/libstdc++-v3/include/bits/stl_construct.h
+++ b/libstdc++-v3/include/bits/stl_construct.h
@@ -181,6 +181,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX20_CONSTEXPR inline void
_Destroy(_ForwardIterator __first, _ForwardIterator __last)
{
+#ifdef __OPTIMIZE__
+ // Rely on the compiler to optimize the loop away for trivial types.
+ return _Destroy_aux<false>::__destroy(__first, __last);
+#else
typedef typename iterator_traits<_ForwardIterator>::value_type
_Value_type;
#if __cplusplus >= 201103L
@@ -194,6 +198,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif
std::_Destroy_aux<__has_trivial_destructor(_Value_type)>::
__destroy(__first, __last);
+#endif
}
template<bool>
But this doesn't change the generated code ¯\_(ツ)_/¯
> OK, then how do we arrange things so the compiler knows it can completely
> elide the unused std::vector? Or do you say it's not valid to do that?
I think it's valid in theory. I don't know if it's possible for GCC to do it in
practice. There doesn't seem to be anything the library can do to help, so
WONTFIX.
