Bug 1651323 - Add temporary lifetime bound checker. r=andi,xpcom-reviewers,nika

Differential Revision: https://phabricator.services.mozilla.com/D82680

build/clang-plugin/Checks.inc

@@ -38,5 +38,6 @@ CHECK(RefCountedCopyConstructorChecker, "refcounted-copy-constructor")
 CHECK(RefCountedInsideLambdaChecker, "refcounted-inside-lambda")
 CHECK(ScopeChecker, "scope")
 CHECK(SprintfLiteralChecker, "sprintf-literal")
+CHECK(TemporaryLifetimeBoundChecker, "temporary-lifetime-bound")
 CHECK(TrivialCtorDtorChecker, "trivial-constructor-destructor")
 CHECK(TrivialDtorChecker, "trivial-destructor")

build/clang-plugin/ChecksIncludes.inc

@@ -39,5 +39,6 @@
 #include "RefCountedInsideLambdaChecker.h"
 #include "ScopeChecker.h"
 #include "SprintfLiteralChecker.h"
+#include "TemporaryLifetimeBoundChecker.h"
 #include "TrivialCtorDtorChecker.h"
 #include "TrivialDtorChecker.h"

build/clang-plugin/CustomAttributes.inc

@@ -26,5 +26,6 @@ ATTR(moz_required_base_method)
 ATTR(moz_stack_class)
 ATTR(moz_static_local_class)
 ATTR(moz_temporary_class)
+ATTR(moz_lifetime_bound)
 ATTR(moz_trivial_ctor_dtor)
 ATTR(moz_trivial_dtor)

build/clang-plugin/CustomMatchers.h

@@ -395,6 +395,11 @@ AST_MATCHER(FunctionDecl, isMozMustReturnFromCaller) {
          hasCustomAttribute<moz_must_return_from_caller_if_this_is_arg>(Decl);
 }
 
+AST_MATCHER(FunctionDecl, isMozTemporaryLifetimeBound) {
+  const FunctionDecl *Decl = Node.getCanonicalDecl();
+  return Decl && hasCustomAttribute<moz_lifetime_bound>(Decl);
+}
+
 /// This matcher will select default args which have nullptr as the value.
 AST_MATCHER(CXXDefaultArgExpr, isNullDefaultArg) {
   const Expr *Expr = Node.getExpr();

build/clang-plugin/TemporaryLifetimeBoundChecker.cpp

@@ -0,0 +1,91 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "TemporaryLifetimeBoundChecker.h"
+#include "CustomMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+void TemporaryLifetimeBoundChecker::registerMatchers(MatchFinder *AstMatcher) {
+  // Look for a call to a MOZ_LIFETIME_BOUND member function
+  auto isTemporaryLifetimeBoundCall =
+      cxxMemberCallExpr(
+          onImplicitObjectArgument(anyOf(has(cxxTemporaryObjectExpr()),
+                                         has(materializeTemporaryExpr()))),
+          callee(functionDecl(isMozTemporaryLifetimeBound())))
+          .bind("call");
+
+  // XXX This definitely does not catch everything relevant. In particular, the
+  // matching on conditionalOperator would need to be recursive. But it's a
+  // start.
+  auto hasTemporaryLifetimeBoundCall =
+      anyOf(isTemporaryLifetimeBoundCall,
+            conditionalOperator(
+                anyOf(hasFalseExpression(isTemporaryLifetimeBoundCall),
+                      hasTrueExpression(isTemporaryLifetimeBoundCall))));
+
+  AstMatcher->addMatcher(
+      returnStmt(hasReturnValue(
+                     allOf(exprWithCleanups().bind("expr-with-cleanups"),
+                           ignoringParenCasts(hasTemporaryLifetimeBoundCall))))
+          .bind("return-stmt"),
+      this);
+
+  AstMatcher->addMatcher(
+      varDecl(hasType(references(cxxRecordDecl())),
+              hasInitializer(
+                  allOf(exprWithCleanups(),
+                        ignoringParenCasts(hasTemporaryLifetimeBoundCall))))
+          .bind("var-decl"),
+      this);
+}
+
+void TemporaryLifetimeBoundChecker::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+  const auto *ReturnStatement =
+      Result.Nodes.getNodeAs<ReturnStmt>("return-stmt");
+  const auto *ReferenceVarDecl = Result.Nodes.getNodeAs<VarDecl>("var-decl");
+
+  const char ErrorReturn[] =
+      "cannot return result of lifetime-bound function %0 on "
+      "temporary of type %1";
+
+  const char ErrorBindToReference[] =
+      "cannot bind result of lifetime-bound function %0 on "
+      "temporary of type %1 to reference, does not extend lifetime";
+
+  const char NoteCalledFunction[] = "member function declared here";
+
+  // We are either a return statement...
+  if (ReturnStatement) {
+    const auto *ExprWithCleanups =
+        Result.Nodes.getNodeAs<Expr>("expr-with-cleanups");
+    if (!ExprWithCleanups->isLValue()) {
+      return;
+    }
+
+    const auto Range = ReturnStatement->getSourceRange();
+
+    diag(Range.getBegin(), ErrorReturn, DiagnosticIDs::Error)
+        << Range << Call->getMethodDecl()
+        << Call->getImplicitObjectArgument()
+               ->getType()
+               .withoutLocalFastQualifiers();
+  }
+
+  // ... or a variable declaration that declare a reference
+  if (ReferenceVarDecl) {
+    const auto Range = ReferenceVarDecl->getSourceRange();
+
+    diag(Range.getBegin(), ErrorBindToReference, DiagnosticIDs::Error)
+        << Range << Call->getMethodDecl()
+        << Call->getImplicitObjectArgument()
+               ->getType()
+               .withoutLocalFastQualifiers();
+  }
+
+  const auto *MethodDecl = Call->getMethodDecl();
+  diag(MethodDecl->getCanonicalDecl()->getLocation(), NoteCalledFunction,
+       DiagnosticIDs::Note);
+}

