Skip to content
This repository has been archived by the owner on Mar 28, 2020. It is now read-only.

Commit

Permalink
[Sema] Change addr space diagnostics in casts to follow C++ style.
Browse files Browse the repository at this point in the history 
This change adds a new diagnostic for mismatching address spaces
to be used for C++ casts (only enabled in C style cast for now,
the rest will follow!).

The change extends C-style cast rules to account for address spaces.
It also adds a separate function for address space cast checking that
can be used to map from a separate address space cast operator
addrspace_cast (to be added as a follow up patch).

Note, that after this change clang will no longer allows arbitrary
address space conversions in reinterpret_casts because they can lead
to accidental errors. The implicit safe conversions would still be
allowed.

Differential Revision: https://reviews.llvm.org/D58346



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@355609 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
Anastasia Stulova committed Mar 7, 2019
1 parent f384fbb commit d8ac2ca
Show file tree
Hide file tree
Showing 6 changed files with 232 additions and 58 deletions.
4 changes: 4 additions & 0 deletions include/clang/Basic/DiagnosticSemaKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6271,6 +6271,10 @@ def err_bad_cxx_cast_bitfield : Error<
def err_bad_cxx_cast_qualifiers_away : Error<
"%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
"functional-style cast}0 from %1 to %2 casts away qualifiers">;
def err_bad_cxx_cast_addr_space_mismatch : Error<
"%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
"functional-style cast}0 from %1 to %2 converts between mismatching address"
" spaces">;
def ext_bad_cxx_cast_qualifiers_away_incoherent : ExtWarn<
"ISO C++ does not allow "
"%select{const_cast|static_cast|reinterpret_cast|dynamic_cast|C-style cast|"
Expand Down
76 changes: 60 additions & 16 deletions lib/Sema/SemaCast.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2212,7 +2212,15 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
/*CheckObjCLifetime=*/CStyle))
SuccessResult = getCastAwayConstnessCastKind(CACK, msg);

if (IsLValueCast) {
if (IsAddressSpaceConversion(SrcType, DestType)) {
Kind = CK_AddressSpaceConversion;
assert(SrcType->isPointerType() && DestType->isPointerType());
if (!CStyle &&
!DestType->getPointeeType().getQualifiers().isAddressSpaceSupersetOf(
SrcType->getPointeeType().getQualifiers())) {
SuccessResult = TC_Failed;
}
} else if (IsLValueCast) {
Kind = CK_LValueBitCast;
} else if (DestType->isObjCObjectPointerType()) {
Kind = Self.PrepareCastToObjCObjectPointer(SrcExpr);
Expand All @@ -2222,8 +2230,6 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
} else {
Kind = CK_BitCast;
}
} else if (IsAddressSpaceConversion(SrcType, DestType)) {
Kind = CK_AddressSpaceConversion;
} else {
Kind = CK_BitCast;
}
Expand Down Expand Up @@ -2278,6 +2284,41 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
return SuccessResult;
}

static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, bool CStyle,
unsigned &msg) {
if (!Self.getLangOpts().OpenCL)
// FIXME: As compiler doesn't have any information about overlapping addr
// spaces at the moment we have to be permissive here.
return TC_NotApplicable;
// Even though the logic below is general enough and can be applied to
// non-OpenCL mode too, we fast-path above because no other languages
// define overlapping address spaces currently.
auto SrcType = SrcExpr.get()->getType();
auto SrcPtrType = SrcType->getAs<PointerType>();
if (!SrcPtrType)
return TC_NotApplicable;
auto DestPtrType = DestType->getAs<PointerType>();
if (!DestPtrType)
return TC_NotApplicable;
auto SrcPointeeType = SrcPtrType->getPointeeType();
auto DestPointeeType = DestPtrType->getPointeeType();
if (SrcPointeeType.getAddressSpace() == DestPointeeType.getAddressSpace())
return TC_NotApplicable;
if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
msg = diag::err_bad_cxx_cast_addr_space_mismatch;
return TC_Failed;
}
auto SrcPointeeTypeWithoutAS =
Self.Context.removeAddrSpaceQualType(SrcPointeeType.getCanonicalType());
auto DestPointeeTypeWithoutAS =
Self.Context.removeAddrSpaceQualType(DestPointeeType.getCanonicalType());
return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,
DestPointeeTypeWithoutAS)
? TC_Success
: TC_NotApplicable;
}

