Skip to content

[WebKit checkers] Add an annotation for pointer conversion. #141277

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 2 commits into from
Jun 9, 2025

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented May 23, 2025

This PR adds the WebKit checker support for [[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the function is treated as safe to call anywhere and the return value's pointer origin is the argument.`

This PR adds the WebKit checker support for [[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the function is treated as safe to
call anywhere and the return value's pointer origin is the argument.`
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels May 23, 2025
@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

This PR adds the WebKit checker support for [[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the function is treated as safe to call anywhere and the return value's pointer origin is the argument.`


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

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+12)
  • (modified) clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp (+9-1)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 4ddd11495f534..cd33476344a34 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -468,6 +468,18 @@ bool isPtrConversion(const FunctionDecl *F) {
       FunctionName == "checked_objc_cast")
     return true;
 
+  auto ReturnType = F->getReturnType();
+  if (auto *Type = ReturnType.getTypePtrOrNull()) {
+    if (auto *AttrType = dyn_cast<AttributedType>(Type)) {
+      if (auto *Attr = AttrType->getAttr()) {
+        if (auto *AnnotateType = dyn_cast<AnnotateTypeAttr>(Attr)) {
+          if (AnnotateType->getAnnotation() == "webkit.pointerconversion")
+            return true;
+        }
+      }
+    }
+  }
+
   return false;
 }
 
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
index a87446564870c..9f6dbade3c746 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
-// expected-no-diagnostics
 
 class Base {
 public:
@@ -44,6 +43,12 @@ inline Target* uncheckedDowncast(Source* source)
     return static_cast<Target*>(source);
 }
 
+template<typename Target, typename Source>
+Target* [[clang::annotate_type("webkit.pointerconversion")]] newCastFunction(Source*);
+
+template<typename Target, typename Source>
+Target* [[clang::annotate_type("unrelated-annotation")]] badCastFunction(Source*);
+
 template<typename... Types>
 String toString(const Types&... values);
 
@@ -52,5 +57,8 @@ void foo(OtherObject* other)
     dynamicDowncast<SubDerived>(other->obj());
     checkedDowncast<SubDerived>(other->obj());
     uncheckedDowncast<SubDerived>(other->obj());
+    newCastFunction<SubDerived>(other->obj());
+    badCastFunction<SubDerived>(other->obj());
+    // expected-warning@-1{{Call argument is uncounted and unsafe}}
     toString(other->obj());
 }

@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

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

Author: Ryosuke Niwa (rniwa)

Changes

This PR adds the WebKit checker support for [[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the function is treated as safe to call anywhere and the return value's pointer origin is the argument.`


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

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+12)
  • (modified) clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp (+9-1)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 4ddd11495f534..cd33476344a34 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -468,6 +468,18 @@ bool isPtrConversion(const FunctionDecl *F) {
       FunctionName == "checked_objc_cast")
     return true;
 
+  auto ReturnType = F->getReturnType();
+  if (auto *Type = ReturnType.getTypePtrOrNull()) {
+    if (auto *AttrType = dyn_cast<AttributedType>(Type)) {
+      if (auto *Attr = AttrType->getAttr()) {
+        if (auto *AnnotateType = dyn_cast<AnnotateTypeAttr>(Attr)) {
+          if (AnnotateType->getAnnotation() == "webkit.pointerconversion")
+            return true;
+        }
+      }
+    }
+  }
+
   return false;
 }
 
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
index a87446564870c..9f6dbade3c746 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
-// expected-no-diagnostics
 
 class Base {
 public:
@@ -44,6 +43,12 @@ inline Target* uncheckedDowncast(Source* source)
     return static_cast<Target*>(source);
 }
 
+template<typename Target, typename Source>
+Target* [[clang::annotate_type("webkit.pointerconversion")]] newCastFunction(Source*);
+
+template<typename Target, typename Source>
+Target* [[clang::annotate_type("unrelated-annotation")]] badCastFunction(Source*);
+
 template<typename... Types>
 String toString(const Types&... values);
 
@@ -52,5 +57,8 @@ void foo(OtherObject* other)
     dynamicDowncast<SubDerived>(other->obj());
     checkedDowncast<SubDerived>(other->obj());
     uncheckedDowncast<SubDerived>(other->obj());
+    newCastFunction<SubDerived>(other->obj());
+    badCastFunction<SubDerived>(other->obj());
+    // expected-warning@-1{{Call argument is uncounted and unsafe}}
     toString(other->obj());
 }

@rniwa rniwa requested a review from t-rasmud May 24, 2025 00:19
@@ -44,6 +43,12 @@ inline Target* uncheckedDowncast(Source* source)
return static_cast<Target*>(source);
}

template<typename Target, typename Source>
Target* [[clang::annotate_type("webkit.pointerconversion")]] newCastFunction(Source*);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this function is a member function, will you accidentally take the implicit this object as the original argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. I added a test case for that.

Copy link
Contributor

@ziqingluo-90 ziqingluo-90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rniwa
Copy link
Contributor Author

rniwa commented Jun 9, 2025

Thanks for the review!

@rniwa rniwa merged commit de96199 into llvm:main Jun 9, 2025
7 checks passed
@rniwa rniwa deleted the annotation-type-webkit-pointerconversion branch June 9, 2025 14:33
rniwa added a commit to rniwa/llvm-project that referenced this pull request Jun 9, 2025
)

This PR adds the WebKit checker support for
[[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the
function is treated as safe to call anywhere and the return value's
pointer origin is the argument.`
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
)

This PR adds the WebKit checker support for
[[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the
function is treated as safe to call anywhere and the return value's
pointer origin is the argument.`
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
)

This PR adds the WebKit checker support for
[[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the
function is treated as safe to call anywhere and the return value's
pointer origin is the argument.`
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
)

This PR adds the WebKit checker support for
[[clang::annotate_type("webkit.pointerconversion")]].

When this attribute is set on the return value of a function, the
function is treated as safe to call anywhere and the return value's
pointer origin is the argument.`
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