Skip to content

Commit 32babdb

Browse files
authored
[-Wunsafe-buffer-usage][BoundsSafety] support __null_terminated in the interop mode (#10060) (#10103)
For a __null_terminated pointer 'p', only a __null_terminated pointer, a string literal or a call to `std::string::c_str/data` can be assigned to (/passed to/used to initialize, etc.) 'p'. Otherwise, an error will be emitted. The implementation reuses Bounds Safety's diagnostics. (rdar://138798346)
1 parent b3a9408 commit 32babdb

File tree

7 files changed

+275
-13
lines changed

7 files changed

+275
-13
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13053,8 +13053,10 @@ def warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by : Warning
1305313053
"sending type to parameter of incompatible type}0,1|"
1305413054
"%diff{casting $ to incompatible type $|"
1305513055
"casting type to incompatible type}0,1|"
13056-
"}2 is an unsafe operation; use '%select{__unsafe_terminated_by_from_indexable|__unsafe_null_terminated_from_indexable}3()' "
13057-
"or '%select{__unsafe_forge_terminated_by|__unsafe_forge_null_terminated}3()' to perform this conversion">, InGroup<BoundsSafetyStrictTerminatedByCast>, DefaultError;
13056+
"}2 is an unsafe operation"
13057+
"%select{; use '__unsafe_terminated_by_from_indexable()' or '__unsafe_forge_terminated_by()' to perform this conversion|"
13058+
"; use '__unsafe_null_terminated_from_indexable()' or '__unsafe_forge_null_terminated()' to perform this conversion|"
13059+
"}3">, InGroup<BoundsSafetyStrictTerminatedByCast>, DefaultError;
1305813060
def err_bounds_safety_incompatible_non_terminated_by_to_terminated_by : Error<warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by.Summary>;
1305913061
def err_bounds_safety_incompatible_terminated_by_to_non_terminated_by_mismatch : Error<
1306013062
"%select{"

clang/include/clang/Sema/Sema.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1563,6 +1563,13 @@ class Sema final : public SemaBase {
15631563
/// be controlled with \#pragma clang abi_ptr_attr set(*ATTR*). Defaults to
15641564
/// __single in user code and __unsafe_indexable in system headers.
15651565
BoundsSafetyPointerAttributes CurPointerAbi;
1566+
1567+
/// \return true iff `-Wunsafe-buffer-usage` is enabled for `Loc` and Bounds
1568+
/// Safety attribute-only mode is on.
1569+
bool isCXXSafeBuffersBoundsSafetyInteropEnabledAt(SourceLocation Loc) const {
1570+
return LangOpts.CPlusPlus && LangOpts.isBoundsSafetyAttributeOnlyMode() &&
1571+
!Diags.isIgnored(diag::warn_unsafe_buffer_operation, Loc);
1572+
}
15661573
/* TO_UPSTREAM(BoundsSafety) OFF */
15671574

15681575
/// pragma clang section kind

clang/lib/Sema/SemaCast.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ namespace {
204204
/// int *__bidi_indexable p1;
205205
/// int *__indexable p2 = (int *__indexable)p1;
206206
/// \endcode
207-
if (Self.getLangOpts().BoundsSafety) {
207+
if (Self.getLangOpts().BoundsSafety ||
208+
Self.isCXXSafeBuffersBoundsSafetyInteropEnabledAt(
209+
SrcExpr.get()->getBeginLoc())) {
208210
unsigned DiagKind = 0;
209211
bool isInvalid = false;
210212
// The type error may be nested, so any pointer can result in VT errors
@@ -237,7 +239,9 @@ namespace {
237239
const auto *DstPointerType = DestType->getAs<ValueTerminatedType>();
238240
IsDstNullTerm =
239241
DstPointerType->getTerminatorValue(Self.Context).isZero();
240-
if (Self.getLangOpts().BoundsSafetyRelaxedSystemHeaders) {
242+
if (Self.getLangOpts().BoundsSafetyRelaxedSystemHeaders ||
243+
Self.isCXXSafeBuffersBoundsSafetyInteropEnabledAt(
244+
OpRange.getBegin())) {
241245
DiagKind = diag::
242246
warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by;
243247
isInvalid = false;
@@ -282,8 +286,11 @@ namespace {
282286
warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by) {
283287
auto FDiag = Self.Diag(OpRange.getBegin(), DiagKind)
284288
<< SrcType << DestType << Sema::AA_Casting
285-
<< (IsDstNullTerm ? /*null_terminated*/ 1
286-
: /*terminated_by*/ 0);
289+
<< (!Self.isCXXSafeBuffersBoundsSafetyInteropEnabledAt(
290+
OpRange.getBegin())
291+
? (IsDstNullTerm ? /*null_terminated*/ 1
292+
: /*terminated_by*/ 0)
293+
: 2 /* cut message irrelevant to that mode*/);
287294
} else {
288295
Self.Diag(OpRange.getBegin(), DiagKind)
289296
<< SrcType << DestType << Sema::AA_Casting;

clang/lib/Sema/SemaExpr.cpp

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12090,13 +12090,39 @@ Sema::CheckValueTerminatedAssignmentConstraints(QualType LHSType,
1209012090
Context.hasSameUnqualifiedType(
1209112091
Context.getCorrespondingSignedType(LPointee),
1209212092
Context.getCorrespondingSignedType(RPointee)));
12093-
if (CompatiblePointees &&
12093+
if (CompatiblePointees && RHSExpr->isPRValue() &&
1209412094
RHSExpr->tryEvaluateTerminatorElement(Evald, Context) &&
1209512095
Evald.Val.isInt() &&
1209612096
llvm::APSInt::isSameValue(LVTT->getTerminatorValue(Context),
1209712097
Evald.Val.getInt())) {
1209812098
return Sema::Compatible;
1209912099
}
12100+
// If in C++ Safe Buffers/Bounds Safety interoperation mode, the RHS can
12101+
// be 'std::string::c_str/data':
12102+
bool isNullTerm = LVTT->getTerminatorValue(Context).isZero();
12103+
12104+
if (isNullTerm && isCXXSafeBuffersBoundsSafetyInteropEnabledAt(
12105+
RHSExpr->getBeginLoc())) {
12106+
if (const auto *MCE =
12107+
dyn_cast<CXXMemberCallExpr>(RHSExpr->IgnoreParenImpCasts())) {
12108+
IdentifierInfo *MethodDeclII = nullptr, *ObjTypeII = nullptr;
12109+
12110+
if (MCE->getMethodDecl())
12111+
MethodDeclII = MCE->getMethodDecl()->getIdentifier();
12112+
if (const auto *RecordDecl =
12113+
MCE->getObjectType()->getAsRecordDecl();
12114+
RecordDecl && RecordDecl->isInStdNamespace())
12115+
// If not in std namespace, let `ObjTypeII` be null so that the
12116+
// match fails:
12117+
ObjTypeII = RecordDecl->getIdentifier();
12118+
if (MethodDeclII && ObjTypeII &&
12119+
(MethodDeclII->getName() == "c_str" ||
12120+
// std::string::data is also null-terminated since C++11:
12121+
MethodDeclII->getName() == "data") &&
12122+
ObjTypeII->getName() == "basic_string")
12123+
return Sema::Compatible;
12124+
}
12125+
}
1210012126
}
1210112127
return Nested
1210212128
? Sema::
@@ -20535,9 +20561,11 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
2053520561
const auto *DstPointerType = DstType->getAs<ValueTerminatedType>();
2053620562
IsDstNullTerm =
2053720563
DstPointerType->getTerminatorValue(getASTContext()).isZero();
20538-
if (getLangOpts().BoundsSafetyRelaxedSystemHeaders) {
20539-
DiagKind =
20540-
diag::warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by;
20564+
20565+
if (getLangOpts().BoundsSafetyRelaxedSystemHeaders ||
20566+
isCXXSafeBuffersBoundsSafetyInteropEnabledAt(Loc)) {
20567+
DiagKind = diag::
20568+
warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by;
2054120569
isInvalid = false;
2054220570
} else {
2054320571
DiagKind =
@@ -20813,8 +20841,11 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
2081320841
diag::
2081420842
warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by) {
2081520843
FDiag << FirstType << SecondType << ActionForDiag
20816-
<< SrcExpr->getSourceRange() <<
20817-
(IsDstNullTerm ? /*null_terminated*/ 1 : /*terminated_by*/ 0);
20844+
<< SrcExpr->getSourceRange()
20845+
<< (!isCXXSafeBuffersBoundsSafetyInteropEnabledAt(Loc)
20846+
? (IsDstNullTerm ? /*null_terminated*/ 1
20847+
: /*terminated_by*/ 0)
20848+
: 2 /* cut message irrelevant to that mode*/);
2081820849
} else {
2081920850
FDiag << FirstType << SecondType << ActionForDiag
2082020851
<< SrcExpr->getSourceRange();

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4200,6 +4200,15 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
42004200
const ImplicitConversionSequence &ICS,
42014201
AssignmentAction Action,
42024202
CheckedConversionKind CCK) {
4203+
if (ToType->isPointerType() &&
4204+
isCXXSafeBuffersBoundsSafetyInteropEnabledAt(From->getBeginLoc())) {
4205+
// In C++ Safe Buffers/Bounds Safety interop mode, warn about implicit
4206+
// conversions from non-`__null_terminated` to `__null_terminated`:
4207+
AssignConvertType ConvTy =
4208+
CheckValueTerminatedAssignmentConstraints(ToType, From);
4209+
DiagnoseAssignmentResult(ConvTy, From->getExprLoc(), ToType,
4210+
From->getType(), From, Action);
4211+
}
42034212
// C++ [over.match.oper]p7: [...] operands of class type are converted [...]
42044213
if (CCK == CheckedConversionKind::ForBuiltinOverloadedOp &&
42054214
!From->getType()->isRecordType())

clang/lib/Sema/SemaInit.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8865,7 +8865,8 @@ ExprResult InitializationSequence::Perform(Sema &S,
88658865
Entity.getKind() == InitializedEntity::EK_Result);
88668866

88678867
/* TO_UPSTREAM(BoundsSafety) ON*/
8868-
if (S.LangOpts.BoundsSafety) {
8868+
if (S.LangOpts.BoundsSafety || S.isCXXSafeBuffersBoundsSafetyInteropEnabledAt(
8869+
CurInit.get()->getBeginLoc())) {
88698870
auto *Init = CurInit.get();
88708871
const auto K = Entity.getKind();
88718872
if (Init && Entity.getParent() == nullptr &&
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
// RUN: %clang_cc1 -fsyntax-only -std=c++20 -Wno-all -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions -fexperimental-bounds-safety-attributes -verify %s
2+
3+
#include <ptrcheck.h>
4+
5+
typedef unsigned size_t;
6+
namespace std {
7+
template <typename CharT>
8+
struct basic_string {
9+
const CharT *data() const noexcept;
10+
CharT *data() noexcept;
11+
const CharT *c_str() const noexcept;
12+
size_t size() const noexcept;
13+
size_t length() const noexcept;
14+
};
15+
16+
typedef basic_string<char> string;
17+
18+
template <typename CharT>
19+
struct basic_string_view {
20+
basic_string_view(basic_string<CharT> str);
21+
const CharT *data() const noexcept;
22+
size_t size() const noexcept;
23+
size_t length() const noexcept;
24+
};
25+
26+
typedef basic_string_view<char> string_view;
27+
} // namespace std
28+
29+
void nt_parm(const char * __null_terminated);
30+
const char * __null_terminated get_nt(const char *, size_t);
31+
32+
void basics(const char * cstr, size_t cstr_len, std::string cxxstr) {
33+
const char * __null_terminated p = "hello"; // safe init
34+
35+
nt_parm(p);
36+
nt_parm(get_nt(cstr, cstr_len));
37+
38+
const char * __null_terminated p2 = cxxstr.c_str(); // safe init
39+
const char * __null_terminated p3;
40+
41+
p3 = cxxstr.c_str(); // safe assignment
42+
p3 = "hello"; // safe assignment
43+
p3 = p; // safe assignment
44+
p3 = get_nt(cstr, cstr_len); // safe assignment
45+
46+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'const char *' is an unsafe operation}}
47+
const char * __null_terminated p4 = cstr; // warn
48+
49+
// expected-error@+1 {{passing 'const char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
50+
nt_parm(cstr); // warn
51+
// expected-error@+1 {{assigning to 'const char * __terminated_by(0)' (aka 'const char *') from incompatible type 'const char *' is an unsafe operation}}
52+
p4 = cstr; // warn
53+
54+
std::string_view view{cxxstr};
55+
56+
// expected-error@+1 {{assigning to 'const char * __terminated_by(0)' (aka 'const char *') from incompatible type 'const char *' is an unsafe operation}}
57+
p4 = view.data(); // warn
58+
// expected-error@+1 {{passing 'const char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
59+
nt_parm(view.data()); // warn
60+
61+
const char * __null_terminated p5 = 0; // nullptr is ok
62+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'const char *' is an unsafe operation}}
63+
const char * __null_terminated p6 = (const char *)1; // other integer literal is unsafe
64+
65+
// (non-0)-terminated pointer is NOT compatible with 'std::string::c_str':
66+
// expected-error@+1 {{initializing 'const char * __terminated_by(42)' (aka 'const char *') with an expression of incompatible type 'const char *' is an unsafe operation}}
67+
const char * __terminated_by(42) p7 = cxxstr.c_str();
68+
}
69+
70+
void test_explicit_cast(char * p, const char * q) {
71+
// expected-error@+1 {{casting 'char *' to incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
72+
const char * __null_terminated nt = (const char * __null_terminated) p;
73+
const char * __null_terminated nt2 = reinterpret_cast<const char * __null_terminated> (p); // FP for now
74+
75+
// expected-error@+1 {{casting 'char *' to incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
76+
nt = (const char * __null_terminated) p;
77+
nt2 = reinterpret_cast<const char * __null_terminated> (p); // FP for now
78+
// expected-error@+1 {{casting 'char *' to incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
79+
nt2 = static_cast<const char * __null_terminated> (p); // FP for now
80+
81+
// expected-error@+1 {{casting 'const char *' to incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
82+
const char * __null_terminated nt3 = (const char * __null_terminated) q;
83+
const char * __null_terminated nt4 = reinterpret_cast<const char * __null_terminated> (q); // FP for now
84+
85+
// expected-error@+1 {{casting 'const char *' to incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
86+
nt3 = (const char * __null_terminated) q;
87+
nt4 = reinterpret_cast<const char * __null_terminated> (q); // FP for now
88+
// expected-error@+1 {{casting 'const char *' to incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
89+
nt4 = static_cast<const char * __null_terminated> (q);
90+
91+
// OK cases for C-style casts
92+
char * __null_terminated nt_p;
93+
const char * __null_terminated nt_p2;
94+
int * __null_terminated nt_p3;
95+
const int * __null_terminated nt_p4;
96+
97+
const char * __null_terminated nt5 = (const char * __null_terminated) nt_p;
98+
const char * __null_terminated nt6 = (const char * __null_terminated) nt_p2;
99+
const char * __null_terminated nt7 = (const char * __null_terminated) nt_p3;
100+
const char * __null_terminated nt8 = (const char * __null_terminated) nt_p4;
101+
102+
nt5 = (const char * __null_terminated) nt_p;
103+
nt6 = (const char * __null_terminated) nt_p2;
104+
nt7 = (const char * __null_terminated) nt_p3;
105+
nt8 = (const char * __null_terminated) nt_p4;
106+
107+
108+
char * __null_terminated nt9 = (char * __null_terminated) nt_p;
109+
char * __null_terminated nt10 = (char * __null_terminated) nt_p2;
110+
char * __null_terminated nt11 = (char * __null_terminated) nt_p3;
111+
char * __null_terminated nt12 = (char * __null_terminated) nt_p4;
112+
113+
nt9 = (char * __null_terminated) nt_p;
114+
nt10 = (char * __null_terminated) nt_p2;
115+
nt11 = (char * __null_terminated) nt_p3;
116+
nt12 = (char * __null_terminated) nt_p4;
117+
}
118+
119+
const char * __null_terminated test_return(const char * p, char * q, std::string &str) {
120+
if (p)
121+
return p; // expected-error {{returning 'const char *' from a function with incompatible result type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
122+
if (q)
123+
return q; // expected-error {{returning 'char *' from a function with incompatible result type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
124+
return str.c_str();
125+
}
126+
127+
void test_array(char * cstr) {
128+
const char arr[__null_terminated 3] = {'h', 'i', '\0'};
129+
// expected-error@+1 {{array 'arr2' with '__terminated_by' attribute is initialized with an incorrect terminator (expected: 0; got 'i')}}
130+
const char arr2[__null_terminated 2] = {'h', 'i'};
131+
const char * __null_terminated arr3[] = {"hello", "world"};
132+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
133+
const char * __null_terminated arr4[] = {"hello", "world", cstr};
134+
135+
// expected-error@+1 {{assigning to 'const char * __terminated_by(0)' (aka 'const char *') from incompatible type 'char *' is an unsafe operation}}
136+
arr3[0] = cstr;
137+
}
138+
139+
struct T {
140+
int a;
141+
const char * __null_terminated p;
142+
struct TT {
143+
int a;
144+
const char * __null_terminated p;
145+
} tt;
146+
};
147+
void test_compound(char * cstr) {
148+
std::string str;
149+
T t = {42, "hello"};
150+
T t2 = {.a = 42};
151+
T t3 = {.p = str.c_str()};
152+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
153+
T t4 = {42, "hello", {.p = cstr}};
154+
155+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
156+
t4 = (struct T){42, "hello", {.p = cstr}};
157+
}
158+
159+
// expected-error@+3 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
160+
class C {
161+
const char * __null_terminated p;
162+
const char * __null_terminated q = (char *) 1; // warn
163+
struct T t;
164+
public:
165+
// expected-error@+1 2{{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
166+
C(char * p): p(p), t({0, p}) {};
167+
C(const char * __null_terminated p, struct T t);
168+
};
169+
170+
void f(const C &c);
171+
C test_class(char * cstr) {
172+
// expected-error@+2 {{passing 'char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
173+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
174+
C c{cstr, {0, cstr}};
175+
// expected-error@+2 {{passing 'char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
176+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
177+
C c1(cstr, {0, cstr});
178+
// expected-error@+2 {{passing 'char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
179+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
180+
C *c2 = new C(cstr, {0, cstr});
181+
// expected-error@+2 {{passing 'char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
182+
// expected-error@+1 {{initializing 'const char * __terminated_by(0)' (aka 'const char *') with an expression of incompatible type 'char *' is an unsafe operation}}
183+
f(C{cstr, {0, cstr}});
184+
185+
C("hello", {0, "hello"});
186+
if (1-1)
187+
return {cstr, {}}; // expected-error {{passing 'char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
188+
return {"hello", {}};
189+
}
190+
191+
192+
// Test input/output __null_terminated parameter.
193+
// expected-note@+1 {{candidate function not viable:}}
194+
void g(const char * __null_terminated *p);
195+
void test_output(const char * __null_terminated p) {
196+
const char * __null_terminated local_nt = p;
197+
const char * const_local;
198+
char * local;
199+
200+
g(&local_nt); // safe
201+
// expected-error@+1 {{passing 'const char **' to parameter of incompatible type 'const char * __terminated_by(0)*' (aka 'const char **') that adds '__terminated_by' attribute is not allowed}}
202+
g(&const_local);
203+
// expected-error@+1 {{no matching function for call to 'g'}}
204+
g(&local);
205+
}

0 commit comments

Comments
 (0)