1651323 - [Static Analysis][Clang-Plugin] Add an annotation and static analysis for lifetime-bound member functions

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, task)

Description

[Simon Giesecke [:sg] [he/him]]

There are functions that cannot be called safely on a temporary in some contexts, but cannot be restricted to be called on lvalues either because in some contexts their use is required.

Examples of this are nsTLiteralString::AsString() and particular the implicit conversion operator nsTLiteralString::operator const nsTString<T>&.

The result of these functions when called on a temporary cannot be returned or bound to a reference, since it depends on the temporary (and in the latter case, the lifetime of the temporary is not extended as it were without the user-defined conversion):

const nsString& Get() {
  return u""_ns; // undefined behaviour
} 

void Do() {
  const nsString&x = u""_ns;

  x.BeginReading(); // undefined behaviour
} 

However, typically the result of these functions is passed as a const-reference parameter (which is actually a major purpose of the implicit conversion operator):

void Do(const nsString& x);

void Foo() {
  Do(u"Bar"_ns);
}

The same might apply to other types or member functions, so a generic MOZ_TEMPORARY_LIFETIME_BOUND annotation for such member functions is suggested, along with a static analysis that prevents the introduction of erroneous uses into the codebae.

Comment 1

[Simon Giesecke [:sg] [he/him]]

When I now try do do this, e.g.:

const nsACString& nsChromeRegistryChrome::nsProviderArray::GetSelected(
    const nsACString& aPreferred, MatchType aType) {
  ProviderEntry* entry = GetProvider(aPreferred, aType);

  if (entry) return entry->provider;

  return ""_ns;
}

I correctly get:

 0:12.32 In file included from Unified_cpp_chrome0.cpp:20:
 0:12.32 /home/simon/work/refactor/chrome/nsChromeRegistryChrome.cpp:392:3: error: cannot return result of MOZ_TEMPORARY_LIFETIME_BOUND function 'operator const nsTString<char> &' on temporary of type 'nsTLiteralString<char>'
 0:12.32   return ""_ns;
 0:12.32   ^~~~~~~~~~~~
 0:12.32 /home/simon/work/refactor/obj-x86_64-pc-linux-gnu/dist/include/nsTLiteralString.h:61:32: note: member function declared here
 0:12.32   MOZ_TEMPORARY_LIFETIME_BOUND operator const nsTString<T>&() const {
 0:12.32                                ^
 0:12.34 1 error generated.

Comment 2

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1651323 - Add temporary lifetime bound checker. r=andi,#xpcom-reviewers

Comment 3

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1651323 - Add MOZ_LIFETIME_BOUND attribute. r=andi,#xpcom-reviewers

Depends on D82680

Comment 4

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1651323 - Add MOZ_LIFETIME_BOUND attribute to nsTLiteralString AsString and conversion operator. r=#xpcom-reviewers

Depends on D82681

Comment 5

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1651323 - Invert annotation MOZ_TEMPORARY_LIFETIME_BOUND to MOZ_NOT_LIFETIME_BOUND. r=andi

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd5811c1e588
Add temporary lifetime bound checker. r=andi,xpcom-reviewers,nika
https://hg.mozilla.org/integration/autoland/rev/7ec79f7da940
Add MOZ_LIFETIME_BOUND attribute. r=andi
https://hg.mozilla.org/integration/autoland/rev/751da4969c50
Add MOZ_LIFETIME_BOUND attribute to nsTLiteralString AsString and conversion operator. r=xpcom-reviewers,nika

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/fd5811c1e588
https://hg.mozilla.org/mozilla-central/rev/7ec79f7da940
https://hg.mozilla.org/mozilla-central/rev/751da4969c50

Comment 8

[Phabricator Automation]

Comment on attachment 9162171 [details]
Bug 1651323 - Invert annotation MOZ_TEMPORARY_LIFETIME_BOUND to MOZ_NOT_LIFETIME_BOUND. r=andi

Revision D82726 was moved to bug 1651960. Setting attachment 9162171 [details] to obsolete.