Skip to content

Fixing the final compatibility issues #338

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

Merged
merged 18 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions change_notes/2023-08-02-a8-4-13-false-positives.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `A8-4-13`
- Address false positives caused by missing modelling of modifying operations for smart pointers for some standard libraries (such as libstdc++).
5 changes: 5 additions & 0 deletions change_notes/2023-08-02-smart-pointers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- `A20-8-1`/`MEM56-CPP`
- Address false negatives caused by lack of modelling of flow through smart pointers.
- Reduce flow paths through standard library headers to simplify results.
- `A18-1-4`
- Address false positives caused by missing modelling of modifying operations for smart pointers for some standard libraries (such as libstdc++).
2 changes: 2 additions & 0 deletions change_notes/2023-08-03-string-replace.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `STR51-CPP`
- Address false negatives caused by incomplete modelling of the `std::string::replace()` function.
3 changes: 3 additions & 0 deletions change_notes/2023-08-04-a15-5-1-noexcept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `A15-5-1`
- Rephrase alert message for `noalert(false)` special functions to clarify that this permits exceptions.
- Additional results for implicit `noexcept(true)` special functions highlighting that the specification should be made explicit.
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,18 @@ class DeleteWrapperFunction extends Function {
class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
Constructor c;

ExceptionThrownInConstructor() { exists(getAFunctionThrownType(c, this)) }
ExceptionThrownInConstructor() {
exists(getAFunctionThrownType(c, this)) and
// The constructor is within the users source code
exists(c.getFile().getRelativePath())
}

Constructor getConstructor() { result = c }
}

/**
* Add the `nodes` predicate to ensure results with an empty path are still reported.
*/
query predicate nodes(ExceptionFlowNode node) { any() }

from
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,25 @@ import codingstandards.cpp.exceptions.ExceptionSpecifications
from SpecialFunction f, string message
where
not isExcluded(f, Exceptions2Package::specialFunctionMissingNoExceptSpecificationQuery()) and
not isNoExceptTrue(f) and
not isFDENoExceptTrue(f.getDefinition()) and
not f.isCompilerGenerated() and
not f.isDeleted() and
not f.isDefaulted() and
(
isNoExceptExplicitlyFalse(f) and
message = f.getQualifiedName() + " should not be noexcept(false)."
message =
"Special function " + f.getQualifiedName() +
" has a noexcept(false) specification that permits exceptions."
or
isNoExceptTrue(f) and
message =
f.getQualifiedName() +
" has an implicit noexcept(true) specification but should make that explicit."
or
not isNoExceptTrue(f) and
not isNoExceptExplicitlyFalse(f) and
message = f.getQualifiedName() + " is implicitly noexcept(false) and might throw."
message =
"Special function " + f.getQualifiedName() +
" has an implicit noexcept(false) specification that permits exceptions."
)
select f, message
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,28 @@ class SingleObjectSmartPointerArrayConstructionConfig extends TaintTracking::Con
(
sp.getAConstructorCallWithExternalObjectConstruction().getAnArgument() = sink.asExpr()
or
sink.asExpr() =
any(FunctionCall fc, MemberFunction mf |
mf = fc.getTarget() and
mf.getDeclaringType() = sp and
mf.getName() = "reset"
|
fc.getArgument(0)
)
sink.asExpr() = sp.getAResetCall().getArgument(0)
)
)
}

override predicate isAdditionalTaintStep(DataFlow::Node source, DataFlow::Node sink) {
exists(AutosarUniquePointer sp, FunctionCall fc |
fc = sp.getAReleaseCall() and
source.asExpr() = fc.getQualifier() and
sink.asExpr() = fc
)
}

override predicate isSanitizerIn(DataFlow::Node node) {
// Exclude flow into header files outside the source archive which are summarized by the
// additional taint steps above.
exists(AutosarUniquePointer sp |
sp.getAReleaseCall().getTarget() = node.asExpr().(ThisExpr).getEnclosingFunction()
|
not exists(node.getLocation().getFile().getRelativePath())
)
}
}

from
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,15 @@ import cpp
import codingstandards.cpp.autosar
import codingstandards.cpp.SmartPointers

