Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit1566129

Browse files
authored
Merge pull request#765 from github/lcartey/rule-8-13-copies
`RULE-8-13`: Address false positive issues
2 parents5d247be +460fb26 commit1566129

File tree

6 files changed

+94
-27
lines changed

6 files changed

+94
-27
lines changed

‎c/misra/src/rules/RULE-8-13/PointerShouldPointToConstTypeWhenPossible.ql‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,54 @@ import cpp
1818
import codingstandards.c.misra
1919
import codingstandards.cpp.Pointers
2020
import codingstandards.cpp.SideEffect
21+
import codingstandards.cpp.alertreporting.HoldsForAllCopies
2122

22-
fromVariableptr,PointerOrArrayTypetype
23+
classNonConstPointerVariableCandidateextendsVariable{
24+
NonConstPointerVariableCandidate(){
25+
// Ignore parameters in functions without bodies
26+
(thisinstanceofParameterimpliesexists(this.(Parameter).getFunction().getBlock()))and
27+
// Ignore variables in functions that use ASM commands
28+
notexists(AsmStmta|
29+
a.getEnclosingFunction()=this.(LocalScopeVariable).getFunction()
30+
or
31+
// In a type declared locally
32+
this.(Field).getDeclaringType+().getEnclosingFunction()=a.getEnclosingFunction()
33+
)and
34+
exists(PointerOrArrayTypetype|
35+
// include only pointers which point to a const-qualified type
36+
this.getType()=typeand
37+
nottype.isDeeplyConstBelow()
38+
)and
39+
// exclude pointers passed as arguments to functions which take a
40+
// parameter that points to a non-const-qualified type
41+
notexists(FunctionCallfc,inti|
42+
fc.getArgument(i)=this.getAnAccess()and
43+
notfc.getTarget().getParameter(i).getType().isDeeplyConstBelow()
44+
)and
45+
// exclude any pointers which have their underlying data modified
46+
notexists(VariableEffecteffect|
47+
effect.getTarget()=thisand
48+
// but not pointers that are only themselves modified
49+
noteffect.(AssignExpr).getLValue()=this.getAnAccess()and
50+
noteffect.(CrementOperation).getOperand()=this.getAnAccess()
51+
)and
52+
// exclude pointers assigned to another pointer to a non-const-qualified type
53+
notexists(Variablea|
54+
a.getAnAssignedValue()=this.getAnAccess()and
55+
nota.getType().(PointerOrArrayType).isDeeplyConstBelow()
56+
)
57+
}
58+
}
59+
60+
/**
61+
* Ensure that all copies of a variable are considered to be missing const qualification to avoid
62+
* false positives where a variable is only used/modified in a single copy.
63+
*/
64+
classNonConstPointerVariable=
65+
HoldsForAllCopies<NonConstPointerVariableCandidate,Variable>::LogicalResultElement;
66+
67+
fromNonConstPointerVariableptr
2368
where
24-
notisExcluded(ptr, Pointers1Package::pointerShouldPointToConstTypeWhenPossibleQuery())and
25-
// include only pointers which point to a const-qualified type
26-
ptr.getType()=typeand
27-
nottype.isDeeplyConstBelow()and
28-
// exclude pointers passed as arguments to functions which take a
29-
// parameter that points to a non-const-qualified type
30-
notexists(FunctionCallfc,inti|
31-
fc.getArgument(i)=ptr.getAnAccess()and
32-
notfc.getTarget().getParameter(i).getType().isDeeplyConstBelow()
33-
)and
34-
// exclude any pointers which have their underlying data modified
35-
notexists(VariableEffecteffect|
36-
effect.getTarget()=ptrand
37-
// but not pointers that are only themselves modified
38-
noteffect.(AssignExpr).getLValue()=effect.getAnAccess()and
39-
noteffect.(CrementOperation).getOperand()=effect.getAnAccess()
40-
)and
41-
// exclude pointers assigned to another pointer to a non-const-qualified type
42-
notexists(Variablea|
43-
a.getAnAssignedValue()=ptr.getAnAccess()and
44-
nota.getType().(PointerOrArrayType).isDeeplyConstBelow()
45-
)
46-
selectptr,"$@ points to a non-const-qualified type.",ptr,ptr.getName()
69+
notisExcluded(ptr.getAnElementInstance(),
70+
Pointers1Package::pointerShouldPointToConstTypeWhenPossibleQuery())
71+
selectptr,"$@ points to a non-const-qualified type.",ptr,ptr.getAnElementInstance().getName()

