Rewrite most Foo* field_ pointer fields to raw_ptr<Foo> field_

[magreenblatt]

Original report by me.

---

Chromium has introduced a new raw_ptr<> type that potentially offers protection against use-after-free bugs. We should look at using this type in CEF (libcef/ code) as well.

For more information, see MiraclePtr One Pager [1], the PSA at chromium-dev@ [2], and the raw_ptr documentation at //base/memory/raw_ptr.md.

There is a script for automatically performing the conversion here and related Chromium changes in issue #1073933.

[1] https://docs.google.com/document/d/1pnnOAIz_DMWDI4oIOFoMAqLnf_MZ2GsrJNb_dbQ3ZBg/edit?usp=sharing
[2] https://groups.google.com/a/chromium.org/g/chromium-dev/c/vAEeVifyf78/m/SkBUc6PhBAAJ

[magreenblatt]

The benefits here may require PartitionAlloc, in which case we won’t get them when the allocator shim is disabled (see #3061)

[magreenblatt]

We can also restore the unretained dangling pointer detector crash-by-default behavior after this issue is resolved. See #3693.

[magreenblatt]

According to this chart, PartitionAlloc-Everywhere (PA-E) (use_partition_alloc_as_malloc) is not available in component builds, meaning that build support for Use-after-Free protection via BackupRefPtr (BRP) (enable_backup_ref_ptr_support) will require non-component builds. The actual requirement appears to be is_win && !is_component_build && !is_debug (_default_use_allocator_shim). Other platforms don't seem to have this requirement.

[magreenblatt]

Generating a Release build on Windows (M125) with is_component_build=false dcheck_always_on=true provides the following default values:

> gn args out\Release_GN_x64 --list=use_partition_alloc_as_malloc
use_partition_alloc_as_malloc
    Current value (from the default) = true
      From //base/allocator/partition_allocator/partition_alloc.gni:88

    PartitionAlloc-Everywhere (PA-E). Causes allocator_shim.cc to route
    calls to PartitionAlloc, rather than some other platform allocator.

> gn args out\Release_GN_x64 --list=enable_backup_ref_ptr_support
enable_backup_ref_ptr_support
    Current value (from the default) = true
      From //base/allocator/partition_allocator/partition_alloc.gni:147

> gn args out\Release_GN_x64 --list=enable_backup_ref_ptr_slow_checks
enable_backup_ref_ptr_slow_checks
    Current value (from the default) = false
      From //base/allocator/partition_allocator/partition_alloc.gni:189

> gn args out\Release_GN_x64 --list=enable_dangling_raw_ptr_checks
enable_dangling_raw_ptr_checks
    Current value (from the default) = true
      From //base/allocator/partition_allocator/partition_alloc.gni:204

> gn args out\Release_GN_x64 --list=backup_ref_ptr_poison_oob_ptr
backup_ref_ptr_poison_oob_ptr
    Current value (from the default) = false
      From //base/allocator/partition_allocator/partition_alloc.gni:223

> gn args out\Release_GN_x64 --list=enable_backup_ref_ptr_instance_tracer
enable_backup_ref_ptr_instance_tracer
    Current value (from the default) = false
      From //base/allocator/partition_allocator/partition_alloc.gni:216

[magreenblatt]

Some problems trying to build the rewrite tool on Windows: https://issues.chromium.org/issues/327699485#comment4

[magreenblatt]

Building the rewrite tool works on Ubuntu 22. The "Make sure out/rewrite/gen/…../some-generated-header.h files are built" step fails due to too many arguments. Using this version instead to build 1000 targets at a time (executes ~20k steps vs ~65k for a full build):

GEN_H_TARGETS=`ninja -C out/rewrite -t targets all | grep '^gen/.*\(\.h\|inc\|css_tokenizer_codepoints.cc\)' | cut -d : -f 1`
echo $GEN_H_TARGETS > targets.txt
cat targets.txt | xargs -n 1000 ninja -C out/rewrite

Also need to generate the CEF headers:

ninja -C out/rewrite cef_make_headers

[magreenblatt]

The clang scripts ignore directories that are not part of the main Chromium src repository by default, so we need to modify them a bit to work with the cef directory (apply clang_scripts.patch). We can then run with the following extract of rewrite.sh:

#!/bin/bash

set -e  # makes the script quit on any command failure
set -u  # unset variables are quit-worthy errors

OUT_DIR="out/rewrite"
COMPILE_DIRS=cef/libcef
EDIT_DIRS=cef/libcef

# A preliminary rewriter run in a special mode that generates a list of fields
# to ignore. These fields would likely lead to compiler errors if rewritten.
echo "*** Generating the ignore list ***"
time tools/clang/scripts/run_tool.py \
    --tool rewrite_raw_ptr_fields \
    --generate-compdb \
    -p $OUT_DIR \
    $COMPILE_DIRS > ~/scratch/rewriter.out
cat ~/scratch/rewriter.out \
    | sed '/^==== BEGIN FIELD FILTERS ====$/,/^==== END FIELD FILTERS ====$/{//!b};d' \
    | sort | uniq > ~/scratch/automated-fields-to-ignore.txt
cat ~/scratch/automated-fields-to-ignore.txt \
    tools/clang/rewrite_raw_ptr_fields/manual-fields-to-ignore.txt \
    | grep -v "base::FileDescriptorWatcher::Controller::watcher_" \
    > ~/scratch/combined-fields-to-ignore.txt

# Main rewrite.
echo "*** Running the main rewrite phase ***"
time tools/clang/scripts/run_tool.py \
    --tool rewrite_raw_ptr_fields \
    --tool-arg=--exclude-fields=$HOME/scratch/combined-fields-to-ignore.txt \
    -p $OUT_DIR \
    $COMPILE_DIRS > ~/scratch/rewriter.main.out

# Apply edits generated by the main rewrite.
echo "*** Applying edits ***"
cat ~/scratch/rewriter.main.out | \
    tools/clang/scripts/extract_edits.py | \
    tools/clang/scripts/apply_edits.py -p $OUT_DIR $EDIT_DIRS

Result:

*** Generating the ignore list ***
Evaluating 584 files
Processed 274 files with /home/marshall/code/chromium_git/chromium/src/third_party/llvm-build/Release+Asserts/bin/rewrite_raw_ptr_fields tool (0 failures) [100.00%]

real	12m3.214s
user	87m36.712s
sys	2m48.719s
*** Running the main rewrite phase ***
Evaluating 584 files
Processed 274 files with /home/marshall/code/chromium_git/chromium/src/third_party/llvm-build/Release+Asserts/bin/rewrite_raw_ptr_fields tool (0 failures) [100.00%]

real	11m29.496s
user	85m21.222s
sys	2m29.357s
*** Applying edits ***
Evaluating 584 files
Applied 273 edits (0 errors) to 108 files [100.00%]

[magreenblatt]

  # - enable_backup_ref_ptr_instance_tracer: use a global table to track all
  #   live raw_ptr/raw_ref instances to help debug dangling pointers at test
  #   end.

This is a very useful feature that we'll enable by default in non-Official builds where BackupRefPtr is supported. From the commit, the runtime penalty for this is <10%.