[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh created this revision.
thejh added reviewers: leonardchan, rsmith, aaron.ballman, mcgrathr.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
thejh requested review of this revision.

The noderef attribute is for catching code that accesses pointers in
a different address space. Unevaluated code is always safe in that regard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91828

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Frontend/noderef.c


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -63,6 +63,12 @@
   p = &s->a;
   p = &(*s).b;
 
+  // Nothing in sizeof() actually accesses memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -63,6 +63,12 @@
   p = &s->a;
   p = &(*s).b;
 
+  // Nothing in sizeof() actually accesses memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh updated this revision to Diff 306555.
thejh added a comment.

re-uploading to trigger a new build, since the build error looks unrelated.
maybe current trunk is flaky?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91828/new/

https://reviews.llvm.org/D91828

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Frontend/noderef.c


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -63,6 +63,12 @@
   p = &s->a;
   p = &(*s).b;
 
+  // Nothing in sizeof() actually accesses memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -63,6 +63,12 @@
   p = &s->a;
   p = &(*s).b;
 
+  // Nothing in sizeof() actually accesses memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh updated this revision to Diff 306792.
thejh marked an inline comment as done.
thejh added a comment.

As requested by @aaron.ballman, added tests for edgecases where
sizeof()/typeid() can cause memory access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91828/new/

https://reviews.llvm.org/D91828

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.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
@@ -6,6 +6,11 @@
 
 #define NODEREF __attribute__((noderef))
 
+// Stub out types for 'typeid' to work.
+namespace std {
+class type_info {};
+} // namespace std
+
 void Normal() {
   int NODEREF i;// expected-warning{{'noderef' can only be used on an 
array or pointer type}}
   int NODEREF *i_ptr;   // expected-note 2 {{i_ptr declared here}}
@@ -102,6 +107,18 @@
   return child->func();   // expected-warning{{dereferencing 
child; was declared with a 'noderef' type}}
 }
 
+std::type_info TypeIdPolymorphic(NODEREF A *a) { // expected-note{{a declared 
here}}
+  return typeid(*a); // 
expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+class SimpleClass {
+  int a;
+};
+
+std::type_info TypeIdNonPolymorphic(NODEREF SimpleClass *simple) {
+  return typeid(*simple);
+}
+
 template 
 class B {
   Ty NODEREF *member;
Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -57,12 +57,19 @@
   p = &*(p + 1);
 
   // Struct member access
-  struct S NODEREF *s;  // expected-note 2 {{s declared here}}
+  struct S NODEREF *s; // expected-note 3 {{s declared here}}
   x = s->a;   // expected-warning{{dereferencing s; was declared with a 
'noderef' type}}
   x = (*s).b; // expected-warning{{dereferencing s; was declared with a 
'noderef' type}}
   p = &s->a;
   p = &(*s).b;
 
+  // Most things in sizeof() can't actually access memory
+  x = sizeof(s->a);  // ok
+  x = sizeof(*s);// ok
+  x = sizeof(s[0]);  // ok
+  x = sizeof(s->a + (s->b)); // ok
+  x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was 
declared with a 'noderef' type}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1734,6 +1734,9 @@
 }
 
 void Sema::CheckMemberAccessOfNoDeref(const MemberExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
 
   // Do not warn on member accesses to arrays since this returns an array
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4751,6 +4751,9 @@
 }
 
 void Sema::CheckSubscriptAccessOfNoDeref(const ArraySubscriptExpr *E) {
+  if (isUnevaluatedContext())
+return;
+
   QualType ResultTy = E->getType();
   ExpressionEvaluationContextRecord &LastRecord = ExprEvalContexts.back();
 
@@ -14666,7 +14669,8 @@
 OpLoc, CanOverflow, CurFPFeatureOverrides());
 
   if (Opc == UO_Deref && UO->getType()->hasAttr(attr::NoDeref) &&
-  !isa(UO->getType().getDesugaredType(Context)))
+  !isa(UO->getType().getDesugaredType(Context)) &&
+  !isUnevaluatedContext())
 ExprEvalContexts.back().PossibleDerefs.insert(UO);
 
   // Convert the result back to a half vector.


Index: clang/test/Frontend/noderef.cpp
===
--- clang/test/Frontend/noderef.cpp
+++ clang/test/Frontend/noderef.cpp
@@ -6,6 +6,11 @@
 
 #define NODEREF __attribute__((noderef))
 
+// Stub out types for 'typeid' to work.
+namespace std {
+class type_info {};
+} // namespace std
+
 void Normal() {
   int NODEREF i;// expected-warning{{'noderef' can only be used on an array or pointer type}}
   int NODEREF *i_ptr;   // expected-note 2 {{i_ptr declared here}}
@@ -102,6 +107,18 @@
   return child->func();   // expected-warning{{dereferencing child; was declared with a 'noderef' type}}
 }
 
