diff --git a/tools/clang/blink_gc_plugin/JsonWriter.h b/tools/clang/blink_gc_plugin/JsonWriter.h index dd904e4f..d4737f63 100644 --- a/tools/clang/blink_gc_plugin/JsonWriter.h +++ b/tools/clang/blink_gc_plugin/JsonWriter.h
@@ -42,15 +42,15 @@ } void Write(const std::string& val) { Separator(); - *os_ << "\"" << val << "\""; + *os_ << "\"" << Escape(val) << "\""; } void Write(const std::string& key, const size_t val) { Separator(); - *os_ << "\"" << key << "\":" << val; + *os_ << "\"" << Escape(key) << "\":" << val; } void Write(const std::string& key, const std::string& val) { Separator(); - *os_ << "\"" << key << "\":\"" << val << "\""; + *os_ << "\"" << Escape(key) << "\":\"" << Escape(val) << "\""; } private: JsonWriter(std::unique_ptr<llvm::raw_ostream> os) : os_(std::move(os)) {} @@ -63,6 +63,15 @@ } state_.top() = true; } + std::string Escape(const std::string& s) { + std::string copy = s; + size_t i = 0; + while ((i = copy.find('\\', i)) != std::string::npos) { + copy.replace(i, 1, "\\\\"); + i += 2; + } + return copy; + } std::unique_ptr<llvm::raw_ostream> os_; std::stack<bool> state_; };
diff --git a/tools/clang/plugins/FindBadConstructsAction.cpp b/tools/clang/plugins/FindBadConstructsAction.cpp index 472fb8b..8d5c67a7 100644 --- a/tools/clang/plugins/FindBadConstructsAction.cpp +++ b/tools/clang/plugins/FindBadConstructsAction.cpp
@@ -25,6 +25,14 @@ // that matches paths that should be excluded from the raw_ptr checks. const char kRawPtrExcludePathArgPrefix[] = "raw-ptr-exclude-path="; +// Name of a cmdline parameter that can be used to add a regular expressions +// that matches function names that should be excluded from the bad raw_ptr cast +// checks. All implicit casts in CallExpr to the specified functions are +// excluded from the check. Use if you know that function does not break a +// reference count. +const char kCheckBadRawPtrCastExcludeFuncArgPrefix[] = + "check-bad-raw-ptr-cast-exclude-func="; + } // namespace namespace chrome_checker { @@ -64,6 +72,9 @@ } else if (arg.startswith(kRawPtrExcludePathArgPrefix)) { options_.raw_ptr_paths_to_exclude_lines.push_back( arg.substr(strlen(kRawPtrExcludePathArgPrefix)).str()); + } else if (arg.startswith(kCheckBadRawPtrCastExcludeFuncArgPrefix)) { + options_.check_bad_raw_ptr_cast_exclude_funcs.push_back( + arg.substr(strlen(kCheckBadRawPtrCastExcludeFuncArgPrefix)).str()); } else if (arg == "check-base-classes") { // TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed. options_.check_base_classes = true; @@ -86,7 +97,7 @@ } else if (arg == "check-raw-ref-fields") { options_.check_raw_ref_fields = true; } else if (arg == "raw-ptr-fix-crbug-1449812") { - options_.raw_ptr_fix_crbug_1449812 = true; + // TODO(mikt): Now enabled by default. Remove this path. } else { llvm::errs() << "Unknown clang plugin argument: " << arg << "\n"; return false;
diff --git a/tools/clang/plugins/FindBadRawPtrPatterns.cpp b/tools/clang/plugins/FindBadRawPtrPatterns.cpp index ffbad54..c6742383 100644 --- a/tools/clang/plugins/FindBadRawPtrPatterns.cpp +++ b/tools/clang/plugins/FindBadRawPtrPatterns.cpp
@@ -35,8 +35,12 @@ class BadCastMatcher : public MatchFinder::MatchCallback { public: - explicit BadCastMatcher(clang::CompilerInstance& compiler) - : compiler_(compiler) { + explicit BadCastMatcher(clang::CompilerInstance& compiler, + const FilterFile& exclude_files, + const FilterFile& exclude_functions) + : compiler_(compiler), + exclude_files_(exclude_files), + exclude_functions_(exclude_functions) { error_bad_cast_signature_ = compiler_.getDiagnostics().getCustomDiagID( clang::DiagnosticsEngine::Error, kBadCastDiagnostic); note_bad_cast_signature_explanation_ = @@ -47,7 +51,8 @@ } void Register(MatchFinder& match_finder) { - auto cast_matcher = BadRawPtrCastExpr(casting_unsafe_predicate_); + auto cast_matcher = BadRawPtrCastExpr(casting_unsafe_predicate_, + exclude_files_, exclude_functions_); match_finder.addMatcher(cast_matcher, this); } @@ -101,6 +106,8 @@ private: clang::CompilerInstance& compiler_; + const FilterFile& exclude_files_; + const FilterFile& exclude_functions_; CastingUnsafePredicate casting_unsafe_predicate_; unsigned error_bad_cast_signature_; unsigned note_bad_cast_signature_explanation_; @@ -233,16 +240,14 @@ clang::CompilerInstance& compiler) { MatchFinder match_finder; - BadCastMatcher bad_cast_matcher(compiler); - if (options.check_bad_raw_ptr_cast) - bad_cast_matcher.Register(match_finder); - std::vector<std::string> paths_to_exclude_lines; + std::vector<std::string> separate_repository_paths; for (auto* const line : kRawPtrManualPathsToIgnore) { paths_to_exclude_lines.push_back(line); } for (auto* const line : kSeparateRepositoryPaths) { paths_to_exclude_lines.push_back(line); + separate_repository_paths.push_back(line); } paths_to_exclude_lines.insert(paths_to_exclude_lines.end(), options.raw_ptr_paths_to_exclude_lines.begin(), @@ -250,10 +255,19 @@ FilterFile exclude_fields(options.exclude_fields_file, "exclude-fields"); FilterFile exclude_lines(paths_to_exclude_lines); + FilterFile exclude_separate_repositories(separate_repository_paths); + FilterFile check_bad_raw_ptr_cast_exclude_funcs( + options.check_bad_raw_ptr_cast_exclude_funcs); StackAllocatedPredicate stack_allocated_predicate; RawPtrAndRefExclusionsOptions exclusion_options{ &exclude_fields, &exclude_lines, options.check_raw_ptr_to_stack_allocated, - &stack_allocated_predicate, options.raw_ptr_fix_crbug_1449812}; + &stack_allocated_predicate}; + + BadCastMatcher bad_cast_matcher(compiler, exclude_separate_repositories, + check_bad_raw_ptr_cast_exclude_funcs); + if (options.check_bad_raw_ptr_cast) { + bad_cast_matcher.Register(match_finder); + } RawPtrFieldMatcher field_matcher(compiler, exclusion_options); if (options.check_raw_ptr_fields) {
diff --git a/tools/clang/plugins/Options.h b/tools/clang/plugins/Options.h index 46fb07fd..e7efe86 100644 --- a/tools/clang/plugins/Options.h +++ b/tools/clang/plugins/Options.h
@@ -21,9 +21,9 @@ bool check_stack_allocated = false; bool check_raw_ref_fields = false; bool check_raw_ptr_to_stack_allocated = false; - bool raw_ptr_fix_crbug_1449812 = false; std::string exclude_fields_file; std::vector<std::string> raw_ptr_paths_to_exclude_lines; + std::vector<std::string> check_bad_raw_ptr_cast_exclude_funcs; }; } // namespace chrome_checker
diff --git a/tools/clang/plugins/RawPtrHelpers.cpp b/tools/clang/plugins/RawPtrHelpers.cpp index 0e41198d..585862f 100644 --- a/tools/clang/plugins/RawPtrHelpers.cpp +++ b/tools/clang/plugins/RawPtrHelpers.cpp
@@ -123,55 +123,24 @@ // } clang::ast_matchers::internal::Matcher<clang::NamedDecl> PtrAndRefExclusions( const RawPtrAndRefExclusionsOptions& options) { - if (!options.fix_crbug_1449812) { - // Before fix for crbug.com/1449812 - // - File exclusion is performed based on the `SourceLocation` obtained - // via `getBeginLoc()`. - // - System header check is based on expansion location. - if (!options.should_exclude_stack_allocated_records) { - return anyOf( - isExpansionInSystemHeader(), isInExternCContext(), - isRawPtrExclusionAnnotated(), isBeginInThirdPartyLocation(), - isBeginInGeneratedLocation(), - isBeginInLocationListedInFilterFile(options.paths_to_exclude), - isFieldDeclListedInFilterFile(options.fields_to_exclude), - ImplicitFieldDeclaration(), isObjCSynthesize()); - } else { - return anyOf( - isExpansionInSystemHeader(), isInExternCContext(), - isRawPtrExclusionAnnotated(), isBeginInThirdPartyLocation(), - isBeginInGeneratedLocation(), - isBeginInLocationListedInFilterFile(options.paths_to_exclude), - isFieldDeclListedInFilterFile(options.fields_to_exclude), - ImplicitFieldDeclaration(), isObjCSynthesize(), - hasDescendant( - StackAllocatedQualType(options.stack_allocated_predicate)), - isDeclaredInStackAllocated(*options.stack_allocated_predicate)); - } + if (!options.should_exclude_stack_allocated_records) { + return anyOf(isSpellingInSystemHeader(), isInExternCContext(), + isRawPtrExclusionAnnotated(), isInThirdPartyLocation(), + isInGeneratedLocation(), isNotSpelledInSource(), + isInLocationListedInFilterFile(options.paths_to_exclude), + isFieldDeclListedInFilterFile(options.fields_to_exclude), + ImplicitFieldDeclaration(), isObjCSynthesize()); } else { - // After fix for crbug.com/1449812 - // - File exclusion is performed based on the `SourceLocation` obtained - // via `getLocation()`. - // - System header check is based on spelling location. - if (!options.should_exclude_stack_allocated_records) { - return anyOf(isSpellingInSystemHeader(), isInExternCContext(), - isRawPtrExclusionAnnotated(), isInThirdPartyLocation(), - isInGeneratedLocation(), isNotSpelledInSource(), - isInLocationListedInFilterFile(options.paths_to_exclude), - isFieldDeclListedInFilterFile(options.fields_to_exclude), - ImplicitFieldDeclaration(), isObjCSynthesize()); - } else { - return anyOf( - isSpellingInSystemHeader(), isInExternCContext(), - isRawPtrExclusionAnnotated(), isInThirdPartyLocation(), - isInGeneratedLocation(), isNotSpelledInSource(), - isInLocationListedInFilterFile(options.paths_to_exclude), - isFieldDeclListedInFilterFile(options.fields_to_exclude), - ImplicitFieldDeclaration(), isObjCSynthesize(), - hasDescendant( - StackAllocatedQualType(options.stack_allocated_predicate)), - isDeclaredInStackAllocated(*options.stack_allocated_predicate)); - } + return anyOf( + isSpellingInSystemHeader(), isInExternCContext(), + isRawPtrExclusionAnnotated(), isInThirdPartyLocation(), + isInGeneratedLocation(), isNotSpelledInSource(), + isInLocationListedInFilterFile(options.paths_to_exclude), + isFieldDeclListedInFilterFile(options.fields_to_exclude), + ImplicitFieldDeclaration(), isObjCSynthesize(), + hasDescendant( + StackAllocatedQualType(options.stack_allocated_predicate)), + isDeclaredInStackAllocated(*options.stack_allocated_predicate)); } } @@ -211,23 +180,12 @@ fieldDecl(hasType(pointerType(pointee(qualType(allOf( isConstQualified(), hasUnqualifiedDesugaredType(anyCharType()))))))); - if (!options.fix_crbug_1449812) { - auto field_decl_matcher = - fieldDecl(allOf(hasType(supported_pointer_types_matcher), - unless(anyOf(const_char_pointer_matcher, - isBeginInScratchSpace(), - PtrAndRefExclusions(options))))) - .bind("affectedFieldDecl"); - return field_decl_matcher; - } else { - // `isBeginInScratchSpace()` check is done inside `PtrAndRefExclusions`. auto field_decl_matcher = fieldDecl(allOf(hasType(supported_pointer_types_matcher), unless(anyOf(const_char_pointer_matcher, PtrAndRefExclusions(options))))) .bind("affectedFieldDecl"); return field_decl_matcher; - } } clang::ast_matchers::internal::Matcher<clang::Decl> AffectedRawRefFieldDecl( @@ -318,7 +276,9 @@ } clang::ast_matchers::internal::Matcher<clang::Stmt> BadRawPtrCastExpr( - const CastingUnsafePredicate& casting_unsafe_predicate) { + const CastingUnsafePredicate& casting_unsafe_predicate, + const FilterFile& exclude_files, + const FilterFile& exclude_functions) { // Matches anything contains |raw_ptr<T>| / |raw_ref<T>|. auto src_type = type(isCastingUnsafe(casting_unsafe_predicate)).bind("srcType"); @@ -332,13 +292,76 @@ hasCastKind(clang::CK_PointerToIntegral), hasCastKind(clang::CK_IntegralToPointer))); - // |__bit/bit_cast.h| header is excluded to perform checking on - // |std::bit_cast<T>|. - auto exclusions = anyOf(isSpellingInSystemHeader(), isInRawPtrCastHeader()); + // Matches implicit casts happening in invocation inside template context. + // void f(int v); + // void f(void* p); + // template <typename T> + // void call_f(T t) { f(t); } + // ^ implicit cast here if |T| = |int*| + // We exclude this cast from check because we cannot apply + // |base::unsafe_raw_ptr_*_cast<void*>(t)| here. + auto in_template_invocation_ctx = implicitCastExpr( + allOf(isInTemplateInstantiation(), hasParent(invocation()))); + + // Matches implicit casts happening in comparison. + // int* x; + // void* y; + // if (x < y) f(); + // ^~~~~ |x| is implicit casted into |void*| here + // This cast is guaranteed to be safe because it cannot break ref count. + auto in_comparison_ctx = + implicitCastExpr(hasParent(binaryOperator(isComparisonOperator()))); + + // Matches implicit casts happening in invocation to allow-listed + // declarations. + auto in_allowlisted_invocation_ctx = + implicitCastExpr(hasParent(invocation(hasDeclaration( + namedDecl(isFieldDeclListedInFilterFile(&exclude_functions)))))); + + // Matches casts to const pointer types pointing to built-in types. + // e.g. matches |const char*| and |const void*| but neither |const int**| nor + // |int* const*|. + // They are safe as long as const qualifier is kept because const means we + // shouldn't be writing to the memory and won't mutate the value in a way that + // causes BRP's refcount inconsistency. + auto const_builtin_pointer_type = + type(hasUnqualifiedDesugaredType(pointerType( + pointee(qualType(allOf(isConstQualified(), builtinType())))))); + auto cast_expr_to_const_pointer = anyOf( + implicitCastExpr(hasImplicitDestinationType(const_builtin_pointer_type)), + explicitCastExpr(hasDestinationType(const_builtin_pointer_type))); + + // Unsafe castings are allowed if: + // - In locations developers have no control + // - In system headers + // - In third party libraries + // - In non-source locations (e.g. <scratch space>) + // - In separate repository locations (e.g. //internal) + // - In locations that are likely to be safe + // - In pointer comparison context + // - In allowlisted function/constructor invocations + // - To const-qualified void/char pointers + // - In cases that the cast is indispensable and developers can guarantee it + // will not break BRP's refcount + // - In |base::unsafe_raw_ptr_static_cast<T>(...)| + // - In |base::unsafe_raw_ptr_reinterpret_cast<T>(...)| + // - In |base::unsafe_raw_ptr_bit_cast<T>(...)| + // - In cases that the cast is indispensable but developers cannot use the + // cast exclusion listed above + // - Implicit casts inside template context as there can be multiple + // destination types depending on how template is instantiated + auto exclusions = + anyOf(isSpellingInSystemHeader(), isInThirdPartyLocation(), + isNotSpelledInSource(), + isInLocationListedInFilterFile(&exclude_files), in_comparison_ctx, + in_allowlisted_invocation_ctx, cast_expr_to_const_pointer, + isInRawPtrCastHeader(), in_template_invocation_ctx); // Implicit/explicit casting from/to |raw_ptr<T>| matches. // Both casting direction is unsafe. // https://godbolt.org/z/zqKMzcKfo + // |__bit/bit_cast.h| header is configured to bypass exclusions to perform + // checking on |std::bit_cast<T>|. auto cast_matcher = castExpr( allOf(anyOf(hasSourceExpression(hasType(src_type)),
diff --git a/tools/clang/plugins/RawPtrHelpers.h b/tools/clang/plugins/RawPtrHelpers.h index 62b924a..0ceb5bd 100644 --- a/tools/clang/plugins/RawPtrHelpers.h +++ b/tools/clang/plugins/RawPtrHelpers.h
@@ -70,8 +70,6 @@ FilterFile* paths_to_exclude; bool should_exclude_stack_allocated_records; chrome_checker::StackAllocatedPredicate* stack_allocated_predicate; - // Enable a fix for https://crbug.com/1449812 when true. - bool fix_crbug_1449812; }; AST_MATCHER(clang::Type, anyCharType) { @@ -95,19 +93,6 @@ source_manager.isWrittenInScratchSpace(loc); } -// TODO(mikt): Remove after option `raw-ptr-fix-crbug-1449812` is fully enabled. -AST_MATCHER(clang::Decl, isBeginInScratchSpace) { - const clang::SourceManager& source_manager = - Finder->getASTContext().getSourceManager(); - clang::SourceLocation location = Node.getSourceRange().getBegin(); - if (location.isInvalid()) { - return false; - } - clang::SourceLocation spelling_location = - source_manager.getSpellingLoc(location); - return source_manager.isWrittenInScratchSpace(spelling_location); -} - AST_POLYMORPHIC_MATCHER(isInThirdPartyLocation, AST_POLYMORPHIC_SUPPORTED_TYPES(clang::Decl, clang::Stmt, @@ -126,22 +111,6 @@ return filename.find("/third_party/") != std::string::npos; } -// TODO(mikt): Remove after option `raw-ptr-fix-crbug-1449812` is fully enabled. -AST_MATCHER(clang::Decl, isBeginInThirdPartyLocation) { - std::string filename = GetFilename(Finder->getASTContext().getSourceManager(), - Node.getSourceRange().getBegin()); - - // Blink is part of the Chromium git repo, even though it contains - // "third_party" in its path. - if (filename.find("/third_party/blink/") != std::string::npos) { - return false; - } - // Otherwise, just check if the paths contains the "third_party" substring. - // We don't want to rewrite content of such paths even if they are in the main - // Chromium git repository. - return filename.find("/third_party/") != std::string::npos; -} - AST_MATCHER(clang::Stmt, isInStdBitCastHeader) { std::string filename = GetFilename(Finder->getASTContext().getSourceManager(), Node.getSourceRange().getBegin()); @@ -167,15 +136,6 @@ filename.rfind("gen/", 0) == 0; } -// TODO(mikt): Remove after option `raw-ptr-fix-crbug-1449812` is fully enabled. -AST_MATCHER(clang::Decl, isBeginInGeneratedLocation) { - std::string filename = GetFilename(Finder->getASTContext().getSourceManager(), - Node.getSourceRange().getBegin()); - - return filename.find("/gen/") != std::string::npos || - filename.rfind("gen/", 0) == 0; -} - AST_MATCHER_P(clang::NamedDecl, isFieldDeclListedInFilterFile, const FilterFile*, @@ -183,10 +143,12 @@ return Filter->ContainsLine(Node.getQualifiedNameAsString()); } -AST_MATCHER_P(clang::Decl, - isInLocationListedInFilterFile, - const FilterFile*, - Filter) { +AST_POLYMORPHIC_MATCHER_P(isInLocationListedInFilterFile, + AST_POLYMORPHIC_SUPPORTED_TYPES(clang::Decl, + clang::Stmt, + clang::TypeLoc), + const FilterFile*, + Filter) { clang::SourceLocation loc = getRepresentativeLocation(Node); if (loc.isInvalid()) { return false; @@ -196,20 +158,6 @@ return Filter->ContainsSubstringOf(file_path); } -// TODO(mikt): Remove after option `raw-ptr-fix-crbug-1449812` is fully enabled. -AST_MATCHER_P(clang::Decl, - isBeginInLocationListedInFilterFile, - const FilterFile*, - Filter) { - clang::SourceLocation loc = Node.getSourceRange().getBegin(); - if (loc.isInvalid()) { - return false; - } - std::string file_path = - GetFilename(Finder->getASTContext().getSourceManager(), loc); - return Filter->ContainsSubstringOf(file_path); -} - AST_MATCHER(clang::Decl, isInExternCContext) { return Node.getLexicalDeclContext()->isExternCContext(); } @@ -311,7 +259,9 @@ const chrome_checker::StackAllocatedPredicate* predicate); clang::ast_matchers::internal::Matcher<clang::Stmt> BadRawPtrCastExpr( - const CastingUnsafePredicate& casting_unsafe_predicate); + const CastingUnsafePredicate& casting_unsafe_predicate, + const FilterFile& exclude_files, + const FilterFile& exclude_functions); // If `field_decl` declares a field in an implicit template specialization, then // finds and returns the corresponding FieldDecl from the template definition.
diff --git a/tools/clang/plugins/Util.h b/tools/clang/plugins/Util.h index 9fa14352..7da2c7bd 100644 --- a/tools/clang/plugins/Util.h +++ b/tools/clang/plugins/Util.h
@@ -33,8 +33,20 @@ inline clang::SourceLocation getRepresentativeLocation( const clang::Stmt& node) { // clang::Stmt has T::getBeginLoc() and T::getEndLoc(). - // As the former may refer to modifiers, we use the latter one. - return node.getEndLoc(); + // Usually the former one does better represent the location. + // + // e.g. clang::IfStmt + // if (foo) {} else {} + // ^ ^ + // | getEndLoc() + // getBeginLoc() + // + // e.g. clang::CastExpr + // int x = static_cast<int>(123ll); + // ^ ^ + // | getEndLoc() + // getBeginLoc() + return node.getBeginLoc(); } inline clang::SourceLocation getRepresentativeLocation( const clang::TypeLoc& node) {
diff --git a/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.cpp b/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.cpp new file mode 100644 index 0000000..6454aabc --- /dev/null +++ b/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.cpp
@@ -0,0 +1,65 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/memory/raw_ptr.h" + +struct RawPtrWrapper { + raw_ptr<int> ptr; +}; + +void OverloadedFunction(void* p) {} +void OverloadedFunction(int v) {} + +namespace test { +void NormalFunc(void* p) {} +void AllowlistedFunc(void* p) {} +struct AllowlistedConstructor { + explicit AllowlistedConstructor(void* p) {} +}; +struct NormalConstructor { + explicit NormalConstructor(void* p) {} +}; +} // namespace test + +// 'unsafe_raw_ptr_*_cast' should not emit errors. +void CastToCastingUnsafeExclusion() { + void* p = nullptr; + RawPtrWrapper* q = nullptr; + + // Base case: should error. + (void)reinterpret_cast<RawPtrWrapper*>(p); + (void)reinterpret_cast<void*>(q); + + // Casts to const built-in typed pointers should be excluded. + (void)reinterpret_cast<const void*>(q); + (void)reinterpret_cast<const char*>(q); + + // Casts in allowlisted invocation context should be excluded. + test::NormalFunc(q); // Not allowlisted + test::AllowlistedFunc(q); // Allowlisted + (void)test::NormalConstructor(q); // Not allowlisted + (void)test::AllowlistedConstructor(q); // Allowlisted + + // Casts in comparison context should be excluded. + (void)(p == q); + // ^ implicit cast from |RawPtrWrapper*| to |void*| here. + + // Implicit casts in invocation inside template context should be excluded. + auto f = [](auto* x) { OverloadedFunction(x); }; + f(p); + f(q); + + // Casts that |isNotSpelledInSource()| should be excluded. +#define ANY_CAST(type) type##_cast + (void)ANY_CAST(reinterpret)<RawPtrWrapper*>(p); + // ^~~~~~~~ token "reinterpret_cast" is in <scratch space>. + + // Casts that |isInThirdPartyLocation()| should be excluded. +#line 1 "../../third_party/fake_loc/bad_raw_ptr_cast_implicit_exclusion.cpp" + (void)reinterpret_cast<RawPtrWrapper*>(p); + + // Casts that |isInLocationListedInFilterFile(...)| should be excluded. +#line 1 "../../internal/fake_loc/bad_raw_ptr_cast_implicit_exclusion.cpp" + (void)reinterpret_cast<RawPtrWrapper*>(p); +}
diff --git a/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.flags b/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.flags new file mode 100644 index 0000000..cf4bfa4 --- /dev/null +++ b/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.flags
@@ -0,0 +1 @@ +-Xclang -plugin-arg-find-bad-constructs -Xclang check-bad-raw-ptr-cast -Xclang -plugin-arg-find-bad-constructs -Xclang check-bad-raw-ptr-cast-exclude-func=test::AllowlistedFunc -Xclang -plugin-arg-find-bad-constructs -Xclang check-bad-raw-ptr-cast-exclude-func=test::AllowlistedConstructor::AllowlistedConstructor
diff --git a/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.txt b/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.txt new file mode 100644 index 0000000..e469507 --- /dev/null +++ b/tools/clang/plugins/tests/bad_raw_ptr_cast_implicit_exclusion.txt
@@ -0,0 +1,29 @@ +bad_raw_ptr_cast_implicit_exclusion.cpp:31:43: error: [chromium-style] casting 'void *' to 'RawPtrWrapper * is not allowed. + (void)reinterpret_cast<RawPtrWrapper*>(p); + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:31:43: note: [chromium-style] 'RawPtrWrapper *' manages BackupRefPtr refcounts; bypassing its C++ interface or treating it as a POD will lead to memory safety errors. +bad_raw_ptr_cast_implicit_exclusion.cpp:8:3: note: [chromium-style] 'RawPtrWrapper' manages BackupRefPtr or its container here. + raw_ptr<int> ptr; + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:32:34: error: [chromium-style] casting 'RawPtrWrapper *' to 'void * is not allowed. + (void)reinterpret_cast<void*>(q); + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:32:34: note: [chromium-style] 'RawPtrWrapper *' manages BackupRefPtr refcounts; bypassing its C++ interface or treating it as a POD will lead to memory safety errors. +bad_raw_ptr_cast_implicit_exclusion.cpp:8:3: note: [chromium-style] 'RawPtrWrapper' manages BackupRefPtr or its container here. + raw_ptr<int> ptr; + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:39:20: error: [chromium-style] casting 'RawPtrWrapper *' to 'void * is not allowed. + test::NormalFunc(q); // Not allowlisted + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:39:20: note: [chromium-style] 'RawPtrWrapper *' manages BackupRefPtr refcounts; bypassing its C++ interface or treating it as a POD will lead to memory safety errors. +bad_raw_ptr_cast_implicit_exclusion.cpp:8:3: note: [chromium-style] 'RawPtrWrapper' manages BackupRefPtr or its container here. + raw_ptr<int> ptr; + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:41:33: error: [chromium-style] casting 'RawPtrWrapper *' to 'void * is not allowed. + (void)test::NormalConstructor(q); // Not allowlisted + ^ +bad_raw_ptr_cast_implicit_exclusion.cpp:41:33: note: [chromium-style] 'RawPtrWrapper *' manages BackupRefPtr refcounts; bypassing its C++ interface or treating it as a POD will lead to memory safety errors. +bad_raw_ptr_cast_implicit_exclusion.cpp:8:3: note: [chromium-style] 'RawPtrWrapper' manages BackupRefPtr or its container here. + raw_ptr<int> ptr; + ^ +4 errors generated.
diff --git a/tools/clang/plugins/tests/raw_ptr_fields_macro.flags b/tools/clang/plugins/tests/raw_ptr_fields_macro.flags index 6f010e60..9883af5 100644 --- a/tools/clang/plugins/tests/raw_ptr_fields_macro.flags +++ b/tools/clang/plugins/tests/raw_ptr_fields_macro.flags
@@ -1 +1 @@ --Xclang -plugin-arg-find-bad-constructs -Xclang check-raw-ptr-fields -Xclang -plugin-arg-find-bad-constructs -Xclang raw-ptr-fix-crbug-1449812 -Wno-unknown-attributes -ferror-limit=0 -DCMD_INT=int -DCMD_INTP=int* -DCMD_CONST=const -DCMD_ATTR=[[fake_attribute]] -DCMD_INTP_FIELD()=int*macro_ptr -DCMD_TYPE_WITH_SUFFIX(TYP)=TYP##Suffix -DCMD_SYMBOL(SYM)=SYM -DCMD_SYMBOL_WITH_SUFFIX(SYM)=SYM##_suffix -DCMD_EQ== -DCMD_NULLPTR=nullptr +-Xclang -plugin-arg-find-bad-constructs -Xclang check-raw-ptr-fields -Wno-unknown-attributes -ferror-limit=0 -DCMD_INT=int -DCMD_INTP=int* -DCMD_CONST=const -DCMD_ATTR=[[fake_attribute]] -DCMD_INTP_FIELD()=int*macro_ptr -DCMD_TYPE_WITH_SUFFIX(TYP)=TYP##Suffix -DCMD_SYMBOL(SYM)=SYM -DCMD_SYMBOL_WITH_SUFFIX(SYM)=SYM##_suffix -DCMD_EQ== -DCMD_NULLPTR=nullptr
diff --git a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.cpp b/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.cpp deleted file mode 100644 index 47957f6..0000000 --- a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.cpp +++ /dev/null
@@ -1,11 +0,0 @@ -// Copyright 2022 The Chromium Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// // Based on third_party/brotli/enc/metablock_inc.h - -class HistogramLiteral; - -#define FN(X) X##Literal -#include "raw_ptr_fields_scratch_space_inc.h" /* NOLINT(build/include) */ -#undef FN
diff --git a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.flags b/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.flags deleted file mode 100644 index cb3c8b65..0000000 --- a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.flags +++ /dev/null
@@ -1 +0,0 @@ --Xclang -plugin-arg-find-bad-constructs -Xclang check-raw-ptr-fields
diff --git a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.txt b/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.txt deleted file mode 100644 index e69de29..0000000 --- a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space.txt +++ /dev/null
diff --git a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space_inc.h b/tools/clang/plugins/tests/raw_ptr_fields_scratch_space_inc.h deleted file mode 100644 index f9dc447..0000000 --- a/tools/clang/plugins/tests/raw_ptr_fields_scratch_space_inc.h +++ /dev/null
@@ -1,20 +0,0 @@ -// Copyright 2022 The Chromium Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// NOLINT(build/header_guard) -// no-include-guard-because-multiply-included - -// FN is a macro that appends a suffix to a given name. In the real code (in -// metablock.c), FN is redefined, then this file is included reapeatedly to -// generate the same code for names with different suffixes. - -// For this test, HistogramType will expand to HistogramLiteral. -#define HistogramType FN(Histogram) - -// For this test, FN(BlockSplitter) will expand to BlockSplitterLiteral. -struct FN(BlockSplitter) { - // No error expected, as the source location for this field declaration will - // be "<scratch space>" and the real file path cannot be detected. - HistogramType* histograms_; -};
diff --git a/tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp b/tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp index f431e7c6..701e1ed 100644 --- a/tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp +++ b/tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp
@@ -34,6 +34,7 @@ #include "RawPtrHelpers.h" #include "RawPtrManualPathsToIgnore.h" +#include "SeparateRepositoryPaths.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -1213,10 +1214,6 @@ llvm::cl::desc("Exclude pointers/references to `STACK_ALLOCATED` objects " "from the rewrite")); - llvm::cl::opt<bool> raw_ptr_fix_crbug_1449812( - "raw_ptr_fix_crbug_1449812", llvm::cl::init(true), - llvm::cl::desc("Apply a fix for crbug.com/1449812")); - llvm::Expected<clang::tooling::CommonOptionsParser> options = clang::tooling::CommonOptionsParser::create(argc, argv, category); assert(static_cast<bool>(options)); // Should not return an error. @@ -1238,6 +1235,9 @@ for (auto* const line : kRawPtrManualPathsToIgnore) { paths_to_exclude_lines.push_back(line); } + for (auto* const line : kSeparateRepositoryPaths) { + paths_to_exclude_lines.push_back(line); + } paths_to_exclude = std::make_unique<FilterFile>(paths_to_exclude_lines); } else { paths_to_exclude = @@ -1248,7 +1248,7 @@ chrome_checker::StackAllocatedPredicate stack_allocated_checker; RawPtrAndRefExclusionsOptions exclusion_options{ &fields_to_exclude, paths_to_exclude.get(), exclude_stack_allocated, - &stack_allocated_checker, raw_ptr_fix_crbug_1449812}; + &stack_allocated_checker}; RawPtrRewriter raw_ptr_rewriter(&output_helper, match_finder, exclusion_options);
diff --git a/tools/clang/scripts/build.py b/tools/clang/scripts/build.py index 2e84b050..e931b3d 100755 --- a/tools/clang/scripts/build.py +++ b/tools/clang/scripts/build.py
@@ -721,11 +721,6 @@ global CLANG_REVISION, PACKAGE_VERSION, LLVM_BUILD_DIR - # TODO(crbug.com/1467585): Remove in next Clang roll. - if args.llvm_force_head_revision: - global RELEASE_VERSION - RELEASE_VERSION = '18' - if (args.pgo or args.thinlto) and not args.bootstrap: print('--pgo/--thinlto requires --bootstrap') return 1
diff --git a/tools/clang/scripts/update.py b/tools/clang/scripts/update.py index 82f9c4f..20c4147 100755 --- a/tools/clang/scripts/update.py +++ b/tools/clang/scripts/update.py
@@ -35,11 +35,11 @@ # https://chromium.googlesource.com/chromium/src/+/main/docs/updating_clang.md # Reverting problematic clang rolls is safe, though. # This is the output of `git describe` and is usable as a commit-ish. -CLANG_REVISION = 'llvmorg-17-init-16420-g0c545a44' -CLANG_SUB_REVISION = 8 +CLANG_REVISION = 'llvmorg-18-init-4631-gd50b56d1' +CLANG_SUB_REVISION = 1 PACKAGE_VERSION = '%s-%s' % (CLANG_REVISION, CLANG_SUB_REVISION) -RELEASE_VERSION = '17' +RELEASE_VERSION = '18' # TODO(crbug.com/1467585): Bump to 18 in next Clang roll. CDS_URL = os.environ.get('CDS_CLANG_BUCKET_OVERRIDE',
diff --git a/tools/clang/scripts/upload_revision.py b/tools/clang/scripts/upload_revision.py index 83080f0..b48e63e 100755 --- a/tools/clang/scripts/upload_revision.py +++ b/tools/clang/scripts/upload_revision.py
@@ -72,7 +72,7 @@ Cq-Include-Trybots: chromium/try:win-official,win32-official Cq-Include-Trybots: chromium/try:linux-swangle-try-x64,win-swangle-try-x86 Cq-Include-Trybots: chrome/try:iphone-device,ipad-device -Cq-Include-Trybots: chrome/try:linux-chromeos-chrome +Cq-Include-Trybots: chrome/try:linux-chromeos-chrome,win-arm64-rel Cq-Include-Trybots: chrome/try:win-chrome,win64-chrome,linux-chrome,mac-chrome Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win32-pgo,win64-pgo'''