[analyzer] do not crash with assertion on processing locations of bodyfarmed functions

[george.karpenkov]

Summary

This addresses an issue introduced in r183451: since removePiecesWithInvalidLocations is called *after* adjustCallLocations, it is not necessary, and in fact harmful, to have this assertion in adjustCallLocations.

Addresses rdar://36170689

[george.karpenkov]

The error is triggered when analyzer finds a bug *inside* the callable captured by the body farm.
Probably we didn't see the crash in the wild because either
a) the clang build did not have assertions enabled
or
b) the HTML output was not enabled

and the bug needs both to trigger.
In any case it does not make sense to assert that invalid locations are not present on the path if right after we are calling a subroutine which removes all diagnostics which do have invalid locations.

[NoQ]

Yep, so it still doesn't answer if removePiecesWithInvalidLocations() was dead code to begin with (considering that the assertion in adjustCallLocations() says that everything but call locations is valid by now, and call locations are fixed immediately after that). The piece with invalid location seems to be a control flow piece (grey piece / arrow) which is indeed not to be displayed to the user in this case. I'm still not sure how/if-at-all this worked before. But the new behavior looks correct and i don't see these questions as the most pressing issues we're having at the moment :)

lib/StaticAnalyzer/Core/BugReporter.cpp

 /// valid locations.
 static void
 adjustCallLocations(PathPieces &Pieces,
                     PathDiagnosticLocation *LastCallLocation = nullptr) {
   for (PathPieces::iterator I = Pieces.begin(), E = Pieces.end(); I != E; ++I) {
     PathDiagnosticCallPiece *Call = dyn_cast<PathDiagnosticCallPiece>(I->get());
 
     if (!Call) {
-      assert((*I)->getLocation().asLocation().isValid());
       continue;
     }
 
     if (LastCallLocation) {
       bool CallerIsImplicit = hasImplicitBody(Call->getCaller());
       if (CallerIsImplicit || !Call->callEnter.asLocation().isValid())
         Call->callEnter = *LastCallLocation;
       if (CallerIsImplicit || !Call->callReturn.asLocation().isValid())

test/Analysis/call_once.cpp

-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBSTDCPP -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -verify %s -o %t.report
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBSTDCPP -verify %s -o %t.report
 
 // We do NOT model libcxx03 implementation, but the analyzer should still
 // not crash.
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBCXX03 -verify %s
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBCXX03 -DEMULATE_LIBSTDCPP -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBCXX03 -verify %s -o %t.report
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBCXX03 -DEMULATE_LIBSTDCPP -verify %s -o %t.report
+// RUN: rm -rf %t.report
 
 void clang_analyzer_eval(bool);
 
 // Faking std::std::call_once implementation.
 namespace std {
 
 #ifndef EMULATE_LIBSTDCPP
 typedef struct once_flag_s {