+std::type_info TypeIdPolymorphic(NODEREF A *a) { // expected-note{{a declared here}}
+  return typeid(*a); // expected-warning{{dereferencing a; was declared with a 'noderef' type}}
+}
+
+class SimpleClass {
+  int a;
+};
+
+std::type_info TypeIdNonPolymorphic(NODEREF SimpleClass *simple) {
+  return typeid(*simple);
+}
+
 template 
 class B {
   Ty NODEREF *member;
In

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh added inline comments.



Comment at: clang/test/Frontend/noderef.c:71
+  x = sizeof(s->a + (s->b)); // ok
+
   // Nested struct access

aaron.ballman wrote:
> Can you add tests for the weird situations where the expression actually is 
> evaluated? e.g.,
> ```
> struct S {
>   virtual ~S(); // Make S polymorphic
> };
> 
> S NODEREF *s;
> typeid(*s); // Actually evaluates *s at runtime
> 
> struct T {
>   unsigned i;
> };
> 
> T t1;
> T NODEREF *t2 = &t1;
> 
> sizeof(int[++t2->i]); // Actually evaluates t2->i at runtime
> ```
Oh jeez, good point. I'm extremely happy that other people have done the hard 
work of building the `ExpressionEvaluationContext` logic... I've added tests 
for the cases you described.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91828/new/

https://reviews.llvm.org/D91828

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

@aaron.ballman Can you land it for me? I don't have commit access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91828/new/

https://reviews.llvm.org/D91828

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

In D91828#2411207 , @aaron.ballman 
wrote:

> In D91828#2411203 , @thejh wrote:
>
>> @aaron.ballman Can you land it for me? I don't have commit access.
>
> Happy to do so -- are you okay with "Jann Horn " for author 
> attribution?

Yes, thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91828/new/

https://reviews.llvm.org/D91828

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92140: Fix noderef for array member of deref expr

[Jann Horn via Phabricator via cfe-commits]

thejh created this revision.
thejh added reviewers: leonardchan, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
thejh requested review of this revision.

Given an __attribute__((noderef)) pointer "p" to the struct

  struct s { int a[2]; };

ensure that the following expressions are treated the same way by the
noderef logic:

  p->a
  (*p).a

Until now, the first expression would be treated correctly (nothing is
added to PossibleDerefs because CheckMemberAccessOfNoDeref() bails out
on array members), but the second expression would incorrectly warn
because "*p" creates a PossibleDerefs entry.

Handle this case the same way as for the AddrOf operator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92140

Files:
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Frontend/noderef.c


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -73,6 +73,7 @@
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
+  p = (*s2_noderef).a; // ok since result is an array in a struct
   p = s2_noderef->a2; // ok
   p = s2_noderef->b;  // expected-warning{{dereferencing s2_noderef; was 
declared with a 'noderef' type}}
   p = s2_noderef->b2; // expected-warning{{dereferencing s2_noderef; was 
declared with a 'noderef' type}}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1739,12 +1739,23 @@
 
   QualType ResultTy = E->getType();
 
