Closed Bug 1536719 Opened 8 months ago Closed 8 months ago

Fix handling of member calls on operator calls in static analysis

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

The patch description will explain what's going on and why the patch fixes it, but the short story is that this will fix the FIXME at https://searchfox.org/mozilla-central/rev/358f816f63da072145c593e9e2ac36b7250ecd25/build/clang-plugin/tests/TestCanRunScript.cpp#196

Assignee: nobody → bzbarsky
Depends on: 1536336

The old code for member method calls did the following:

  1. Find the member method calls.
  2. Look at their "this" expression.
  3. If the "this" is an operator call, check for any of the arguments of the
    operator call being invalid.
  4. Otherwise (if not an operator call) check for the "this" value being
    invalid.

This wasn't right, because the "is invalid" check checks the type and only
considers refcounted things. So if the code looked something like
"foo[i]->call_method()", we would look at the types of "foo" and "i" and
determine that none of those are refcounted types so there is nothing invalid
here (since "foo" is some sort of array type and "i" is an integer). The new
setup just checks whether the "this" value is invalid, which does the type
check on the "this" value itself; in the "foo[i]->call_method()" case on
"foo[i]". We then adjust the exclusions in InvalidArg to consider operator->
on known-live things valid, to allow the thing that we were really trying to
accomplish with the "check for an operator call" bits:
"stackRefPtr->some_method()".

The test coverage being added for the made-up TArray type is meant to catch
things like the geolocation issue that was being hidden by the buggy behavior.
I'm not using nsTArray itself because some header included by nsTArray.h
tries to define operator new/delete bits inline and that triggers warnings that
then cause a clang-plugin test failure, because they're unexpected.

Blocks: 1536724
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f497ce3b7419
Fix handling of member method calls in the MOZ_CAN_RUN_SCRIPT analysis.  r=andi
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.