Dangling Pointer Guide

A dangling pointer has been found in your patch? This doc will help you fix it.

This document can also be commented upon: https://docs.google.com/document/d/19yyWdCQB5KwYtRE5cwadNf30jNSSWwMbim1KJjc-eZQ/edit#

See also the general instructions about the dangling pointer detector: docs/dangling_ptr.md

Table of content

Case 1 I don’t own the affected component

If you do not directly own the affected component, you don't need to solve the issue yourself… though doing so is a great way to learn about and improve the codebase.

Please annotate the pointer with DanglingUntriaged, and indicate the test case that can be used to reproduce.

// Dangling when executing TestSuite.TestCase on Windows:
raw_ptr<T, DanglingUntriaged> ptr_;

Opening and filling a P2 bug is a nice thing to do, but it is not required.

Rationale: Engineers might uncover new dangling pointers, by testing new code paths. Knowing about dangling pointers is a purely positive increment. In some cases, the affected component belongs to a different team. We don’t want to disrupt engineers achieving their primary goal, if they only “discover” a dangling pointer. Annotating the pointer makes the issue visible directly in the code, improving our knowledge of Chrome.

Case 2 The dangling pointer does not own the deleted object

Incorrect destruction order

This represents ~25% of the dangling pointers.

In the majority of cases, this happens when dependent objects are declared in the wrong order in a class, causing the dependency to be released first, thus creating a dangling pointer in the other.

It is important to reorder them correctly to prevent pre-existing and future UAF in destructors.

See Fix member declaration order and Fix reset ordering examples.

Observer callback

This represents ~4% of the dangling pointers.

It is important to clear the pointer when the object is about to be deleted. Chrome uses the observer pattern heavily. In some cases, the observer does not clear its pointer toward the observed class when notified of its destruction.

Challenging lifespan

It can be challenging to deal with an object's lifespan. Sometimes, the lifetime of two objects are completely different.

Removing the pointer may be a good thing to do. Sometimes, it can be replaced by:

Fallback solution

As a last resort, when the situation is perfectly understood, and you believe it is better to let the pointer dangle, the raw_ptr can be annotated with DisableDanglingPtrDetection. A comment explaining why this is safe must be added.

// DisableDanglingPtrDetection option for raw_ptr annotates
// "intentional-and-safe" dangling pointers. It is meant to be used at the
// margin, only if there is no better way to re-architecture the code.
//
// Usage:
// raw_ptr<T, DisableDanglingPtrDetection> dangling_ptr;
//
// When using it, please provide a justification about what guarantees that it
// will never be dereferenced after becoming dangling.

In emergency situations: DanglingUntriaged can be used similarly, in case your patch needs to land as soon as possible.

Case 3 The pointer manages ownership over the object

raw_ptr, just like raw pointers T*, are not meant to keep an object alive. It is preferable not to manage memory manually using them and new/delete. Calling delete on a raw_ptr will cause the raw_ptr to become immediately dangling.

Smart pointers

First, consider replacing the raw_ptr with a smart pointer:

  • std::unique_ptr (See example)
  • scoped_refptr

Object vended from C API

In some cases, the object is vended by a C API. It means new/delete are not used, but some equivalent API. In this case, it still makes sense to use a unique_ptr, but with a custom deleter: example.

For most common objects, a wrapper already exists: base::ScopedFILE, base::ScopedTempDir, ..

Object conditionally owned

In some cases, the raw_ptr conditionally owns the memory, depending on some logic or some is_owned boolean. This can still use a unique_ptr (example)

std::unique_ptr<T> t_owned_;
raw_ptr<T> t_; // Reference `t_owned_` or an external object.

Fallback solution

If no solution with a smart pointer is found:

You can use raw_ptr::ClearAndDelete() or raw_ptr::ClearAndDeleteArray() to clear the pointer and free the object in a single operation.

BeforeAfter
delete ptr_ptr_.ClearAndDelete();
delete array_ptr_.ClearAndDeleteArray();

When delete is not used, but the deletion happens through some C API or some external manager, the raw_ptr::ExtractAsDangling() can be used to clear the pointer, and return the pointer to be passed to the API. The return value must not be saved, thus outliving the line where it was called.

This method should be used wisely, and only if there is no other way to rework the dangling raw_ptr.

BeforeAfter
ExternalAPIDelete(ptr_);ExternalAPIDelete(ptr_.ExtractAsDangling());