Skip to content

Commit 45ecf12

Browse files
authored
[C++ Safe Buffers][BoundsSafety] Fix that some interop diagnostics cannot be suppressed by __unsafe_buffer_usage_begin/end (llvm#10206)
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) (cherry picked from commit 87da2d5)
1 parent 1c65036 commit 45ecf12

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
@@ -3018,6 +3018,11 @@ class Preprocessor {
30183018
} LoadedSafeBufferOptOutMap;
30193019

30203020
public:
3021+
// FIXME: The result of saving Safe Buffers opt-out regions in PP is that
3022+
// clang needs to check two places---PP and DiagnosticsEngine, in order to
3023+
// confirm if C++ Safe Buffers is enabled at a location. This might not be
3024+
// ideal as developers can forget to check one of the places.
3025+
//
30213026
/// \return true iff the given `Loc` is in a "-Wunsafe-buffer-usage" opt-out
30223027
/// region. This `Loc` must be a source location that has been pre-processed.
30233028
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
@@ -1566,10 +1566,7 @@ class Sema final : public SemaBase {
15661566

15671567
/// \return true iff `-Wunsafe-buffer-usage` is enabled for `Loc` and Bounds
15681568
/// 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-
}
1569+
bool isCXXSafeBuffersBoundsSafetyInteropEnabledAt(SourceLocation Loc) const;
15731570
/* TO_UPSTREAM(BoundsSafety) OFF */
15741571

15751572
/// pragma clang section kind

clang/lib/Sema/Sema.cpp

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

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

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)