[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f6bb2a69215: [clang][Attribute] Fix noderef attribute
false-negatives (authored by leonardchan).
Changed prior to commit:
https://reviews.llvm.org/D77836?vs=267684=269951#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Frontend/noderef.cpp
Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -80,12 +80,28 @@
int member;
int NODEREF *member2;
int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+ int *member4;
+
+ int func() { return member; }
+ virtual int func_virt() { return member; }
+
+ A(NODEREF int *x) : member4(x) {} // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
};
+class Child : public A {};
+
void MemberPointer() {
int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
}
+int MethodCall(NODEREF A *a) { // expected-note{{a declared here}}
+ return a->func();// expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+int ChildCall(NODEREF Child *child) { // expected-note{{child declared here}}
+ return child->func(); // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
+}
+
template
class B {
Ty NODEREF *member;
@@ -100,3 +116,53 @@
int NODEREF *glob_ptr; // expected-note{{glob_ptr declared here}}
int glob_int = *glob_ptr; // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}
+
+void cast_from_void_ptr(NODEREF void *x) {
+ int *a = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+
+ // Allow regular C-style casts and C-style through reinterpret_casts to be holes
+ int *b = reinterpret_cast(x);
+ int *c = (int *)x;
+}
+
+void conversion_sequences() {
+ NODEREF int *x;
+ int *x2 = x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *x3 = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *x4 = reinterpret_cast(x);
+
+ // Functional cast - This is exactly equivalent to a C-style cast.
+ typedef int *INT_PTR;
+ int *x5 = INT_PTR(x);
+
+ NODEREF Child *child;
+ Child *child2 = dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *static_cast_from_same_ptr_type(NODEREF int *x) {
+ return static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_up(NODEREF Child *child) {
+ return dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+Child *dynamic_cast_down(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_side(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+void *dynamic_cast_to_void_ptr(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *const_cast_check(NODEREF const int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+const int *const_cast_check(NODEREF int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8200,9 +8200,13 @@
if (const auto *ToPtrType = Step->Type->getAs()) {
if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
+ S.Diag(CurInit.get()->getExprLoc(),
+ diag::warn_noderef_to_dereferenceable_pointer)
+ << CurInit.get()->getSourceRange();
+}
}
}
}
Index: clang/lib/Sema/SemaCast.cpp
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan added a comment. Lemme know if there's any more feedback. Will aim for committing this sometime at the end of the day. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77836/new/ https://reviews.llvm.org/D77836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan updated this revision to Diff 267684.
leonardchan added a comment.
Rebased
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Frontend/noderef.cpp
Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -80,12 +80,28 @@
int member;
int NODEREF *member2;
int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+ int *member4;
+
+ int func() { return member; }
+ virtual int func_virt() { return member; }
+
+ A(NODEREF int *x) : member4(x) {} // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
};
+class Child : public A {};
+
void MemberPointer() {
int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
}
+int MethodCall(NODEREF A *a) { // expected-note{{a declared here}}
+ return a->func();// expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+int ChildCall(NODEREF Child *child) { // expected-note{{child declared here}}
+ return child->func(); // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
+}
+
template
class B {
Ty NODEREF *member;
@@ -100,3 +116,53 @@
int NODEREF *glob_ptr; // expected-note{{glob_ptr declared here}}
int glob_int = *glob_ptr; // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}
+
+void cast_from_void_ptr(NODEREF void *x) {
+ int *a = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+
+ // Allow regular C-style casts and C-style through reinterpret_casts to be holes
+ int *b = reinterpret_cast(x);
+ int *c = (int *)x;
+}
+
+void conversion_sequences() {
+ NODEREF int *x;
+ int *x2 = x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *x3 = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *x4 = reinterpret_cast(x);
+
+ // Functional cast - This is exactly equivalent to a C-style cast.
+ typedef int *INT_PTR;
+ int *x5 = INT_PTR(x);
+
+ NODEREF Child *child;
+ Child *child2 = dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *static_cast_from_same_ptr_type(NODEREF int *x) {
+ return static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_up(NODEREF Child *child) {
+ return dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+Child *dynamic_cast_down(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_side(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+void *dynamic_cast_to_void_ptr(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *const_cast_check(NODEREF const int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+const int *const_cast_check(NODEREF int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8195,9 +8195,13 @@
if (const auto *ToPtrType = Step->Type->getAs()) {
if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast().
+if (!Kind.isStaticCast()) {
+ S.Diag(CurInit.get()->getExprLoc(),
+ diag::warn_noderef_to_dereferenceable_pointer)
+ << CurInit.get()->getSourceRange();
+}
}
}
}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -161,6 +161,30 @@
PlaceholderKind = (BuiltinType::Kind) 0;
}
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM, but please wait for a few days in case @rsmith has further concerns.
Comment at: clang/lib/Sema/SemaInit.cpp:8182
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
The comment should end with a period.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan added a comment. *ping* Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77836/new/ https://reviews.llvm.org/D77836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan updated this revision to Diff 261519.
leonardchan marked 6 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Frontend/noderef.cpp
Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -80,12 +80,28 @@
int member;
int NODEREF *member2;
int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+ int *member4;
+
+ int func() { return member; }
+ virtual int func_virt() { return member; }
+
+ A(NODEREF int *x) : member4(x) {} // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
};
+class Child : public A {};
+
void MemberPointer() {
int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
}
+int MethodCall(NODEREF A *a) { // expected-note{{a declared here}}
+ return a->func();// expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+int ChildCall(NODEREF Child *child) { // expected-note{{child declared here}}
+ return child->func(); // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
+}
+
template
class B {
Ty NODEREF *member;
@@ -100,3 +116,53 @@
int NODEREF *glob_ptr; // expected-note{{glob_ptr declared here}}
int glob_int = *glob_ptr; // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}
+
+void cast_from_void_ptr(NODEREF void *x) {
+ int *a = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+
+ // Allow regular C-style casts and C-style through reinterpret_casts to be holes
+ int *b = reinterpret_cast(x);
+ int *c = (int *)x;
+}
+
+void conversion_sequences() {
+ NODEREF int *x;
+ int *x2 = x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *x3 = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *x4 = reinterpret_cast(x);
+
+ // Functional cast - This is exactly equivalent to a C-style cast.
+ typedef int *INT_PTR;
+ int *x5 = INT_PTR(x);
+
+ NODEREF Child *child;
+ Child *child2 = dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *static_cast_from_same_ptr_type(NODEREF int *x) {
+ return static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_up(NODEREF Child *child) {
+ return dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+Child *dynamic_cast_down(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_side(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+void *dynamic_cast_to_void_ptr(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *const_cast_check(NODEREF const int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+const int *const_cast_check(NODEREF int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8179,9 +8179,13 @@
if (const auto *ToPtrType = Step->Type->getAs()) {
if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
+ S.Diag(CurInit.get()->getExprLoc(),
+ diag::warn_noderef_to_dereferenceable_pointer)
+ << CurInit.get()->getSourceRange();
+}
}
}
}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -159,6 +159,30 @@
PlaceholderKind = (BuiltinType::Kind) 0;
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:2557-2558
return;
+CheckNoDeref(Self, SrcExpr.get()->getType(), ResultType,
+ OpRange.getBegin());
}
rsmith wrote:
> Should we be checking this for a C-style cast? You previously said you were
> going to turn off the checks for C-style casts. Did you change your mind on
> that?
No, you're right. I accidentally left this in.
Comment at: clang/lib/Sema/SemaCast.cpp:2982
+ CheckNoDeref(*this, Op.SrcExpr.get()->getType(), Op.ResultType, LPLoc);
+
rsmith wrote:
> As above, should we be checking this for a C-style cast? (It also looks like
> we're doing this check twice in C++.)
Also accidentally left this in.
Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
rsmith wrote:
> leonardchan wrote:
> > leonardchan wrote:
> > > rsmith wrote:
> > > > Are there any `static_cast` cases that don't get here? I'm also a
> > > > little surprised that `static_cast` would get here but the
> > > > `static_cast` interpretation of a C-style or functional cast would not.
> > > I'll double check on this. The reason I added this here was because the
> > > cast in `static_cast_from_same_ptr_type()` triggered 2 of the
> > > `warn_noderef_to_dereferenceable_pointer` warnings and one of them was
> > > triggered here.
> > Yeah it seems the `int *a = static_cast(x);` case in
> > `cast_from_void_ptr()` is an example of a static_cast that doesn't go
> > through here.
> I see, this is only reached for the part of `static_cast` that considers
> implicit conversion sequences. OK.
>
> I expect there's similar overlap for all the other kinds of cast too. Should
> we be suppressing this check for *all* casts, given that we perform the check
> in `SemaCast` instead?
Hmm. I think for this particular location, `static_cast` contexts are all we
need to suppress it for to prevent duplicate warnings from showing.
We don't need to suppress for these since the attribute is allowed to pass
through these:
```
IC_CStyleCast
IC_FunctionalCast
```
We perform the check for implicit casts here since (AFAICT) only explicit casts
(C-style, functional, and C++ style) seem to be covered in `SemaCast`:
```
IC_Implicit
```
This would cover cases like `int *ptr = noderef_ptr;`.
I'm not sure about these ones, but none of the cases I have seem to fail if I
suppress only them:
```
IC_Normal
IC_ExplicitConvs
```
It could be though that there's a particular case I'm not covering in my tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
rsmith added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:734
void CastOperation::CheckDynamicCast() {
+ CheckNoDerefRAII noderef_check(*this);
+
Please use UpperCamelCase for local variables.
Comment at: clang/lib/Sema/SemaCast.cpp:2557-2558
return;
+CheckNoDeref(Self, SrcExpr.get()->getType(), ResultType,
+ OpRange.getBegin());
}
Should we be checking this for a C-style cast? You previously said you were
going to turn off the checks for C-style casts. Did you change your mind on
that?
Comment at: clang/lib/Sema/SemaCast.cpp:2982
+ CheckNoDeref(*this, Op.SrcExpr.get()->getType(), Op.ResultType, LPLoc);
+
As above, should we be checking this for a C-style cast? (It also looks like
we're doing this check twice in C++.)
Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
leonardchan wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > Are there any `static_cast` cases that don't get here? I'm also a little
> > > surprised that `static_cast` would get here but the `static_cast`
> > > interpretation of a C-style or functional cast would not.
> > I'll double check on this. The reason I added this here was because the
> > cast in `static_cast_from_same_ptr_type()` triggered 2 of the
> > `warn_noderef_to_dereferenceable_pointer` warnings and one of them was
> > triggered here.
> Yeah it seems the `int *a = static_cast(x);` case in
> `cast_from_void_ptr()` is an example of a static_cast that doesn't go through
> here.
I see, this is only reached for the part of `static_cast` that considers
implicit conversion sequences. OK.
I expect there's similar overlap for all the other kinds of cast too. Should we
be suppressing this check for *all* casts, given that we perform the check in
`SemaCast` instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan updated this revision to Diff 257419.
leonardchan added a comment.
Rebased
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Frontend/noderef.c
clang/test/Frontend/noderef.cpp
Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -80,12 +80,28 @@
int member;
int NODEREF *member2;
int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+ int *member4;
+
+ int func() { return member; }
+ virtual int func_virt() { return member; }
+
+ A(NODEREF int *x) : member4(x) {} // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
};
+class Child : public A {};
+
void MemberPointer() {
int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
}
+int MethodCall(NODEREF A *a) { // expected-note{{a declared here}}
+ return a->func();// expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+int ChildCall(NODEREF Child *child) { // expected-note{{child declared here}}
+ return child->func(); // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
+}
+
template
class B {
Ty NODEREF *member;
@@ -100,3 +116,37 @@
int NODEREF *glob_ptr; // expected-note{{glob_ptr declared here}}
int glob_int = *glob_ptr; // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}
+
+void cast_from_void_ptr(NODEREF void *x) {
+ int *a = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *b = reinterpret_cast(x); // Allow C-style casts through reinterpret_casts to be a hole
+ int *c = (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *static_cast_from_same_ptr_type(NODEREF int *x) {
+ return static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_up(NODEREF Child *child) {
+ return dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+Child *dynamic_cast_down(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_side(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+void *dynamic_cast_to_void_ptr(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *const_cast_check(NODEREF const int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+const int *const_cast_check(NODEREF int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -207,3 +207,7 @@
do {} while (*p); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
return *p; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
}
+
+int *explicit_cast(NODEREF int *x) {
+ return (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8179,9 +8179,13 @@
if (const auto *ToPtrType = Step->Type->getAs()) {
if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
+ S.Diag(CurInit.get()->getExprLoc(),
+ diag::warn_noderef_to_dereferenceable_pointer)
+ << CurInit.get()->getSourceRange();
+}
}
}
}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -159,6 +159,30 @@
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan updated this revision to Diff 257153.
leonardchan marked an inline comment as done.
leonardchan added a comment.
Updated to address comments and added an RAII class for easier checking before
exiting functions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Frontend/noderef.c
clang/test/Frontend/noderef.cpp
Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -80,12 +80,28 @@
int member;
int NODEREF *member2;
int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+ int *member4;
+
+ int func() { return member; }
+ virtual int func_virt() { return member; }
+
+ A(NODEREF int *x) : member4(x) {} // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
};
+class Child : public A {};
+
void MemberPointer() {
int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
}
+int MethodCall(NODEREF A *a) { // expected-note{{a declared here}}
+ return a->func();// expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+int ChildCall(NODEREF Child *child) { // expected-note{{child declared here}}
+ return child->func(); // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
+}
+
template
class B {
Ty NODEREF *member;
@@ -100,3 +116,37 @@
int NODEREF *glob_ptr; // expected-note{{glob_ptr declared here}}
int glob_int = *glob_ptr; // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}
+
+void cast_from_void_ptr(NODEREF void *x) {
+ int *a = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *b = reinterpret_cast(x); // Allow C-style casts through reinterpret_casts to be a hole
+ int *c = (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *static_cast_from_same_ptr_type(NODEREF int *x) {
+ return static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_up(NODEREF Child *child) {
+ return dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+Child *dynamic_cast_down(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_side(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+void *dynamic_cast_to_void_ptr(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *const_cast_check(NODEREF const int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+const int *const_cast_check(NODEREF int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -207,3 +207,7 @@
do {} while (*p); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
return *p; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
}
+
+int *explicit_cast(NODEREF int *x) {
+ return (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8179,9 +8179,13 @@
if (const auto *ToPtrType = Step->Type->getAs()) {
if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
+ S.Diag(CurInit.get()->getExprLoc(),
+ diag::warn_noderef_to_dereferenceable_pointer)
+ << CurInit.get()->getSourceRange();
+}
}
}
}
Index: clang/lib/Sema/SemaCast.cpp
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan added inline comments.
Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
leonardchan wrote:
> rsmith wrote:
> > Are there any `static_cast` cases that don't get here? I'm also a little
> > surprised that `static_cast` would get here but the `static_cast`
> > interpretation of a C-style or functional cast would not.
> I'll double check on this. The reason I added this here was because the cast
> in `static_cast_from_same_ptr_type()` triggered 2 of the
> `warn_noderef_to_dereferenceable_pointer` warnings and one of them was
> triggered here.
Yeah it seems the `int *a = static_cast(x);` case in
`cast_from_void_ptr()` is an example of a static_cast that doesn't go through
here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan marked 3 inline comments as done.
leonardchan added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:264
+ CheckNoderef(*this, E->getType(), TInfo->getType(), OpLoc);
+
rsmith wrote:
> Warning on this seems reasonable for `static_cast` and `dynamic_cast`, but
> should `reinterpret_cast` (or the `reinterpret_cast` interpretation of a
> C-style cast) warn? Presumably we want to leave open some type system holes
> for the case where the programmer really does know what they're doing.
My initial idea for this was that users could explicitly disable the warning
with `-Wno-noderef` or pragmas for line-level granularity, but I can see how
this could be less convenient/not consistent with other casts. Will update to
allow C-style + reinterpret_casts.
Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
rsmith wrote:
> Are there any `static_cast` cases that don't get here? I'm also a little
> surprised that `static_cast` would get here but the `static_cast`
> interpretation of a C-style or functional cast would not.
I'll double check on this. The reason I added this here was because the cast in
`static_cast_from_same_ptr_type()` triggered 2 of the
`warn_noderef_to_dereferenceable_pointer` warnings and one of them was
triggered here.
Comment at: clang/test/Frontend/noderef.c:211
+
+int *implicit_cast(NODEREF int *x) {
+ return (int *)x; // expected-warning{{casting to dereferenceable pointer
removes 'noderef' attribute}}
aaron.ballman wrote:
> The name of the function is a bit odd given that there's an explicit cast.
Woops. Will update the name.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
aaron.ballman added inline comments.
Comment at: clang/test/Frontend/noderef.c:211
+
+int *implicit_cast(NODEREF int *x) {
+ return (int *)x; // expected-warning{{casting to dereferenceable pointer
removes 'noderef' attribute}}
The name of the function is a bit odd given that there's an explicit cast.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
rsmith added inline comments.
Comment at: clang/lib/Sema/SemaCast.cpp:264
+ CheckNoderef(*this, E->getType(), TInfo->getType(), OpLoc);
+
Warning on this seems reasonable for `static_cast` and `dynamic_cast`, but
should `reinterpret_cast` (or the `reinterpret_cast` interpretation of a
C-style cast) warn? Presumably we want to leave open some type system holes for
the case where the programmer really does know what they're doing.
Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
Are there any `static_cast` cases that don't get here? I'm also a little
surprised that `static_cast` would get here but the `static_cast`
interpretation of a C-style or functional cast would not.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77836/new/
https://reviews.llvm.org/D77836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives
leonardchan created this revision.
leonardchan added reviewers: rsmith, aaron.ballman.
leonardchan added a project: clang.
`noderef` was failing to trigger warnings in some cases related to c++ style
casting. This patch addresses them.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77836
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Frontend/noderef.c
clang/test/Frontend/noderef.cpp
Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -80,12 +80,28 @@
int member;
int NODEREF *member2;
int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+ int *member4;
+
+ int func() { return member; }
+ virtual int func_virt() { return member; }
+
+ A(NODEREF int *x) : member4(x) {} // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
};
+class Child : public A {};
+
void MemberPointer() {
int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
}
+int MethodCall(NODEREF A *a) { // expected-note{{a declared here}}
+ return a->func();// expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+int ChildCall(NODEREF Child *child) { // expected-note{{child declared here}}
+ return child->func(); // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
+}
+
template
class B {
Ty NODEREF *member;
@@ -100,3 +116,37 @@
int NODEREF *glob_ptr; // expected-note{{glob_ptr declared here}}
int glob_int = *glob_ptr; // expected-warning{{dereferencing glob_ptr; was declared with a 'noderef' type}}
+
+void cast_from_void_ptr(NODEREF void *x) {
+ int *a = static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *b = reinterpret_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+ int *c = (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *static_cast_from_same_ptr_type(NODEREF int *x) {
+ return static_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_up(NODEREF Child *child) {
+ return dynamic_cast(child); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+Child *dynamic_cast_down(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+A *dynamic_cast_side(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+void *dynamic_cast_to_void_ptr(NODEREF A *a) {
+ return dynamic_cast(a); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+int *const_cast_check(NODEREF const int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
+
+const int *const_cast_check(NODEREF int *x) {
+ return const_cast(x); // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -207,3 +207,7 @@
do {} while (*p); // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
return *p; // expected-warning{{dereferencing p; was declared with a 'noderef' type}}
}
+
+int *implicit_cast(NODEREF int *x) {
+ return (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8179,9 +8179,13 @@
if (const auto *ToPtrType = Step->Type->getAs()) {
if (FromPtrType->getPointeeType()->hasAttr(attr::NoDeref) &&
!ToPtrType->getPointeeType()->hasAttr(attr::NoDeref)) {
-S.Diag(CurInit.get()->getExprLoc(),
- diag::warn_noderef_to_dereferenceable_pointer)
-<< CurInit.get()->getSourceRange();
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {
+ S.Diag(CurInit.get()->getExprLoc(),
+ diag::warn_noderef_to_dereferenceable_pointer)
+ << CurInit.get()->getSourceRange();
+}
}
}
}
Index: clang/lib/Sema/SemaCast.cpp
