[llvm] r255258 - Avoid undefined behavior when vector is empty.

[David Blaikie via llvm-commits]

Ah, I just hit that myself while working on the type units in llvm-dwp.

SetVector does have toArrayRef which is probably the better option than
what you have (what you have there would be UB if the container is empty,
for example).

But yeah, I wouldn't mind if SetVector just had an "operator ArrayRef<T>".
That seems reasonable, unambiguous, obvious goodness.

On Tue, Dec 15, 2015 at 5:27 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> On 13 December 2015 at 19:13, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Thu, Dec 10, 2015 at 8:35 AM, Rafael Espindola via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> Author: rafael
> >> Date: Thu Dec 10 10:35:06 2015
> >> New Revision: 255258
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=255258&view=rev
> >> Log:
> >> Avoid undefined behavior when vector is empty.
> >>
> >> Found by ubsan.
> >>
> >> Modified:
> >>     llvm/trunk/include/llvm/ADT/SetVector.h
> >>     llvm/trunk/lib/Linker/LinkModules.cpp
> >>
> >> Modified: llvm/trunk/include/llvm/ADT/SetVector.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SetVector.h?rev=255258&r1=255257&r2=255258&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/include/llvm/ADT/SetVector.h (original)
> >> +++ llvm/trunk/include/llvm/ADT/SetVector.h Thu Dec 10 10:35:06 2015
> >> @@ -58,6 +58,8 @@ public:
> >>      insert(Start, End);
> >>    }
> >>
> >> +  ArrayRef<T> getArrayRef() const { return vector_; }
> >> +
> >>    /// \brief Determine if the SetVector is empty or not.
> >>    bool empty() const {
> >>      return vector_.empty();
> >>
> >> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=255258&r1=255257&r2=255258&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> >> +++ llvm/trunk/lib/Linker/LinkModules.cpp Thu Dec 10 10:35:06 2015
> >> @@ -772,8 +772,7 @@ bool ModuleLinker::run() {
> >>        Internalize.insert(GV->getName());
> >>    }
> >>
> >> -  if (Mover.move(SrcM,
> >> -                 makeArrayRef(&*ValuesToLink.begin(),
> >> ValuesToLink.size()),
> >
> >
> > Isn't vector already implicitly convertible to ArrayRef? (I guess there's
> > something else at work heer - 'move' is a function template & has weird
> arg
> > deduction issues? something else?)
>
> ValuesToLink is a SetVector which is not implicitly convertible to
> ArrayRef. Should it be?
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151215/29c21c27/attachment.html>