1757080 - nullptr warning when constructing mozilla::TimeDuration with constant 0

Status: NEW

(Developer Infrastructure :: Source Code Analysis, defect, P4)

Description

[Steve Fink [:sfink] [:s:]]

I believe clang-tidy is incorrect about these complaints:

// static
void nsJSContext::PokeGC(JS::GCReason aReason, JSObject* aObj,
                         TimeDuration aDelay) {
  MOZ_ASSERT(aReason != JS::GCReason::NO_REASON);
  sScheduler.PokeGC(aReason, aObj, aDelay);
}

// static
void nsJSContext::MaybePokeGC() {
  if (sShuttingDown) {
    return;
  }

  JSRuntime* rt = CycleCollectedJSRuntime::Get()->Runtime();
  JS::GCReason reason = JS::WantEagerMinorGC(rt);
  if (reason != JS::GCReason::NO_REASON) {
    MOZ_ASSERT(reason == JS::GCReason::EAGER_NURSERY_COLLECTION);
    PokeGC(reason, nullptr, 0); // WARNING: use nullptr
  }
  reason = JS::WantEagerMajorGC(rt);
  if (reason != JS::GCReason::NO_REASON) {
    PokeGC(reason, nullptr, 0); // WARNING: use nullptr

https://phabricator.services.mozilla.com/D100368 currently shows the above two warnings. But you can see the signature of PokeGC just above: it takes a reason, a pointer, and a TimeDuration. I'm guessing that the analysis sees that I am passing a 0 to a function that takes a pointer value -- except it's for a different parameter.

One thing that might factor in here is that the 3rd parameter has a default value?

static void PokeGC(JS::GCReason aReason, JSObject* aObj,
                     mozilla::TimeDuration aDelay = 0);

Or maybe it just thinks that the autoconversion from 0 to TimeDuration is the same as autoconverting 0 to nullptr?

Comment 1

[Steve Fink [:sfink] [:s:]]

Ok, I have another instance, and it's much simpler than what I was hypothesizing:

  EnsureGCRunner(0);

where

  void EnsureGCRunner(TimeDuration aDelay);

so it's complaining purely about passing a 0 for a TimeDuration. So actually this is not a bug in the static analysis, it's a warning for a weird hack that allows 0 and only 0 to be used to initialize a TImeDuration.

Comment 2

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:andi, could you have a look please?

For more information, please visit auto_nag documentation.