void CastOperation::checkAddressSpaceCast(QualType SrcType, QualType DestType) {
// In OpenCL only conversions between pointers to objects in overlapping
// addr spaces are allowed. v2.0 s6.5.5 - Generic addr space overlaps
Expand Down Expand Up @@ -2372,30 +2413,35 @@ void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
// listed above, the interpretation that appears first in the list is used,
// even if a cast resulting from that interpretation is ill-formed.
// In plain language, this means trying a const_cast ...
// Note that for address space we check compatibility after const_cast.
unsigned msg = diag::err_bad_cxx_cast_generic;
TryCastResult tcr = TryConstCast(Self, SrcExpr, DestType,
/*CStyle*/true, msg);
/*CStyle*/ true, msg);
if (SrcExpr.isInvalid())
return;
if (isValidCast(tcr))
Kind = CK_NoOp;

Sema::CheckedConversionKind CCK
= FunctionalStyle? Sema::CCK_FunctionalCast
: Sema::CCK_CStyleCast;
Sema::CheckedConversionKind CCK =
FunctionalStyle ? Sema::CCK_FunctionalCast : Sema::CCK_CStyleCast;
if (tcr == TC_NotApplicable) {
// ... or if that is not possible, a static_cast, ignoring const, ...
tcr = TryStaticCast(Self, SrcExpr, DestType, CCK, OpRange,
msg, Kind, BasePath, ListInitialization);
tcr = TryAddressSpaceCast(Self, SrcExpr, DestType, /*CStyle*/ true, msg);
if (SrcExpr.isInvalid())
return;

if (tcr == TC_NotApplicable) {
// ... and finally a reinterpret_cast, ignoring const.
tcr = TryReinterpretCast(Self, SrcExpr, DestType, /*CStyle*/true,
OpRange, msg, Kind);
// ... or if that is not possible, a static_cast, ignoring const, ...
tcr = TryStaticCast(Self, SrcExpr, DestType, CCK, OpRange, msg, Kind,
BasePath, ListInitialization);
if (SrcExpr.isInvalid())
return;

if (tcr == TC_NotApplicable) {
// ... and finally a reinterpret_cast, ignoring const.
tcr = TryReinterpretCast(Self, SrcExpr, DestType, /*CStyle*/ true,
OpRange, msg, Kind);
if (SrcExpr.isInvalid())
return;
}
}
}

Expand Down Expand Up @@ -2426,8 +2472,6 @@ void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
}
}

checkAddressSpaceCast(SrcExpr.get()->getType(), DestType);

