Remove freed InvalidDomains from InvalidDomainMap.

[maxf]

Summary

Since r306667, propagateInvalidStmtDomains gets a reference to an
InvalidDomainMap. As part of the branch leading to return false, the respective
domain is freed. It is, however, not removed from the InvalidDomainMap, leaking
a pointer to a freed object which results in a use-after-free. Fix this be
removing the domain from the map before returning.

[grosser]

Very nice.

@maxf: Any chance we can get a test case here.

This might already be covered by existing test cases, but is possibly not visible in normal test runs, but only with -DPOLLY_TEST_USE_VG=ON. Can you try this and in this case point out the test cases that already cover this. In case no test case cover this, is it easy to add a test case that covers this in POLLY_TEST_USE_VG mode at least?

If this is the case, we might consider adding a corresponding buildbot.

[maxf]

I looked into providing a testcase and it seems to be infeasible, because the only test cases I can reliably reproduce this on are

• MultiSource/Applications/JM/lencod/lencod
• MultiSource/Applications/SIBsim4/SIBsim4
• MultiSource/Benchmarks/MallocBench/gs/gs
• MultiSource/Benchmarks/MiBench/automotive-susan/automotive-susan
• MultiSource/Benchmarks/Ptrdist/bc/bc
• MultiSource/Benchmarks/mafft/pairlocalalign

when running with "-polly-process-unprofitable". Those are all big multi-source tests and bugpoint also seems unable to reduce them by much. This seems to be kind of logical since this is essentially a heap corruption which is detected because glibc notices the corrupted free list, therefore we're most likely to detect it only if we're finding lots of ScOPs and then only as a side-effect, so I'm not sure how I would create a test that's suitable for our regression test suite in this case. I mean, I can include one of the above, but they are about 250KB.
POLLY_TEST_USE_VG results in make check-polly complaining, but as far as I can see due to a different issue that I have yet to track down...

[grosser]

Thank you Maximilian for investigating!

polly/trunk/lib/Analysis/ScopInfo.cpp

for (unsigned u = 0; u < NumSuccs; u++) {
       InvalidDomainMap[SuccBB] = SuccInvalidDomain;
 
       // Check if the maximal number of domain disjunctions was reached.
       // In case this happens we will bail.
       if (NumConjucts < MaxDisjunctsInDomain)
         continue;
 
       isl_set_free(InvalidDomain);
+      InvalidDomainMap.erase(BB);
       invalidate(COMPLEXITY, TI->getDebugLoc());
       return false;
     }
 
     InvalidDomainMap[BB] = InvalidDomain;
   }
 
   return true;