Commit de344db5 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Chromium LUCI CQ

Document how fields order leads to dereferencing a destructed CheckedPtr

Bug: 1157988
Change-Id: I655e6b332382b6863c5a7484557f976cae4604f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593736
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837344}
parent 57e7c8da
......@@ -234,6 +234,34 @@ int** ptr_to_raw_int_ptr = reinterpret_cast<int**>(&checked_int_ptr);
*ptr_to_raw_int_ptr = new int(123);
```
#### Fields order leading to dereferencing a destructed CheckedPtr
Fields are destructed in the reverse order of their declarations:
```cpp
struct S {
Bar bar_; // Bar is destructed last.
CheckedPtr<Foo> foo_ptr_; // CheckedPtr (not Foo) is destructed first.
};
```
If destructor of `Bar` has a pointer to `S`, then it may try to dereference
`s->foo_ptr_` after `CheckedPtr` has been already destructed.
In practice this will lead to a null dereference and a crash
(e.g. see https://crbug.com/1157988).
Note that this code pattern would have resulted in an Undefined Behavior,
even if `foo_ptr_` was a raw `Foo*` pointer (see the
[memory-safete-dev@ discussion](https://groups.google.com/a/chromium.org/g/memory-safety-dev/c/3sEmSnFc61I/m/Ng6PyqDiAAAJ)
for more details).
Possible solutions (in no particular order):
- Declare the `bar_` field as the very last field.
- Declare the `foo_` field (and other POD or raw-pointer-like fields)
before any other fields.
- Avoid accessing `S` from the destructor of `Bar`
(and in general, avoid doing significant work from destructors).
#### Other
TODO(bartekn): Document runtime errors encountered by BackupRefPtr
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment