Backed out 2 changesets (bug 1878108) for causing sm bustages in Attributes.h. CLOSED TREE

Backed out changeset 05e2d2bc1260 (bug 1878108)
Backed out changeset bb356c39c0e2 (bug 1878108)

build/clang-plugin/Checks.inc

@@ -40,5 +40,6 @@ CHECK(RefCountedInsideLambdaChecker, "refcounted-inside-lambda")
 CHECK(RefCountedThisInsideConstructorChecker, "refcount-within-constructor")
 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

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

build/clang-plugin/CustomAttributes.inc

@@ -27,5 +27,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

@@ -428,6 +428,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

@@ -45,6 +45,7 @@ HOST_SOURCES += [
     "RefCountedThisInsideConstructorChecker.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

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

mfbt/Attributes.h

@@ -446,23 +446,6 @@
 #  define MOZ_NO_STACK_PROTECTOR /* no support */
 #endif
 
-/**
- * MOZ_LIFETIME_BOUND indicates that objects that are referred to by that
- * parameter may also be referred to by the return value of the annotated
- * function (or, for a parameter of a constructor, by the value of the
- * constructed object).
- * See: https://clang.llvm.org/docs/AttributeReference.html#lifetimebound
- */
-#ifdef __has_cpp_attribute
-#  if __has_cpp_attribute(clang::lifetimebound)
-#    define MOZ_LIFETIME_BOUND [[clang::lifetimebound]]
-#  else
-#    define MOZ_LIFETIME_BOUND /* nothing */
-#  endif
-#else
-#  define MOZ_LIFETIME_BOUND /* nothing */
-#endif
-
 #ifdef __cplusplus
 
 /**
@@ -762,6 +745,9 @@
  * MOZ_MAY_CALL_AFTER_MUST_RETURN: Applies to function or method declarations.
  *   Calls to these methods may be made in functions after calls a
  *   MOZ_MUST_RETURN_FROM_CALLER_IF_THIS_IS_ARG method.
+ * MOZ_LIFETIME_BOUND: Applies to method declarations.
+ *   The result of calling these functions on temporaries may not be returned as
+ *   a reference or bound to a reference variable.
  * MOZ_UNANNOTATED/MOZ_ANNOTATED: Applies to Mutexes/Monitors and variations on
  *   them. MOZ_UNANNOTATED indicates that the Mutex/Monitor/etc hasn't been
  *   examined and annotated using macros from mfbt/ThreadSafety --
@@ -852,6 +838,7 @@
       __attribute__((annotate("moz_must_return_from_caller_if_this_is_arg")))
 #    define MOZ_MAY_CALL_AFTER_MUST_RETURN \
       __attribute__((annotate("moz_may_call_after_must_return")))
+#    define MOZ_LIFETIME_BOUND __attribute__((annotate("moz_lifetime_bound")))
 #    define MOZ_KNOWN_LIVE __attribute__((annotate("moz_known_live")))
 #    ifndef XGILL_PLUGIN
 #      define MOZ_UNANNOTATED __attribute__((annotate("moz_unannotated")))
@@ -911,6 +898,7 @@
 #    define MOZ_REQUIRED_BASE_METHOD                        /* nothing */
 #    define MOZ_MUST_RETURN_FROM_CALLER_IF_THIS_IS_ARG      /* nothing */
 #    define MOZ_MAY_CALL_AFTER_MUST_RETURN                  /* nothing */
+#    define MOZ_LIFETIME_BOUND                              /* nothing */
 #    define MOZ_KNOWN_LIVE                                  /* nothing */
 #    define MOZ_UNANNOTATED                                 /* nothing */
 #    define MOZ_ANNOTATED                                   /* nothing */

mfbt/FunctionRef.h

@@ -174,7 +174,7 @@ class MOZ_TEMPORARY_CLASS FunctionRef<Ret(Params...)> {
             typename std::enable_if_t<!std::is_same_v<
                 std::remove_cv_t<std::remove_reference_t<Callable>>,
                 FunctionRef>>* = nullptr>
-  MOZ_IMPLICIT FunctionRef(Callable&& aCallable MOZ_LIFETIME_BOUND) noexcept
+  MOZ_IMPLICIT FunctionRef(Callable&& aCallable) noexcept
       : mAdaptor([](const Payload& aPayload, Params... aParams) {
           auto& func = *static_cast<std::remove_reference_t<Callable>*>(aPayload.mObject);
           return static_cast<Ret>(func(std::forward<Params>(aParams)...));

xpcom/string/nsTLiteralString.h

@@ -56,11 +56,11 @@ class nsTLiteralString : public mozilla::detail::nsTStringRepr<T> {
    * Use sparingly. If possible, rewrite code to use const ns[C]String&
    * and the implicit cast will just work.
    */
-  const nsTString<T>& AsString() const MOZ_LIFETIME_BOUND {
+  MOZ_LIFETIME_BOUND const nsTString<T>& AsString() const {
     return *reinterpret_cast<const nsTString<T>*>(this);
   }
 
-  operator const nsTString<T>&() const MOZ_LIFETIME_BOUND { return AsString(); }
+  MOZ_LIFETIME_BOUND operator const nsTString<T>&() const { return AsString(); }
 
   template <typename N, typename Dummy>
   struct raw_type {