-  // Do not warn on member accesses to arrays since this returns an array
-  // lvalue and does not actually dereference memory.
-  if (isa(ResultTy))
-return;
-
-  if (E->isArrow()) {
+  // Member accesses have four cases:
+  // 1: non-array member via "->": dereferences
+  // 2: non-array member via ".": nothing interesting happens
+  // 3: array member access via "->": nothing interesting happens
+  //(this returns an array lvalue and does not actually dereference memory)
+  // 4: array member access via ".": *adds* a layer of indirection
+  if (ResultTy->isArrayType()) {
+if (!E->isArrow()) {
+  // This might be something like:
+  // (*structPtr).arrayMember
+  // which behaves roughly like:
+  // &(*structPtr).pointerMember
+  // in that the apparent dereference in the base expression does not
+  // actually happen.
+  CheckAddressOfNoDeref(E->getBase());
+}
+  } else if (E->isArrow()) {
 if (const auto *Ptr = dyn_cast(
 E->getBase()->getType().getDesugaredType(Context))) {
   if (Ptr->getPointeeType()->hasAttr(attr::NoDeref))


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -73,6 +73,7 @@
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
+  p = (*s2_noderef).a; // ok since result is an array in a struct
   p = s2_noderef->a2; // ok
   p = s2_noderef->b;  // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
   p = s2_noderef->b2; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1739,12 +1739,23 @@
 
   QualType ResultTy = E->getType();
 
-  // Do not warn on member accesses to arrays since this returns an array
-  // lvalue and does not actually dereference memory.
-  if (isa(ResultTy))
-return;
-
-  if (E->isArrow()) {
+  // Member accesses have four cases:
+  // 1: non-array member via "->": dereferences
+  // 2: non-array member via ".": nothing interesting happens
+  // 3: array member access via "->": nothing interesting happens
+  //(this returns an array lvalue and does not actually dereference memory)
+  // 4: array member access via ".": *adds* a layer of indirection
+  if (ResultTy->isArrayType()) {
+if (!E->isArrow()) {
+  // This might be something like:
+  // (*structPtr).arrayMember
+  // which behaves roughly like:
+  // &(*structPtr).pointerMember
+  // in that the apparent dereference in the base expression does not
+  // actually happen.
+  CheckAddressOfNoDeref(E->getBase());
+}
+  } else if (E->isArrow()) {
 if (const auto *Ptr = dyn_cast(
 E->getBase()->getType().getDesugaredType(Context))) {
   if (Ptr->getPointeeType()->hasAttr(attr::NoDeref))
_

[PATCH] D92141: Fix noderef for AddrOf on MemberExpr

[Jann Horn via Phabricator via cfe-commits]

thejh created this revision.
thejh added reviewers: leonardchan, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
thejh requested review of this revision.

As part of this change, one existing test case has to be adjusted
because it accidentally stripped the NoDeref attribute without
getting caught.

Depends on D92140 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92141

Files:
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Frontend/noderef.c


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -70,6 +70,14 @@
   x = sizeof(s->a + (s->b)); // ok
   x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was 
declared with a 'noderef' type}}
 
+  // Struct member access should carry NoDeref type information through to an
+  // enclosing AddrOf.
+  p = &s->a;// ok
+  p = &(*s).a;  // ok
+  p2 = &s->a;   // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  x = *&s->a;   // expected-warning{{dereferencing expression marked as 
'noderef'}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
@@ -113,7 +121,7 @@
 
   p = s2_arr[1]->a;
   p = s2_arr[1]->b; // expected-warning{{dereferencing expression marked as 
'noderef'}}
-  int **bptr = &s2_arr[1]->b;
+  int *NODEREF *bptr = &s2_arr[1]->b;
 
   x = s2->s2->a;// expected-warning{{dereferencing expression marked 
as 'noderef'}}
   x = s2_noderef->a[1]; // expected-warning{{dereferencing s2_noderef; was 
declared with a 'noderef' type}}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1810,6 +1810,14 @@
 Qualifiers Combined = BaseQuals + MemberQuals;
 if (Combined != MemberQuals)
   MemberType = Context.getQualifiedType(MemberType, Combined);
+
+// Pick up NoDeref from the base in case we end up using AddrOf on the
+// result. E.g. the expression
+// &someNoDerefPtr->pointerMember
+// should be a noderef pointer again.
+if (BaseType->hasAttr(attr::NoDeref))
+  MemberType =
+  Context.getAttributedType(attr::NoDeref, MemberType, MemberType);
   }
 
   auto *CurMethod = dyn_cast(CurContext);


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -70,6 +70,14 @@
   x = sizeof(s->a + (s->b)); // ok
   x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was declared with a 'noderef' type}}
 
+  // Struct member access should carry NoDeref type information through to an
+  // enclosing AddrOf.
+  p = &s->a;// ok
+  p = &(*s).a;  // ok
+  p2 = &s->a;   // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  x = *&s->a;   // expected-warning{{dereferencing expression marked as 'noderef'}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
@@ -113,7 +121,7 @@
 
   p = s2_arr[1]->a;
   p = s2_arr[1]->b; // expected-warning{{dereferencing expression marked as 'noderef'}}
-  int **bptr = &s2_arr[1]->b;
+  int *NODEREF *bptr = &s2_arr[1]->b;
 
   x = s2->s2->a;// expected-warning{{dereferencing expression marked as 'noderef'}}
   x = s2_noderef->a[1]; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1810,6 +1810,14 @@
 Qualifiers Combined = BaseQuals + MemberQuals;
 if (Combined != MemberQuals)
   MemberType = Context.getQualifiedType(MemberType, Combined);
+
+// Pick up NoDeref from the base in case we end up using AddrOf on the
+// result. E.g. the expression
+// &someNoDerefPtr->pointerMember
+// should be a noderef pointer again.
+if (BaseType->hasAttr(attr::NoDeref))
+  MemberType =
+  Context.getAttributedType(attr::NoDeref, MemberType, MemberType);
   }
 
   auto *CurMethod = dyn_cast(CurContext);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92141: Fix noderef for AddrOf on MemberExpr

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

