[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd16b3fe2559: [BPF] Restrict preserve_access_index attribute 
to C only (authored by yonghong-song).

Changed prior to commit:
  https://reviews.llvm.org/D70257?vs=229355=229406#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70257

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/Sema/bpf-attr-preserve-access-index.cpp


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 
-fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if (DeclRefCast) {
-const VarDecl *VarDef = dyn_cast(DeclRefCast->getDecl());
+  const Expr *E = ArrayBase->IgnoreImpCasts();
+  if (const auto *ME = dyn_cast(E))
+return ME->getMemberDecl()->hasAttr();
+
+  if (const auto *DRE = dyn_cast(E)) {
+const auto *VarDef = dyn_cast(DRE->getDecl());
 if (!VarDef)
   return false;
 
-const auto *PtrT = dyn_cast(VarDef->getType().getTypePtr());
+const auto *PtrT = VarDef->getType()->getAs();
 if (!PtrT)
   return false;
-const auto *PointeeT = PtrT->getPointeeType().getTypePtr();
-
-// Peel off typedef's
-const auto *TypedefT = dyn_cast(PointeeT);
-while (TypedefT) {
-  PointeeT = TypedefT->desugar().getTypePtr();
-  TypedefT = dyn_cast(PointeeT);
-}
-
-// Not a typedef any more, it should be an elaborated type.
-const auto ElaborateT = dyn_cast(PointeeT);
-if (!ElaborateT)
-  return false;
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType()
+ ->getUnqualifiedDesugaredType();
+if (const auto *RecT = dyn_cast(PointeeT))
+  return RecT->getDecl()->hasAttr();
+return false;
   }
 
   return false;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1584,6 +1584,7 @@
   let Spellings = [Clang<"preserve_access_index">];
   let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [BPFPreserveAccessIndexDocs];
+  let LangOpts = [COnly];
 }
 
 def WebAssemblyImportModule : InheritableAttr,


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if 

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song marked an inline comment as done.
yonghong-song added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3431
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType().getTypePtr()
+ ->getUnqualifiedDesugaredType();

aaron.ballman wrote:
> You can drop the `getTypePtr()` here and just use the magic `->` overload.
Thanks! This indeed better:
```
+const auto *PointeeT = PtrT->getPointeeType()
+ ->getUnqualifiedDesugaredType();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70257



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

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a small nit. Thank you!




Comment at: clang/lib/CodeGen/CGExpr.cpp:3431
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType().getTypePtr()
+ ->getUnqualifiedDesugaredType();

You can drop the `getTypePtr()` here and just use the magic `->` overload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70257



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

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.
yonghong-song retitled this revision from "[BPF] Restrict preserve_access_index 
to C only" to "[BPF] Restrict preserve_access_index attribute to C only".

This patch is a follow-up for commit 4e2ce228ae79 


  [BPF] Add preserve_access_index attribute for record definition

to restrict attribute for C only. A new test case is added
to check for this restriction.

Additional code polishing is done based on
Aaron Ballman's suggestion in https://reviews.llvm.org/D69759/new/.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70257

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/Sema/bpf-attr-preserve-access-index.cpp


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 
-fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if (DeclRefCast) {
-const VarDecl *VarDef = dyn_cast(DeclRefCast->getDecl());
+  const Expr *E = ArrayBase->IgnoreImpCasts();
+  if (const auto *ME = dyn_cast(E))
+return ME->getMemberDecl()->hasAttr();
+
+  if (const auto *DRE = dyn_cast(E)) {
+const auto *VarDef = dyn_cast(DRE->getDecl());
 if (!VarDef)
   return false;
 
-const auto *PtrT = dyn_cast(VarDef->getType().getTypePtr());
+const auto *PtrT = VarDef->getType()->getAs();
 if (!PtrT)
   return false;
-const auto *PointeeT = PtrT->getPointeeType().getTypePtr();
-
-// Peel off typedef's
-const auto *TypedefT = dyn_cast(PointeeT);
-while (TypedefT) {
-  PointeeT = TypedefT->desugar().getTypePtr();
-  TypedefT = dyn_cast(PointeeT);
-}
-
-// Not a typedef any more, it should be an elaborated type.
-const auto ElaborateT = dyn_cast(PointeeT);
-if (!ElaborateT)
-  return false;
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType().getTypePtr()
+ ->getUnqualifiedDesugaredType();
+if (const auto *RecT = dyn_cast(PointeeT))
+  return RecT->getDecl()->hasAttr();
+return false;
   }
 
   return false;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1584,6 +1584,7 @@
   let Spellings = [Clang<"preserve_access_index">];
   let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [BPFPreserveAccessIndexDocs];
+  let LangOpts = [COnly];
 }
 
 def WebAssemblyImportModule : InheritableAttr,


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //