mirror of
https://github.com/github/codeql.git
synced 2026-04-28 02:05:14 +02:00
Merge master into next.
JavaScript semantic conflicts fixed by referring to the `LegacyLanguage` enum. C++ conflicts fixed by accepting Qltest output.
This commit is contained in:
@@ -4,5 +4,5 @@ void fillRect(int x, int y, int w, int h,
|
||||
int r2, int g2, int b2, int a2,
|
||||
gradient_type grad, unsigned int flags, bool border)
|
||||
{
|
||||
// ...
|
||||
// ...
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
int find(int start, char *str, char goal)
|
||||
{
|
||||
int len = strlen(str);
|
||||
//Potential buffer overflow
|
||||
//Potential buffer overflow
|
||||
for (int i = start; str[i] != 0 && i < len; i++) {
|
||||
if (str[i] == goal)
|
||||
return i;
|
||||
@@ -12,7 +12,7 @@ int find(int start, char *str, char goal)
|
||||
int findRangeCheck(int start, char *str, char goal)
|
||||
{
|
||||
int len = strlen(str);
|
||||
//Range check protects against buffer overflow
|
||||
//Range check protects against buffer overflow
|
||||
for (int i = start; i < len && str[i] != 0 ; i++) {
|
||||
if (str[i] == goal)
|
||||
return i;
|
||||
|
||||
@@ -1,16 +1,16 @@
|
||||
void sanitize(Fields[] record) {
|
||||
//The number of fields here can be put in a const
|
||||
for (fieldCtr = 0; field < 7; field++) {
|
||||
sanitize(fields[fieldCtr]);
|
||||
}
|
||||
for (fieldCtr = 0; field < 7; field++) {
|
||||
sanitize(fields[fieldCtr]);
|
||||
}
|
||||
}
|
||||
|
||||
#define NUM_FIELDS 7
|
||||
|
||||
void process(Fields[] record) {
|
||||
//This avoids using a magic constant by using the macro instead
|
||||
for (fieldCtr = 0; field < NUM_FIELDS; field++) {
|
||||
process(fields[fieldCtr]);
|
||||
}
|
||||
//This avoids using a magic constant by using the macro instead
|
||||
for (fieldCtr = 0; field < NUM_FIELDS; field++) {
|
||||
process(fields[fieldCtr]);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
//start of file
|
||||
static void f() { //static function f() is unused in the file
|
||||
//...
|
||||
//...
|
||||
}
|
||||
static void g() {
|
||||
//...
|
||||
//...
|
||||
}
|
||||
void public_func() { //non-static function public_func is not called in file,
|
||||
//but could be visible in other files
|
||||
//...
|
||||
g(); //call to g()
|
||||
//...
|
||||
//...
|
||||
g(); //call to g()
|
||||
//...
|
||||
}
|
||||
//end of file
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
typedef struct Names {
|
||||
char first[100];
|
||||
char last[100];
|
||||
char first[100];
|
||||
char last[100];
|
||||
} Names;
|
||||
|
||||
int doFoo(Names n) { //wrong: n is passed by value (meaning the entire structure
|
||||
//is copied onto the stack, instead of just a pointer)
|
||||
...
|
||||
...
|
||||
}
|
||||
|
||||
int doBar(Names &n) { //better, only a reference is passed
|
||||
...
|
||||
...
|
||||
}
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
Record records[SIZE] = ...;
|
||||
|
||||
int f() {
|
||||
int recordIdx = 0;
|
||||
recordIdx = readUserInput(); //recordIdx is returned from a function
|
||||
int recordIdx = 0;
|
||||
recordIdx = readUserInput(); //recordIdx is returned from a function
|
||||
// there is no check so it could be negative
|
||||
doFoo(&(records[recordIdx])); //but is not checked before use as an array offset
|
||||
doFoo(&(records[recordIdx])); //but is not checked before use as an array offset
|
||||
}
|
||||
|
||||
|
||||
@@ -11,8 +11,8 @@
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* security
|
||||
* external/cwe/190
|
||||
* external/cwe/192
|
||||
* external/cwe/cwe-190
|
||||
* external/cwe/cwe-192
|
||||
*/
|
||||
|
||||
import cpp
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
if (!flags & SOME_BIT) { //wrong: '!' has higher precedence than '&', so this
|
||||
// is bracketed as '(!flags) & SOME_BIT', and does not
|
||||
// check whether a particular bit is set.
|
||||
// ...
|
||||
// is bracketed as '(!flags) & SOME_BIT', and does not
|
||||
// check whether a particular bit is set.
|
||||
// ...
|
||||
}
|
||||
|
||||
if ((p != NULL) & p->f()) { //wrong: The use of '&' rather than '&&' will still
|
||||
// de-reference the pointer even if it is NULL.
|
||||
// ...
|
||||
// de-reference the pointer even if it is NULL.
|
||||
// ...
|
||||
}
|
||||
|
||||
int bits = (s > 8) & 0xff; //wrong: Invalid attempt to get the 8 most significant
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
#define FLAGS 0x4004
|
||||
|
||||
void f_warning(int i)
|
||||
{
|
||||
// The usage of the logical not operator in this case is unlikely to be correct
|
||||
// as the output is being used as an operator for a bit-wise and operation
|
||||
if (i & !FLAGS)
|
||||
{
|
||||
// code
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
void f_fixed(int i)
|
||||
{
|
||||
if (i & ~FLAGS) // Changing the logical not operator for the bit-wise not operator would fix this logic
|
||||
{
|
||||
// code
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,26 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>This rule finds logical-not operator usage as an operator for in a bit-wise operation.</p>
|
||||
|
||||
<p>Due to the nature of logical operation result value, only the lowest bit could possibly be set, and it is unlikely to be intent in bitwise opeartions. Violations are often indicative of a typo, using a logical-not (<code>!</code>) opeartor instead of the bit-wise not (<code>~</code>) operator. </p>
|
||||
<p>This rule is restricted to analyze bit-wise and (<code>&</code>) and bit-wise or (<code>|</code>) operation in order to provide better precision.</p>
|
||||
<p>This rule ignores instances where a double negation (<code>!!</code>) is explicitly used as the opeartor of the bitwise operation, as this is a commonly used as a mechanism to normalize an integer value to either 1 or 0.</p>
|
||||
<p>NOTE: It is not recommended to use this rule in kernel code or older C code as it will likely find several false positive instances.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Carefully inspect the flagged expressions. Consider the intent in the code logic, and decide whether it is necessary to change the not operator.</p>
|
||||
</recommendation>
|
||||
|
||||
<example><sample src="IncorrectNotOperatorUsage.cpp" /></example>
|
||||
|
||||
<references>
|
||||
<li>
|
||||
<a href="https://docs.microsoft.com/en-us/visualstudio/code-quality/c6317?view=vs-2017">warning C6317: incorrect operator: logical-not (!) is not interchangeable with ones-complement (~)</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,35 @@
|
||||
/**
|
||||
* @name Incorrect Not Operator Usage
|
||||
* @description Usage of a logical-not (!) operator as an operand for a bit-wise operation.
|
||||
* This commonly indicates the usage of an incorrect operator instead of the bit-wise not (~) operator,
|
||||
* also known as ones' complement operator.
|
||||
* @kind problem
|
||||
* @id cpp/incorrect-not-operator-usage
|
||||
* @problem.severity warning
|
||||
* @precision medium
|
||||
* @tags security
|
||||
* external/cwe/cwe-480
|
||||
* external/microsoft/c6317
|
||||
*/
|
||||
|
||||
import cpp
|
||||
|
||||
/**
|
||||
* It's common in some projects to use "a double negation" to normalize the boolean
|
||||
* result to either 1 or 0.
|
||||
* This predciate is intended to filter explicit usage of a double negation as it typically
|
||||
* indicates the explicit purpose to normalize the result for bit-wise or arithmetic purposes.
|
||||
*/
|
||||
predicate doubleNegationNormalization( NotExpr notexpr ){
|
||||
notexpr.getAnOperand() instanceof NotExpr
|
||||
}
|
||||
|
||||
from BinaryBitwiseOperation binbitwop
|
||||
where exists( NotExpr notexpr |
|
||||
binbitwop.getAnOperand() = notexpr
|
||||
and not doubleNegationNormalization(notexpr)
|
||||
and ( binbitwop instanceof BitwiseAndExpr
|
||||
or binbitwop instanceof BitwiseOrExpr )
|
||||
)
|
||||
select binbitwop, "Usage of a logical not (!) expression as a bitwise operator."
|
||||
|
||||
@@ -14,53 +14,56 @@ import cpp
|
||||
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
|
||||
import semmle.code.cpp.dataflow.DataFlow
|
||||
|
||||
predicate illDefinedDecrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
|
||||
v.getAnAssignedValue() = initialCondition
|
||||
and
|
||||
exists(
|
||||
RelationalOperation rel |
|
||||
rel = forstmt.getCondition() |
|
||||
terminalCondition = rel.getGreaterOperand()
|
||||
and v.getAnAccess() = rel.getLesserOperand()
|
||||
and
|
||||
DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(rel.getLesserOperand()))
|
||||
/**
|
||||
* A `for` statement whose update is a crement operation on a variable.
|
||||
*/
|
||||
predicate candidateForStmt(ForStmt forStmt, Variable v, CrementOperation update, RelationalOperation rel) {
|
||||
update = forStmt.getUpdate() and
|
||||
update.getAnOperand() = v.getAnAccess() and
|
||||
rel = forStmt.getCondition()
|
||||
}
|
||||
|
||||
predicate illDefinedDecrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
|
||||
exists(DecrementOperation dec, RelationalOperation rel |
|
||||
// decrementing for loop
|
||||
candidateForStmt(forstmt, v, dec, rel) and
|
||||
|
||||
// condition is `v < terminalCondition`
|
||||
terminalCondition = rel.getGreaterOperand() and
|
||||
v.getAnAccess() = rel.getLesserOperand() and
|
||||
|
||||
// `initialCondition` is a value of `v` in the for loop
|
||||
v.getAnAssignedValue() = initialCondition and
|
||||
DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(rel.getLesserOperand())) and
|
||||
|
||||
// `initialCondition` < `terminalCondition`
|
||||
(
|
||||
( upperBound(initialCondition) < lowerBound(terminalCondition) )
|
||||
or
|
||||
( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue() )
|
||||
)
|
||||
and
|
||||
exists(
|
||||
DecrementOperation dec |
|
||||
dec = forstmt.getUpdate().(DecrementOperation)
|
||||
and dec.getAnOperand() = v.getAnAccess()
|
||||
)
|
||||
and
|
||||
(
|
||||
( upperBound(initialCondition) < lowerBound(terminalCondition) )
|
||||
or
|
||||
( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue() )
|
||||
)
|
||||
}
|
||||
|
||||
predicate illDefinedIncrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
|
||||
v.getAnAssignedValue() = initialCondition
|
||||
and
|
||||
exists(
|
||||
RelationalOperation rel |
|
||||
rel = forstmt.getCondition() |
|
||||
terminalCondition = rel.getLesserOperand()
|
||||
and v.getAnAccess() = rel.getGreaterOperand()
|
||||
and
|
||||
DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(rel.getGreaterOperand()))
|
||||
)
|
||||
and
|
||||
exists( IncrementOperation incr |
|
||||
incr = forstmt.getUpdate().(IncrementOperation)
|
||||
and
|
||||
incr.getAnOperand() = v.getAnAccess()
|
||||
)
|
||||
and
|
||||
(
|
||||
( upperBound(terminalCondition) < lowerBound(initialCondition))
|
||||
or
|
||||
( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue())
|
||||
predicate illDefinedIncrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
|
||||
exists(IncrementOperation inc, RelationalOperation rel |
|
||||
// incrementing for loop
|
||||
candidateForStmt(forstmt, v, inc, rel) and
|
||||
|
||||
// condition is `v > terminalCondition`
|
||||
terminalCondition = rel.getLesserOperand() and
|
||||
v.getAnAccess() = rel.getGreaterOperand() and
|
||||
|
||||
// `initialCondition` is a value of `v` in the for loop
|
||||
v.getAnAssignedValue() = initialCondition and
|
||||
DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(rel.getGreaterOperand())) and
|
||||
|
||||
// `terminalCondition` < `initialCondition`
|
||||
(
|
||||
( upperBound(terminalCondition) < lowerBound(initialCondition) )
|
||||
or
|
||||
( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue() )
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -104,9 +104,13 @@ where
|
||||
// Some of the functions operate on a larger char type, like `wchar_t`, so we
|
||||
// need to take this into account in the fixed size case.
|
||||
charSize = f.getParameter(argDest).getType().getUnspecifiedType().(PointerType).getBaseType().getSize() and
|
||||
if exists (fc.getArgument(argLimit).getValue().toInt()) then (
|
||||
if exists(fc.getArgument(argLimit).getValue().toInt()) then (
|
||||
// Fixed sized case
|
||||
arrayExprFixedSize(copyDest) < charSize * fc.getArgument(argLimit).getValue().toInt()
|
||||
exists(int size |
|
||||
size = arrayExprFixedSize(copyDest) and
|
||||
size < charSize * fc.getArgument(argLimit).getValue().toInt() and
|
||||
size != 0 // if the array has zero size, something special is going on
|
||||
)
|
||||
) else exists (Access takenSizeOf, BufferSizeExpr sizeExpr, int plus |
|
||||
// Variable sized case
|
||||
sizeExpr = fc.getArgument(argLimit).getAChild*() and
|
||||
|
||||
@@ -1,16 +1,16 @@
|
||||
int x1 = 0;
|
||||
for (x1 = 0; x1 < 100; x1++) {
|
||||
int x2 = 0;
|
||||
for (x1 = 0; x1 < 300; x1++) {
|
||||
int x2 = 0;
|
||||
for (x1 = 0; x1 < 300; x1++) {
|
||||
// this is most likely a typo
|
||||
// the outer loop will exit immediately
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (x1 = 0; x1 < 100; x1++) {
|
||||
if(x1 == 10 && condition) {
|
||||
for (; x1 < 75; x1++) {
|
||||
for (; x1 < 75; x1++) {
|
||||
// this should be written as a while loop
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,31 +1,31 @@
|
||||
class Base {
|
||||
public:
|
||||
Resource *p;
|
||||
Base() {
|
||||
p = createResource();
|
||||
}
|
||||
//...
|
||||
~Base() {
|
||||
//wrong: this destructor is non-virtual, but Base has a derived class
|
||||
// with a non-virtual destructor
|
||||
freeResource(p);
|
||||
}
|
||||
Resource *p;
|
||||
Base() {
|
||||
p = createResource();
|
||||
}
|
||||
//...
|
||||
~Base() {
|
||||
//wrong: this destructor is non-virtual, but Base has a derived class
|
||||
//with a non-virtual destructor
|
||||
freeResource(p);
|
||||
}
|
||||
};
|
||||
|
||||
class Derived: public Base {
|
||||
public:
|
||||
Resource *dp;
|
||||
Derived() {
|
||||
dp = createResource2();
|
||||
}
|
||||
~Derived() {
|
||||
freeResource2(dp);
|
||||
}
|
||||
Resource *dp;
|
||||
Derived() {
|
||||
dp = createResource2();
|
||||
}
|
||||
~Derived() {
|
||||
freeResource2(dp);
|
||||
}
|
||||
};
|
||||
|
||||
int f() {
|
||||
Base *b = new Derived(); //creates resources for both Base::p and Derived::dp
|
||||
//...
|
||||
delete b; //will only call Base::~Base(), leaking the resource dp.
|
||||
Base *b = new Derived(); //creates resources for both Base::p and Derived::dp
|
||||
//...
|
||||
delete b; //will only call Base::~Base(), leaking the resource dp.
|
||||
// Change both destructors to virtual to ensure they are both called.
|
||||
}
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
class Base {
|
||||
public:
|
||||
Resource *p;
|
||||
Base() {
|
||||
p = createResource();
|
||||
}
|
||||
virtual void f() { //has virtual function
|
||||
//...
|
||||
}
|
||||
//...
|
||||
~Base() { //wrong: is non-virtual
|
||||
freeResource(p);
|
||||
}
|
||||
};
|
||||
|
||||
class Derived: public Base {
|
||||
public:
|
||||
Resource *dp;
|
||||
Derived() {
|
||||
dp = createResource2();
|
||||
}
|
||||
~Derived() {
|
||||
freeResource2(dp);
|
||||
}
|
||||
};
|
||||
|
||||
int f() {
|
||||
Base *b = new Derived(); //creates resources for both Base::p and Derived::dp
|
||||
//...
|
||||
|
||||
//will only call Base::~Base(), leaking the resource dp.
|
||||
//Change both destructors to virtual to ensure they are both called.
|
||||
delete b;
|
||||
}
|
||||
@@ -0,0 +1,38 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
This rule finds classes with virtual functions but no virtual destructor. Deleting a class without a virtual destructor will
|
||||
only call the destructor of the type of the pointer being deleted. This can cause a defect if the pointer type is a base
|
||||
type while the object instance is a derived type.
|
||||
</p>
|
||||
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>
|
||||
Make sure that all classes with virtual functions also have a virtual destructor, especially if other classes derive from them.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example><sample src="NonVirtualDestructorInBaseClass.cpp" />
|
||||
|
||||
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>
|
||||
S. Meyers. <em>Effective C++ 3d ed.</em> pp 40-44. Addison-Wesley Professional, 2005.
|
||||
</li>
|
||||
<li>
|
||||
<a href="http://blogs.msdn.com/b/oldnewthing/archive/2004/05/07/127826.aspx">When should your destructor be virtual?</a>
|
||||
</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
27
cpp/ql/src/Likely Bugs/OO/NonVirtualDestructorInBaseClass.ql
Normal file
27
cpp/ql/src/Likely Bugs/OO/NonVirtualDestructorInBaseClass.ql
Normal file
@@ -0,0 +1,27 @@
|
||||
/**
|
||||
* @name Non-virtual destructor in base class
|
||||
* @description All base classes with a virtual function should define a virtual destructor. If an application attempts to delete a derived class object through a base class pointer, the result is undefined if the base class destructor is non-virtual.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @precision high
|
||||
* @id cpp/virtual-destructor
|
||||
* @tags reliability
|
||||
* readability
|
||||
* language-features
|
||||
*/
|
||||
import cpp
|
||||
|
||||
// find classes with virtual functions that have a destructor that is not virtual and for which there exists a derived class
|
||||
// when calling the destructor of a derived class the destructor in the base class may not be called
|
||||
|
||||
from Class c
|
||||
where exists(VirtualFunction f | f.getDeclaringType() = c)
|
||||
and exists(Destructor d | d.getDeclaringType() = c and
|
||||
// Ignore non-public destructors, which prevent an object of the declaring class from being deleted
|
||||
// directly (except from within the class itself). This is a common pattern in real-world code.
|
||||
d.hasSpecifier("public") and
|
||||
not d.isVirtual() and
|
||||
not d.isDeleted() and
|
||||
not d.isCompilerGenerated())
|
||||
and exists(ClassDerivation d | d.getBaseClass() = c)
|
||||
select c, "A base class with a virtual function should define a virtual destructor."
|
||||
@@ -1,35 +1,35 @@
|
||||
class Base {
|
||||
protected:
|
||||
Resource* resource;
|
||||
Resource* resource;
|
||||
public:
|
||||
virtual void init() {
|
||||
resource = createResource();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResource(resource);
|
||||
}
|
||||
virtual void init() {
|
||||
resource = createResource();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResource(resource);
|
||||
}
|
||||
};
|
||||
|
||||
class Derived: public Base {
|
||||
virtual void init() {
|
||||
resource = createResourceV2();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResourceV2(resource);
|
||||
}
|
||||
virtual void init() {
|
||||
resource = createResourceV2();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResourceV2(resource);
|
||||
}
|
||||
};
|
||||
|
||||
Base::Base() {
|
||||
this->init();
|
||||
this->init();
|
||||
}
|
||||
Base::~Base() {
|
||||
this->release();
|
||||
this->release();
|
||||
}
|
||||
|
||||
int f() {
|
||||
// this will call Base::Base() and then Derived::Derived(), but this->init()
|
||||
// inBase::Base() will resolve to Base::init(), not Derived::init()
|
||||
// The reason for this is that when Base::Base is called, the object being
|
||||
// created is still of type Base (including the vtable)
|
||||
Derived* d = new Derived();
|
||||
// this will call Base::Base() and then Derived::Derived(), but this->init()
|
||||
// inBase::Base() will resolve to Base::init(), not Derived::init()
|
||||
// The reason for this is that when Base::Base is called, the object being
|
||||
// created is still of type Base (including the vtable)
|
||||
Derived* d = new Derived();
|
||||
}
|
||||
|
||||
@@ -1,35 +1,35 @@
|
||||
class Base {
|
||||
protected:
|
||||
Resource* resource;
|
||||
Resource* resource;
|
||||
public:
|
||||
virtual void init() {
|
||||
resource = createResource();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResource(resource);
|
||||
}
|
||||
virtual void init() {
|
||||
resource = createResource();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResource(resource);
|
||||
}
|
||||
};
|
||||
|
||||
class Derived: public Base {
|
||||
virtual void init() {
|
||||
resource = createResourceV2();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResourceV2(resource);
|
||||
}
|
||||
virtual void init() {
|
||||
resource = createResourceV2();
|
||||
}
|
||||
virtual void release() {
|
||||
freeResourceV2(resource);
|
||||
}
|
||||
};
|
||||
|
||||
Base::Base() {
|
||||
this->init();
|
||||
this->init();
|
||||
}
|
||||
Base::~Base() {
|
||||
this->release();
|
||||
this->release();
|
||||
}
|
||||
|
||||
int f() {
|
||||
// this will call Base::Base() and then Derived::Derived(), but this->init()
|
||||
// inBase::Base() will resolve to Base::init(), not Derived::init()
|
||||
// The reason for this is that when Base::Base is called, the object being
|
||||
// created is still of type Base (including the vtable)
|
||||
Derived* d = new Derived();
|
||||
// this will call Base::Base() and then Derived::Derived(), but this->init()
|
||||
// inBase::Base() will resolve to Base::init(), not Derived::init()
|
||||
// The reason for this is that when Base::Base is called, the object being
|
||||
// created is still of type Base (including the vtable)
|
||||
Derived* d = new Derived();
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
struct X {
|
||||
//This struct will have a compiler-generated copy constructor
|
||||
//This struct will have a compiler-generated copy constructor
|
||||
X(const X&, int);
|
||||
...
|
||||
};
|
||||
@@ -7,7 +7,7 @@ struct X {
|
||||
//However, if this is declared later, it will override the compiler-generated
|
||||
//constructor
|
||||
X::X(const X& x, int i =0) {
|
||||
this-> i = i; //uses the i parameter, instead of x.i
|
||||
this-> i = i; //uses the i parameter, instead of x.i
|
||||
}
|
||||
|
||||
C c(1);
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @precision high
|
||||
* @id cpp/virtual-destructor
|
||||
* @id cpp/jsf/av-rule-78
|
||||
* @tags reliability
|
||||
* readability
|
||||
* language-features
|
||||
|
||||
@@ -2,17 +2,17 @@
|
||||
// cannot close the file
|
||||
class ResourceLeak {
|
||||
private:
|
||||
int sockfd;
|
||||
FILE* file;
|
||||
int sockfd;
|
||||
FILE* file;
|
||||
public:
|
||||
C() {
|
||||
sockfd = socket(AF_INET, SOCK_STREAM, 0);
|
||||
}
|
||||
C() {
|
||||
sockfd = socket(AF_INET, SOCK_STREAM, 0);
|
||||
}
|
||||
|
||||
void f() {
|
||||
file = fopen("foo.txt", "r");
|
||||
...
|
||||
}
|
||||
void f() {
|
||||
file = fopen("foo.txt", "r");
|
||||
...
|
||||
}
|
||||
};
|
||||
|
||||
// This class relies on its client to release any stream it
|
||||
|
||||
@@ -83,6 +83,7 @@ predicate assignOperatorWithWrongType(Operator op, string msg) {
|
||||
predicate assignOperatorWithWrongResult(Operator op, string msg) {
|
||||
op.hasName("operator=")
|
||||
and not returnsDereferenceThis(op)
|
||||
and exists(op.getBlock())
|
||||
and not op.getType() instanceof VoidType
|
||||
and not assignOperatorWithWrongType(op, _)
|
||||
and msg = "Assignment operator in class " + op.getDeclaringType().getName() + " does not return a reference to *this."
|
||||
|
||||
@@ -6,13 +6,13 @@ class C : protected Superclass,
|
||||
public InterfaceA, public InterfaceB,
|
||||
private ImplementationA, private ImplementationB
|
||||
{
|
||||
//implementation
|
||||
//implementation
|
||||
};
|
||||
|
||||
//wrong: multiple protected bases
|
||||
class D : protected Superclass1, protected Superclass2,
|
||||
public Interface, private Implementation
|
||||
{
|
||||
//implementation
|
||||
//implementation
|
||||
};
|
||||
|
||||
|
||||
@@ -18,13 +18,17 @@ import cpp
|
||||
programmer, we can flag it anyway, since this is arguably a bug.) */
|
||||
|
||||
predicate functionsMissingReturnStmt(Function f, ControlFlowNode blame) {
|
||||
f.fromSource() and
|
||||
exists(Type returnType |
|
||||
returnType = f.getType().getUnderlyingType().getUnspecifiedType() and
|
||||
not returnType instanceof VoidType and
|
||||
not returnType instanceof TemplateParameter
|
||||
) and
|
||||
exists(ReturnStmt s | f.getAPredecessor() = s | blame = s.getAPredecessor())}
|
||||
f.fromSource() and
|
||||
exists(Type returnType |
|
||||
returnType = f.getType().getUnderlyingType().getUnspecifiedType() and
|
||||
not returnType instanceof VoidType and
|
||||
not returnType instanceof TemplateParameter
|
||||
) and
|
||||
exists(ReturnStmt s |
|
||||
f.getAPredecessor() = s and
|
||||
blame = s.getAPredecessor()
|
||||
)
|
||||
}
|
||||
|
||||
/* If a function has a value-carrying return statement, but the extractor hit a snag
|
||||
whilst parsing the value, then the control flow graph will not include the value.
|
||||
|
||||
@@ -542,5 +542,5 @@ query predicate edges(PrintASTNode source, PrintASTNode target, string key, stri
|
||||
}
|
||||
|
||||
query predicate graphProperties(string key, string value) {
|
||||
key = "semmle.graphKind" and value = "tree"
|
||||
key = "semmle.graphKind" and value = "tree"
|
||||
}
|
||||
|
||||
@@ -57,7 +57,9 @@ int getBufferSize(Expr bufferExpr, Element why) {
|
||||
// buffer is a fixed size array
|
||||
result = bufferVar.getType().getUnspecifiedType().(ArrayType).getSize() and
|
||||
why = bufferVar and
|
||||
not memberMayBeVarSize(_, bufferVar)
|
||||
not memberMayBeVarSize(_, bufferVar) and
|
||||
not result = 0 // zero sized arrays are likely to have special usage, for example
|
||||
// behaving a bit like a 'union' overlapping other fields.
|
||||
) or (
|
||||
// buffer is an initialized array
|
||||
// e.g. int buffer[] = {1, 2, 3};
|
||||
|
||||
@@ -240,13 +240,18 @@ class BasicBlock extends ControlFlowNodeBase {
|
||||
|
||||
/**
|
||||
* Holds if control flow may reach this basic block from a function entry
|
||||
* point or a `catch` clause of a reachable `try` statement.
|
||||
* point or any handler of a reachable `try` statement.
|
||||
*/
|
||||
predicate isReachable() {
|
||||
exists(Function f | f.getBlock() = this)
|
||||
or
|
||||
exists(TryStmt t, BasicBlock tryblock | this = t.getACatchClause() and tryblock.isReachable() and tryblock.contains(t))
|
||||
or
|
||||
exists(TryStmt t, BasicBlock tryblock |
|
||||
// a `Handler` preceeds the `CatchBlock`, and is always the beginning
|
||||
// of a new `BasicBlock` (see `primitive_basic_block_entry_node`).
|
||||
this.(Handler).getTryStmt() = t and
|
||||
tryblock.isReachable() and
|
||||
tryblock.contains(t)
|
||||
) or
|
||||
exists(BasicBlock pred | pred.getASuccessor() = this and pred.isReachable())
|
||||
}
|
||||
|
||||
|
||||
@@ -95,7 +95,7 @@ private cached module Cached {
|
||||
or
|
||||
reachable(n.getAPredecessor())
|
||||
or
|
||||
n instanceof CatchBlock
|
||||
n instanceof Handler
|
||||
}
|
||||
|
||||
/** Holds if `condition` always evaluates to a nonzero value. */
|
||||
|
||||
@@ -136,6 +136,28 @@ private predicate impossibleDefaultSwitchEdge(Block switchBlock, DefaultCase dc)
|
||||
val <= switchCaseRangeEnd(sc))))
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if the function `f` never returns due to not containing a return
|
||||
* statement (explicit or compiler generated). This can be thought of as
|
||||
* a lightweight `potentiallyReturningFunction`- reachability of return
|
||||
* statements is not checked.
|
||||
*/
|
||||
private predicate nonReturningFunction(Function f)
|
||||
{
|
||||
exists(f.getBlock()) and
|
||||
not exists(ReturnStmt ret | ret.getEnclosingFunction() = f) and
|
||||
not getOptions().exits(f)
|
||||
}
|
||||
|
||||
/**
|
||||
* An edge from a call to a function that never returns is impossible.
|
||||
*/
|
||||
private predicate impossibleFunctionReturn(FunctionCall fc, Node succ) {
|
||||
nonReturningFunction(fc.getTarget()) and
|
||||
not fc.isVirtual() and
|
||||
successors_extended(fc, succ)
|
||||
}
|
||||
|
||||
/**
|
||||
* If `pred` is a function call with (at least) one function target,
|
||||
* (at least) one such target must be potentially returning.
|
||||
@@ -158,6 +180,7 @@ cached predicate successors_adapted(Node pred, Node succ) {
|
||||
and not impossibleTrueEdge(pred, succ)
|
||||
and not impossibleSwitchEdge(pred, succ)
|
||||
and not impossibleDefaultSwitchEdge(pred, succ)
|
||||
and not impossibleFunctionReturn(pred, succ)
|
||||
and not getOptions().exprExits(pred)
|
||||
}
|
||||
|
||||
|
||||
@@ -44,6 +44,14 @@ private cached module Cached {
|
||||
// that the node have at least one successor.
|
||||
or
|
||||
(not successors_extended(_, node) and successors_extended(node, _))
|
||||
|
||||
// An exception handler is always the start of a new basic block. We
|
||||
// don't generate edges for [possible] exceptions, but in practice control
|
||||
// flow could reach the handler from anywhere inside the try block that
|
||||
// could throw an exception of a corresponding type. A `Handler` usually
|
||||
// needs to be considered reachable (see also `BasicBlock.isReachable`).
|
||||
or
|
||||
node instanceof Handler
|
||||
}
|
||||
|
||||
/** Holds if `n2` follows `n1` in a `PrimitiveBasicBlock`. */
|
||||
|
||||
@@ -221,9 +221,7 @@ module FlowVar_internal {
|
||||
BlockVar() { this = TBlockVar(sbb, v) }
|
||||
|
||||
override VariableAccess getAnAccess() {
|
||||
result.getTarget() = v and
|
||||
result = getAReachedBlockVarSBB(this).getANode() and
|
||||
not overwrite(result, _)
|
||||
variableAccessInSBB(v, getAReachedBlockVarSBB(this), result)
|
||||
}
|
||||
|
||||
override predicate definedByInitialValue(LocalScopeVariable lsv) {
|
||||
@@ -373,6 +371,15 @@ module FlowVar_internal {
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `va` is a read access to `v` in `sbb`, where `v` is modeled by `BlockVar`. */
|
||||
pragma[noinline]
|
||||
private predicate variableAccessInSBB(Variable v, SubBasicBlock sbb, VariableAccess va) {
|
||||
exists(TBlockVar(_, v)) and
|
||||
va.getTarget() = v and
|
||||
va = sbb.getANode() and
|
||||
not overwrite(va, _)
|
||||
}
|
||||
|
||||
/**
|
||||
* A local variable that is uninitialized immediately after its declaration.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user