Skip more scenarios via blocklist, rather than unconditionally.

Similarly to r778581, after this CL, the rewriter will no longer
unconditionally skip fields in the following scenarios:
- Field pointing to a struct/class with deleted operator new
- Fields in generated code

Instead, the rewriter will emit such fields as candidates for the
--field-filter-file blocklist - in the following section of the output:

    ==== BEGIN FIELD FILTERS ====
    GeneratedStruct::ptr_field  # generated-code
    MyStruct::ptr_to_stack  # pointee-has-no-operator-new
    NoNewOperator::field  # embedder-has-no-operator-new
    ==== END FIELD FILTERS ====

Skipping via filter list is desirable, because it helps with auditing
and counting the filtered fields.  In particular:
- operator-new-based cases should be vetted to ensure the heuristic
  is sound and check what kind of cases are present outside of Blink
- generated code should eventually be fixed (to use CheckedPtr as
  appropriate) after the rewrite and explicit auditing will help scope
  this work

Note that the "embedder-has-no-operator-new" case was not present (i.e.
was not unconditionally skipped) before this CL - this case is new.

Bug: 1069567
Change-Id: Ic67ac7a94225d0e11e1e405e75dc41a7248ab67f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2488744Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819402}

tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp

@@ -853,16 +853,11 @@ int main(int argc, const char* argv[]) {
   //     int (*func_ptr)();
   //     int (MyStruct::* member_func_ptr)(char);
   //     int (*ptr_to_array_of_ints)[123]
-  //     StructOrClassWithDeletedOperatorNew* stack_or_gc_ptr;
   //   };
   // matches |int*|, but not the other types.
-  auto record_with_deleted_allocation_operator_type_matcher =
-      recordType(hasDeclaration(cxxRecordDecl(
-          hasMethod(allOf(hasOverloadedOperatorName("new"), isDeleted())))));
   auto supported_pointer_types_matcher =
       pointerType(unless(pointee(hasUnqualifiedDesugaredType(
-          anyOf(record_with_deleted_allocation_operator_type_matcher,
-                functionType(), memberPointerType(), arrayType())))));
+          anyOf(functionType(), memberPointerType(), arrayType())))));
 
   // Implicit field declarations =========
   // Matches field declarations that do not explicitly appear in the source
@@ -898,7 +893,7 @@ int main(int argc, const char* argv[]) {
       fieldDecl(
           allOf(hasType(supported_pointer_types_matcher),
                 unless(anyOf(isExpansionInSystemHeader(), isInExternCContext(),
-                             isInThirdPartyLocation(), isInGeneratedLocation(),
+                             isInThirdPartyLocation(),
                              isInLocationListedInFilterFile(&paths_to_exclude),
                              isFieldDeclListedInFilterFile(&fields_to_exclude),
                              implicit_field_decl_matcher))))
@@ -1063,6 +1058,46 @@ int main(int argc, const char* argv[]) {
                                               "global-destructor");
   match_finder.addMatcher(global_destructor_matcher, &global_destructor_writer);
 
+  // Matches rewritable fields in generated code - see the testcase in
+  // tests/gen-generated-code-test.cc
+  auto field_in_generated_code_matcher =
+      fieldDecl(allOf(field_decl_matcher, isInGeneratedLocation()));
+  FilteredExprWriter field_in_generated_code_writer(&output_helper,
+                                                    "generated-code");
+  match_finder.addMatcher(field_in_generated_code_matcher,
+                          &field_in_generated_code_writer);
+
+  // Matches CXXRecordDecls with a deleted operator new - e.g.
+  // StructWithNoOperatorNew below:
+  //     struct StructWithNoOperatorNew {
+  //       void* operator new(size_t) = delete;
+  //     };
+  auto record_with_deleted_allocation_operator_type_matcher = cxxRecordDecl(
+      hasMethod(allOf(hasOverloadedOperatorName("new"), isDeleted())));
+  // Matches rewritable fields inside structs with no operator new.  See the
+  // testcase in tests/gen-deleted-operator-new-test.cc
+  auto field_in_record_with_deleted_operator_new_matcher = fieldDecl(
+      allOf(field_decl_matcher,
+            hasParent(record_with_deleted_allocation_operator_type_matcher)));
+  FilteredExprWriter field_in_record_with_deleted_operator_new_writer(
+      &output_helper, "embedder-has-no-operator-new");
+  match_finder.addMatcher(field_in_record_with_deleted_operator_new_matcher,
+                          &field_in_record_with_deleted_operator_new_writer);
+  // Matches rewritable fields that contain a pointer, pointing to a pointee
+  // with no operator new.  See the testcase in
+  // tests/gen-deleted-operator-new-test.cc
+  auto field_pointing_to_record_with_deleted_operator_new_matcher =
+      fieldDecl(allOf(
+          field_decl_matcher,
+          hasType(pointerType(
+              pointee(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+                  record_with_deleted_allocation_operator_type_matcher))))))));
+  FilteredExprWriter field_pointing_to_record_with_deleted_operator_new_writer(
+      &output_helper, "pointee-has-no-operator-new");
+  match_finder.addMatcher(
+      field_pointing_to_record_with_deleted_operator_new_matcher,
+      &field_pointing_to_record_with_deleted_operator_new_writer);
+
   // Matches fields in unions - see the testcases in tests/gen-unions-test.cc.
   auto union_field_decl_matcher = fieldDecl(
       allOf(field_decl_matcher, hasParent(decl(recordDecl(isUnion())))));

