Fix assertion failure "PathDiagnosticSpotPiece's must have a valid location." in ReturnPtrRange checker on builtin functions

[arseniy-sonar]

Summary

Fixes #55347
Builtin functions (such ast std::move, std::forward, std::as_const) have a body generated during the analysis not related to any source file so their statements have no valid source locations.
ReturnPtrRange checker should not report issues for these builtin functions because they only forward its parameter and do not create any new pointers.

[steakhal]

I already reviewed this internally. For me, it looks great.

@arseniy-sonar consider adding the Fixes #55347 marker to the summary, so the commit will auto-reference the fixed issue.
Also mention the commit author sign to which we should accredit this revision if accepted.

[NoQ]

LGTM, thanks!

This patch seems obvious but please upload with context next time (https://llvm.org/docs/Phabricator.html#phabricator-request-review-web).

clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp

48

I suspect that you might run into more similar problems with functions coming from "body farms". A direct check like

RetE->getBeginLoc().isValid()

might be more reliable.

You might need to check the entire range though. We probably need a unified solution for such checks, because many checkers end up implementing them.

[steakhal]

clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp

48

We were actually considering it. We decided against it to keep the impact of this fix minimal.

That being said, I wonder if a similar check should be at some higher level API, lets say inside the emitReport.
That way no chevker would experience such crashes. WDYT?

[steakhal]

I think we should backport this to release/16.x.
Here is the ticket for it: https://github.com/llvm/llvm-project/issues/61115

clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp

   if (!RetE)
     return;
 
+  // Skip calls to built-in functions because such functions might feature
+  // a return statement with no valid source location.
+  if (const CallExpr *CE =
+          dyn_cast_or_null<CallExpr>(C.getStackFrame()->getCallSite());
+      CE && CE->getBuiltinCallee() != 0)
+    return;
+
   SVal V = C.getSVal(RetE);
   const MemRegion *R = V.getAsRegion();
 

clang/test/Analysis/return-ptr-range.cpp

 
 }
 
+namespace std {
+// A builtin function with the body generated on the fly.
+template <typename T> T&& move(T &&) noexcept;
+} // namespace std
+
+char buf[2];
+
+void top() {
+  // see https://github.com/llvm/llvm-project/issues/55347
+  (void)std::move(*(buf + 3)); // no-crash
+}