r257947 - Avoid self-assignment of SmallString, trigger UB behavior down the road.

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 15 14:59:16 PST 2016


On Fri, Jan 15, 2016 at 2:56 PM, Benjamin Kramer <benny.kra at gmail.com>
wrote:

> On Fri, Jan 15, 2016 at 11:52 PM, David Blaikie via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> >
> > On Fri, Jan 15, 2016 at 2:29 PM, Joerg Sonnenberger via cfe-commits
> > <cfe-commits at lists.llvm.org> wrote:
> >>
> >> Author: joerg
> >> Date: Fri Jan 15 16:29:34 2016
> >> New Revision: 257947
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=257947&view=rev
> >> Log:
> >> Avoid self-assignment of SmallString, trigger UB behavior down the road.
> >
> >
> > Shouldn't we be fixing that in SmallString? (self /move/ assignment
> arguably
> > can be UB, but self copy assign probably shouldn't be)
>
> It's a partial self-copy via StringRef. I don't think guarding against
> that very special case is worth it.
>

Seems pretty subtle/likely to cause pain in the future, perhaps.

I assume std::string is resilient to this?


>
> >>
> >>
> >> Modified:
> >>     cfe/trunk/tools/driver/driver.cpp
> >>
> >> Modified: cfe/trunk/tools/driver/driver.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=257947&r1=257946&r2=257947&view=diff
> >>
> >>
> ==============================================================================
> >> --- cfe/trunk/tools/driver/driver.cpp (original)
> >> +++ cfe/trunk/tools/driver/driver.cpp Fri Jan 15 16:29:34 2016
> >> @@ -290,9 +290,9 @@ static void SetInstallDir(SmallVectorImp
> >>    if (CanonicalPrefixes)
> >>      llvm::sys::fs::make_absolute(InstalledPath);
> >>
> >> -  InstalledPath = llvm::sys::path::parent_path(InstalledPath);
> >> -  if (llvm::sys::fs::exists(InstalledPath.c_str()))
> >> -    TheDriver.setInstalledDir(InstalledPath);
> >> +  StringRef
> >> InstalledPathParent(llvm::sys::path::parent_path(InstalledPath));
> >> +  if (llvm::sys::fs::exists(InstalledPathParent))
> >> +    TheDriver.setInstalledDir(InstalledPathParent);
> >>  }
> >>
> >>  static int ExecuteCC1Tool(ArrayRef<const char *> argv, StringRef Tool)
> {
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160115/b9910bb9/attachment-0001.html>


More information about the cfe-commits mailing list