Skip to content

Commit 87da2d5

Browse files
committed
[C++ Safe Buffers][BoundsSafety] Fix that some interop diagnostics cannot be suppressed by __unsafe_buffer_usage_begin/end
The interop diagnostics emitted in Sema cannot be suppressed by `#pragma clang unsafe_buffer_usage begin/end`. This is because the opt-out regions marked by those pragmas are stored in Preprocessor not DiagnosticsEngine. We need query both the Preprocessor and DiagnosticsEngine to confirm if C++ Safe Buffers is enabled at a location. This commit fixes this problem. (rdar://146438364)
1 parent 293ff41 commit 87da2d5

File tree

4 files changed

+51
-4
lines changed

4 files changed

+51
-4
lines changed

clang/include/clang/Lex/Preprocessor.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3019,6 +3019,11 @@ class Preprocessor {
30193019
} LoadedSafeBufferOptOutMap;
30203020

30213021
public:
3022+
// FIXME: The result of saving Safe Buffers opt-out regions in PP is that
3023+
// clang needs to check two places---PP and DiagnosticsEngine, in order to
3024+
// confirm if C++ Safe Buffers is enabled at a location. This might not be
3025+
// ideal as developers can forget to check one of the places.
3026+
//
30223027
/// \return true iff the given `Loc` is in a "-Wunsafe-buffer-usage" opt-out
30233028
/// region. This `Loc` must be a source location that has been pre-processed.
30243029
bool isSafeBufferOptOut(const SourceManager&SourceMgr, const SourceLocation &Loc) const;

clang/include/clang/Sema/Sema.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1563,10 +1563,7 @@ class Sema final : public SemaBase {
15631563

15641564
/// \return true iff `-Wunsafe-buffer-usage` is enabled for `Loc` and Bounds
15651565
/// Safety attribute-only mode is on.
1566-
bool isCXXSafeBuffersBoundsSafetyInteropEnabledAt(SourceLocation Loc) const {
1567-
return LangOpts.CPlusPlus && LangOpts.isBoundsSafetyAttributeOnlyMode() &&
1568-
!Diags.isIgnored(diag::warn_unsafe_buffer_operation, Loc);
1569-
}
1566+
bool isCXXSafeBuffersBoundsSafetyInteropEnabledAt(SourceLocation Loc) const;
15701567
/* TO_UPSTREAM(BoundsSafety) OFF */
15711568

15721569
/// pragma clang section kind

clang/lib/Sema/Sema.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,20 @@
7979
using namespace clang;
8080
using namespace sema;
8181

82+
/* TO_UPSTREAM(BoundsSafety) ON */
83+
bool Sema::isCXXSafeBuffersBoundsSafetyInteropEnabledAt(
84+
SourceLocation Loc) const {
85+
// Informations of '#pragma clang unsafe_buffer_usage begin/end' are stored
86+
// Preprocessor. So we need also check
87+
// `getPreprocessor().isSafeBufferOptOut`.
88+
bool NotSupressedByPragmas =
89+
Loc.isInvalid() || !getPreprocessor().isSafeBufferOptOut(SourceMgr, Loc);
90+
return LangOpts.CPlusPlus && LangOpts.isBoundsSafetyAttributeOnlyMode() &&
91+
NotSupressedByPragmas &&
92+
!Diags.isIgnored(diag::warn_unsafe_buffer_operation, Loc);
93+
}
94+
/* TO_UPSTREAM(BoundsSafety) OFF */
95+
8296
SourceLocation Sema::getLocForEndOfToken(SourceLocation Loc, unsigned Offset) {
8397
return Lexer::getLocForEndOfToken(Loc, Offset, SourceMgr, LangOpts);
8498
}

clang/test/SemaCXX/warn-unsafe-buffer-usage-null-terminated.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,37 @@ C test_class(char * cstr) {
188188
return {"hello", {}};
189189
}
190190

191+
#pragma clang unsafe_buffer_usage begin
192+
void basics_suppressed(const char * cstr, size_t cstr_len, std::string cxxstr) {
193+
const char * __null_terminated p = "hello"; // safe init
194+
195+
nt_parm(p);
196+
nt_parm(get_nt(cstr, cstr_len));
197+
198+
const char * __null_terminated p2 = cxxstr.c_str(); // safe init
199+
const char * __null_terminated p3;
200+
201+
p3 = cxxstr.c_str(); // safe assignment
202+
p3 = "hello"; // safe assignment
203+
p3 = p; // safe assignment
204+
p3 = get_nt(cstr, cstr_len); // safe assignment
205+
206+
const char * __null_terminated p4 = cstr; // warn
207+
208+
nt_parm(cstr); // warn
209+
p4 = cstr; // warn
210+
211+
std::string_view view{cxxstr};
212+
213+
p4 = view.data(); // warn
214+
nt_parm(view.data()); // warn
215+
216+
const char * __null_terminated p5 = 0; // nullptr is ok
217+
const char * __null_terminated p6 = (const char *)1; // other integer literal is unsafe
218+
// (non-0)-terminated pointer is NOT compatible with 'std::string::c_str':
219+
const char * __terminated_by(42) p7 = cxxstr.c_str();
220+
}
221+
#pragma clang unsafe_buffer_usage end
191222

192223
// Test input/output __null_terminated parameter.
193224
// expected-note@+1 {{candidate function not viable:}}

0 commit comments

Comments
 (0)