build/clang-plugin/TemporaryLifetimeBoundChecker.h

@@ -0,0 +1,22 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef TemporaryLifetimeBoundChecker_h__
+#define TemporaryLifetimeBoundChecker_h__
+
+#include "plugin.h"
+
+class TemporaryLifetimeBoundChecker : public BaseCheck {
+public:
+  TemporaryLifetimeBoundChecker(StringRef CheckName,
+                                ContextType *Context = nullptr)
+      : BaseCheck(CheckName, Context) {}
+
+  void registerMatchers(MatchFinder *AstMatcher) override;
+  void check(const MatchFinder::MatchResult &Result) override;
+
+private:
+};
+
+#endif

build/clang-plugin/moz.build

@@ -43,6 +43,7 @@ HOST_SOURCES += [
     'RefCountedInsideLambdaChecker.cpp',
     'ScopeChecker.cpp',
     'SprintfLiteralChecker.cpp',
+    'TemporaryLifetimeBoundChecker.cpp',
     'TrivialCtorDtorChecker.cpp',
     'TrivialDtorChecker.cpp',
     'VariableUsageHelpers.cpp',

build/clang-plugin/tests/TestTemporaryLifetimeBound.cpp

@@ -0,0 +1,126 @@
+#define MOZ_LIFETIME_BOUND __attribute__((annotate("moz_lifetime_bound")))
+
+struct Foo {};
+
+struct Bar {
+  MOZ_LIFETIME_BOUND const Foo &AsFoo() const; // expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}}
+  MOZ_LIFETIME_BOUND operator const Foo &() const; // expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}} expected-note {{member function declared here}}
+};
+
+Bar MakeBar() { return Bar(); }
+
+Bar testReturnsInstance_Constructed() { return Bar(); }
+
+const Foo &testReturnsReference_Static() {
+  static constexpr auto bar = Bar{};
+  return bar;
+}
+
+/* TODO This is bad as well... but not related to a temporary.
+const Foo& testReturnsReference_Local() {
+  constexpr auto bar = Bar{};
+  return bar;
+}
+*/
+
+const Foo &testReturnsReferenceToTemporaryViaLifetimeBound_Constructed() {
+  return Bar(); // expected-error {{cannot return result of lifetime-bound function 'operator const Foo &' on temporary of type 'Bar'}}
+}
+
+const Foo &testReturnsReferenceToTemporaryViaLifetimeBound2_Constructed() {
+  return static_cast<const Foo &>(Bar()); // expected-error {{cannot return result of lifetime-bound function 'operator const Foo &' on temporary of type 'Bar'}}
+}
+
+const Foo &testReturnsReferenceToTemporaryViaLifetimeBound3_Constructed() {
+  return Bar().AsFoo(); // expected-error {{cannot return result of lifetime-bound function 'AsFoo' on temporary of type 'Bar'}}
+}
+
+const Foo &
+testReturnsReferenceToTemporaryViaLifetimeBound4_Constructed(bool aCond) {
+  static constexpr Foo foo;
+  return aCond ? foo : Bar().AsFoo(); // expected-error {{cannot return result of lifetime-bound function 'AsFoo' on temporary of type 'Bar'}}
+}
+
+Foo testReturnsValueViaLifetimeBoundFunction_Constructed() { return Bar(); }
+
+Foo testReturnsValueViaLifetimeBoundFunction2_Constructed() {
+  return static_cast<const Foo &>(Bar());
+}
+
+Foo testReturnsValueViaLifetimeBoundFunction3_Constructed() {
+  return Bar().AsFoo();
+}
+
+Bar testReturnInstance_Returned() { return MakeBar(); }
+
+const Foo &testReturnsReferenceToTemporaryViaLifetimeBound_Returned() {
+  return MakeBar(); // expected-error {{cannot return result of lifetime-bound function 'operator const Foo &' on temporary of type 'Bar'}}
+}
+
+const Foo &testReturnsReferenceToTemporaryViaLifetimeBound2_Returned() {
+  return static_cast<const Foo &>(MakeBar()); // expected-error {{cannot return result of lifetime-bound function 'operator const Foo &' on temporary of type 'Bar'}}
+}
+
+const Foo &testReturnsReferenceToTemporaryViaLifetimeBound3_Returned() {
+  return MakeBar().AsFoo(); // expected-error {{cannot return result of lifetime-bound function 'AsFoo' on temporary of type 'Bar'}}
+}
+
+Foo testReturnsValueViaLifetimeBoundFunction_Returned() { return MakeBar(); }
+
+Foo testReturnsValueViaLifetimeBoundFunction2_Returned() {
+  return static_cast<const Foo &>(MakeBar());
+}
+
+Foo testReturnsValueViaLifetimeBoundFunction3_Returned() {
+  return MakeBar().AsFoo();
+}
+
+void testNoLifetimeExtension() {
+  const Foo &foo = Bar(); // expected-error {{cannot bind result of lifetime-bound function 'operator const Foo &' on temporary of type 'Bar' to reference, does not extend lifetime}}
+}
+
+void testNoLifetimeExtension2() {
+  const auto &foo = static_cast<const Foo &>(MakeBar()); // expected-error {{cannot bind result of lifetime-bound function 'operator const Foo &' on temporary of type 'Bar' to reference, does not extend lifetime}}
+}
+
+void testNoLifetimeExtension3() {
+  const Foo &foo = Bar().AsFoo(); // expected-error {{cannot bind result of lifetime-bound function 'AsFoo' on temporary of type 'Bar' to reference, does not extend lifetime}}
+}
+
+void testNoLifetimeExtension4(bool arg) {
+  const Foo foo;
+  const Foo &fooRef = arg ? foo : Bar().AsFoo(); // expected-error {{cannot bind result of lifetime-bound function 'AsFoo' on temporary of type 'Bar' to reference, does not extend lifetime}}
+}
+
+// While this looks similar to testNoLifetimeExtension4, this is actually fine,
+// as the coerced type of the conditional operator is `Foo` here rather than
+// `const Foo&`, and thus an implicit copy of `Bar().AsFoo()` is created, whose
+// lifetime is actually extended.
+void testLifetimeExtension(bool arg) {
+  const Foo &foo = arg ? Foo() : Bar().AsFoo();
+}
+
+void testConvertToValue() { const Foo foo = Bar(); }
+
+Foo testReturnConvertToValue() {
+  return static_cast<Foo>(Bar());
+}
+
+void FooFunc(const Foo &aFoo);
+
+// We want to allow binding to parameters of the target reference type though.
+// This is the very reason the annotation is required, and the function cannot
+// be restricted to lvalues. Lifetime is not an issue here, as the temporary's
+// lifetime is until the end of the full expression anyway.
+
+void testBindToParameter() {
+  FooFunc(Bar());
+  FooFunc(static_cast<const Foo &>(Bar()));
+  FooFunc(Bar().AsFoo());
+  FooFunc(MakeBar());
+}
+
+// This should be OK, because the return value isn't necessarily coming from the
+// argument (and it should be OK for any type).
+const Foo &RandomFunctionCall(const Foo &aFoo);
+const Foo &testReturnFunctionCall() { return RandomFunctionCall(Bar()); }

build/clang-plugin/tests/moz.build

@@ -49,6 +49,7 @@ SOURCES += [
     'TestStackClass.cpp',
     'TestStaticLocalClass.cpp',
     'TestTemporaryClass.cpp',
+    'TestTemporaryLifetimeBound.cpp',
     'TestTrivialCtorDtor.cpp',
     'TestTrivialDtor.cpp',
 ]