Skip to content

A5-0-1: False positive related to overeager alias analysis #1047

@MichaelRFairhurst

Description

@MichaelRFairhurst

Affected rules

  • A5-0-1
  • Likely others such as RULE-13-2 under more atypical conditions

Description

We can produce false positives related to unsequenced side effects from overeager alias analysis.

The SideEffects library attempts to capture side effects across function boundaries:

void f(int *i) {
  (*i)++; // side-effect (*i)++
}

void g() {
  int i;
  f(&i); // side-effect: (*i)++
}

Detecting the latter case requires flow analysis. Currently, we perform flow analysis on all address-of operators, not just to parameters or through "other" functions. This leads to weird behavior:

void h() {
  int i;
  int j = &i;
  i = 1;
  j; // according to our analysis, this has a side-effect: i = 1
  j + j; // reported as unsequenced side-effects
}

Our side effect analysis should be split into two distinct phases: side-effects within expressions, and side-effects from function calls.

It looks like we mostly do this, but incorrectly.

Side-effects within expressions

Should only use flow analysis to find aliases for cases such as:

void f() {
  int i = 0;
  int j = &i;

  int x = i-- + (*j++); // unsequenced effects due to aliasing
}

That is, we can detect that *j is an alias of i and effectively consider the side-effects of i-- + i++. This is different than declaring *j has side-effects.

This is not currently handled correctly, as we inaccurately assign side-effects via localFlow.

Side-effects from function calls

void f(int *x) {
  (*x)++;
}

We should recursively find parameters that become the subjects of side-effects. We should trace parameter x to the effect on x. Now f(x) can be discovered in the above phase as an effect on x -- without flow analysis.

We implement this as AliasParameter, but we don't find local flow from a parameter to its effect, we just look for effects on param.getAnAccess()

Example

Real code example in pandas

      while (*step) {
         stbtt__active_edge * z = *step;
         if (z->ey <= scan_y_top) {
            *step = z->next; // delete from list
            STBTT_assert(z->direction);
            z->direction = 0;
            stbtt__hheap_free(&hh, z);
         } else {
            step = &((*step)->next); // advance through list
         }
      }

Metadata

Metadata

Assignees

No one assigned

    Labels

    false positive/false negativeAn issue related to observed false positives or false negatives.

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions