Guard against self-assignment in InputArgList

[andrew.w.kaylor]

Summary

The InputArgList move assignment operator releases memory before copying fields, but it doesn't check against self-assignment. I haven't seen the self-assignment happen, but it doesn't look safe.

[dblaikie]

Any chance of adding unit test coverage for this?

llvm/include/llvm/Option/ArgList.h

428–429

I'd probably reduce indentation with an early return, even if it means duplicating the (fairly simple) return expression. But no big deal either way.

[andrew.w.kaylor]

Add unit test and refactor to use early return

[dblaikie]

awesome :)

llvm/include/llvm/Option/ArgList.h

public:
   InputArgList(const char* const *ArgBegin, const char* const *ArgEnd);
 
   InputArgList(InputArgList &&RHS)
       : ArgList(std::move(RHS)), ArgStrings(std::move(RHS.ArgStrings)),
         SynthesizedStrings(std::move(RHS.SynthesizedStrings)),
         NumInputArgStrings(RHS.NumInputArgStrings) {}
 
   InputArgList &operator=(InputArgList &&RHS) {
+    if (this == &RHS)
+      return *this;
     releaseMemory();
     ArgList::operator=(std::move(RHS));
     ArgStrings = std::move(RHS.ArgStrings);
     SynthesizedStrings = std::move(RHS.SynthesizedStrings);
     NumInputArgStrings = RHS.NumInputArgStrings;
     return *this;
   }
 
   ~InputArgList() { releaseMemory(); }
 
   const char *getArgString(unsigned Index) const override {
     return ArgStrings[Index];
   }
 

llvm/unittests/Option/OptionParsingTest.cpp

TYPED_TEST(OptTableTest, IgnoreCase) {
   unsigned MAI, MAC;
 
   const char *MyArgs[] = { "-a", "-joo" };
   InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
   EXPECT_TRUE(AL.hasArg(OPT_A));
   EXPECT_TRUE(AL.hasArg(OPT_B));
 }
 
+TYPED_TEST(OptTableTest, InputArgListSelfAssign) {
+  TypeParam T;
+  unsigned MAI, MAC;
+  InputArgList AL = T.ParseArgs(Args, MAI, MAC,
+                                /*FlagsToInclude=*/0,
+                                /*FlagsToExclude=*/OptFlag3);
+  EXPECT_TRUE(AL.hasArg(OPT_A));
+  EXPECT_TRUE(AL.hasArg(OPT_C));
+  EXPECT_FALSE(AL.hasArg(OPT_SLASH_C));
+
+  AL = std::move(AL);
+
+  EXPECT_TRUE(AL.hasArg(OPT_A));
+  EXPECT_TRUE(AL.hasArg(OPT_C));
+  EXPECT_FALSE(AL.hasArg(OPT_SLASH_C));
+}
+
 TYPED_TEST(OptTableTest, DoNotIgnoreCase) {
   TypeParam T;
   unsigned MAI, MAC;
 
   const char *MyArgs[] = { "-a", "-joo" };
   InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
   EXPECT_FALSE(AL.hasArg(OPT_A));
   EXPECT_FALSE(AL.hasArg(OPT_B));