From ec182fe2e5a309646451c880c76df184ab72c385 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 15 Sep 2014 18:39:24 +0000 Subject: [PATCH] Fix memory leak in error paths in YAMLTraits by using unique_ptr There's some other cleanup that could happen here, but this is at least the mechanical transformation to unique_ptr. Derived from a patch by Anton Yartsev. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@217803 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/YAMLTraits.h | 12 ++++--- lib/Support/YAMLTraits.cpp | 52 +++++++++++++------------------ 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/include/llvm/Support/YAMLTraits.h b/include/llvm/Support/YAMLTraits.h index a23faf65bb5..023dcee7d54 100644 --- a/include/llvm/Support/YAMLTraits.h +++ b/include/llvm/Support/YAMLTraits.h @@ -943,16 +943,17 @@ private: }; class MapHNode : public HNode { + virtual void anchor(); + public: MapHNode(Node *n) : HNode(n) { } - virtual ~MapHNode(); static inline bool classof(const HNode *n) { return MappingNode::classof(n->_node); } static inline bool classof(const MapHNode *) { return true; } - typedef llvm::StringMap NameToNode; + typedef llvm::StringMap> NameToNode; bool isValidKey(StringRef key); @@ -961,19 +962,20 @@ private: }; class SequenceHNode : public HNode { + virtual void anchor(); + public: SequenceHNode(Node *n) : HNode(n) { } - virtual ~SequenceHNode(); static inline bool classof(const HNode *n) { return SequenceNode::classof(n->_node); } static inline bool classof(const SequenceHNode *) { return true; } - std::vector Entries; + std::vector> Entries; }; - Input::HNode *createHNodes(Node *node); + std::unique_ptr createHNodes(Node *node); void setError(HNode *hnode, const Twine &message); void setError(Node *node, const Twine &message); diff --git a/lib/Support/YAMLTraits.cpp b/lib/Support/YAMLTraits.cpp index 526667fc59e..81edca2c0b5 100644 --- a/lib/Support/YAMLTraits.cpp +++ b/lib/Support/YAMLTraits.cpp @@ -63,6 +63,8 @@ std::error_code Input::error() { return EC; } void Input::HNode::anchor() {} void Input::EmptyHNode::anchor() {} void Input::ScalarHNode::anchor() {} +void Input::MapHNode::anchor() {} +void Input::SequenceHNode::anchor() {} bool Input::outputting() { return false; @@ -82,7 +84,7 @@ bool Input::setCurrentDocument() { ++DocIterator; return setCurrentDocument(); } - TopNode.reset(this->createHNodes(N)); + TopNode = this->createHNodes(N); CurrentNode = TopNode.get(); return true; } @@ -133,7 +135,7 @@ bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault, return false; } MN->ValidKeys.push_back(Key); - HNode *Value = MN->Mapping[Key]; + HNode *Value = MN->Mapping[Key].get(); if (!Value) { if (Required) setError(CurrentNode, Twine("missing required key '") + Key + "'"); @@ -159,7 +161,7 @@ void Input::endMapping() { return; for (const auto &NN : MN->Mapping) { if (!MN->isValidKey(NN.first())) { - setError(NN.second, Twine("unknown key '") + NN.first() + "'"); + setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'"); break; } } @@ -180,7 +182,7 @@ bool Input::preflightElement(unsigned Index, void *&SaveInfo) { return false; if (SequenceHNode *SQ = dyn_cast(CurrentNode)) { SaveInfo = CurrentNode; - CurrentNode = SQ->Entries[Index]; + CurrentNode = SQ->Entries[Index].get(); return true; } return false; @@ -202,7 +204,7 @@ bool Input::preflightFlowElement(unsigned index, void *&SaveInfo) { return false; if (SequenceHNode *SQ = dyn_cast(CurrentNode)) { SaveInfo = CurrentNode; - CurrentNode = SQ->Entries[index]; + CurrentNode = SQ->Entries[index].get(); return true; } return false; @@ -253,8 +255,8 @@ bool Input::bitSetMatch(const char *Str, bool) { return false; if (SequenceHNode *SQ = dyn_cast(CurrentNode)) { unsigned Index = 0; - for (HNode *N : SQ->Entries) { - if (ScalarHNode *SN = dyn_cast(N)) { + for (auto &N : SQ->Entries) { + if (ScalarHNode *SN = dyn_cast(N.get())) { if (SN->value().equals(Str)) { BitValuesUsed[Index] = true; return true; @@ -277,7 +279,7 @@ void Input::endBitSetScalar() { assert(BitValuesUsed.size() == SQ->Entries.size()); for (unsigned i = 0; i < SQ->Entries.size(); ++i) { if (!BitValuesUsed[i]) { - setError(SQ->Entries[i], "unknown bit value"); + setError(SQ->Entries[i].get(), "unknown bit value"); return; } } @@ -302,7 +304,7 @@ void Input::setError(Node *node, const Twine &message) { EC = make_error_code(errc::invalid_argument); } -Input::HNode *Input::createHNodes(Node *N) { +std::unique_ptr Input::createHNodes(Node *N) { SmallString<128> StringStorage; if (ScalarNode *SN = dyn_cast(N)) { StringRef KeyStr = SN->getValue(StringStorage); @@ -313,18 +315,18 @@ Input::HNode *Input::createHNodes(Node *N) { memcpy(Buf, &StringStorage[0], Len); KeyStr = StringRef(Buf, Len); } - return new ScalarHNode(N, KeyStr); + return llvm::make_unique(N, KeyStr); } else if (SequenceNode *SQ = dyn_cast(N)) { - SequenceHNode *SQHNode = new SequenceHNode(N); + auto SQHNode = llvm::make_unique(N); for (Node &SN : *SQ) { - HNode *Entry = this->createHNodes(&SN); + auto Entry = this->createHNodes(&SN); if (EC) break; - SQHNode->Entries.push_back(Entry); + SQHNode->Entries.push_back(std::move(Entry)); } - return SQHNode; + return std::move(SQHNode); } else if (MappingNode *Map = dyn_cast(N)) { - MapHNode *mapHNode = new MapHNode(N); + auto mapHNode = llvm::make_unique(N); for (KeyValueNode &KVN : *Map) { Node *KeyNode = KVN.getKey(); ScalarNode *KeyScalar = dyn_cast(KeyNode); @@ -341,14 +343,14 @@ Input::HNode *Input::createHNodes(Node *N) { memcpy(Buf, &StringStorage[0], Len); KeyStr = StringRef(Buf, Len); } - HNode *ValueHNode = this->createHNodes(KVN.getValue()); + auto ValueHNode = this->createHNodes(KVN.getValue()); if (EC) break; - mapHNode->Mapping[KeyStr] = ValueHNode; + mapHNode->Mapping[KeyStr] = std::move(ValueHNode); } - return mapHNode; + return std::move(mapHNode); } else if (isa(N)) { - return new EmptyHNode(N); + return llvm::make_unique(N); } else { setError(N, "unknown node kind"); return nullptr; @@ -371,18 +373,6 @@ bool Input::canElideEmptySequence() { return false; } -Input::MapHNode::~MapHNode() { - for (auto &N : Mapping) - delete N.second; -} - -Input::SequenceHNode::~SequenceHNode() { - for (HNode *N : Entries) - delete N; -} - - - //===----------------------------------------------------------------------===// // Output //===----------------------------------------------------------------------===//