[PATCH] D92141: Fix noderef for AddrOf on MemberExpr

[Leonard Chan via Phabricator via cfe-commits]

leonardchan closed this revision.
leonardchan added a comment.

Ok this time I remembered the email. Unsure why the bug isn't automatically 
closed though.


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 marked an inline comment as done.
thejh added inline comments.



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

leonardchan wrote:
> These two can probably be removed since we have
> 
> ```
>   p = >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] 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 = >a;   // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  x = *>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 = _arr[1]->b;
+  int *NODEREF *bptr = _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
+// >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 = >a;   // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  x = *>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 = _arr[1]->b;
+  int *NODEREF *bptr = _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
+// >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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan accepted this revision.
leonardchan added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks




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

These two can probably be removed since we have

```
  p = >a;
  p = &(*s).b;
```

above. 


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 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 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 = >a;// ok
+  p = &(*s).a;  // ok
+  p2 = >a;   // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}
+  x = *>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 = _arr[1]->b;
+  int *NODEREF *bptr = _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
+// >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 = >a;// ok
+  p = &(*s).a;  // ok
+  p2 = >a;   // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  p2 = &(*s).a; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}}
+  x = *>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 = _arr[1]->b;
+  int *NODEREF *bptr = _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
+// >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