From 1006956ed50cabafe7f8b49a0955dab748c31e7c Mon Sep 17 00:00:00 2001 From: josh11b Date: Thu, 27 Jul 2023 13:39:54 -0700 Subject: [PATCH] Fix `class_value_ptr` invariant bug found by fuzzer (#3025) Invariant is `*class_value_ptr == this` when the static type matches the dynamic type. --- explorer/ast/value.cpp | 22 ++++-- explorer/ast/value.h | 3 +- .../linked_list/fuzzed_linked_list.carbon | 69 +++++++++++++++++++ 3 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 explorer/testdata/linked_list/fuzzed_linked_list.carbon diff --git a/explorer/ast/value.cpp b/explorer/ast/value.cpp index 08ab579c1dd4..01a01d1540c6 100644 --- a/explorer/ast/value.cpp +++ b/explorer/ast/value.cpp @@ -19,7 +19,6 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Casting.h" -#include "llvm/Support/Error.h" namespace Carbon { @@ -128,6 +127,7 @@ NominalClassValue::NominalClassValue( inits_(inits), base_(base), class_value_ptr_(class_value_ptr) { + CARBON_CHECK(!base || (*base)->class_value_ptr() == class_value_ptr); // Update ancestors's class value to point to latest child. *class_value_ptr_ = this; } @@ -375,15 +375,27 @@ static auto SetFieldImpl( if (auto inits = SetFieldImpl(arena, &object.inits(), path_begin, path_end, field_value, source_loc); inits.ok()) { - return arena->New( - &object.type(), *inits, object.base(), object.class_value_ptr()); + auto* class_value_ptr = arena->New(); + std::vector base_path; + for (auto base = object.base(); base; base = (*base)->base()) { + base_path.push_back(*base); + } + std::optional> base; + for (auto* base_path_elem : llvm::reverse(base_path)) { + base = arena->New(&base_path_elem->type(), + &base_path_elem->inits(), base, + class_value_ptr); + } + return arena->New(&object.type(), *inits, base, + class_value_ptr); } else if (object.base().has_value()) { auto new_base = SetFieldImpl(arena, object.base().value(), path_begin, path_end, field_value, source_loc); if (new_base.ok()) { + auto as_nominal_class_value = cast(*new_base); return arena->New( - &object.type(), &object.inits(), - cast(*new_base), object.class_value_ptr()); + &object.type(), &object.inits(), as_nominal_class_value, + as_nominal_class_value->class_value_ptr()); } } // Failed to match, show full object content diff --git a/explorer/ast/value.h b/explorer/ast/value.h index 6ec59bdfc937..4963c9847efa 100644 --- a/explorer/ast/value.h +++ b/explorer/ast/value.h @@ -400,7 +400,8 @@ class NominalClassValue : public Value { // NominalClassValue*, that must be common to all NominalClassValue of the // same object. The pointee is updated, when `NominalClassValue`s are // constructed, to point to the `NominalClassValue` corresponding to the - // child-most class type. + // child-most class type. Sets *class_value_ptr = this, which corresponds to + // the static type of the value matching its dynamic type. NominalClassValue(Nonnull type, Nonnull inits, std::optional> base, Nonnull class_value_ptr); diff --git a/explorer/testdata/linked_list/fuzzed_linked_list.carbon b/explorer/testdata/linked_list/fuzzed_linked_list.carbon new file mode 100644 index 000000000000..6128118ab62e --- /dev/null +++ b/explorer/testdata/linked_list/fuzzed_linked_list.carbon @@ -0,0 +1,69 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// CHECK:STDOUT: result: 0 + +package ExplorerTest api; + +class ListElement { + fn Create(value: i32) -> ListElement { + return {.element = Optional(i32).Create(value), + .next = Optional(ListElement*).CreateEmpty()}; + } + + fn set[addr self: Self*](value: Optional(ListElement*)) { + (*self).next = value; + } + + var element: Optional(i32); + var next: Optional(ListElement*); +} + +class List { + fn Create() -> List { + return {.next = Optional(ListElement*).CreateEmpty()}; + } + + fn insert[addr self: (Self*)](value: i32) { + if (not (*self).next.HasValue()) { + (*self).next = Optional(ListElement*).Create( + heap.New(ListElement.Create(value))); + return; + } + + var iter: Optional(ListElement*) = (*self).next; + var node: auto = iter.Get(); + while ((*node).next.HasValue()) { + iter = (*node).next; + node = iter.Get(); + } + + var v: ListElement* = iter.Get(); + var x: Optional(ListElement*) + = Optional(ListElement*).Create(heap.New((*node))); + (*v).set(x); + } + + fn clean[addr self: (Self*)]() { + var head: auto = (*self).next; + while (head.HasValue()) { + var tmp: auto = head; + head = (*head.Get()).next; + heap.Delete(tmp.Get()); + } + + (*self).next = Optional(ListElement*).CreateEmpty(); + } + + var next: Optional(ListElement*); +} + +fn Main() -> i32 { + var list: List = List.Create(); + list.insert(2); + list.insert(3); + list.clean(); + return 0; +}