[polly] r225326 - Do not use a const Twine here

[David Blaikie]

On Tue, Jan 6, 2015 at 4:55 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> > On Jan 6, 2015, at 4:39 PM, Tobias Grosser <tobias at grosser.es> wrote:
> >
> > On 07.01.2015 01:33, David Blaikie wrote:
> >>
> >>
> >> On Tue, Jan 6, 2015 at 4:30 PM, Tobias Grosser <tobias at grosser.es
> >> <mailto:tobias at grosser.es>> wrote:
> >>
> >>    Author: grosser
> >>    Date: Tue Jan  6 18:30:01 2015
> >>    New Revision: 225326
> >>
> >>    URL: http://llvm.org/viewvc/llvm-project?rev=225326&view=rev
> >>    Log:
> >>    Do not use a const Twine here
> >>
> >>    This has caused segfaults when using Polly in the context of Julia,
> >>    that are
> >>    not reproducible on my machine in 'make check-polly'.
> >>
> >>    Modified:
> >>         polly/trunk/lib/CodeGen/IslExprBuilder.cpp
> >>
> >>    Modified: polly/trunk/lib/CodeGen/IslExprBuilder.cpp
> >>    URL:
> >>
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/IslExprBuilder.cpp?rev=225326&r1=225325&r2=225326&view=diff
> >>
> ==============================================================================
> >>    --- polly/trunk/lib/CodeGen/IslExprBuilder.cpp (original)
> >>    +++ polly/trunk/lib/CodeGen/IslExprBuilder.cpp Tue Jan  6 18:30:01
> 2015
> >>    @@ -112,7 +112,7 @@ Value *IslExprBuilder::createAccessAddre
> >>        const ScopArrayInfo *SAI = ScopArrayInfo::getFromId(BaseId);
> >>        Base = SAI->getBasePtr();
> >>        assert(Base->getType()->isPointerTy() && "Access base should be
> >>    a pointer");
> >>    -  const Twine &BaseName = Base->getName();
> >>    +  auto BaseName = Base->getName();
> >>
> >>
> >> So what type does getName return? StringRef?
> >
> > Base is an llvm::Value and llvm::Value's getName() function returns a
> StringRef, yes.
> >
> > Are you saying I should not use 'auto' here? Or do you have an
> explanation for the memory corruption I am seeing?
>
> Isn’t it possible that the issue was taking a Twine by reference?
> I mean the return object is a temporary StringRef, it will be destroyed
> unless bound to a temporary. However there is a Twine constructor which is
> called for the conversion here, so the temp is bound to the argument of the
> Twine ctor and its lifetime ends with the ctor.
> You Twine reference a destroyed StringRef after that.
>

The rest of what you said is correct, but making the named local a Twine
value instead of a Twine reference would not fix/help. The StringRef would
still be temporary and destroyed by the time the Twine was used.


>
> Mehdi
>
>
>
> >
> > Cheers,
> > Tobias
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150106/c50cdf85/attachment.html>