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

RULE-8-13: Address false positive issues#765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolaswill merged 10 commits intomainfromlcartey/rule-8-13-copies
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,29 +18,54 @@ import cpp
import codingstandards.c.misra
import codingstandards.cpp.Pointers
import codingstandards.cpp.SideEffect
import codingstandards.cpp.alertreporting.HoldsForAllCopies

from Variable ptr, PointerOrArrayType type
class NonConstPointerVariableCandidate extends Variable {
NonConstPointerVariableCandidate() {
// Ignore parameters in functions without bodies
(this instanceof Parameter implies exists(this.(Parameter).getFunction().getBlock())) and
// Ignore variables in functions that use ASM commands
not exists(AsmStmt a |
a.getEnclosingFunction() = this.(LocalScopeVariable).getFunction()
or
// In a type declared locally
this.(Field).getDeclaringType+().getEnclosingFunction() = a.getEnclosingFunction()
) and
exists(PointerOrArrayType type |
// include only pointers which point to a const-qualified type
this.getType() = type and
not type.isDeeplyConstBelow()
) and
// exclude pointers passed as arguments to functions which take a
// parameter that points to a non-const-qualified type
not exists(FunctionCall fc, int i |
fc.getArgument(i) = this.getAnAccess() and
not fc.getTarget().getParameter(i).getType().isDeeplyConstBelow()
) and
// exclude any pointers which have their underlying data modified
not exists(VariableEffect effect |
effect.getTarget() = this and
// but not pointers that are only themselves modified
not effect.(AssignExpr).getLValue() = this.getAnAccess() and
not effect.(CrementOperation).getOperand() = this.getAnAccess()
) and
// exclude pointers assigned to another pointer to a non-const-qualified type
not exists(Variable a |
a.getAnAssignedValue() = this.getAnAccess() and
not a.getType().(PointerOrArrayType).isDeeplyConstBelow()
)
}
}

/**
* Ensure that all copies of a variable are considered to be missing const qualification to avoid
* false positives where a variable is only used/modified in a single copy.
*/
class NonConstPointerVariable =
HoldsForAllCopies<NonConstPointerVariableCandidate, Variable>::LogicalResultElement;

from NonConstPointerVariable ptr
where
not isExcluded(ptr, Pointers1Package::pointerShouldPointToConstTypeWhenPossibleQuery()) and
// include only pointers which point to a const-qualified type
ptr.getType() = type and
not type.isDeeplyConstBelow() and
// exclude pointers passed as arguments to functions which take a
// parameter that points to a non-const-qualified type
not exists(FunctionCall fc, int i |
fc.getArgument(i) = ptr.getAnAccess() and
not fc.getTarget().getParameter(i).getType().isDeeplyConstBelow()
) and
// exclude any pointers which have their underlying data modified
not exists(VariableEffect effect |
effect.getTarget() = ptr and
// but not pointers that are only themselves modified
not effect.(AssignExpr).getLValue() = effect.getAnAccess() and
not effect.(CrementOperation).getOperand() = effect.getAnAccess()
) and
// exclude pointers assigned to another pointer to a non-const-qualified type
not exists(Variable a |
a.getAnAssignedValue() = ptr.getAnAccess() and
not a.getType().(PointerOrArrayType).isDeeplyConstBelow()
)
select ptr, "$@ points to a non-const-qualified type.", ptr, ptr.getName()
not isExcluded(ptr.getAnElementInstance(),
Pointers1Package::pointerShouldPointToConstTypeWhenPossibleQuery())
select ptr, "$@ points to a non-const-qualified type.", ptr, ptr.getAnElementInstance().getName()
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,3 +12,4 @@
| test.c:66:23:66:24 | p1 | $@ points to a non-const-qualified type. | test.c:66:23:66:24 | p1 | p1 |
| test.c:71:17:71:18 | p1 | $@ points to a non-const-qualified type. | test.c:71:17:71:18 | p1 | p1 |
| test.c:75:15:75:16 | p1 | $@ points to a non-const-qualified type. | test.c:75:15:75:16 | p1 | p1 |
| test.c:103:30:103:30 | s | $@ points to a non-const-qualified type. | test.c:103:30:103:30 | s | s |
34 changes: 34 additions & 0 deletionsc/misra/test/rules/RULE-8-13/test.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,4 +75,38 @@ char *f16(char *p1) { // NON_COMPLIANT
int f17(char *p1) { // NON_COMPLIANT
p1++;
return 0;
}

#include <stdint.h>

int16_t
test_r(int16_t *value) { // COMPLIANT - ignored because of the use of ASM
int16_t result;
struct S {
int *x; // COMPLIANT - ignored because of the use of ASM
struct S2 {
int *y; // COMPLIANT - ignored because of the use of ASM
} s2;
};
__asm__("movb %bh (%eax)");
return result;
}

struct S {
int x;
};

void test_struct(struct S *s) { // COMPLIANT
s->x = 1;
}

void test_struct_2(struct S *s) { // NON_COMPLIANT - could be const
s = 0;
}

void test_no_body(int *p); // COMPLIANT - no body, so cannot evaluate whether it
// should be const

void increment(int *p) { // COMPLIANT
*p++ = 1;
}
7 changes: 7 additions & 0 deletionschange_notes/2024-10-20-8-13-fixes.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
- `RULE-8-13` - `PointerShouldPointToConstTypeWhenPossible.ql`
- 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.
- Exclude results for local scope variables in functions that use assembly code, as CodeQL cannot determine the impact of the assembly.
- Exclude false positives when an assignment is made to a struct field.
- Exclude false positives where the object pointed to by the variable is modified using `*p++ = ...`.
- Exclude false positives for functions without bodies.
- 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`.
2 changes: 2 additions & 0 deletionscpp/common/src/codingstandards/cpp/SideEffect.qll
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -190,6 +190,8 @@ Expr getAnEffect(Expr base) {
or
exists(PointerDereferenceExpre|e.getOperand()=base|result=getAnEffect(e))
or
exists(CrementOperationc|c.getOperand()=base|result=getAnEffect(c))
or
// local alias analysis, assume alias when data flows to derived type (pointer/reference)
// auto ptr = &base;
exists(VariableAccessva,AddressOfExpraddressOf|
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -85,11 +85,9 @@ module HoldsForAllCopies<CandidateElementSig CandidateElement, ElementSetSig Ele
string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
exists(CandidateElement s |
// Only consider candidates where we can match up the location
isNotWithinMacroExpansion(s) and
hasLocation(s, filepath, startline, startcolumn, endline, endcolumn) and
// All relevant elements that occur at the same location are candidates
forex(RelevantElement relevantElement | s = relevantElement.getCandidateElement() |
forall(RelevantElement relevantElement | s = relevantElement.getCandidateElement() |
relevantElement instanceof CandidateElement
)
)
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp