diff --git a/c/cert/src/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.ql b/c/cert/src/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.ql index 4ae6619227..7974c4d601 100644 --- a/c/cert/src/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.ql +++ b/c/cert/src/rules/PRE31-C/SideEffectsInArgumentsToUnsafeMacros.ql @@ -15,7 +15,6 @@ import cpp import codingstandards.c.cert import codingstandards.cpp.Macro import codingstandards.cpp.SideEffect -import codingstandards.cpp.StructuralEquivalence import codingstandards.cpp.sideeffect.DefaultEffects import codingstandards.cpp.sideeffect.Customizations import semmle.code.cpp.valuenumbering.HashCons diff --git a/change_notes/2025-02-10-improve-perf-a5-1-9.md b/change_notes/2025-02-10-improve-perf-a5-1-9.md new file mode 100644 index 0000000000..5d355bab49 --- /dev/null +++ b/change_notes/2025-02-10-improve-perf-a5-1-9.md @@ -0,0 +1,3 @@ + - `A5-1-9` - `IdenticalLambdaExpressions.ql`: + - Performance has been improved. + - False positives due to repeated invocation of macros containing lambdas have been excluded. \ No newline at end of file diff --git a/cpp/autosar/src/rules/A5-1-9/IdenticalLambdaExpressions.ql b/cpp/autosar/src/rules/A5-1-9/IdenticalLambdaExpressions.ql index 8717fd000e..1520955716 100644 --- a/cpp/autosar/src/rules/A5-1-9/IdenticalLambdaExpressions.ql +++ b/cpp/autosar/src/rules/A5-1-9/IdenticalLambdaExpressions.ql @@ -24,6 +24,14 @@ where not lambdaExpression = otherLambdaExpression and not lambdaExpression.isFromTemplateInstantiation(_) and not otherLambdaExpression.isFromTemplateInstantiation(_) and - getLambdaHashCons(lambdaExpression) = getLambdaHashCons(otherLambdaExpression) + getLambdaHashCons(lambdaExpression) = getLambdaHashCons(otherLambdaExpression) and + // Do not report lambdas produced by the same macro in different invocations + not exists(Macro m, MacroInvocation m1, MacroInvocation m2 | + m1 = m.getAnInvocation() and + m2 = m.getAnInvocation() and + not m1 = m2 and // Lambdas in the same macro can be reported + m1.getAnExpandedElement() = lambdaExpression and + m2.getAnExpandedElement() = otherLambdaExpression + ) select lambdaExpression, "Lambda expression is identical to $@ lambda expression.", otherLambdaExpression, "this" diff --git a/cpp/autosar/src/rules/A5-1-9/LambdaEquivalence.qll b/cpp/autosar/src/rules/A5-1-9/LambdaEquivalence.qll index 040777e321..cab93608c5 100644 --- a/cpp/autosar/src/rules/A5-1-9/LambdaEquivalence.qll +++ b/cpp/autosar/src/rules/A5-1-9/LambdaEquivalence.qll @@ -211,7 +211,7 @@ private module HashCons { private newtype HC_Params = HC_NoParams() or - HC_ParamCons(HashConsExpr hc, int i, HC_Params list) { mk_ParamCons(hc, i, list, _) } + HC_ParamCons(Type t, string name, int i, HC_Params list) { mk_ParamCons(t, name, i, list, _) } /** * HashConsExpr is the hash-cons of an expression. The relationship between `Expr` @@ -624,11 +624,21 @@ private module HashCons { strictcount(access.getTarget()) = 1 } + /** + * Gets the name of a variable. + * + * Extracted for performance reasons, to avoid magic, which was causing performance issues in getParameter(int i). + */ + pragma[nomagic] + private string getVariableName(Variable v) { result = v.getName() } + /* Note: This changed from the original HashCons module to be able to find structural equivalent expression. */ private predicate mk_Variable(Type t, string name, VariableAccess access) { analyzableVariable(access) and exists(Variable v | - v = access.getTarget() and t = v.getUnspecifiedType() and name = v.getName() + v = access.getTarget() and + t = v.getUnspecifiedType() and + name = getVariableName(v) ) } @@ -1104,7 +1114,14 @@ private module HashCons { nee.getExpr().getFullyConverted() = child.getAnExpr() } - private predicate mk_StmtCons(HashConsStmt hc, int i, HC_Stmts list, BlockStmt block) { + private class LambdaBlockStmt extends BlockStmt { + LambdaBlockStmt() { + // Restricting to statements inside a lambda expressions. + this.getParentScope*() = any(LambdaExpression le).getLambdaFunction() + } + } + + private predicate mk_StmtCons(HashConsStmt hc, int i, HC_Stmts list, LambdaBlockStmt block) { hc = hashConsStmt(block.getStmt(i)) and ( exists(HashConsStmt head, HC_Stmts tail | @@ -1118,13 +1135,13 @@ private module HashCons { } private predicate mk_StmtConsInner( - HashConsStmt head, HC_Stmts tail, int i, HC_Stmts list, BlockStmt block + HashConsStmt head, HC_Stmts tail, int i, HC_Stmts list, LambdaBlockStmt block ) { list = HC_StmtCons(head, i, tail) and mk_StmtCons(head, i, tail, block) } - private predicate mk_BlockStmtCons(HC_Stmts hc, BlockStmt s) { + private predicate mk_BlockStmtCons(HC_Stmts hc, LambdaBlockStmt s) { if s.getNumStmt() > 0 then exists(HashConsStmt head, HC_Stmts tail | @@ -1275,13 +1292,14 @@ private module HashCons { mk_DeclConsInner(_, _, s.getNumDeclarations() - 1, hc, s) } - private predicate mk_ParamCons(HashConsExpr hc, int i, HC_Params list, Function f) { - hc = hashConsExpr(f.getParameter(i).getAnAccess()) and - ( - exists(HashConsExpr head, HC_Params tail | - mk_ParamConsInner(head, tail, i - 1, list, f) and - i > 0 - ) + private predicate mk_ParamCons(Type t, string name, int i, HC_Params list, Function f) { + exists(Parameter p | + p = f.getParameter(i) and + t = p.getType() and + name = p.getName() + | + mk_ParamConsInner(_, _, _, i - 1, list, f) and + i > 0 or i = 0 and list = HC_NoParams() @@ -1289,10 +1307,10 @@ private module HashCons { } private predicate mk_ParamConsInner( - HashConsExpr head, HC_Params tail, int i, HC_Params list, Function f + Type t, string name, HC_Params tail, int i, HC_Params list, Function f ) { - list = HC_ParamCons(head, i, tail) and - mk_ParamCons(head, i, tail, f) + list = HC_ParamCons(t, name, i, tail) and + mk_ParamCons(t, name, i, tail, f) } private predicate mk_FunctionCons( @@ -1302,7 +1320,7 @@ private module HashCons { name = f.getName() and body = hashConsStmt(f.getBlock()) and if f.getNumberOfParameters() > 0 - then mk_ParamConsInner(_, _, f.getNumberOfParameters() - 1, params, f) + then mk_ParamConsInner(_, _, _, f.getNumberOfParameters() - 1, params, f) else params = HC_NoParams() } @@ -1486,8 +1504,6 @@ private module HashCons { cached HashConsStmt hashConsStmt(Stmt s) { - // Restricting to statements inside a lambda expressions. - s.getParentScope*() = any(LambdaExpression le).getLambdaFunction() and exists(HC_Stmts list | mk_BlockStmtCons(list, s) and result = HC_BlockStmt(list) diff --git a/cpp/autosar/test/rules/A5-1-9/test.cpp b/cpp/autosar/test/rules/A5-1-9/test.cpp index 466cf14dfa..511be302a0 100644 --- a/cpp/autosar/test/rules/A5-1-9/test.cpp +++ b/cpp/autosar/test/rules/A5-1-9/test.cpp @@ -104,4 +104,10 @@ class Test_issue468 { LogError("Error"); LogFatal("Fatal"); } -}; \ No newline at end of file +}; + +#define MACRO() [](int i) -> int { return i + 3; } +void test_macros() { + MACRO(); // COMPLIANT + MACRO(); // COMPLIANT - no duplication +} \ No newline at end of file