[clang][dataflow] Don't propagate result objects inside decltype.

Trying to do so can cause crashes -- see newly added test and the comments in
the fix.

We're starting to see a repeating pattern here: We're getting crashes because
`ResultObjectVisitor` and `getReferencedDecls()` don't agree on which parts of
the AST to visit and, hence, which fields should be modeled.

I think we should ensure consistency between these two parts of the code by
using a `RecursiveASTVisitor` in `getReferencedDecls()`[^1]; the
`Traverse...()` functions that control which parts of the AST we visit would go
in a common base class that would be used for both `ResultObjectVisitor` and
`getReferencedDecls()`.

I'd like to focus this PR, however, on a targeted fix for the current crash and
postpone the refactoring to a later PR (which will be easier to revert if there
are unintended side-effects).

[^1]: As an added bonus, this would make the code better structured and more
efficient than the current sequence of `if (dyn_cast<T>(...))` statements).

Author: martinboehme

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -350,6 +350,13 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
     return RecursiveASTVisitor<ResultObjectVisitor>::TraverseDecl(D);
   }
 
+  bool TraverseDecltypeTypeLoc(DecltypeTypeLoc Node) {
+    // Don't traverse `decltype` because we don't analyze its argument (as it
+    // isn't executed) and hence don't model fields that are only used in the
+    // argument of a `decltype`.
+    return true;
+  }
+
   bool TraverseBindingDecl(BindingDecl *BD) {
     // `RecursiveASTVisitor` doesn't traverse holding variables for
     // `BindingDecl`s by itself, so we need to tell it to.

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

@@ -3331,6 +3331,23 @@ TEST(TransferTest, ResultObjectLocationDontVisitNestedRecordDecl) {
          ASTContext &ASTCtx) {});
 }
 
+TEST(TransferTest, ResultObjectLocationDontVisitDeclTypeArg) {
+  // This is a crash repro.
+  // We used to crash because when propagating result objects, we would visit
+  // the argument of `decltype()`, but we don't model fields used only in these.
+  std::string Code = R"cc(
+    struct S1 {};
+    struct S2 { S1 s1; };
+    void target() {
+        decltype(S2{ S1() }) Dummy;
+    }
+  )cc";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {});
+}
+
 TEST(TransferTest, StaticCast) {
   std::string Code = R"(
     void target(int Foo) {