‎c/misra/test/rules/RULE-8-13/PointerShouldPointToConstTypeWhenPossible.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@
1212
| test.c:66:23:66:24 | p1 | $@ points to a non-const-qualified type. | test.c:66:23:66:24 | p1 | p1 |
1313
| test.c:71:17:71:18 | p1 | $@ points to a non-const-qualified type. | test.c:71:17:71:18 | p1 | p1 |
1414
| test.c:75:15:75:16 | p1 | $@ points to a non-const-qualified type. | test.c:75:15:75:16 | p1 | p1 |
15+
| test.c:103:30:103:30 | s | $@ points to a non-const-qualified type. | test.c:103:30:103:30 | s | s |

‎c/misra/test/rules/RULE-8-13/test.c‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,38 @@ char *f16(char *p1) { // NON_COMPLIANT
7575
intf17(char*p1) {// NON_COMPLIANT
7676
p1++;
7777
return0;
78+
}
79+
80+
#include<stdint.h>
81+
82+
int16_t
83+
test_r(int16_t*value) {// COMPLIANT - ignored because of the use of ASM
84+
int16_tresult;
85+
structS {
86+
int*x;// COMPLIANT - ignored because of the use of ASM
87+
structS2 {
88+
int*y;// COMPLIANT - ignored because of the use of ASM
89+
}s2;
90+
};
91+
__asm__("movb %bh (%eax)");
92+
returnresult;
93+
}
94+
95+
structS {
96+
intx;
97+
};
98+
99+
voidtest_struct(structS*s) {// COMPLIANT
100+
s->x=1;
101+
}
102+
103+
voidtest_struct_2(structS*s) {// NON_COMPLIANT - could be const
104+
s=0;
105+
}
106+
107+
voidtest_no_body(int*p);// COMPLIANT - no body, so cannot evaluate whether it
108+
// should be const
109+
110+
voidincrement(int*p) {// COMPLIANT
111+
*p++=1;
78112
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-`RULE-8-13` -`PointerShouldPointToConstTypeWhenPossible.ql`
2+
- Exclude false positives where a variable occurs in a file compiled multiple times, but where it may only be const in some of those scenarios.
3+
- Exclude results for local scope variables in functions that use assembly code, as CodeQL cannot determine the impact of the assembly.
4+
- Exclude false positives when an assignment is made to a struct field.
5+
- Exclude false positives where the object pointed to by the variable is modified using`*p++ = ...`.
6+
- Exclude false positives for functions without bodies.
7+
- Rules that rely on the determination of side-effects of an expression may change as a result of considering`*p++ = ...` as having a side-effect on`p`.

‎cpp/common/src/codingstandards/cpp/SideEffect.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ Expr getAnEffect(Expr base) {
190190
or
191191
exists(PointerDereferenceExpre|e.getOperand()=base|result=getAnEffect(e))
192192
or
193+
exists(CrementOperationc|c.getOperand()=base|result=getAnEffect(c))
194+
or
193195
// local alias analysis, assume alias when data flows to derived type (pointer/reference)
194196
// auto ptr = &base;
195197
exists(VariableAccessva,AddressOfExpraddressOf|

‎cpp/common/src/codingstandards/cpp/alertreporting/HoldsForAllCopies.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,9 @@ module HoldsForAllCopies<CandidateElementSig CandidateElement, ElementSetSig Ele
8585
stringfilepath,intstartline,intstartcolumn,intendline,intendcolumn
8686
){
8787
exists(CandidateElements|
88-
// Only consider candidates where we can match up the location
89-
isNotWithinMacroExpansion(s)and
9088
hasLocation(s,filepath,startline,startcolumn,endline,endcolumn)and
9189
// All relevant elements that occur at the same location are candidates
92-
forex(RelevantElementrelevantElement|s=relevantElement.getCandidateElement()|
90+
forall(RelevantElementrelevantElement|s=relevantElement.getCandidateElement()|
9391
relevantElementinstanceofCandidateElement
9492
)
9593
)

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp