llvm dev - Aug 2010 - [LLVMdev] Usage of pointers to elements of a std::vector that might be reallocated

Hello,
I was trying to interface a custom backend instruction scheduler with llvm
code when I realize something terrible. The scheduling code builds a graph
made up of SUnit * nodes (see ScheduleDAG*.{cpp,h}). These SUnits nodes are
allocated via a std::vector< SUnit >.
This isn't a problem as long as the pointers are taken after the vector is
fully filled and the vector never changes its size. But the problem is that
is can happen !
Indeed, in some rare cases, the scheduler needs to duplicate a SUnit and
thus allocate a new one. This gives code this:

ScheduleDAGSNodes.cpp:

#ifndef NDEBUG
  const SUnit *Addr = 0;
  if (!SUnits.empty())
    Addr = &SUnits[0];
#endif
  SUnits.push_back(SUnit(N, (unsigned)SUnits.size()));
  assert((Addr == 0 || Addr == &SUnits[0]) &&

Not only this code does not compile with NDEBUG set but it could trigger an
extermely reliable assertion failure in some cases. One could think that
this is too rare to happen but it does happen with my scheduler and I'm not
quite embarassed because I can't do anything apart from hacking llvm source
code. I feel that triggering an assertion failure just because the user made
the mistake of having not luck is not great for such a big framework as LLVM
:)

Shouldn't LLVM use a custom vector implementation for such cases, an
implementation that does not invalidate pointers when growing ?
I have such an implementation at hand that I'm willing to provide if needed.

Regards


Amaury Pouly
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100808/4dd2161a/attachment.html>

> Not only this code does not compile with NDEBUG set
I may be missing something, but why does it not compile with -DNDEBUG?
assert() macro expands to noop when NDEBUG is set.

Eugene

On Sun, Aug 8, 2010 at 2:19 PM, Amaury Pouly <amaury.pouly at gmail.com>
wrote:
> Hello,
> I was trying to interface a custom backend instruction scheduler with llvm
> code when I realize something terrible. The scheduling code builds a graph
> made up of SUnit * nodes (see ScheduleDAG*.{cpp,h}). These SUnits nodes are
> allocated via a std::vector< SUnit >.
> This isn't a problem as long as the pointers are taken after the vector
is
> fully filled and the vector never changes its size. But the problem is that
> is can happen !
> Indeed, in some rare cases, the scheduler needs to duplicate a SUnit and
> thus allocate a new one. This gives code this:
>
> ScheduleDAGSNodes.cpp:
>
> #ifndef NDEBUG
>   const SUnit *Addr = 0;
>   if (!SUnits.empty())
>     Addr = &SUnits[0];
> #endif
>   SUnits.push_back(SUnit(N, (unsigned)SUnits.size()));
>   assert((Addr == 0 || Addr == &SUnits[0]) &&
>
> Not only this code does not compile with NDEBUG set but it could trigger an
> extermely reliable assertion failure in some cases. One could think that
> this is too rare to happen but it does happen with my scheduler and I'm
not
> quite embarassed because I can't do anything apart from hacking llvm
source
> code. I feel that triggering an assertion failure just because the user
made
> the mistake of having not luck is not great for such a big framework as
LLVM
> :)
>
> Shouldn't LLVM use a custom vector implementation for such cases, an
> implementation that does not invalidate pointers when growing ?
> I have such an implementation at hand that I'm willing to provide if
needed.
>
> Regards
>
>
> Amaury Pouly
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>

Oh yes you're right, I missed that :) But the point still hold.

Amaury Pouly


2010/8/8 Eugene Toder <eltoder at gmail.com>
> > Not only this code does not compile with NDEBUG set
>
> I may be missing something, but why does it not compile with -DNDEBUG?
> assert() macro expands to noop when NDEBUG is set.
>
> Eugene
>
> On Sun, Aug 8, 2010 at 2:19 PM, Amaury Pouly <amaury.pouly at
gmail.com>
> wrote:
> > Hello,
> > I was trying to interface a custom backend instruction scheduler with
> llvm
> > code when I realize something terrible. The scheduling code builds a
> graph
> > made up of SUnit * nodes (see ScheduleDAG*.{cpp,h}). These SUnits
nodes
> are
> > allocated via a std::vector< SUnit >.
> > This isn't a problem as long as the pointers are taken after the
vector
> is
> > fully filled and the vector never changes its size. But the problem is
> that
> > is can happen !
> > Indeed, in some rare cases, the scheduler needs to duplicate a SUnit
and
> > thus allocate a new one. This gives code this:
> >
> > ScheduleDAGSNodes.cpp:
> >
> > #ifndef NDEBUG
> >   const SUnit *Addr = 0;
> >   if (!SUnits.empty())
> >     Addr = &SUnits[0];
> > #endif
> >   SUnits.push_back(SUnit(N, (unsigned)SUnits.size()));
> >   assert((Addr == 0 || Addr == &SUnits[0]) &&
> >
> > Not only this code does not compile with NDEBUG set but it could
trigger
> an
> > extermely reliable assertion failure in some cases. One could think
that
> > this is too rare to happen but it does happen with my scheduler and
I'm
> not
> > quite embarassed because I can't do anything apart from hacking
llvm
> source
> > code. I feel that triggering an assertion failure just because the
user
> made
> > the mistake of having not luck is not great for such a big framework
as
> LLVM
> > :)
> >
> > Shouldn't LLVM use a custom vector implementation for such cases,
an
> > implementation that does not invalidate pointers when growing ?
> > I have such an implementation at hand that I'm willing to provide
if
> needed.
> >
> > Regards
> >
> >
> > Amaury Pouly
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100808/95fc8e35/attachment.html>