[llvm] r240417 - [Option] Plug a leak when move-assigning an InputArgList.

[David Blaikie]

On Tue, Jun 23, 2015 at 8:28 AM, Benjamin Kramer <benny.kra at googlemail.com>
wrote:

> Author: d0k
> Date: Tue Jun 23 10:28:10 2015
> New Revision: 240417
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240417&view=rev
> Log:
> [Option] Plug a leak when move-assigning an InputArgList.
>
> The class has a non-trivial dtor so we have to clean up before we move
> in new members. Remove misleading comment as a default move assignment
> operator will never be synthesized for this class.
>

Thanks for the fix!

(& now extra incentive (I wouldn't say extra motivated... ) to pull the
container out of ArgList into the derived classes so that it doesn't have
that conditional ownership & can just use raw pointers in DerivedArgList
and unique_ptrs in InputArgList)


>
> Modified:
>     llvm/trunk/include/llvm/Option/ArgList.h
>     llvm/trunk/lib/Option/ArgList.cpp
>
> Modified: llvm/trunk/include/llvm/Option/ArgList.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Option/ArgList.h?rev=240417&r1=240416&r2=240417&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Option/ArgList.h (original)
> +++ llvm/trunk/include/llvm/Option/ArgList.h Tue Jun 23 10:28:10 2015
> @@ -325,22 +325,24 @@ private:
>    /// The number of original input argument strings.
>    unsigned NumInputArgStrings;
>
> +  /// Release allocated arguments.
> +  void releaseMemory();
> +
>  public:
>    InputArgList(const char* const *ArgBegin, const char* const *ArgEnd);
> -  // Default move operations implemented for the convenience of MSVC.
> Nothing
> -  // special here.
>    InputArgList(InputArgList &&RHS)
>        : ArgList(std::move(RHS)), ArgStrings(std::move(RHS.ArgStrings)),
>          SynthesizedStrings(std::move(RHS.SynthesizedStrings)),
>          NumInputArgStrings(RHS.NumInputArgStrings) {}
>    InputArgList &operator=(InputArgList &&RHS) {
> +    releaseMemory();
>      ArgList::operator=(std::move(RHS));
>      ArgStrings = std::move(RHS.ArgStrings);
>      SynthesizedStrings = std::move(RHS.SynthesizedStrings);
>      NumInputArgStrings = RHS.NumInputArgStrings;
>      return *this;
>    }
> -  ~InputArgList();
> +  ~InputArgList() { releaseMemory(); }
>
>    const char *getArgString(unsigned Index) const override {
>      return ArgStrings[Index];
>
> Modified: llvm/trunk/lib/Option/ArgList.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Option/ArgList.cpp?rev=240417&r1=240416&r2=240417&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Option/ArgList.cpp (original)
> +++ llvm/trunk/lib/Option/ArgList.cpp Tue Jun 23 10:28:10 2015
> @@ -315,18 +315,18 @@ const char *ArgList::GetOrMakeJoinedArgS
>
>  //
>
> +void InputArgList::releaseMemory() {
> +  // An InputArgList always owns its arguments.
> +  for (Arg *A : *this)
> +    delete A;
> +}
> +
>  InputArgList::InputArgList(const char* const *ArgBegin,
>                             const char* const *ArgEnd)
>    : NumInputArgStrings(ArgEnd - ArgBegin) {
>    ArgStrings.append(ArgBegin, ArgEnd);
>  }
>
> -InputArgList::~InputArgList() {
> -  // An InputArgList always owns its arguments.
> -  for (iterator it = begin(), ie = end(); it != ie; ++it)
> -    delete *it;
> -}
> -
>  unsigned InputArgList::MakeIndex(StringRef String0) const {
>    unsigned Index = ArgStrings.size();
>
>
>
> _______________________________________________
> 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/20150623/d4f24347/attachment.html>