if (isValidCast(tcr)) {
if (Kind == CK_BitCast)
checkCastAlign();
Expand Down
14 changes: 14 additions & 0 deletions test/CodeGenOpenCLCXX/address-space-castoperators.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s

void test_reinterpret_cast(){
__private float x;
__private float& y = x;
// We don't need bitcast to cast pointer type and
// address space at the same time.
//CHECK: addrspacecast float* %x to i32 addrspace(4)*
//CHECK: [[REG:%[0-9]+]] = load float*, float** %y
//CHECK: addrspacecast float* [[REG]] to i32 addrspace(4)*
//CHECK-NOT: bitcast
__generic int& rc1 = reinterpret_cast<__generic int&>(x);
__generic int& rc2 = reinterpret_cast<__generic int&>(y);
}
28 changes: 14 additions & 14 deletions test/SemaCXX/address-space-conversion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,24 +131,24 @@ void test_dynamic_cast(A_ptr ap, A_ptr_1 ap1, A_ptr_2 ap2,
void test_reinterpret_cast(void_ptr vp, void_ptr_1 vp1, void_ptr_2 vp2,
A_ptr ap, A_ptr_1 ap1, A_ptr_2 ap2,
B_ptr bp, B_ptr_1 bp1, B_ptr_2 bp2,
const void __attribute__((address_space(1))) *cvp1) {
// reinterpret_cast can be used to cast to a different address space.
(void)reinterpret_cast<A_ptr>(ap1);
(void)reinterpret_cast<A_ptr>(ap2);
const void __attribute__((address_space(1))) * cvp1) {
// reinterpret_cast can't be used to cast to a different address space unless they are matching (i.e. overlapping).
(void)reinterpret_cast<A_ptr>(ap1); // expected-error{{reinterpret_cast from 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') to 'A_ptr' (aka 'A *') is not allowed}}
(void)reinterpret_cast<A_ptr>(ap2); // expected-error{{reinterpret_cast from 'A_ptr_2' (aka '__attribute__((address_space(2))) A *') to 'A_ptr' (aka 'A *') is not allowed}}
(void)reinterpret_cast<A_ptr>(bp);
(void)reinterpret_cast<A_ptr>(bp1);
(void)reinterpret_cast<A_ptr>(bp2);
(void)reinterpret_cast<A_ptr>(bp1); // expected-error{{reinterpret_cast from 'B_ptr_1' (aka '__attribute__((address_space(1))) B *') to 'A_ptr' (aka 'A *') is not allowed}}
(void)reinterpret_cast<A_ptr>(bp2); // expected-error{{reinterpret_cast from 'B_ptr_2' (aka '__attribute__((address_space(2))) B *') to 'A_ptr' (aka 'A *') is not allowed}}
(void)reinterpret_cast<A_ptr>(vp);
(void)reinterpret_cast<A_ptr>(vp1);
(void)reinterpret_cast<A_ptr>(vp2);
(void)reinterpret_cast<A_ptr_1>(ap);
(void)reinterpret_cast<A_ptr_1>(ap2);
(void)reinterpret_cast<A_ptr_1>(bp);
(void)reinterpret_cast<A_ptr>(vp1); // expected-error{{reinterpret_cast from 'void_ptr_1' (aka '__attribute__((address_space(1))) void *') to 'A_ptr' (aka 'A *') is not allowed}}
(void)reinterpret_cast<A_ptr>(vp2); // expected-error{{reinterpret_cast from 'void_ptr_2' (aka '__attribute__((address_space(2))) void *') to 'A_ptr' (aka 'A *') is not allowed}}
(void)reinterpret_cast<A_ptr_1>(ap); // expected-error{{reinterpret_cast from 'A_ptr' (aka 'A *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
(void)reinterpret_cast<A_ptr_1>(ap2); // expected-error{{reinterpret_cast from 'A_ptr_2' (aka '__attribute__((address_space(2))) A *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
(void)reinterpret_cast<A_ptr_1>(bp); // expected-error{{reinterpret_cast from 'B_ptr' (aka 'B *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
(void)reinterpret_cast<A_ptr_1>(bp1);
(void)reinterpret_cast<A_ptr_1>(bp2);
(void)reinterpret_cast<A_ptr_1>(vp);
(void)reinterpret_cast<A_ptr_1>(bp2); // expected-error{{reinterpret_cast from 'B_ptr_2' (aka '__attribute__((address_space(2))) B *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
(void)reinterpret_cast<A_ptr_1>(vp); // expected-error{{reinterpret_cast from 'void_ptr' (aka 'void *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}
(void)reinterpret_cast<A_ptr_1>(vp1);
(void)reinterpret_cast<A_ptr_1>(vp2);
(void)reinterpret_cast<A_ptr_1>(vp2); // expected-error{{reinterpret_cast from 'void_ptr_2' (aka '__attribute__((address_space(2))) void *') to 'A_ptr_1' (aka '__attribute__((address_space(1))) A *') is not allowed}}

// ... but don't try to cast away constness!
(void)reinterpret_cast<A_ptr_2>(cvp1); // expected-error{{casts away qualifiers}}
Expand Down
60 changes: 50 additions & 10 deletions test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,27 +129,47 @@ void test_conversion(__global int *arg_glob, __local int *arg_loc,

AS int *var_cast1 = (AS int *)arg_glob;
#ifdef CONSTANT
// expected-error@-2{{casting '__global int *' to type '__constant int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error@-3{{casting '__global int *' to type '__constant int *' changes address space of pointer}}
#else
// expected-error@-5{{C-style cast from '__global int *' to '__constant int *' converts between mismatching address spaces}}
#endif
#endif

AS int *var_cast2 = (AS int *)arg_loc;
#ifndef GENERIC
// expected-error-re@-2{{casting '__local int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error-re@-3{{casting '__local int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#else
// expected-error-re@-5{{C-style cast from '__local int *' to '__{{global|constant}} int *' converts between mismatching address spaces}}
#endif
#endif

AS int *var_cast3 = (AS int *)arg_const;
#ifndef CONSTANT
// expected-error-re@-2{{casting '__constant int *' to type '__{{global|generic}} int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error-re@-3{{casting '__constant int *' to type '__{{global|generic}} int *' changes address space of pointer}}
#else
// expected-error-re@-5{{C-style cast from '__constant int *' to '__{{global|generic}} int *' converts between mismatching address spaces}}
#endif
#endif

AS int *var_cast4 = (AS int *)arg_priv;
#ifndef GENERIC
// expected-error-re@-2{{casting 'int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error-re@-3{{casting 'int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#else
// expected-error-re@-5{{C-style cast from 'int *' to '__{{global|constant}} int *' converts between mismatching address spaces}}
#endif
#endif

AS int *var_cast5 = (AS int *)arg_gen;
#ifdef CONSTANT
// expected-error@-2{{casting '__generic int *' to type '__constant int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error@-3{{casting '__generic int *' to type '__constant int *' changes address space of pointer}}
#else
// expected-error@-5{{C-style cast from '__generic int *' to '__constant int *' converts between mismatching address spaces}}
#endif
#endif

AS int *var_impl;
Expand Down Expand Up @@ -200,27 +220,47 @@ void test_conversion(__global int *arg_glob, __local int *arg_loc,

var_cast1 = (AS int *)arg_glob;
#ifdef CONSTANT
// expected-error@-2{{casting '__global int *' to type '__constant int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error@-3{{casting '__global int *' to type '__constant int *' changes address space of pointer}}
#else
// expected-error@-5{{C-style cast from '__global int *' to '__constant int *' converts between mismatching address spaces}}
#endif
#endif

var_cast2 = (AS int *)arg_loc;
#ifndef GENERIC
// expected-error-re@-2{{casting '__local int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error-re@-3{{casting '__local int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#else
// expected-error-re@-5{{C-style cast from '__local int *' to '__{{global|constant}} int *' converts between mismatching address spaces}}
#endif
#endif

var_cast3 = (AS int *)arg_const;
#ifndef CONSTANT
// expected-error-re@-2{{casting '__constant int *' to type '__{{global|generic}} int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error-re@-3{{casting '__constant int *' to type '__{{global|generic}} int *' changes address space of pointer}}
#else
// expected-error-re@-5{{C-style cast from '__constant int *' to '__{{global|generic}} int *' converts between mismatching address spaces}}
#endif
#endif

var_cast4 = (AS int *)arg_priv;
#ifndef GENERIC
// expected-error-re@-2{{casting 'int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error-re@-3{{casting 'int *' to type '__{{global|constant}} int *' changes address space of pointer}}
#else
// expected-error-re@-5{{C-style cast from 'int *' to '__{{global|constant}} int *' converts between mismatching address spaces}}
#endif
#endif

var_cast5 = (AS int *)arg_gen;
#ifdef CONSTANT
// expected-error@-2{{casting '__generic int *' to type '__constant int *' changes address space of pointer}}
#if !__OPENCL_CPP_VERSION__
// expected-error@-3{{casting '__generic int *' to type '__constant int *' changes address space of pointer}}
#else
// expected-error@-5{{C-style cast from '__generic int *' to '__constant int *' converts between mismatching address spaces}}
#endif
#endif

AS int *var_cmp;
Expand Down
Loading

0 comments on commit d8ac2ca

Please sign in to comment.