[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
This revision was automatically updated to reflect the committed changes.
Closed by commit rG854f5f332af4: [Sema] Teach -Wcast-align to compute an
accurate alignment using the alignment… (authored by ahatanak).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/warn-cast-align.cpp
Index: clang/test/SemaCXX/warn-cast-align.cpp
===
--- clang/test/SemaCXX/warn-cast-align.cpp
+++ clang/test/SemaCXX/warn-cast-align.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wcast-align -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wno-unused-value -Wcast-align -verify %s
// Simple casts.
void test0(char *P) {
@@ -43,3 +43,115 @@
typedef int *IntPtr;
c = IntPtr(P);
}
+
+struct __attribute__((aligned(16))) A {
+ char m0[16];
+ char m1[16];
+};
+
+struct B0 {
+ char m0[16];
+};
+
+struct B1 {
+ char m0[16];
+};
+
+struct C {
+ A
+ B0
+ A m2;
+};
+
+struct __attribute__((aligned(16))) D0 : B0, B1 {
+};
+
+struct __attribute__((aligned(16))) D1 : virtual B0 {
+};
+
+struct B2 {
+ char m0[8];
+};
+
+struct B3 {
+ char m0[8];
+};
+
+struct B4 {
+ char m0[8];
+};
+
+struct D2 : B2, B3 {
+};
+
+struct __attribute__((aligned(16))) D3 : B4, D2 {
+};
+
+struct __attribute__((aligned(16))) D4 : virtual D2 {
+};
+
+struct D5 : virtual D0 {
+ char m0[16];
+};
+
+struct D6 : virtual D5 {
+};
+
+struct D7 : virtual D3 {
+};
+
+void test2(int n, A *a2) {
+ __attribute__((aligned(16))) char m[sizeof(A) * 2];
+ char(_ref)[sizeof(A) * 2] = m;
+ extern char(_ref_noinit)[sizeof(A) * 2];
+ __attribute__((aligned(16))) char vararray[10][n];
+ A t0;
+ B0 t1;
+ C t2 = {.m0 = t0, .m1 = t1};
+ __attribute__((aligned(16))) char t3[5][5][5];
+ __attribute__((aligned(16))) char t4[4][16];
+ D0 t5;
+ D1 t6;
+ D3 t7;
+ D4 t8;
+ D6 t9;
+ __attribute__((aligned(1))) D7 t10;
+
+ A *a;
+ a = (A *)
+ a = (A *)(m + sizeof(A));
+ a = (A *)(sizeof(A) + m);
+ a = (A *)((sizeof(A) * 2 + m) - sizeof(A));
+ a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)&*[sizeof(A)];
+ a = (A *)(0, 0, [sizeof(A)]);
+ a = (A *)&(0, 0, *[sizeof(A)]);
+ a = (A *)[n]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)_ref;
+ a = (A *)_ref_noinit;// expected-warning {{cast from 'char (*)[64]' to 'A *'}}
+ a = (A *)([4][0]);// expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(t2.m2.m1);
+ a = (A *)([3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)([2][2][4]);
+ a = (A *)([0][n][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)[n][0];
+ a = (A *)[n][1]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(t4 + 1);
+ a = (A *)(t4 + n);
+ a = (A *)(static_cast());
+ a = (A *)(&(static_cast(t5)));
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast());
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B3 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'D5 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'D3 *' to 'A *'}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -30,6 +30,7 @@
#include "clang/AST/NSAPI.h"
#include "clang/AST/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
@@ -13105,17 +13106,226 @@
return HasInvalidParm;
}
-/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
-/// or MemberExpr.
-static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
- ASTContext ) {
- if (const auto *DRE = dyn_cast(E))
-return Context.getDeclAlign(DRE->getDecl());
+Optional>
+static getBaseAlignmentAndOffsetFromPtr(const Expr *E, ASTContext );
+
+/// Compute the alignment and offset of the base class object given the
+/// derived-to-base cast
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: clang/lib/Sema/SemaChecking.cpp:13122
+if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > Oh, this — and all the other places that do presumed alignment based on
> > > > a pointee type — needs a special case for C++ records with virtual
> > > > bases, because you need to get its presumed alignment as a base
> > > > sub-object, not its presumed alignment as a complete object, which is
> > > > what `getTypeAlignInChars` will return. The right way to merge that
> > > > information is to get the normal alignment — which may be lower than
> > > > expected if there's a typedef in play — and then `min` that with the
> > > > base sub-object alignment.
> > > I think the base sub-object alignment in this case is the
> > > `NonVirtualAlignment` of the class (the `CXXRecordDecl` of `Base` in this
> > > function), but it's not clear to me what the normal alignment is. I don't
> > > think it's what `Ctx.getTypeAlignInChars(Base->getType())` returns, is
> > > it? I see `CodeGenModule::getDynamicOffsetAlignment` seems to be doing
> > > what you are suggesting, but I'm not sure whether that's what we should
> > > be doing here. It looks like it's comparing the alignment of the derived
> > > class and the non-virtual alignment of the base class.
> > The base sub-object alignment is the class's non-virtual alignment, right.
> >
> > By the normal alignment, I mean the alignment you're starting from for the
> > outer object — if that's less than the base's alignment, the base may be
> > misaligned as well. For the non-base-conversion case, that's probably not
> > necessary to consider.
> >
> Let me know if the comment explaining why we are taking the minimum makes
> sense. I added a test case for this too (``).
Looks good.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak added inline comments.
Comment at: clang/lib/Sema/SemaChecking.cpp:13122
+if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Oh, this — and all the other places that do presumed alignment based on a
> > > pointee type — needs a special case for C++ records with virtual bases,
> > > because you need to get its presumed alignment as a base sub-object, not
> > > its presumed alignment as a complete object, which is what
> > > `getTypeAlignInChars` will return. The right way to merge that
> > > information is to get the normal alignment — which may be lower than
> > > expected if there's a typedef in play — and then `min` that with the base
> > > sub-object alignment.
> > I think the base sub-object alignment in this case is the
> > `NonVirtualAlignment` of the class (the `CXXRecordDecl` of `Base` in this
> > function), but it's not clear to me what the normal alignment is. I don't
> > think it's what `Ctx.getTypeAlignInChars(Base->getType())` returns, is it?
> > I see `CodeGenModule::getDynamicOffsetAlignment` seems to be doing what you
> > are suggesting, but I'm not sure whether that's what we should be doing
> > here. It looks like it's comparing the alignment of the derived class and
> > the non-virtual alignment of the base class.
> The base sub-object alignment is the class's non-virtual alignment, right.
>
> By the normal alignment, I mean the alignment you're starting from for the
> outer object — if that's less than the base's alignment, the base may be
> misaligned as well. For the non-base-conversion case, that's probably not
> necessary to consider.
>
Let me know if the comment explaining why we are taking the minimum makes
sense. I added a test case for this too (``).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak updated this revision to Diff 264157.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Fix alignment computation of virtual bases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/warn-cast-align.cpp
Index: clang/test/SemaCXX/warn-cast-align.cpp
===
--- clang/test/SemaCXX/warn-cast-align.cpp
+++ clang/test/SemaCXX/warn-cast-align.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wcast-align -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wno-unused-value -Wcast-align -verify %s
// Simple casts.
void test0(char *P) {
@@ -43,3 +43,115 @@
typedef int *IntPtr;
c = IntPtr(P);
}
+
+struct __attribute__((aligned(16))) A {
+ char m0[16];
+ char m1[16];
+};
+
+struct B0 {
+ char m0[16];
+};
+
+struct B1 {
+ char m0[16];
+};
+
+struct C {
+ A
+ B0
+ A m2;
+};
+
+struct __attribute__((aligned(16))) D0 : B0, B1 {
+};
+
+struct __attribute__((aligned(16))) D1 : virtual B0 {
+};
+
+struct B2 {
+ char m0[8];
+};
+
+struct B3 {
+ char m0[8];
+};
+
+struct B4 {
+ char m0[8];
+};
+
+struct D2 : B2, B3 {
+};
+
+struct __attribute__((aligned(16))) D3 : B4, D2 {
+};
+
+struct __attribute__((aligned(16))) D4 : virtual D2 {
+};
+
+struct D5 : virtual D0 {
+ char m0[16];
+};
+
+struct D6 : virtual D5 {
+};
+
+struct D7 : virtual D3 {
+};
+
+void test2(int n, A *a2) {
+ __attribute__((aligned(16))) char m[sizeof(A) * 2];
+ char(_ref)[sizeof(A) * 2] = m;
+ extern char(_ref_noinit)[sizeof(A) * 2];
+ __attribute__((aligned(16))) char vararray[10][n];
+ A t0;
+ B0 t1;
+ C t2 = {.m0 = t0, .m1 = t1};
+ __attribute__((aligned(16))) char t3[5][5][5];
+ __attribute__((aligned(16))) char t4[4][16];
+ D0 t5;
+ D1 t6;
+ D3 t7;
+ D4 t8;
+ D6 t9;
+ __attribute__((aligned(1))) D7 t10;
+
+ A *a;
+ a = (A *)
+ a = (A *)(m + sizeof(A));
+ a = (A *)(sizeof(A) + m);
+ a = (A *)((sizeof(A) * 2 + m) - sizeof(A));
+ a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)&*[sizeof(A)];
+ a = (A *)(0, 0, [sizeof(A)]);
+ a = (A *)&(0, 0, *[sizeof(A)]);
+ a = (A *)[n]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)_ref;
+ a = (A *)_ref_noinit;// expected-warning {{cast from 'char (*)[64]' to 'A *'}}
+ a = (A *)([4][0]);// expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(t2.m2.m1);
+ a = (A *)([3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)([2][2][4]);
+ a = (A *)([0][n][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)[n][0];
+ a = (A *)[n][1]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(t4 + 1);
+ a = (A *)(t4 + n);
+ a = (A *)(static_cast());
+ a = (A *)(&(static_cast(t5)));
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast());
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B3 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'D5 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'D3 *' to 'A *'}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -30,6 +30,7 @@
#include "clang/AST/NSAPI.h"
#include "clang/AST/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
@@ -13105,17 +13106,226 @@
return HasInvalidParm;
}
-/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
-/// or MemberExpr.
-static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
- ASTContext ) {
- if (const auto *DRE = dyn_cast(E))
-return Context.getDeclAlign(DRE->getDecl());
+Optional>
+static getBaseAlignmentAndOffsetFromPtr(const Expr *E, ASTContext );
+
+/// Compute the alignment and offset of the base class object given the
+/// derived-to-base cast expression and the alignment and offset of the derived
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
rjmccall added a comment.
Other changes look good to me, thanks.
Comment at: clang/lib/Sema/SemaChecking.cpp:13122
+if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
ahatanak wrote:
> rjmccall wrote:
> > Oh, this — and all the other places that do presumed alignment based on a
> > pointee type — needs a special case for C++ records with virtual bases,
> > because you need to get its presumed alignment as a base sub-object, not
> > its presumed alignment as a complete object, which is what
> > `getTypeAlignInChars` will return. The right way to merge that information
> > is to get the normal alignment — which may be lower than expected if
> > there's a typedef in play — and then `min` that with the base sub-object
> > alignment.
> I think the base sub-object alignment in this case is the
> `NonVirtualAlignment` of the class (the `CXXRecordDecl` of `Base` in this
> function), but it's not clear to me what the normal alignment is. I don't
> think it's what `Ctx.getTypeAlignInChars(Base->getType())` returns, is it? I
> see `CodeGenModule::getDynamicOffsetAlignment` seems to be doing what you are
> suggesting, but I'm not sure whether that's what we should be doing here. It
> looks like it's comparing the alignment of the derived class and the
> non-virtual alignment of the base class.
The base sub-object alignment is the class's non-virtual alignment, right.
By the normal alignment, I mean the alignment you're starting from for the
outer object — if that's less than the base's alignment, the base may be
misaligned as well. For the non-base-conversion case, that's probably not
necessary to consider.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak added inline comments.
Comment at: clang/lib/Sema/SemaChecking.cpp:13122
+if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
rjmccall wrote:
> Oh, this — and all the other places that do presumed alignment based on a
> pointee type — needs a special case for C++ records with virtual bases,
> because you need to get its presumed alignment as a base sub-object, not its
> presumed alignment as a complete object, which is what `getTypeAlignInChars`
> will return. The right way to merge that information is to get the normal
> alignment — which may be lower than expected if there's a typedef in play —
> and then `min` that with the base sub-object alignment.
I think the base sub-object alignment in this case is the `NonVirtualAlignment`
of the class (the `CXXRecordDecl` of `Base` in this function), but it's not
clear to me what the normal alignment is. I don't think it's what
`Ctx.getTypeAlignInChars(Base->getType())` returns, is it? I see
`CodeGenModule::getDynamicOffsetAlignment` seems to be doing what you are
suggesting, but I'm not sure whether that's what we should be doing here. It
looks like it's comparing the alignment of the derived class and the
non-virtual alignment of the base class.
Comment at: clang/lib/Sema/SemaChecking.cpp:13266
+ if (!RHS->isIntegerConstantExpr(RHSRes, Ctx))
+return Optional>();
+ CharUnits Offset = LHSRes->second;
rjmccall wrote:
> This should be handled the same as array subscripting. Maybe you can extract
> a helper for that that's given a pointer expression, an integer expression,
> and a flag indicating whether it's a subtraction?
The offset computation was wrong when the element type wasn't a char. The bug
is fixed in the helper function.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak updated this revision to Diff 264106.
ahatanak marked 8 inline comments as done.
ahatanak added a comment.
Address most of the review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/warn-cast-align.cpp
Index: clang/test/SemaCXX/warn-cast-align.cpp
===
--- clang/test/SemaCXX/warn-cast-align.cpp
+++ clang/test/SemaCXX/warn-cast-align.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wcast-align -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wno-unused-value -Wcast-align -verify %s
// Simple casts.
void test0(char *P) {
@@ -43,3 +43,101 @@
typedef int *IntPtr;
c = IntPtr(P);
}
+
+struct __attribute__((aligned(16))) A {
+ char m0[16];
+ char m1[16];
+};
+
+struct B0 {
+ char m0[16];
+};
+
+struct B1 {
+ char m0[16];
+};
+
+struct C {
+ A
+ B0
+ A m2;
+};
+
+struct __attribute__((aligned(16))) D0 : B0, B1 {
+};
+
+struct __attribute__((aligned(16))) D1 : virtual B0 {
+};
+
+struct B2 {
+ char m0[8];
+};
+
+struct B3 {
+ char m0[8];
+};
+
+struct B4 {
+ char m0[8];
+};
+
+struct D2 : B2, B3 {
+};
+
+struct __attribute__((aligned(16))) D3 : B4, D2 {
+};
+
+struct __attribute__((aligned(16))) D4 : virtual D2 {
+};
+
+void test2(int n, A *a2) {
+ __attribute__((aligned(16))) char m[sizeof(A) * 2];
+ char(_ref)[sizeof(A) * 2] = m;
+ extern char(_ref_noinit)[sizeof(A) * 2];
+ __attribute__((aligned(16))) char vararray[10][n];
+ A t0;
+ B0 t1;
+ C t2 = {.m0 = t0, .m1 = t1};
+ __attribute__((aligned(16))) char t3[5][5][5];
+ __attribute__((aligned(16))) char t4[4][16];
+ D0 t5;
+ D1 t6;
+ D3 t7;
+ D4 t8;
+
+ A *a;
+ a = (A *)
+ a = (A *)(m + sizeof(A));
+ a = (A *)(sizeof(A) + m);
+ a = (A *)((sizeof(A) * 2 + m) - sizeof(A));
+ a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)&*[sizeof(A)];
+ a = (A *)(0, 0, [sizeof(A)]);
+ a = (A *)&(0, 0, *[sizeof(A)]);
+ a = (A *)[n]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)_ref;
+ a = (A *)_ref_noinit;// expected-warning {{cast from 'char (*)[64]' to 'A *'}}
+ a = (A *)([4][0]);// expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(t2.m2.m1);
+ a = (A *)([3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)([2][2][4]);
+ a = (A *)([0][n][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)[n][0];
+ a = (A *)[n][1]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(t4 + 1);
+ a = (A *)(t4 + n);
+ a = (A *)(static_cast());
+ a = (A *)(&(static_cast(t5)));
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast());
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B3 *' to 'A *'}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -30,6 +30,7 @@
#include "clang/AST/NSAPI.h"
#include "clang/AST/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
@@ -13105,17 +13106,219 @@
return HasInvalidParm;
}
-/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
-/// or MemberExpr.
-static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
- ASTContext ) {
- if (const auto *DRE = dyn_cast(E))
-return Context.getDeclAlign(DRE->getDecl());
+Optional>
+static getBaseAlignmentAndOffsetFromPtr(const Expr *E, ASTContext );
+
+/// Compute the alignment and offset of the base class object given the
+/// derived-to-base cast expression and the alignment and offset of the derived
+/// class object.
+static std::pair
+getDerivedToBaseAlignmentAndOffset(const CastExpr *CE, QualType DerivedType,
+ CharUnits BaseAlignment, CharUnits Offset,
+ ASTContext ) {
+ for (auto PathI = CE->path_begin(), PathE = CE->path_end(); PathI != PathE;
+
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaChecking.cpp:13122
+if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
Oh, this — and all the other places that do presumed alignment based on a
pointee type — needs a special case for C++ records with virtual bases, because
you need to get its presumed alignment as a base sub-object, not its presumed
alignment as a complete object, which is what `getTypeAlignInChars` will
return. The right way to merge that information is to get the normal alignment
— which may be lower than expected if there's a typedef in play — and then
`min` that with the base sub-object alignment.
Comment at: clang/lib/Sema/SemaChecking.cpp:13140
+ E = E->IgnoreParens();
+ switch (E->getStmtClass()) {
+ default:
You should add a case for unary `*`.
Comment at: clang/lib/Sema/SemaChecking.cpp:13211
+ }
+ return Optional>();
+}
There's an `llvm::None` which has this effect.
Comment at: clang/lib/Sema/SemaChecking.cpp:13225
+return getBaseAlignmentAndOffsetFromPtr(cast(E)->getSubExpr(),
+Ctx);
+ case Stmt::UnaryOperatorClass: {
I don't think we guarantee that these will be no-op casts; all the explicit
cast nodes should be handled the same as ImplicitCastExpr, in both of these
functions.
Comment at: clang/lib/Sema/SemaChecking.cpp:13254
+ BinaryOperatorKind Kind) {
+ auto LHSRes = getBaseAlignmentAndOffsetFromPtr(LHS, Ctx);
+ if (!LHSRes) {
Can you do this with just a type analysis on the operands instead of eagerly
doing these calls?
Comment at: clang/lib/Sema/SemaChecking.cpp:13266
+ if (!RHS->isIntegerConstantExpr(RHSRes, Ctx))
+return Optional>();
+ CharUnits Offset = LHSRes->second;
This should be handled the same as array subscripting. Maybe you can extract a
helper for that that's given a pointer expression, an integer expression, and a
flag indicating whether it's a subtraction?
Comment at: clang/lib/Sema/SemaChecking.cpp:13279
+ return HandleBinOp(BO->getLHS(), BO->getRHS(), Opcode);
+break;
+ }
You should look through comma expressions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak updated this revision to Diff 263753.
ahatanak marked 6 inline comments as done.
ahatanak added a comment.
Address review comments. Handle derived-to-base cast expressions and array
subscript expressions that don't have constant indices.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/warn-cast-align.cpp
Index: clang/test/SemaCXX/warn-cast-align.cpp
===
--- clang/test/SemaCXX/warn-cast-align.cpp
+++ clang/test/SemaCXX/warn-cast-align.cpp
@@ -43,3 +43,97 @@
typedef int *IntPtr;
c = IntPtr(P);
}
+
+struct __attribute__((aligned(16))) A {
+ char m0[16];
+ char m1[16];
+};
+
+struct B0 {
+ char m0[16];
+};
+
+struct B1 {
+ char m0[16];
+};
+
+struct C {
+ A
+ B0
+ A m2;
+};
+
+struct __attribute__((aligned(16))) D0 : B0, B1 {
+};
+
+struct __attribute__((aligned(16))) D1 : virtual B0 {
+};
+
+struct B2 {
+ char m0[8];
+};
+
+struct B3 {
+ char m0[8];
+};
+
+struct B4 {
+ char m0[8];
+};
+
+struct D2 : B2, B3 {
+};
+
+struct __attribute__((aligned(16))) D3 : B4, D2 {
+};
+
+struct __attribute__((aligned(16))) D4 : virtual D2 {
+};
+
+void test2(int n, A *a2) {
+ __attribute__((aligned(16))) char m[sizeof(A) * 2];
+ char(_ref)[sizeof(A) * 2] = m;
+ extern char(_ref_noinit)[sizeof(A) * 2];
+ __attribute__((aligned(16))) char vararray[10][n];
+ A t0;
+ B0 t1;
+ C t2 = {.m0 = t0, .m1 = t1};
+ __attribute__((aligned(16))) char t3[5][5][5];
+ __attribute__((aligned(16))) char t4[4][16];
+ D0 t5;
+ D1 t6;
+ D3 t7;
+ D4 t8;
+
+ A *a;
+ a = (A *)
+ a = (A *)(m + sizeof(A));
+ a = (A *)(sizeof(A) + m);
+ a = (A *)((sizeof(A) * 2 + m) - sizeof(A));
+ a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)[sizeof(A)];
+ a = (A *)[n]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)_ref;
+ a = (A *)_ref_noinit;// expected-warning {{cast from 'char (*)[64]' to 'A *'}}
+ a = (A *)([4][0]);// expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(t2.m2.m1);
+ a = (A *)([3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)([2][2][4]);
+ a = (A *)([0][n][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)[n][0];
+ a = (A *)[n][1]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(static_cast());
+ a = (A *)(&(static_cast(t5)));
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B0 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast());
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B2 *' to 'A *'}}
+ a = (A *)(static_cast()); // expected-warning {{cast from 'B3 *' to 'A *'}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -30,6 +30,7 @@
#include "clang/AST/NSAPI.h"
#include "clang/AST/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
@@ -13105,17 +13106,192 @@
return HasInvalidParm;
}
-/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
-/// or MemberExpr.
-static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
- ASTContext ) {
- if (const auto *DRE = dyn_cast(E))
-return Context.getDeclAlign(DRE->getDecl());
+Optional>
+static getBaseAlignmentAndOffsetFromPtr(const Expr *E, ASTContext );
+
+/// Compute the alignment and offset of the base class object given the
+/// derived-to-base cast expression and the alignment and offset of the derived
+/// class object.
+static std::pair getDerivedToBaseAlignmentAndOffset(
+const ImplicitCastExpr *ICE, QualType DerivedType, CharUnits BaseAlignment,
+CharUnits Offset, ASTContext ) {
+ for (auto PathI = ICE->path_begin(), PathE = ICE->path_end(); PathI != PathE;
+ ++PathI) {
+const CXXBaseSpecifier *Base = *PathI;
+if (Base->isVirtual()) {
+ BaseAlignment = Ctx.getTypeAlignInChars(Base->getType());
+ Offset = CharUnits::Zero();
+} else {
+ const ASTRecordLayout =
+ Ctx.getASTRecordLayout(DerivedType->getAsCXXRecordDecl());
+ Offset +=
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
rjmccall added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13111 + } + } + return std::make_tuple(false, CharUnits::Zero(), CharUnits::Zero()); ahatanak wrote: > rjmccall wrote: > > Derived-to-base conversions. > Does this include cases where bases are virtual? I think that would be more > complex as it requires tracking the most-derived object. Do you think it's > important to handle virtual bases in this patch or should it be done > separately in a follow-up? You should handle virtual bases, but it's okay to assume they're not within known-complete objects; i.e. you just need to reset the alignment to the alignment of the base. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78767/new/ https://reviews.llvm.org/D78767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13111 + } + } + return std::make_tuple(false, CharUnits::Zero(), CharUnits::Zero()); rjmccall wrote: > Derived-to-base conversions. Does this include cases where bases are virtual? I think that would be more complex as it requires tracking the most-derived object. Do you think it's important to handle virtual bases in this patch or should it be done separately in a follow-up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78767/new/ https://reviews.llvm.org/D78767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaChecking.cpp:13066
+std::tuple
+static getBaseAlignmentAndOffsetFromLValue(const Expr *E, ASTContext ) {
+ E = E->IgnoreParens();
I think an `Optional` would be a more self-documenting type here,
and you'd be able to test the result in an `if`.
Comment at: clang/lib/Sema/SemaChecking.cpp:13082
+ std::get<2>(P) + EltSize *
IdxRes.getExtValue());
+ }
+}
You still need to conservatively adjust the alignment when the index isn't a
constant expression. You can do that by reducing the base result to a simple
alignment, adding the array element size as an offset, and then reducing again.
Comment at: clang/lib/Sema/SemaChecking.cpp:13091
+ if (VD->getType()->isReferenceType())
+return getBaseAlignmentAndOffsetFromLValue(VD->getInit(), Ctx);
+ return std::make_tuple(true, Ctx.getDeclAlign(VD), CharUnits::Zero());
Reference variables don't have to have initializers; e.g. they can be `extern`.
Comment at: clang/lib/Sema/SemaChecking.cpp:13111
+ }
+ }
+ return std::make_tuple(false, CharUnits::Zero(), CharUnits::Zero());
Derived-to-base conversions.
Comment at: clang/lib/Sema/SemaChecking.cpp:13160
+ Opcode == BO_Add)
+HandleBinOp(BO->getRHS(), BO->getLHS(), Opcode, P);
+ return P;
This all seems like it'd be simpler if you just checked for the pointer/integer
operands and then swapped if necessary.
Comment at: clang/lib/Sema/SemaChecking.cpp:13164
+break;
+ }
+ }
Derived-to-base conversions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak updated this revision to Diff 263238.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Define two helper functions in Sema that compute the alignment of a VarDecl and
a constant offset from it and use them instead of the classes for evaluating
lvalue and pointer expressions in ExprConstant.cpp.
Using the classes in ExprConstant.cpp to compute an expression alignment as I
did in the previous patch is probably not a good idea since they are for
evaluating constant expressions. They don't return the lvalue base variables
and offsets in some cases ( for example, `(A *)_ref` in the test case) and
using them for this purpose can make it harder to make changes to the way
constant expressions are evaluated. The current patch is far from perfect as it
misses many cases that could be detected by the classes in ExprConstant.cpp,
but is at least an improvement over what we have now.
I was also thinking about fixing the alignment computation of captured
variables, but I think I should do that separately in a follow-up as it might
not be trivial. It will probably require looking up the captured variables in
all the enclosing `CapturingScopeInfo`s.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78767/new/
https://reviews.llvm.org/D78767
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/warn-cast-align.cpp
Index: clang/test/SemaCXX/warn-cast-align.cpp
===
--- clang/test/SemaCXX/warn-cast-align.cpp
+++ clang/test/SemaCXX/warn-cast-align.cpp
@@ -43,3 +43,53 @@
typedef int *IntPtr;
c = IntPtr(P);
}
+
+struct __attribute__((aligned(16))) A {
+ char m0[16];
+ char m1[16];
+};
+
+struct B {
+ char m0[16];
+};
+
+struct C {
+ A
+ B
+ A m2;
+};
+
+struct D {
+ char m0[8];
+};
+
+void test2(int n, A *a2) {
+ __attribute__((aligned(16))) char m[sizeof(A) * 2];
+ char(_ref)[sizeof(A) * 2] = m;
+ __attribute__((aligned(16))) char vararray[10][n];
+ A t0;
+ B t1;
+ C t2 = {.m0 = t0, .m1 = t1};
+ __attribute__((aligned(16))) char t3[5][5][5];
+
+ A *a;
+ a = (A *)
+ a = (A *)(m + sizeof(A));
+ a = (A *)(sizeof(A) + m);
+ a = (A *)((sizeof(A) * 2 + m) - sizeof(A));
+ a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)[sizeof(A)];
+ a = (A *)[n]; // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)_ref;
+ a = (A *)([4][0]);// expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(); // expected-warning {{cast from 'B *' to 'A *'}}
+ a = (A *)();
+ a = (A *)(t2.m2.m1);
+ a = (A *)([3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}}
+ a = (A *)([2][2][4]);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -30,6 +30,7 @@
#include "clang/AST/NSAPI.h"
#include "clang/AST/NonTrivialTypeVisitor.h"
#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
@@ -13055,17 +13056,126 @@
return HasInvalidParm;
}
-/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
-/// or MemberExpr.
-static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
- ASTContext ) {
- if (const auto *DRE = dyn_cast(E))
-return Context.getDeclAlign(DRE->getDecl());
+std::tuple static getBaseAlignmentAndOffsetFromPtr(
+const Expr *E, ASTContext );
- if (const auto *ME = dyn_cast(E))
-return Context.getDeclAlign(ME->getMemberDecl());
+/// This helper function takes an lvalue expression and returns the alignment of
+/// a VarDecl and a constant offset from the VarDecl. The first element of the
+/// tuple it returns is true if a VarDecl is found and the offset is constant.
+std::tuple
+static getBaseAlignmentAndOffsetFromLValue(const Expr *E, ASTContext ) {
+ E = E->IgnoreParens();
+ switch (E->getStmtClass()) {
+ default:
+break;
+ case Stmt::ArraySubscriptExprClass: {
+if (!E->getType()->isConstantSizeType())
+ break;
+auto *ASE = cast(E);
+auto P = getBaseAlignmentAndOffsetFromPtr(ASE->getBase(), Ctx);
+if (std::get<0>(P)) {
+ llvm::APSInt IdxRes;
+ if (ASE->getIdx()->isIntegerConstantExpr(IdxRes, Ctx)) {
+CharUnits EltSize = Ctx.getTypeSizeInChars(E->getType());
+return std::make_tuple(true, std::get<1>(P),
+
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak planned changes to this revision. ahatanak added a comment. The method has to compute the correct alignment for variables captured by lambdas or blocks too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78767/new/ https://reviews.llvm.org/D78767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:14844 + if (auto *VD = Result.Base.dyn_cast()) +return Ctx.getDeclAlign(VD).alignmentAtOffset(Result.Offset); + rjmccall wrote: > Does this do the right thing if `getDeclAlign` returns 0, or can that never > happen? I looked at `getDeclAlign` and, as far as I can tell, it returns an alignment that is larger than 0. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78767/new/ https://reviews.llvm.org/D78767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
rjmccall added inline comments. Comment at: clang/include/clang/AST/Expr.h:710 + /// possible to compute the alignment, return zero. + CharUnits getAlignmentFromDecl(ASTContext ) const; + Nothing about the name here suggests that it only works on expressions of pointer type. Maybe call it `getPresumedAlignmentOfPointer` or something and then make it also consider the type alignment? Comment at: clang/lib/AST/ExprConstant.cpp:14844 + if (auto *VD = Result.Base.dyn_cast()) +return Ctx.getDeclAlign(VD).alignmentAtOffset(Result.Offset); + Does this do the right thing if `getDeclAlign` returns 0, or can that never happen? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78767/new/ https://reviews.llvm.org/D78767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78767: [Sema] Teach -Wcast-align to compute a more accurate alignment when the source expression has array subscript or pointer arithmetic operators
ahatanak created this revision.
ahatanak added reviewers: arphaman, erik.pilkington, rjmccall.
ahatanak added a project: clang.
Herald added subscribers: ributzka, dexonsmith, jkorous.
This improves upon https://reviews.llvm.org/D21099, which taught -Wcast-align
to look at the aligned attribute of variables. I added a function that computes
a more accurate alignment to `ExprConstant.cpp`.
rdar://problem/59242343
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78767
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/warn-cast-align.c
Index: clang/test/Sema/warn-cast-align.c
===
--- clang/test/Sema/warn-cast-align.c
+++ clang/test/Sema/warn-cast-align.c
@@ -67,3 +67,14 @@
FnTy test5(void) {
return (FnTy)
}
+
+void test6(int n) {
+ __attribute__((aligned(16))) char m[sizeof(struct A) * 2];
+ struct A *a = (struct A *)
+ a = (struct A *)(m + sizeof(struct A));
+ a = (struct A *)(sizeof(struct A) + m);
+ a = (struct A *)(m + 1); // expected-warning {{cast from 'char *' to 'struct A *'}}
+ a = (struct A *)(1 + m); // expected-warning {{cast from 'char *' to 'struct A *'}}
+ a = (struct A *)(m + n); // expected-warning {{cast from 'char *' to 'struct A *'}}
+ a = (struct A *)[sizeof(struct A)];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13055,19 +13055,6 @@
return HasInvalidParm;
}
-/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
-/// or MemberExpr.
-static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
- ASTContext ) {
- if (const auto *DRE = dyn_cast(E))
-return Context.getDeclAlign(DRE->getDecl());
-
- if (const auto *ME = dyn_cast(E))
-return Context.getDeclAlign(ME->getMemberDecl());
-
- return TypeAlign;
-}
-
/// CheckCastAlign - Implements -Wcast-align, which warns when a
/// pointer cast increases the alignment requirements.
void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
@@ -13101,15 +13088,13 @@
// includes 'void'.
if (SrcPointee->isIncompleteType()) return;
- CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointee);
+ // Try to compute an accurate alignment using the alignment of the ValueDecls
+ // in the expression.
+ CharUnits SrcAlign = Op->getAlignmentFromDecl(Context);
- if (auto *CE = dyn_cast(Op)) {
-if (CE->getCastKind() == CK_ArrayToPointerDecay)
- SrcAlign = getDeclAlign(CE->getSubExpr(), SrcAlign, Context);
- } else if (auto *UO = dyn_cast(Op)) {
-if (UO->getOpcode() == UO_AddrOf)
- SrcAlign = getDeclAlign(UO->getSubExpr(), SrcAlign, Context);
- }
+ // If that failed, just use the type's alignment.
+ if (SrcAlign.isZero())
+SrcAlign = Context.getTypeAlignInChars(SrcPointee);
if (SrcAlign >= DestAlign) return;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -14827,3 +14827,21 @@
EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold);
return tryEvaluateBuiltinObjectSize(this, Type, Info, Result);
}
+
+CharUnits Expr::getAlignmentFromDecl(ASTContext ) const {
+ assert(getType()->getAs() &&
+ "expression must be a pointer type");
+
+ Expr::EvalStatus Status;
+ EvalInfo Info(Ctx, Status, EvalInfo::EM_IgnoreSideEffects);
+ LValue Result;
+ PointerExprEvaluator Eval(Info, Result, false);
+
+ if (!Eval.Visit(this) || !Result.Base)
+return CharUnits::Zero();
+
+ if (auto *VD = Result.Base.dyn_cast())
+return Ctx.getDeclAlign(VD).alignmentAtOffset(Result.Offset);
+
+ return CharUnits::Zero();
+}
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -704,6 +704,11 @@
bool tryEvaluateObjectSize(uint64_t , ASTContext ,
unsigned Type) const;
+ /// Compute the alignment of an expression of a pointer type using the
+ /// alignment of a ValueDecl referenced in the expression. If it's not
+ /// possible to compute the alignment, return zero.
+ CharUnits getAlignmentFromDecl(ASTContext ) const;
+
/// Enumeration used to describe the kind of Null pointer constant
/// returned from \c isNullPointerConstant().
enum NullPointerConstantKind {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