tools/clang/rewrite_raw_ptr_fields/tests/affected-expr-expected.cc

@@ -8,7 +8,6 @@
 #include <utility>  // for std::swap
 
 #include "base/memory/checked_ptr.h"
-#include "gen/generated_header.h"
 
 class SomeClass {};
 class DerivedClass : public SomeClass {};
@@ -173,20 +172,6 @@ void foo(int x) {
 
 }  // namespace ternary_operator_tests
 
-namespace generated_code_tests {
-
-void MyPrintf(const char* fmt, ...) {}
-
-void foo() {
-  GeneratedStruct s;
-
-  // No rewrite expected below (i.e. no |.get()| appended), because the field
-  // dereferenced below comes from (simulated) generated code.
-  MyPrintf("%p", s.ptr_field);
-}
-
-}  // namespace generated_code_tests
-
 namespace templated_functions {
 
 template <typename T>

tools/clang/rewrite_raw_ptr_fields/tests/affected-expr-original.cc

@@ -7,8 +7,6 @@
 #include <tuple>    // for std::tie
 #include <utility>  // for std::swap
 
-#include "gen/generated_header.h"
-
 class SomeClass {};
 class DerivedClass : public SomeClass {};
 
@@ -172,20 +170,6 @@ void foo(int x) {
 
 }  // namespace ternary_operator_tests
 
-namespace generated_code_tests {
-
-void MyPrintf(const char* fmt, ...) {}
-
-void foo() {
-  GeneratedStruct s;
-
-  // No rewrite expected below (i.e. no |.get()| appended), because the field
-  // dereferenced below comes from (simulated) generated code.
-  MyPrintf("%p", s.ptr_field);
-}
-
-}  // namespace generated_code_tests
-
 namespace templated_functions {
 
 template <typename T>

tools/clang/rewrite_raw_ptr_fields/tests/gen-deleted-operator-new-expected.txt

+==== BEGIN EDITS ====
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-deleted-operator-new-actual.cc:::-1:::-1:::base/memory/checked_ptr.h
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-deleted-operator-new-actual.cc:::1837:::5:::CheckedPtr<int> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-deleted-operator-new-actual.cc:::2313:::15:::CheckedPtr<NoNewOperator> 
+==== END EDITS ====
+==== BEGIN FIELD FILTERS ====
+MyStruct::pointer_to_pointee_with_no_operator_new  # pointee-has-no-operator-new
+NoNewOperator::field_in_struct_with_no_operator_new  # embedder-has-no-operator-new
+==== END FIELD FILTERS ====

tools/clang/rewrite_raw_ptr_fields/tests/gen-deleted-operator-new-test.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stddef.h>
+
+// This file (and other gen-*-test.cc files) tests generation of output for
+// --field-filter-file and therefore the expectations file
+// (gen-char-expected.txt) needs to be compared against the raw output of the
+// rewriter (rather than against the actual edits result).  This makes the test
+// incompatible with other tests, which require passing --apply-edits switch to
+// test_tool.py and so to disable the test it is named *-test.cc rather than
+// *-original.cc.
+//
+// To run the test use tools/clang/rewrite_raw_ptr_fields/tests/run_all_tests.py
+
+// The class below deletes the |operator new| - this simulate's Blink's
+// STACK_ALLOCATED macro and/or OilPan / GarbageCollected<T> classes.
+//
+// We assume that NoNewOperator classes are never allocated on the heap (i.e.
+// are never managed by PartitionAlloc) and therefore would not benefit from
+// the protection of CheckedPtr.  This assumption/heuristic is generally true,
+// but not always - for details how WTF::Vector can allocate elements without
+// operator new, see (Google-internal)
+// https://groups.google.com/a/google.com/g/chrome-memory-safety/c/GybWkNGqSyk/m/pUOjMvK5CQAJ
+class NoNewOperator {
+  void* operator new(size_t) = delete;
+
+  // Based on the deleted-oparator-new assumption/heuristic above, we assume
+  // that NoNewOperator will always be allocated on the stack or in OilPan and
+  // therefore doesn't need CheckedPtr protection (since it will be protected
+  // either by StackScanning or by OilPan's tracing and GC).  Therefore,
+  // |no_operator_new_struct| should be emitted as candidates for the
+  // --field-filter-file with "embedder-has-no-operator-new" tag.
+  int* field_in_struct_with_no_operator_new;
+};
+
+struct MyStruct {
+  // Based on the deleted-oparator-new assumption/heuristic above, we assume
+  // that NoNewOperator is never allocated on the PartitionAlloc-managed heap.
+  // This means that CheckedPtr would always be disabled for the field below and
+  // therefore the test below checks that |no_operator_new_pointee| is emitted
+  // as a candidate for the --field-filter-file with
+  // "pointee-has-no-operator-new" tag.
+  NoNewOperator* pointer_to_pointee_with_no_operator_new;
+};

tools/clang/rewrite_raw_ptr_fields/tests/gen-generated-code-expected.txt

+==== BEGIN EDITS ====
+include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen/generated_header.h:::-1:::-1:::base/memory/checked_ptr.h
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen/generated_header.h:::552:::5:::CheckedPtr<int> 
+r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen/generated_header.h:::570:::11:::CheckedPtr<SomeClass> 
+==== END EDITS ====
+==== BEGIN FIELD FILTERS ====
+GeneratedStruct::ptr_field  # generated-code
+GeneratedStruct::ptr_field2  # generated-code
+==== END FIELD FILTERS ====

tools/clang/rewrite_raw_ptr_fields/tests/gen-generated-code-test.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This file (and other gen-*-test.cc files) tests generation of output for
+// --field-filter-file and therefore the expectations file
+// (gen-char-expected.txt) needs to be compared against the raw output of the
+// rewriter (rather than against the actual edits result).  This makes the test
+// incompatible with other tests, which require passing --apply-edits switch to
+// test_tool.py and so to disable the test it is named *-test.cc rather than
+// *-original.cc.
+//
+// To run the test use tools/clang/rewrite_raw_ptr_fields/tests/run_all_tests.py
+
+// Fields in "gen/generated_header.h" should be emitted be emitted as candidates
+// for the --field-filter-file.
+#include "gen/generated_header.h"

tools/clang/rewrite_raw_ptr_fields/tests/various-types-expected.cc

@@ -23,16 +23,7 @@ struct SomeTemplate {
   T t;
 };
 
-// The class below deletes the |operator new| - this simulate's Blink's
-// STACK_ALLOCATED macro and/or OilPan / GarbageCollected<T> classes.
-class NoNewOperator {
-  void* operator new(size_t) = delete;
-};
-
 struct MyStruct {
-  // No rewrite expected for classes with no |operator new|.
-  NoNewOperator* no_new_ptr;
-
   // Expected rewrite: CheckedPtr<CheckedPtr<SomeClass>> double_ptr;
   // TODO(lukasza): Handle recursion/nesting.
   CheckedPtr<SomeClass*> double_ptr;

tools/clang/rewrite_raw_ptr_fields/tests/various-types-original.cc

@@ -21,16 +21,7 @@ struct SomeTemplate {
   T t;
 };
 
-// The class below deletes the |operator new| - this simulate's Blink's
-// STACK_ALLOCATED macro and/or OilPan / GarbageCollected<T> classes.
-class NoNewOperator {
-  void* operator new(size_t) = delete;
-};
-
 struct MyStruct {
-  // No rewrite expected for classes with no |operator new|.
-  NoNewOperator* no_new_ptr;
-
   // Expected rewrite: CheckedPtr<CheckedPtr<SomeClass>> double_ptr;
   // TODO(lukasza): Handle recursion/nesting.
   SomeClass** double_ptr;

tools/clang/scripts/test_tool.py

@@ -97,6 +97,7 @@ def _ApplyTool(tools_clang_scripts_directory,
           os.path.join(tools_clang_scripts_directory, 'apply_edits.py'), '-p',
           test_directory_for_tool
       ]
+      args.extend(actual_files)  # Limit edits to the test files.
       processes.append(subprocess.Popen(
           args, stdin=processes[-1].stdout, stdout=subprocess.PIPE))