Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++: Remove FPs from cpp/uninitialized-local when encountered extraction errors #17481

Merged
merged 8 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
11 changes: 11 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/Function.qll
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,17 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
* Gets the nearest enclosing AccessHolder.
*/
override AccessHolder getEnclosingAccessHolder() { result = this.getDeclaringType() }

/**
* Holds if this function has extraction errors that create an `ErrorExpr`.
*/
predicate hasErrors() {
exists(ErrorExpr e |
e.getEnclosingFunction() = this and
// Exclude the first allocator call argument because it is always extracted as `ErrorExpr`.
not exists(NewOrNewArrayExpr new | e = new.getAllocatorCall().getArgument(0))
)
}
}

pragma[noinline]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ where
not declarationHasSideEffects(v) and
not exists(AsmStmt s | f = s.getEnclosingFunction()) and
not v.getAnAttribute().getName() = "unused" and
not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr may use `v`
not f.hasErrors()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment should be retained here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or at least something along those lines.

select v, "Variable " + v.getName() + " is not used."
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
override predicate isSource(Instruction source) {
exists(Function func |
// Rule out FPs caused by extraction errors.
not any(ErrorExpr e).getEnclosingFunction() = func and
not func.hasErrors() and
not intentionallyReturnsStackPointer(func) and
func = source.getEnclosingFunction()
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ predicate isSinkImpl(Instruction sink, VariableAccess va) {
exists(LoadInstruction load |
va = load.getUnconvertedResultExpression() and
not va = commonException() and
not va.getTarget().(LocalVariable).getFunction().hasErrors() and
sink = load.getSourceValue()
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ predicate instructionHasVariable(VariableAddressInstruction vai, StackVariable v
// Pointer-to-member types aren't properly handled in the dbscheme.
not vai.getResultType() instanceof PointerToMemberType and
// Rule out FPs caused by extraction errors.
not any(ErrorExpr e).getEnclosingFunction() = f
not f.hasErrors()
}

/**
Expand Down
2 changes: 1 addition & 1 deletion cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ predicate functionsMissingReturnStmt(Function f, ControlFlowNode blame) {
predicate functionImperfectlyExtracted(Function f) {
exists(CompilerError e | f.getBlock().getLocation().subsumes(e.getLocation()))
or
exists(ErrorExpr ee | ee.getEnclosingFunction() = f)
f.hasErrors()
or
count(f.getType()) > 1
or
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
edges
nodes
| errors.cpp:13:7:13:7 | definition of x | semmle.label | definition of x |
| test.cpp:11:6:11:8 | definition of foo | semmle.label | definition of foo |
| test.cpp:111:6:111:8 | definition of foo | semmle.label | definition of foo |
| test.cpp:226:7:226:7 | definition of x | semmle.label | definition of x |
Expand All @@ -14,6 +15,7 @@ nodes
| test.cpp:472:6:472:6 | definition of x | semmle.label | definition of x |
| test.cpp:479:6:479:6 | definition of x | semmle.label | definition of x |
#select
| errors.cpp:14:18:14:18 | x | errors.cpp:13:7:13:7 | definition of x | errors.cpp:13:7:13:7 | definition of x | The variable $@ may not be initialized at this access. | errors.cpp:13:7:13:7 | x | x |
| test.cpp:12:6:12:8 | foo | test.cpp:11:6:11:8 | definition of foo | test.cpp:11:6:11:8 | definition of foo | The variable $@ may not be initialized at this access. | test.cpp:11:6:11:8 | foo | foo |
| test.cpp:113:6:113:8 | foo | test.cpp:111:6:111:8 | definition of foo | test.cpp:111:6:111:8 | definition of foo | The variable $@ may not be initialized at this access. | test.cpp:111:6:111:8 | foo | foo |
| test.cpp:227:3:227:3 | x | test.cpp:226:7:226:7 | definition of x | test.cpp:226:7:226:7 | definition of x | The variable $@ may not be initialized at this access. | test.cpp:226:7:226:7 | x | x |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// semmle-extractor-options: --expect_errors

int f1() {
int x;
initialize(&x); // error expression - initialize() is not defined
return x; // GOOD - assume x is initialized
}

void * operator new(unsigned long, bool);
void operator delete(void*, bool);

int f2() {
int x;
new(true) int (x); // BAD, ignore implicit error expression
}
Loading