(I marked this as depending on D92140  because 
if you apply this patch to the current HEAD directly, the tests will break.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92141/new/

https://reviews.llvm.org/D92141

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92141: Fix noderef for AddrOf on MemberExpr

[Jann Horn via Phabricator via cfe-commits]

thejh updated this revision to Diff 307728.
thejh added a comment.

Removed duplicate tests as suggested by leonardchan


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92141/new/

https://reviews.llvm.org/D92141

Files:
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Frontend/noderef.c


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -70,6 +70,12 @@
   x = sizeof(s->a + (s->b)); // ok
   x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was 
declared with a 'noderef' type}}
 
+  // Struct member access should carry NoDeref type information through to an
+  // enclosing AddrOf.
+  p2 = &s->a;   // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  x = *&s->a;   // expected-warning{{dereferencing expression marked as 
'noderef'}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared 
here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
@@ -113,7 +119,7 @@
 
   p = s2_arr[1]->a;
   p = s2_arr[1]->b; // expected-warning{{dereferencing expression marked as 
'noderef'}}
-  int **bptr = &s2_arr[1]->b;
+  int *NODEREF *bptr = &s2_arr[1]->b;
 
   x = s2->s2->a;// expected-warning{{dereferencing expression marked 
as 'noderef'}}
   x = s2_noderef->a[1]; // expected-warning{{dereferencing s2_noderef; was 
declared with a 'noderef' type}}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1810,6 +1810,14 @@
 Qualifiers Combined = BaseQuals + MemberQuals;
 if (Combined != MemberQuals)
   MemberType = Context.getQualifiedType(MemberType, Combined);
+
+// Pick up NoDeref from the base in case we end up using AddrOf on the
+// result. E.g. the expression
+// &someNoDerefPtr->pointerMember
+// should be a noderef pointer again.
+if (BaseType->hasAttr(attr::NoDeref))
+  MemberType =
+  Context.getAttributedType(attr::NoDeref, MemberType, MemberType);
   }
 
   auto *CurMethod = dyn_cast(CurContext);


Index: clang/test/Frontend/noderef.c
===
--- clang/test/Frontend/noderef.c
+++ clang/test/Frontend/noderef.c
@@ -70,6 +70,12 @@
   x = sizeof(s->a + (s->b)); // ok
   x = sizeof(int[++s->a]);   // expected-warning{{dereferencing s; was declared with a 'noderef' type}}
 
+  // Struct member access should carry NoDeref type information through to an
+  // enclosing AddrOf.
+  p2 = &s->a;   // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  x = *&s->a;   // expected-warning{{dereferencing expression marked as 'noderef'}}
+
   // Nested struct access
   struct S2 NODEREF *s2_noderef;// expected-note 5 {{s2_noderef declared here}}
   p = s2_noderef->a;  // ok since result is an array in a struct
@@ -113,7 +119,7 @@
 
   p = s2_arr[1]->a;
   p = s2_arr[1]->b; // expected-warning{{dereferencing expression marked as 'noderef'}}
-  int **bptr = &s2_arr[1]->b;
+  int *NODEREF *bptr = &s2_arr[1]->b;
 
   x = s2->s2->a;// expected-warning{{dereferencing expression marked as 'noderef'}}
   x = s2_noderef->a[1]; // expected-warning{{dereferencing s2_noderef; was declared with a 'noderef' type}}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1810,6 +1810,14 @@
 Qualifiers Combined = BaseQuals + MemberQuals;
 if (Combined != MemberQuals)
   MemberType = Context.getQualifiedType(MemberType, Combined);
+
+// Pick up NoDeref from the base in case we end up using AddrOf on the
+// result. E.g. the expression
+// &someNoDerefPtr->pointerMember
+// should be a noderef pointer again.
+if (BaseType->hasAttr(attr::NoDeref))
+  MemberType =
+  Context.getAttributedType(attr::NoDeref, MemberType, MemberType);
   }
 
   auto *CurMethod = dyn_cast(CurContext);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92141: Fix noderef for AddrOf on MemberExpr

[Jann Horn via Phabricator via cfe-commits]

thejh marked an inline comment as done.
thejh added inline comments.



Comment at: clang/test/Frontend/noderef.c:75-76
+  // enclosing AddrOf.
+  p = &s->a;// ok
+  p = &(*s).a;  // ok
+  p2 = &s->a;   // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}

leonardchan wrote:
> These two can probably be removed since we have
> 
> ```
>   p = &s->a;
>   p = &(*s).b;
> ```
> 
> above. 
Good point, I've removed them now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92141/new/

https://reviews.llvm.org/D92141

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92140: Fix noderef for array member of deref expr

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

@leonardchan  I don't have commit access; can you land this change and D92141 
 for me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92140/new/

https://reviews.llvm.org/D92140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92140: Fix noderef for array member of deref expr

[Jann Horn via Phabricator via cfe-commits]

thejh added a comment.

Thanks! :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92140/new/

https://reviews.llvm.org/D92140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits