[llvm] r237614 - [YAML] Plug a memory leak

[David Blaikie]

On Mon, May 18, 2015 at 2:29 PM, Benjamin Kramer <benny.kra at gmail.com>
wrote:

> On Mon, May 18, 2015 at 11:22 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> >
> > On Mon, May 18, 2015 at 2:11 PM, Benjamin Kramer <
> benny.kra at googlemail.com>
> > wrote:
> >>
> >> Author: d0k
> >> Date: Mon May 18 16:11:27 2015
> >> New Revision: 237614
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=237614&view=rev
> >> Log:
> >> [YAML] Plug a memory leak
> >>
> >> The destructor of BlockScalarNode is never called. Store the contained
> >> string in BumpPtrAllocated memory instead.
> >
> >
> > Reckon we could put a static_assert in BumpPtrAllocator that checks that
> > objects allocated with it have trivial destruction? I suppose that'd
> work in
> > SpecificBumpPtrAllocator - but maybe we could template the allocate
> > operation of the non-specificBumpPtrAllocator to catch them there too...
>
> There are places in LLVM that use a BumpPtrAllocator and call the
> destructor manually (crazy, isn't it?). Also global 'operator new'
> overloads don't have any type information, so adding type trait checks
> would require massive code churn :(
>

*nod* :/


>
> - Ben
>
> >>
> >>
> >> Modified:
> >>     llvm/trunk/include/llvm/Support/YAMLParser.h
> >>     llvm/trunk/lib/Support/YAMLParser.cpp
> >>
> >> Modified: llvm/trunk/include/llvm/Support/YAMLParser.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/YAMLParser.h?rev=237614&r1=237613&r2=237614&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/include/llvm/Support/YAMLParser.h (original)
> >> +++ llvm/trunk/include/llvm/Support/YAMLParser.h Mon May 18 16:11:27
> 2015
> >> @@ -235,8 +235,8 @@ class BlockScalarNode : public Node {
> >>
> >>  public:
> >>    BlockScalarNode(std::unique_ptr<Document> &D, StringRef Anchor,
> >> StringRef Tag,
> >> -                  std::string &Value, StringRef RawVal)
> >> -      : Node(NK_BlockScalar, D, Anchor, Tag), Value(std::move(Value)) {
> >> +                  StringRef Value, StringRef RawVal)
> >> +      : Node(NK_BlockScalar, D, Anchor, Tag), Value(Value) {
> >>      SMLoc Start = SMLoc::getFromPointer(RawVal.begin());
> >>      SMLoc End = SMLoc::getFromPointer(RawVal.end());
> >>      SourceRange = SMRange(Start, End);
> >> @@ -250,7 +250,7 @@ public:
> >>    }
> >>
> >>  private:
> >> -  std::string Value;
> >> +  StringRef Value;
> >>  };
> >>
> >>  /// \brief A key and value pair. While not technically a Node under the
> >> YAML
> >>
> >> Modified: llvm/trunk/lib/Support/YAMLParser.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/YAMLParser.cpp?rev=237614&r1=237613&r2=237614&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Support/YAMLParser.cpp (original)
> >> +++ llvm/trunk/lib/Support/YAMLParser.cpp Mon May 18 16:11:27 2015
> >> @@ -2377,11 +2377,13 @@ parse_property:
> >>                  , AnchorInfo.Range.substr(1)
> >>                  , TagInfo.Range
> >>                  , T.Range);
> >> -  case Token::TK_BlockScalar:
> >> +  case Token::TK_BlockScalar: {
> >>      getNext();
> >> +    StringRef StrCopy = StringRef(T.Value).copy(NodeAllocator);
> >>      return new (NodeAllocator)
> >>          BlockScalarNode(stream.CurrentDoc, AnchorInfo.Range.substr(1),
> >> -                        TagInfo.Range, T.Value, T.Range);
> >> +                        TagInfo.Range, StrCopy, T.Range);
> >> +  }
> >>    case Token::TK_Key:
> >>      // Don't eat the TK_Key, KeyValueNode expects it.
> >>      return new (NodeAllocator)
> >>
> >>
> >> _______________________________________________
> >> 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/20150518/608be87a/attachment.html>