`A5-0-1`: False positive related to overeager alias analysis
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 35/100
Research direction
Start with the SideEffects library and the affected A5-0-1 analysis, then inspect the references to localFlow and AliasParameter. Compare the provided examples with the handling of expression side-effects and function-call side-effects, including the RULE-13-2 case. Done means alias analysis no longer reports false unsequenced side-effects while still detecting genuine effects through function parameters.
Written by the indexing model from the issue text.
Description
Affected rules
A5-0-1- Likely others such as
RULE-13-2under 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
}
}
- Dominant language
- CodeQL
- Stars
- 227
- Forks
- 82
- Avg merge
- 6d 7h
- Merged PRs (30d)
- 9
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/codeql-coding-standards
-
false positive/false negative Stardard-MISRA-C++
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
github/codeql-coding-standards#1172 ·
-
Difficulty-Low false positive/false negative false-negative Impact-Low Standard-MISRA-C
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty-Medium false positive/false negative false-positive Impact-Medium Standard-CERT-C
Difficulty 4/5 3-5 days Newbie friendliness 48/100
github/codeql-coding-standards#1200 ·
-
`RULE-0-0-1`: "unreachable statement" false positives due to over-pruning of the control-flow graph Openfalse positive/false negative
Difficulty 4/5 3-5 days Newbie friendliness 48/100
github/codeql-coding-standards#1190 ·
-
false positive/false negative
Difficulty 3/5 1-2 days Newbie friendliness 65/100
github/codeql-coding-standards#1175 ·
All issues in github/codeql-coding-standards
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
ScoopInstaller/Nonportable#639 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
ScoopInstaller/Extras#18800 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
uqbar-project/website-wollok-ts#84 · 2 comments ·
-
on hold T: core-bug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100