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

[analyzer] Simplify CallEvent castArgToParamTypeIfNeeded #120981

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

steakhal
Copy link
Contributor

I noticed recently that this code (that I wrote xD) uses the getRuntimeDefinition() which isn't quite necessary for the simple task this function was designed for.

Why would it be better not using this API here?
I'm experimenting with improving how virtual functions are inlined, where depending on our ability of deducing the dynamic type of the object we may end up with inaccurate type information. Such inaccuracy would mean that we may have multiple runtime definitions. After that, this code would become ambiguous.

To resolve this, I decided to refactor this and use a simpler - but equivalent approach.

I noticed recently that this code (that I wrote xD) uses the
`getRuntimeDefinition()` which isn't quite necessary for the simple task
this function was designed for.

Why would it be better not using this API here?
I'm experimenting with improving how virtual functions are inlined,
where depending on our ability of deducing the dynamic type of the
object we may end up with inaccurate type information.
Such inaccuracy would mean that we may have multiple runtime
definitions. After that, this code would become ambiguous.

To resolve this, I decided to refactor this and use a simpler - but
equivalent approach.
@steakhal steakhal added clang Clang issues not falling into any other category clang:static analyzer labels Dec 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 23, 2024

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Balazs Benics (steakhal)

Changes

I noticed recently that this code (that I wrote xD) uses the getRuntimeDefinition() which isn't quite necessary for the simple task this function was designed for.

Why would it be better not using this API here?
I'm experimenting with improving how virtual functions are inlined, where depending on our ability of deducing the dynamic type of the object we may end up with inaccurate type information. Such inaccuracy would mean that we may have multiple runtime definitions. After that, this code would become ambiguous.

To resolve this, I decided to refactor this and use a simpler - but equivalent approach.


Full diff: https://github.com/llvm/llvm-project/pull/120981.diff

1 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Core/CallEvent.cpp (+7-7)
diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 0fdef7487b9814..bb4a39f68280cd 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -435,27 +435,27 @@ static SVal processArgument(SVal Value, const Expr *ArgumentExpr,
 /// runtime definition don't match in terms of argument and parameter count.
 static SVal castArgToParamTypeIfNeeded(const CallEvent &Call, unsigned ArgIdx,
                                        SVal ArgVal, SValBuilder &SVB) {
-  const FunctionDecl *RTDecl =
-      Call.getRuntimeDefinition().getDecl()->getAsFunction();
   const auto *CallExprDecl = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
-
-  if (!RTDecl || !CallExprDecl)
+  if (!CallExprDecl)
     return ArgVal;
 
+  const FunctionDecl *Definition = CallExprDecl;
+  Definition->hasBody(Definition);
+
   // The function decl of the Call (in the AST) will not have any parameter
   // declarations, if it was 'only' declared without a prototype. However, the
   // engine will find the appropriate runtime definition - basically a
   // redeclaration, which has a function body (and a function prototype).
-  if (CallExprDecl->hasPrototype() || !RTDecl->hasPrototype())
+  if (CallExprDecl->hasPrototype() || !Definition->hasPrototype())
     return ArgVal;
 
   // Only do this cast if the number arguments at the callsite matches with
   // the parameters at the runtime definition.
-  if (Call.getNumArgs() != RTDecl->getNumParams())
+  if (Call.getNumArgs() != Definition->getNumParams())
     return UnknownVal();
 
   const Expr *ArgExpr = Call.getArgExpr(ArgIdx);
-  const ParmVarDecl *Param = RTDecl->getParamDecl(ArgIdx);
+  const ParmVarDecl *Param = Definition->getParamDecl(ArgIdx);
   return SVB.evalCast(ArgVal, Param->getType(), ArgExpr->getType());
 }
 

@steakhal steakhal merged commit 8dbb337 into llvm:main Dec 24, 2024
11 checks passed
@steakhal steakhal deleted the bb/simplify-castArgToParamTypeIfNeeded branch December 24, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants