danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 1 | // Copyright 2024 The Chromium Authors |
| 2 | // Use of this source code is governed by a BSD-style license that can be |
| 3 | // found in the LICENSE file. |
| 4 | |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 5 | #include "Util.h" |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 6 | #include "clang/AST/ASTConsumer.h" |
| 7 | #include "clang/Basic/DiagnosticSema.h" |
| 8 | #include "clang/Frontend/CompilerInstance.h" |
| 9 | #include "clang/Frontend/FrontendAction.h" |
| 10 | #include "clang/Frontend/FrontendPluginRegistry.h" |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 11 | #include "clang/Lex/Pragma.h" |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 12 | #include "llvm/ADT/StringMap.h" |
| 13 | #include "llvm/ADT/StringRef.h" |
| 14 | #include "llvm/Support/MemoryBuffer.h" |
| 15 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 16 | namespace chrome_checker { |
| 17 | |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 18 | enum Disposition { |
| 19 | kSkip = 0, // Do not check for any unsafe operations. |
| 20 | kSkipLibc, // Check for unsafe buffers but not unsafe libc calls. |
| 21 | kCheck, // Check for both unsafe buffers and unsafe libc calls. |
| 22 | }; |
| 23 | |
| 24 | // Stores whether the filename (key) should be checked for errors. |
| 25 | // If the filename is not present, the choice is up to the plugin to |
| 26 | // determine from the path prefixes control file. |
| 27 | llvm::StringMap<Disposition> g_checked_files_cache; |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 28 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 29 | struct CheckFilePrefixes { |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 30 | // `buffer` owns the memory for the strings in `prefix_map`. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 31 | std::unique_ptr<llvm::MemoryBuffer> buffer; |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 32 | std::map<llvm::StringRef, char> prefix_map; |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 33 | bool check_buffers = true; |
Tom Sepez | 639ca5a | 2024-12-17 18:59:59 | [diff] [blame] | 34 | bool check_libc_calls = false; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 35 | }; |
| 36 | |
| 37 | class UnsafeBuffersDiagnosticConsumer : public clang::DiagnosticConsumer { |
| 38 | public: |
| 39 | UnsafeBuffersDiagnosticConsumer(clang::DiagnosticsEngine* engine, |
| 40 | clang::DiagnosticConsumer* next, |
| 41 | clang::CompilerInstance* instance, |
| 42 | CheckFilePrefixes check_file_prefixes) |
| 43 | : engine_(engine), |
| 44 | next_(next), |
| 45 | instance_(instance), |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 46 | check_file_prefixes_(std::move(check_file_prefixes)), |
| 47 | diag_note_link_(engine_->getCustomDiagID( |
| 48 | clang::DiagnosticsEngine::Level::Note, |
| 49 | "See //docs/unsafe_buffers.md for help.")) {} |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 50 | ~UnsafeBuffersDiagnosticConsumer() override = default; |
| 51 | |
| 52 | void clear() override { |
| 53 | if (next_) { |
| 54 | next_->clear(); |
| 55 | NumErrors = next_->getNumErrors(); |
| 56 | NumWarnings = next_->getNumWarnings(); |
| 57 | } |
| 58 | } |
| 59 | |
| 60 | void BeginSourceFile(const clang::LangOptions& opts, |
| 61 | const clang::Preprocessor* pp) override { |
| 62 | if (next_) { |
| 63 | next_->BeginSourceFile(opts, pp); |
| 64 | NumErrors = next_->getNumErrors(); |
| 65 | NumWarnings = next_->getNumWarnings(); |
| 66 | } |
| 67 | } |
| 68 | |
| 69 | void EndSourceFile() override { |
| 70 | if (next_) { |
| 71 | next_->EndSourceFile(); |
| 72 | NumErrors = next_->getNumErrors(); |
| 73 | NumWarnings = next_->getNumWarnings(); |
| 74 | } |
| 75 | } |
| 76 | |
| 77 | void finish() override { |
| 78 | if (next_) { |
| 79 | next_->finish(); |
| 80 | NumErrors = next_->getNumErrors(); |
| 81 | NumWarnings = next_->getNumWarnings(); |
| 82 | } |
| 83 | } |
| 84 | |
| 85 | bool IncludeInDiagnosticCounts() const override { |
| 86 | return next_ && next_->IncludeInDiagnosticCounts(); |
| 87 | } |
| 88 | |
| 89 | void HandleDiagnostic(clang::DiagnosticsEngine::Level level, |
| 90 | const clang::Diagnostic& diag) override { |
| 91 | const unsigned diag_id = diag.getID(); |
| 92 | |
| 93 | if (inside_handle_diagnostic_) { |
| 94 | // Avoid handling the diagnostics which we emit in here. |
| 95 | return PassthroughDiagnostic(level, diag); |
| 96 | } |
| 97 | |
danakj | a8ecec3 | 2024-03-08 19:47:47 | [diff] [blame] | 98 | // The `-Runsafe-buffer-usage-in-container` warning gets enabled along with |
| 99 | // `-Runsafe-buffer-usage`, but it's a hardcoded warning about std::span |
| 100 | // constructor. We don't want to emit these, we instead want the span ctor |
| 101 | // (and our own base::span ctor) to be marked [[clang::unsafe_buffer_usage]] |
| 102 | // and have that work: https://github.com/llvm/llvm-project/issues/80482 |
| 103 | if (diag_id == clang::diag::warn_unsafe_buffer_usage_in_container) { |
| 104 | return; |
| 105 | } |
| 106 | |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 107 | // Drop the note saying "pass -fsafe-buffer-usage-suggestions to receive |
| 108 | // code hardening suggestions" since that's not simple for Chrome devs to |
| 109 | // do anyway. We can provide a GN variable in the future and point to that |
| 110 | // if needed, or just turn it on always in this plugin, if desired. |
| 111 | if (diag_id == clang::diag::note_safe_buffer_usage_suggestions_disabled) { |
| 112 | return; |
| 113 | } |
| 114 | |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 115 | const bool is_buffers_diagnostic = |
| 116 | diag_id == clang::diag::warn_unsafe_buffer_variable || |
| 117 | diag_id == clang::diag::warn_unsafe_buffer_operation || |
| 118 | diag_id == clang::diag::note_unsafe_buffer_operation || |
| 119 | diag_id == clang::diag::note_unsafe_buffer_variable_fixit_group || |
| 120 | diag_id == clang::diag::note_unsafe_buffer_variable_fixit_together || |
| 121 | diag_id == clang::diag::note_safe_buffer_debug_mode; |
| 122 | |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 123 | const bool is_libc_diagnostic = |
| 124 | diag_id == clang::diag::warn_unsafe_buffer_libc_call || |
| 125 | diag_id == clang::diag::note_unsafe_buffer_printf_call; |
| 126 | |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 127 | const bool ignore_diagnostic = |
| 128 | (is_buffers_diagnostic && !check_file_prefixes_.check_buffers) || |
| 129 | (is_libc_diagnostic && !check_file_prefixes_.check_libc_calls); |
| 130 | |
| 131 | if (ignore_diagnostic) { |
| 132 | return; |
| 133 | } |
| 134 | |
| 135 | const bool handle_diagnostic = |
| 136 | (is_buffers_diagnostic && check_file_prefixes_.check_buffers) || |
| 137 | (is_libc_diagnostic && check_file_prefixes_.check_libc_calls); |
| 138 | |
| 139 | if (!handle_diagnostic) { |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 140 | return PassthroughDiagnostic(level, diag); |
| 141 | } |
| 142 | |
| 143 | // Note that we promote from Remark directly to Error, rather than to |
| 144 | // Warning, as -Werror will not get applied to whatever we choose here. |
| 145 | const auto elevated_level = |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 146 | (is_libc_diagnostic || |
Tom Sepez | 639ca5a | 2024-12-17 18:59:59 | [diff] [blame] | 147 | diag_id == clang::diag::warn_unsafe_buffer_variable || |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 148 | diag_id == clang::diag::warn_unsafe_buffer_operation) |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 149 | ? (engine_->getWarningsAsErrors() |
| 150 | ? clang::DiagnosticsEngine::Level::Error |
| 151 | : clang::DiagnosticsEngine::Level::Warning) |
| 152 | : clang::DiagnosticsEngine::Level::Note; |
| 153 | |
| 154 | const clang::SourceManager& sm = instance_->getSourceManager(); |
| 155 | const clang::SourceLocation loc = diag.getLocation(); |
| 156 | |
| 157 | // -Wunsage-buffer-usage errors are omitted conditionally based on what file |
| 158 | // they are coming from. |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 159 | auto disposition = FileHasSafeBuffersWarnings(sm, loc); |
| 160 | if (disposition == kSkip || |
| 161 | (is_libc_diagnostic && disposition == kSkipLibc)) { |
| 162 | return; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 163 | } |
Tom Sepez | ccf2bd08 | 2025-02-21 18:43:10 | [diff] [blame] | 164 | |
| 165 | // More selectively filter the libc calls we enforce. |
| 166 | if (is_libc_diagnostic && IsIgnoredLibcFunction(diag)) { |
| 167 | return; |
| 168 | } |
| 169 | |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 170 | // Elevate the Remark to a Warning, and pass along its Notes without |
| 171 | // changing them. Otherwise, do nothing, and the Remark (and its notes) |
| 172 | // will not be displayed. |
| 173 | // |
| 174 | // We don't count warnings/errors in this DiagnosticConsumer, so we don't |
| 175 | // call up to the base class here. Instead, whenever we pass through to |
| 176 | // the `next_` DiagnosticConsumer, we record its counts. |
| 177 | // |
| 178 | // Construct the StoredDiagnostic before Clear() or we get bad data from |
| 179 | // `diag`. |
| 180 | auto stored = clang::StoredDiagnostic(elevated_level, diag); |
| 181 | inside_handle_diagnostic_ = true; |
| 182 | engine_->Report(stored); |
| 183 | if (elevated_level != clang::DiagnosticsEngine::Level::Note) { |
| 184 | // For each warning, we inject our own Note as well, pointing to docs. |
| 185 | engine_->Report(loc, diag_note_link_); |
| 186 | } |
| 187 | inside_handle_diagnostic_ = false; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 188 | } |
| 189 | |
| 190 | private: |
| 191 | void PassthroughDiagnostic(clang::DiagnosticsEngine::Level level, |
| 192 | const clang::Diagnostic& diag) { |
| 193 | if (next_) { |
| 194 | next_->HandleDiagnostic(level, diag); |
| 195 | NumErrors = next_->getNumErrors(); |
| 196 | NumWarnings = next_->getNumWarnings(); |
| 197 | } |
| 198 | } |
| 199 | |
| 200 | // Depending on where the diagnostic is coming from, we may ignore it or |
| 201 | // cause it to generate a warning. |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 202 | Disposition FileHasSafeBuffersWarnings(const clang::SourceManager& sm, |
| 203 | clang::SourceLocation loc) { |
danakj | 69209bf | 2024-04-19 14:51:05 | [diff] [blame] | 204 | // ClassifySourceLocation() does not report kMacro as the location unless it |
| 205 | // happens to be inside a scratch buffer, which not all macro use does. For |
| 206 | // the unsafe-buffers warning, we want the SourceLocation where the macro is |
| 207 | // expanded to always be the decider about whether to fire a warning or not. |
| 208 | // |
| 209 | // The reason we do this is that the expansion site should be wrapped in |
| 210 | // UNSAFE_BUFFERS() if the unsafety is warranted. It can be done inside the |
| 211 | // macro itself too (in which case the warning will not fire), but the |
| 212 | // finest control is always at each expansion site. |
| 213 | while (loc.isMacroID()) { |
| 214 | loc = sm.getExpansionLoc(loc); |
| 215 | } |
| 216 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 217 | // TODO(crbug.com/40284755): Expand this diagnostic to more code. It should |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 218 | // include everything except kSystem eventually. |
| 219 | LocationClassification loc_class = |
| 220 | ClassifySourceLocation(instance_->getHeaderSearchOpts(), sm, loc); |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 221 | switch (loc_class) { |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 222 | case LocationClassification::kSystem: |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 223 | return kSkip; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 224 | case LocationClassification::kGenerated: |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 225 | return kSkip; |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 226 | case LocationClassification::kThirdParty: |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 227 | case LocationClassification::kChromiumThirdParty: |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 228 | case LocationClassification::kFirstParty: |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 229 | case LocationClassification::kBlink: |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 230 | case LocationClassification::kMacro: |
danakj | 69209bf | 2024-04-19 14:51:05 | [diff] [blame] | 231 | break; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 232 | } |
| 233 | |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 234 | // We default to everything opting into checks (except categories that early |
| 235 | // out above) unless it is removed by the paths control file or by pragma. |
| 236 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 237 | // TODO(danakj): It would be an optimization to find a way to avoid creating |
| 238 | // a std::string here. |
danakj | 69209bf | 2024-04-19 14:51:05 | [diff] [blame] | 239 | std::string filename = GetFilename(sm, loc, FilenameLocationType::kExactLoc, |
| 240 | FilenamesFollowPresumed::kNo); |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 241 | |
| 242 | // Avoid searching `check_file_prefixes_` more than once for a file. |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 243 | auto cache_it = g_checked_files_cache.find(filename); |
| 244 | if (cache_it != g_checked_files_cache.end()) { |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 245 | return cache_it->second; |
| 246 | } |
| 247 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 248 | llvm::StringRef cmp_filename = filename; |
danakj | 4af79fa | 2024-04-23 18:54:35 | [diff] [blame] | 249 | |
| 250 | // If the path is absolute, drop the prefix up to the current working |
| 251 | // directory. Some mac machines are passing absolute paths to source files, |
| 252 | // but it's the absolute path to the build directory (the current working |
| 253 | // directory here) then a relative path from there. |
| 254 | llvm::SmallVector<char> cwd; |
| 255 | if (llvm::sys::fs::current_path(cwd).value() == 0) { |
| 256 | if (cmp_filename.consume_front(llvm::StringRef(cwd.data(), cwd.size()))) { |
| 257 | cmp_filename.consume_front("/"); |
| 258 | } |
| 259 | } |
| 260 | |
| 261 | // Drop the ../ prefixes. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 262 | while (cmp_filename.consume_front("./") || |
| 263 | cmp_filename.consume_front("../")) |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 264 | continue; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 265 | |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 266 | Disposition should_check = kCheck; |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 267 | while (!cmp_filename.empty()) { |
| 268 | auto it = check_file_prefixes_.prefix_map.find(cmp_filename); |
| 269 | if (it != check_file_prefixes_.prefix_map.end()) { |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 270 | should_check = it->second == '+' ? kCheck : kSkip; |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 271 | break; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 272 | } |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 273 | cmp_filename = llvm::sys::path::parent_path(cmp_filename); |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 274 | } |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 275 | g_checked_files_cache.insert({filename, should_check}); |
| 276 | return should_check; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 277 | } |
| 278 | |
Tom Sepez | ccf2bd08 | 2025-02-21 18:43:10 | [diff] [blame] | 279 | bool IsIgnoredLibcFunction(const clang::Diagnostic& diag) const { |
| 280 | // The unsafe libc calls warning is a wee bit overzealous about |
| 281 | // functions which might result in a OOB read only. |
| 282 | if (diag.getNumArgs() < 1) { |
| 283 | return false; |
| 284 | } |
| 285 | if (diag.getArgKind(0) != |
| 286 | clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl) { |
| 287 | return false; |
| 288 | } |
| 289 | auto* decl = reinterpret_cast<clang::NamedDecl*>(diag.getRawArg(0)); |
| 290 | llvm::StringRef name = decl->getName(); |
Tom Sepez | 2ad168c | 2025-03-18 18:27:19 | [diff] [blame] | 291 | return name == "strlen" || name == "wcslen" || name == "atoi" || |
| 292 | name == "atof"; |
Tom Sepez | ccf2bd08 | 2025-02-21 18:43:10 | [diff] [blame] | 293 | } |
| 294 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 295 | // Used to prevent recursing into HandleDiagnostic() when we're emitting a |
| 296 | // diagnostic from that function. |
| 297 | bool inside_handle_diagnostic_ = false; |
| 298 | clang::DiagnosticsEngine* engine_; |
| 299 | clang::DiagnosticConsumer* next_; |
| 300 | clang::CompilerInstance* instance_; |
| 301 | CheckFilePrefixes check_file_prefixes_; |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 302 | unsigned diag_note_link_; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 303 | }; |
| 304 | |
| 305 | class UnsafeBuffersASTConsumer : public clang::ASTConsumer { |
| 306 | public: |
| 307 | UnsafeBuffersASTConsumer(clang::CompilerInstance* instance, |
| 308 | CheckFilePrefixes check_file_prefixes) |
| 309 | : instance_(instance) { |
| 310 | // Replace the DiagnosticConsumer with our own that sniffs diagnostics and |
| 311 | // can omit them. |
| 312 | clang::DiagnosticsEngine& engine = instance_->getDiagnostics(); |
| 313 | old_client_ = engine.getClient(); |
| 314 | old_owned_client_ = engine.takeClient(); |
| 315 | engine.setClient( |
| 316 | new UnsafeBuffersDiagnosticConsumer(&engine, old_client_, instance_, |
| 317 | std::move(check_file_prefixes)), |
| 318 | /*owned=*/true); |
| 319 | |
| 320 | // Enable the -Wunsafe-buffer-usage warning as a remark. This prevents it |
| 321 | // from stopping compilation, even with -Werror. If we see the remark go by, |
| 322 | // we can re-emit it as a warning for the files we want to include in the |
| 323 | // check. |
| 324 | engine.setSeverityForGroup(clang::diag::Flavor::WarningOrError, |
| 325 | "unsafe-buffer-usage", |
| 326 | clang::diag::Severity::Remark); |
Arthur Eubanks | 79d6771 | 2024-09-09 21:23:31 | [diff] [blame] | 327 | |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 328 | // Enable the -Wunsafe-buffer-usage-in-libc-call warning as a remark. This |
| 329 | // prevents it from stopping compilation, even with -Werror. If we see the |
| 330 | // remark go by, we can re-emit it as a warning for the files we want to |
| 331 | // include in the check. |
Arthur Eubanks | 79d6771 | 2024-09-09 21:23:31 | [diff] [blame] | 332 | engine.setSeverityForGroup(clang::diag::Flavor::WarningOrError, |
| 333 | "unsafe-buffer-usage-in-libc-call", |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 334 | clang::diag::Severity::Remark); |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 335 | } |
| 336 | |
| 337 | ~UnsafeBuffersASTConsumer() { |
| 338 | // Restore the original DiagnosticConsumer that we replaced with our own. |
| 339 | clang::DiagnosticsEngine& engine = instance_->getDiagnostics(); |
| 340 | if (old_owned_client_) { |
| 341 | engine.setClient(old_owned_client_.release(), |
| 342 | /*owned=*/true); |
| 343 | } else { |
| 344 | engine.setClient(old_client_, /*owned=*/false); |
| 345 | } |
| 346 | } |
| 347 | |
| 348 | private: |
| 349 | clang::CompilerInstance* instance_; |
| 350 | clang::DiagnosticConsumer* old_client_; |
| 351 | std::unique_ptr<clang::DiagnosticConsumer> old_owned_client_; |
| 352 | }; |
| 353 | |
| 354 | class UnsafeBuffersASTAction : public clang::PluginASTAction { |
| 355 | public: |
| 356 | std::unique_ptr<clang::ASTConsumer> CreateASTConsumer( |
| 357 | clang::CompilerInstance& instance, |
| 358 | llvm::StringRef ref) override { |
| 359 | assert(!moved_prefixes_); // This would mean we move the prefixes twice. |
| 360 | moved_prefixes_ = true; |
| 361 | |
| 362 | // The ASTConsumer can outlive `this`, so we can't give it references to |
| 363 | // members here and must move the `check_file_prefixes_` vector instead. |
| 364 | return std::make_unique<UnsafeBuffersASTConsumer>( |
| 365 | &instance, std::move(check_file_prefixes_)); |
| 366 | } |
| 367 | |
| 368 | bool ParseArgs(const clang::CompilerInstance& instance, |
| 369 | const std::vector<std::string>& args) override { |
| 370 | bool found_file_arg = false; |
Tom Sepez | 639ca5a | 2024-12-17 18:59:59 | [diff] [blame] | 371 | for (const auto& arg : args) { |
| 372 | // Nothing should follow the unsafe buffers path positional argument. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 373 | if (found_file_arg) { |
| 374 | llvm::errs() |
| 375 | << "[unsafe-buffers] Extra argument to unsafe-buffers plugin: '" |
Tom Sepez | 639ca5a | 2024-12-17 18:59:59 | [diff] [blame] | 376 | << arg << ". Usage: [SWITCHES] PATH_TO_CHECK_FILE'\n"; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 377 | return false; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 378 | } |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 379 | |
| 380 | // Switches, if any, would go here. |
| 381 | |
Tom Sepez | 639ca5a | 2024-12-17 18:59:59 | [diff] [blame] | 382 | // Anything not recognized as a switch is the unsafe buffer paths file. |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 383 | found_file_arg = true; |
Tom Sepez | 639ca5a | 2024-12-17 18:59:59 | [diff] [blame] | 384 | if (!LoadCheckFilePrefixes(arg)) { |
| 385 | llvm::errs() << "[unsafe-buffers] Failed to load paths from file '" |
| 386 | << arg << "'\n"; |
| 387 | return false; |
| 388 | } |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 389 | } |
| 390 | return true; |
| 391 | } |
| 392 | |
| 393 | bool LoadCheckFilePrefixes(std::string_view path) { |
| 394 | if (auto buffer = llvm::MemoryBuffer::getFileAsStream(path)) { |
| 395 | check_file_prefixes_.buffer = std::move(buffer.get()); |
| 396 | } else { |
| 397 | llvm::errs() << "[unsafe-buffers] Error reading file: '" |
| 398 | << buffer.getError().message() << "'\n"; |
| 399 | return false; |
| 400 | } |
| 401 | |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 402 | // Parse out the paths into `check_file_prefixes_`. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 403 | // |
| 404 | // The file format is as follows: |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 405 | // * `#` introduces a comment until the end of the line. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 406 | // * Empty lines are ignored. |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 407 | // * A line beginning with a `.` lists diagnostics to enable. These |
| 408 | // are comma-separated and currently allow: `buffers`, `libc`. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 409 | // * Every other line is a path prefix from the source tree root using |
| 410 | // unix-style delimiters. |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 411 | // * Each line either removes a path from checks or adds a path to checks. |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 412 | // * If the line starts with `+` paths matching the line will be added. |
| 413 | // * If the line starts with `-` paths matching the line will removed. |
| 414 | // * Other starting characters are not allowed. |
| 415 | // * Paths naming directories match the entire sub-directory. For instance |
| 416 | // `+a/b/` will match the file at `//a/b/c.h` but will *not* match |
| 417 | // `//other/a/b/c.h`. |
| 418 | // * Paths naming files match the single file and look like `+a/b/c.h`. |
| 419 | // * Trailing slashes for directories are recommended, but not enforced. |
| 420 | // * The longest (most specific) match takes precedence. |
| 421 | // * Files that do not match any of the prefixes file will be checked. |
| 422 | // * Duplicate entries are not allowed and produce compilation errors. |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 423 | // |
| 424 | // Example: |
| 425 | // ``` |
| 426 | // # A file of path prefixes. |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 427 | // # Matches anything under the directory //foo/bar, opting them into |
| 428 | // # checks. |
| 429 | // +foo/bar/ |
| 430 | // # Avoids checks in the //my directory. |
| 431 | // -my/ |
| 432 | // # Matches a specific file at //my/file.cc, overriding the `-my/` above |
| 433 | // # for this one file. |
| 434 | // +my/file.cc |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 435 | // |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 436 | llvm::StringRef string = check_file_prefixes_.buffer->getBuffer(); |
| 437 | while (!string.empty()) { |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 438 | auto [line, remainder] = string.split('\n'); |
| 439 | string = remainder; |
| 440 | auto [active, comment] = line.split('#'); |
| 441 | active = active.trim(); |
| 442 | if (active.empty()) { |
| 443 | continue; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 444 | } |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 445 | char symbol = active[0u]; |
Tom Sepez | b1a8156 | 2025-02-20 17:28:54 | [diff] [blame] | 446 | if (symbol == '.') { |
| 447 | // A "dot" line contains directives to enable. |
| 448 | if (active.contains("buffers")) { |
| 449 | check_file_prefixes_.check_buffers = true; |
| 450 | } |
| 451 | if (active.contains("libc")) { |
| 452 | check_file_prefixes_.check_libc_calls = true; |
| 453 | } |
| 454 | continue; |
| 455 | } |
Tom Sepez | 4157e2d | 2024-12-16 20:25:33 | [diff] [blame] | 456 | if (symbol != '+' && symbol != '-') { |
| 457 | llvm::errs() << "[unsafe-buffers] Invalid line in paths file, must " |
| 458 | << "start with +/-: '" << line << "'\n"; |
| 459 | return false; |
| 460 | } |
| 461 | llvm::StringRef prefix = active.substr(1u).rtrim('/'); |
| 462 | if (prefix.empty()) { |
| 463 | llvm::errs() << "[unsafe-buffers] Invalid line in paths file, path " |
| 464 | << "must immediately follow +/-: '" << line << "'\n"; |
| 465 | return false; |
| 466 | } |
| 467 | auto [ignore, was_inserted] = |
| 468 | check_file_prefixes_.prefix_map.insert({prefix, symbol}); |
| 469 | if (!was_inserted) { |
| 470 | llvm::errs() << "[unsafe-buffers] Duplicate entry in paths file " |
| 471 | "for '" |
| 472 | << line << "'\n"; |
| 473 | return false; |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 474 | } |
| 475 | } |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 476 | return true; |
| 477 | } |
| 478 | |
| 479 | private: |
| 480 | CheckFilePrefixes check_file_prefixes_; |
| 481 | bool moved_prefixes_ = false; |
| 482 | }; |
| 483 | |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 484 | class AllowUnsafeBuffersPragmaHandler : public clang::PragmaHandler { |
| 485 | public: |
| 486 | static constexpr char kName[] = "allow_unsafe_buffers"; |
| 487 | |
| 488 | AllowUnsafeBuffersPragmaHandler() : clang::PragmaHandler(kName) {} |
| 489 | |
| 490 | void HandlePragma(clang::Preprocessor& preprocessor, |
| 491 | clang::PragmaIntroducer introducer, |
| 492 | clang::Token& token) override { |
| 493 | // TODO(danakj): It would be an optimization to find a way to avoid creating |
| 494 | // a std::string here. |
| 495 | std::string filename = |
danakj | 69209bf | 2024-04-19 14:51:05 | [diff] [blame] | 496 | GetFilename(preprocessor.getSourceManager(), introducer.Loc, |
| 497 | FilenameLocationType::kExpansionLoc); |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 498 | // The pragma opts the file out of checks. |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 499 | g_checked_files_cache.insert({filename, kSkip}); |
| 500 | } |
| 501 | }; |
| 502 | |
| 503 | class AllowUnsafeLibcPragmaHandler : public clang::PragmaHandler { |
| 504 | public: |
| 505 | static constexpr char kName[] = "allow_unsafe_libc_calls"; |
| 506 | |
| 507 | AllowUnsafeLibcPragmaHandler() : clang::PragmaHandler(kName) {} |
| 508 | |
| 509 | void HandlePragma(clang::Preprocessor& preprocessor, |
| 510 | clang::PragmaIntroducer introducer, |
| 511 | clang::Token& token) override { |
| 512 | // TODO(danakj): It would be an optimization to find a way to avoid creating |
| 513 | // a std::string here. |
| 514 | std::string filename = |
| 515 | GetFilename(preprocessor.getSourceManager(), introducer.Loc, |
| 516 | FilenameLocationType::kExpansionLoc); |
| 517 | // The pragma opts the file into checks. |
| 518 | g_checked_files_cache.insert({filename, kSkipLibc}); |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 519 | } |
| 520 | }; |
| 521 | |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 522 | static clang::FrontendPluginRegistry::Add<UnsafeBuffersASTAction> X1( |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 523 | "unsafe-buffers", |
| 524 | "Enforces -Wunsafe-buffer-usage during incremental rollout"); |
| 525 | |
danakj | e7db1e3f3 | 2024-04-16 20:43:24 | [diff] [blame] | 526 | static clang::PragmaHandlerRegistry::Add<AllowUnsafeBuffersPragmaHandler> X2( |
| 527 | AllowUnsafeBuffersPragmaHandler::kName, |
| 528 | "Avoid reporting unsafe-buffer-usage warnings in the file"); |
| 529 | |
Tom Sepez | 4474d026 | 2025-01-10 17:41:33 | [diff] [blame] | 530 | static clang::PragmaHandlerRegistry::Add<AllowUnsafeLibcPragmaHandler> X3( |
| 531 | AllowUnsafeLibcPragmaHandler::kName, |
| 532 | "Avoid reporting unsafe-libc-call warnings in the file"); |
| 533 | |
danakj | c06d5f4 | 2024-02-29 17:54:11 | [diff] [blame] | 534 | } // namespace chrome_checker |