From ff90ac764085929338b1e469e39f8fc64d68fb80 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 2 Oct 2025 16:19:43 +0100 Subject: [PATCH] C++: Fix queries I forgot after merging github/codeql#20485. --- .../DoNotCompareFunctionPointersToConstantValues.ql | 3 ++- .../rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql | 2 +- .../DetectAndHandleMemoryAllocationErrors.ql | 13 ++++++++++--- .../DetectAndHandleMemoryAllocationErrors.expected | 11 +++++------ .../FunctionErroneousReturnValueNotTested.qll | 4 +--- .../UnsignedOperationWithConstantOperandsWraps.qll | 2 +- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/c/cert/src/rules/EXP16-C/DoNotCompareFunctionPointersToConstantValues.ql b/c/cert/src/rules/EXP16-C/DoNotCompareFunctionPointersToConstantValues.ql index 812d4d910b..a74d88edbc 100644 --- a/c/cert/src/rules/EXP16-C/DoNotCompareFunctionPointersToConstantValues.ql +++ b/c/cert/src/rules/EXP16-C/DoNotCompareFunctionPointersToConstantValues.ql @@ -53,8 +53,9 @@ class ExplicitComparison extends EffectivelyComparison, FinalComparisonOperation class ImplicitComparison extends EffectivelyComparison, GuardCondition instanceof Expr { ImplicitComparison() { + this.valueControlsEdge(_, _, _) and this instanceof FunctionExpr and - not getParent() instanceof ComparisonOperation + not super.getParent() instanceof ComparisonOperation } override string getExplanation() { result = "$@ undergoes implicit constant comparison." } diff --git a/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql b/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql index a6d7abc456..a93796e150 100644 --- a/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql +++ b/cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql @@ -23,7 +23,7 @@ from InterestingOverflowingOperation e where not isExcluded(e, IntegerConversionPackage::integerExpressionLeadToDataLossQuery()) and // Not within a guard condition - not exists(GuardCondition gc | gc.getAChild*() = e) and + not e.getParent*().(GuardCondition).valueControlsEdge(_, _, _) and // Not guarded by a check, where the check is not an invalid overflow check not e.hasValidPreCheck() and // Covered by `IntMultToLong.ql` instead diff --git a/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql b/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql index ac9281ee9d..79ff7a08a2 100644 --- a/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql +++ b/cpp/cert/src/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.ql @@ -62,8 +62,13 @@ class NoThrowAllocExprWrapperFunction extends Function { NoThrowAllocExprWrapperFunction() { n.getEnclosingFunction() = this and DataFlow::localExprFlow(n, any(ReturnStmt rs).getExpr()) and - // Not checked in this wrapper function - not exists(GuardCondition gc | DataFlow::localExprFlow(n, gc.(Expr).getAChild*())) + // Not checked in this wrapper function. That is, the allocation is not a + // guard condition which guards something inside the function. + not exists(BasicBlock bb | + pragma[only_bind_out](bb.getEnclosingFunction()) = + pragma[only_bind_out](n.getEnclosingFunction()) and + n.(GuardCondition).valueControlsEdge(bb, _, _) + ) } /** Gets the underlying nothrow allocation ultimately being wrapped. */ @@ -84,7 +89,9 @@ module NoThrowNewErrorCheckConfig implements DataFlow::ConfigSig { source.asExpr() instanceof NotWrappedNoThrowAllocExpr } - predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(GuardCondition gc).getAChild*() } + predicate isSink(DataFlow::Node sink) { + sink.asExpr().(GuardCondition).valueControlsEdge(_, _, _) + } } module NoThrowNewErrorCheckFlow = DataFlow::Global; diff --git a/cpp/cert/test/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.expected b/cpp/cert/test/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.expected index 41fa58045f..45b75e6123 100644 --- a/cpp/cert/test/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.expected +++ b/cpp/cert/test/rules/MEM52-CPP/DetectAndHandleMemoryAllocationErrors.expected @@ -1,9 +1,8 @@ WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:64,5-13) -WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:66,36-44) -WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:82,46-54) -WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:83,22-30) -WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:87,20-28) -WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:90,35-43) -WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:95,38-46) +WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:86,46-54) +WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:87,22-30) +WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:91,20-28) +WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:96,35-43) +WARNING: module 'DataFlow' has been deprecated and may be removed in future (DetectAndHandleMemoryAllocationErrors.ql:101,38-46) | test.cpp:24:7:24:34 | new | nothrow new allocation of $@ returns here without a subsequent check to see whether the pointer is valid. | test.cpp:24:7:24:34 | new | StructA * | | test.cpp:40:17:40:38 | call to allocate_without_check | nothrow new allocation of $@ returns here without a subsequent check to see whether the pointer is valid. | test.cpp:35:17:35:44 | new | StructA * | diff --git a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll index 83907c609a..1b130dc187 100644 --- a/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll +++ b/cpp/common/src/codingstandards/cpp/rules/functionerroneousreturnvaluenottested/FunctionErroneousReturnValueNotTested.qll @@ -55,8 +55,6 @@ query predicate problems(FunctionCall fc, string message) { "vwprintf", "vfwprintf", "vswprintf", "vwprintf_s", "vfwprintf_s", "vswprintf_s", "vsnwprintf_s" ]) and - not exists(GuardCondition gc | - DataFlow::localFlow(DataFlow::exprNode(fc), DataFlow::exprNode(gc.(Expr).getAChild*())) - ) and + not fc.(GuardCondition).valueControlsEdge(_, _, _) and message = "Return value from " + fc.getTarget().getName() + " is not tested for errors." } diff --git a/cpp/common/src/codingstandards/cpp/rules/unsignedoperationwithconstantoperandswraps/UnsignedOperationWithConstantOperandsWraps.qll b/cpp/common/src/codingstandards/cpp/rules/unsignedoperationwithconstantoperandswraps/UnsignedOperationWithConstantOperandsWraps.qll index bc0c6d8fc1..98171b4e16 100644 --- a/cpp/common/src/codingstandards/cpp/rules/unsignedoperationwithconstantoperandswraps/UnsignedOperationWithConstantOperandsWraps.qll +++ b/cpp/common/src/codingstandards/cpp/rules/unsignedoperationwithconstantoperandswraps/UnsignedOperationWithConstantOperandsWraps.qll @@ -18,7 +18,7 @@ query predicate problems(InterestingOverflowingOperation op, string message) { not isExcluded(op, getQuery()) and op.getType().getUnderlyingType().(IntegralType).isUnsigned() and // Not within a guard condition - not exists(GuardCondition gc | gc.getAChild*() = op) and + not op.getParent*().(GuardCondition).valueControlsEdge(_, _, _) and // Not guarded by a check, where the check is not an invalid overflow check not op.hasValidPreCheck() and // Is not checked after the operation