Expr underlyingObjectAffectingSharedPointerExpr(Function f) {
result =
any(VariableAccess va, FunctionCall fc |
va.getEnclosingFunction() = f and
// strip the type so as to include reference parameter types
va.getType().stripType() instanceof AutosarSharedPointer and
fc.getTarget().getDeclaringType().stripType() instanceof AutosarSharedPointer and
fc.getQualifier() = va and
// include only calls to methods which modify the underlying object
fc.getTarget().hasName(["operator=", "reset", "swap"])
|
va
)
VariableAccess underlyingObjectAffectingSharedPointerExpr(Function f) {
exists(FunctionCall fc |
// Find a call in the function
fc.getEnclosingFunction() = f and
// include only calls to methods which modify the underlying object
fc = any(AutosarSharedPointer s).getAModifyingCall() and
// Report the qualifier
fc.getQualifier() = result
)
}

predicate flowsToUnderlyingObjectAffectingExpr(Parameter p) {
Expand Down
1 change: 1 addition & 0 deletions cpp/autosar/test/rules/A1-1-2.3/options.clang
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-w
2 changes: 1 addition & 1 deletion cpp/autosar/test/rules/A12-0-2/test.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include <cstdlib>
#include <string>
#include <cstring>

class A {
public:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
| test.cpp:5:3:5:9 | ~ClassA | ClassA::~ClassA should not be noexcept(false). |
| test.cpp:9:3:9:9 | ~ClassB | ClassB::~ClassB should not be noexcept(false). |
| test.cpp:38:6:38:20 | operator delete | operator delete is implicitly noexcept(false) and might throw. |
| test.cpp:43:6:43:20 | operator delete | operator delete is implicitly noexcept(false) and might throw. |
| test.cpp:53:11:53:19 | operator= | ClassF::operator= should not be noexcept(false). |
| test.cpp:63:3:63:8 | ClassH | ClassH::ClassH should not be noexcept(false). |
| test.cpp:68:6:68:9 | swap | swap is implicitly noexcept(false) and might throw. |
| test.cpp:72:6:72:9 | swap | swap should not be noexcept(false). |
| test.cpp:77:8:77:11 | swap | ClassI::swap is implicitly noexcept(false) and might throw. |
| test.cpp:82:8:82:11 | swap | ClassJ::swap is implicitly noexcept(false) and might throw. |
| test.cpp:88:6:88:6 | swap | swap is implicitly noexcept(false) and might throw. |
| test.cpp:5:3:5:9 | ~ClassA | Special function ClassA::~ClassA has a noexcept(false) specification that permits exceptions. |
| test.cpp:9:3:9:9 | ~ClassB | Special function ClassB::~ClassB has a noexcept(false) specification that permits exceptions. |
| test.cpp:38:6:38:20 | operator delete | operator delete has an implicit noexcept(true) specification but should make that explicit. |
| test.cpp:43:6:43:20 | operator delete | operator delete has an implicit noexcept(true) specification but should make that explicit. |
| test.cpp:53:11:53:19 | operator= | Special function ClassF::operator= has a noexcept(false) specification that permits exceptions. |
| test.cpp:63:3:63:8 | ClassH | Special function ClassH::ClassH has a noexcept(false) specification that permits exceptions. |
| test.cpp:68:6:68:9 | swap | Special function swap has an implicit noexcept(false) specification that permits exceptions. |
| test.cpp:72:6:72:9 | swap | Special function swap has a noexcept(false) specification that permits exceptions. |
| test.cpp:77:8:77:11 | swap | Special function ClassI::swap has an implicit noexcept(false) specification that permits exceptions. |
| test.cpp:82:8:82:11 | swap | Special function ClassJ::swap has an implicit noexcept(false) specification that permits exceptions. |
| test.cpp:88:6:88:6 | swap | Special function swap has an implicit noexcept(false) specification that permits exceptions. |
6 changes: 3 additions & 3 deletions cpp/autosar/test/rules/A15-5-1/test.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "stddef.h"
#include <new>
#include <stdexcept>

class ClassA {
~ClassA() noexcept(false) { throw std::exception(); } // NON_COMPLIANT
};
Expand Down Expand Up @@ -36,12 +36,12 @@ class ClassD {
};

void operator delete(void *ptr) { // NON_COMPLIANT
// NOTE: cannot be declared noexcept(false)
// NOTE: defaults to noexcept(true)
throw std::exception();
}

void operator delete(void *ptr, size_t size) { // NON_COMPLIANT
// NOTE: cannot be declared noexcept(false)
// NOTE: defaults to noexcept(true)
throw std::exception();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| test.cpp:1:1:1:19 | #include "test.hpp" | Nothing in this file uses anything from "test.hpp" |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| test.cpp:1:1:1:19 | #include "test.hpp" | Nothing in this file uses anything from "test.hpp" |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| test.cpp:1:1:1:19 | #include "test.hpp" | Nothing in this file uses anything from "test.hpp" |
4 changes: 2 additions & 2 deletions cpp/autosar/test/rules/A16-2-2/test.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include "test.hpp" //NON_COMPLIANT
#include <algorithm> //NON_COMPLIANT
#include "test.hpp" //NON_COMPLIANT
#include <algorithm> //NON_COMPLIANT - redundant but not useless on real compilers
#include <vector> //COMPLIANT

std::vector<int> v;
2 changes: 1 addition & 1 deletion cpp/autosar/test/rules/A18-5-5/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ void *malloc1(int b) { // NON_COMPLIANT - recursion
return malloc1(b - 1);
}

void *malloc2(int b) __attribute__((no_caller_saved_registers, __malloc__));
void *malloc2(int b) __attribute__((__malloc__));
void *malloc2(int b) { // NON_COMPLIANT - execution doesn't depend on b

for (int i = 0; i < 10; i++) {
Expand Down
2 changes: 1 addition & 1 deletion cpp/autosar/test/rules/A18-5-6/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ void *malloc1(int b) __attribute__((malloc));

void *malloc1(int b) { return nullptr; } // NON_COMPLIANT

void *malloc3(int b) __attribute__((no_caller_saved_registers, __malloc__));
void *malloc3(int b) __attribute__((__malloc__));
void *malloc3(int b) { return nullptr; } // NON_COMPLIANT

void h1() {} // NON_COMPLIANT
Expand Down
2 changes: 1 addition & 1 deletion cpp/autosar/test/rules/M18-0-5/test.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <string>
#include <cstring>

void test_unbounded_str_funs() {
char str1[] = "Sample string";
Expand Down
2 changes: 1 addition & 1 deletion cpp/cert/test/rules/EXP62-CPP/test.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <string>
#include <cstring>

struct S1 {
int i, j, k;
Expand Down
2 changes: 1 addition & 1 deletion cpp/cert/test/rules/OOP57-CPP/test.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <string>
#include <cstring>

class trivial {};

Expand Down
17 changes: 16 additions & 1 deletion cpp/common/src/codingstandards/cpp/Concurrency.qll
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ class CPPMutexFunctionCall extends MutexFunctionCall {
/**
* Holds if this `CPPMutexFunctionCall` is a lock.
*/
override predicate isLock() { getTarget().getName() = "lock" }
override predicate isLock() {
not isLockingOperationWithinLockingOperation(this) and
getTarget().getName() = "lock"
}

/**
* Holds if this `CPPMutexFunctionCall` is a speculative lock, defined as calling
Expand Down Expand Up @@ -172,6 +175,7 @@ class CMutexFunctionCall extends MutexFunctionCall {
* Holds if this `CMutexFunctionCall` is a lock.
*/
override predicate isLock() {
not isLockingOperationWithinLockingOperation(this) and
getTarget().getName() = ["mtx_lock", "mtx_timedlock", "mtx_trylock"]
}

Expand Down Expand Up @@ -296,6 +300,16 @@ abstract class LockingOperation extends FunctionCall {
* Holds if this is an unlock operation
*/
abstract predicate isUnlock();

/**
* Holds if this locking operation is really a locking operation within a
* designated locking operation. This library assumes the underlying locking
* operations are implemented correctly in that calling a `LockingOperation`
* results in the creation of a singular lock.
*/
predicate isLockingOperationWithinLockingOperation(LockingOperation inner) {
exists(LockingOperation outer | outer.getTarget() = inner.getEnclosingFunction())
}
}

/**
Expand All @@ -317,6 +331,7 @@ class RAIIStyleLock extends LockingOperation {
* Holds if this is a lock operation
*/
override predicate isLock() {
not isLockingOperationWithinLockingOperation(this) and
this instanceof ConstructorCall and
lock = getArgument(0).getAChild*() and
// defer_locks don't cause a lock
Expand Down
20 changes: 20 additions & 0 deletions cpp/common/src/codingstandards/cpp/SmartPointers.qll
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ abstract class AutosarSmartPointer extends Class {
)
}

FunctionCall getAGetCall() {
result.getTarget().hasName("get") and
result.getQualifier().getType().stripType() = this
}

FunctionCall getAnInitializerExpr() {
result =
any(FunctionCall fc |
Expand All @@ -51,10 +56,25 @@ abstract class AutosarSmartPointer extends Class {
AutosarSmartPointer
)
}

FunctionCall getAResetCall() {
result.getTarget().hasName("reset") and
result.getQualifier().getType().stripType() = this
}

FunctionCall getAModifyingCall() {
result.getTarget().hasName(["operator=", "reset", "swap"]) and
result.getQualifier().getType().stripType() = this
}
}

class AutosarUniquePointer extends AutosarSmartPointer {
AutosarUniquePointer() { this.hasQualifiedName("std", "unique_ptr") }

FunctionCall getAReleaseCall() {
result.getTarget().hasName("release") and
result.getQualifier().getType().stripType() = this
}
}

class AutosarSharedPointer extends AutosarSmartPointer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,34 @@ private class PointerToSmartPointerConstructorFlowConfig extends TaintTracking::
cc.getArgument(0) = sink.asExpr()
)
}

override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
// Summarize flow through constructor calls
exists(AutosarSmartPointer sp, ConstructorCall cc |
sp.getAConstructorCall() = cc and
cc = node2.asExpr() and
cc.getArgument(0) = node1.asExpr()
)
or
// Summarize flow through get() calls
exists(AutosarSmartPointer sp, FunctionCall fc |
sp.getAGetCall() = fc and
fc = node2.asExpr() and
fc.getQualifier() = node1.asExpr()
)
}

override predicate isSanitizerIn(DataFlow::Node node) {
// Exclude flow into header files outside the source archive which are summarized by the
// additional taint steps above.
exists(AutosarSmartPointer sp |
sp.getAConstructorCall().getTarget().getAParameter() = node.asParameter()
or
sp.getAGetCall().getTarget().getAParameter() = node.asParameter()
|
not exists(node.getLocation().getFile().getRelativePath())
)
}
}

query predicate problems(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ predicate getAnOrderedLockPair(
lock1 = node.coveredByLock() and
lock2 = node.coveredByLock() and
not lock1 = lock2 and
lock1.getEnclosingFunction() = lock2.getEnclosingFunction() and
exists(Function f |
lock1.getEnclosingFunction() = f and
lock2.getEnclosingFunction() = f and
node.getBasicBlock().getEnclosingFunction() = f
) and
exists(Location l1Loc, Location l2Loc |
l1Loc = lock1.getLocation() and
l2Loc = lock2.getLocation()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class StdBasicString extends ClassTemplateInstantiation {
Type getConstIteratorType() {
exists(TypedefType t |
t.getDeclaringType() = this and
t.getName() = "const_iterator" and
// Certain compilers user __const_iterator instead of const_iterator.
t.getName() = ["const_iterator", "__const_iterator"] and
result = t
)
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/common/test/includes/standard-library/array
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef _GHLIBCPP_ARRAY
#define _GHLIBCPP_ARRAY
#include "iterator.h"
#include "string.h"
#include <iterator>
#include <string>

// Note: a few features currently unused by tests are commented out
namespace std {
Expand Down
3 changes: 1 addition & 2 deletions cpp/common/test/includes/standard-library/cerrno
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
int __errno;
#define errno __errno
#include <errno.h>
Loading