[tint] Convert legacy suppressions for C-style code Changes the old style TINT_BEGIN/TINT_END suppressions to the new DAWN_UNSAFE_BUFFERS macros for C API calls and pointer/array arithmetic. Bug: 551707095 Change-Id: If808b3788d8336f7e89bbd011b9f79d7d5fd5608 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/335515 Reviewed-by: James Price <jrprice@google.com> Auto-Submit: Ryan Harrison <rharrison@chromium.org> Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/tint/cmd/fuzz/common/init.cc b/src/tint/cmd/fuzz/common/init.cc index 9d6e315..1c0b083 100644 --- a/src/tint/cmd/fuzz/common/init.cc +++ b/src/tint/cmd/fuzz/common/init.cc
@@ -33,16 +33,18 @@ #include "src/tint/cmd/fuzz/common/helper.h" #include "src/tint/utils/command/cli.h" #include "src/tint/utils/containers/vector.h" +#include "src/utils/compiler.h" namespace tint::fuzz::common { -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); int ParseFuzzerOptions(FuzzerType type, int* argc, char*** argv, Options* options) { tint::cli::OptionSet opts; tint::Vector<std::string_view, 8> arguments; for (int i = 1; i < *argc; i++) { - std::string_view arg((*argv)[i]); + // SAFETY: `*argv` comes from program main entry point and has at least `*argc` valid + // elements. + std::string_view arg(DAWN_UNSAFE_BUFFERS((*argv)[i])); if (!arg.empty()) { arguments.Push(arg); } @@ -56,7 +58,9 @@ std::cerr << "Standard libfuzzer "; // libfuzzer will print 'Usage:' static char help[] = "-help=1"; *argc = 2; - (*argv)[1] = help; + // SAFETY: `*argv` is guaranteed by the program entry point to have at least 2 valid + // elements. + DAWN_UNSAFE_BUFFERS((*argv)[1]) = help; }; auto& opt_help = opts.Add<tint::cli::BoolOption>("help", "shows the usage"); @@ -125,6 +129,5 @@ return 0; } -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); } // namespace tint::fuzz::common
diff --git a/src/tint/lang/wgsl/reader/parser/lexer.cc b/src/tint/lang/wgsl/reader/parser/lexer.cc index 5286ecf..07e2e41 100644 --- a/src/tint/lang/wgsl/reader/parser/lexer.cc +++ b/src/tint/lang/wgsl/reader/parser/lexer.cc
@@ -163,13 +163,10 @@ return l[pos]; } -// This pointer is passed into std::from_chars which requires a pointer beyond the end of contiguous -// range, not an end iterator, so will always hit this warning. -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); const char* Lexer::line_end() const { - return &(line()[length() - 1]) + 1; + // SAFETY: line() is a valid string_view, so line().data() + line().size() is bounds-safe. + return DAWN_UNSAFE_BUFFERS(line().data() + line().size()); } -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); std::string_view Lexer::substr(uint32_t offset, uint32_t count) { return line().substr(offset, count);
diff --git a/src/tint/utils/command/args.cc b/src/tint/utils/command/args.cc index fb91a47..77a393a 100644 --- a/src/tint/utils/command/args.cc +++ b/src/tint/utils/command/args.cc
@@ -27,20 +27,24 @@ #include "src/tint/utils/command/args.h" +#include <span> + +#include "src/utils/compiler.h" + namespace tint::args { -// Working with argv is known to always cause this warning to fire -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); tint::Vector<std::string_view, 8> Vectorize(int argc, const char** argv) { tint::Vector<std::string_view, 8> arguments; + // SAFETY: `argv` is guaranteed by the program entry point (argc/argv) to contain at least + // `argc` elements. + auto args = DAWN_UNSAFE_BUFFERS(std::span<const char* const>{argv, static_cast<size_t>(argc)}); for (int i = 1; i < argc; i++) { - std::string_view arg(argv[i]); + std::string_view arg(args[static_cast<size_t>(i)]); if (!arg.empty()) { - arguments.Push(argv[i]); + arguments.Push(arg); } } return arguments; } -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); } // namespace tint::args
diff --git a/src/tint/utils/containers/hashmap_base.h b/src/tint/utils/containers/hashmap_base.h index 9aed7ae..512c60c 100644 --- a/src/tint/utils/containers/hashmap_base.h +++ b/src/tint/utils/containers/hashmap_base.h
@@ -41,6 +41,7 @@ #include "src/tint/utils/math/math.h" #include "src/tint/utils/memory/aligned_storage.h" #include "src/tint/utils/rtti/traits.h" +#include "src/utils/compiler.h" // This file implements a custom STL style container & iterator in a performant manner, using // C-style data access. It is not unexpected that -Wunsafe-buffer-usage triggers in this code, since @@ -48,8 +49,6 @@ // Attempting to change this code in simple ways to quiet these errors either a) negatively affects // the performance by introducing unneeded copes, or b) uses typing shenanigans to work around the // warning that other linters/analyses are unhappy with. -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - namespace tint { /// HashmapKey wraps the comparator type for a Hashmap and Hashset. @@ -668,9 +667,13 @@ nodes_allocation->next = allocations_; allocations_ = nodes_allocation; - auto* nodes = Bitcast<Node*>(memory + kAllocationSize); + // SAFETY: memory is allocated to be kAllocationSize + space for count nodes, so memory + // + kAllocationSize is the boundary of the nodes. If there are 0 nodes it will not be + // used. + auto* nodes = Bitcast<Node*>(DAWN_UNSAFE_BUFFERS(memory + kAllocationSize)); for (size_t i = 0; i < count; i++) { - Add(&nodes[i]); + // SAFETY: nodes has an allocated size of at least `count` elements. + Add(DAWN_UNSAFE_BUFFERS(&nodes[i])); } } }; @@ -690,6 +693,4 @@ } // namespace tint -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - #endif // SRC_TINT_UTILS_CONTAINERS_HASHMAP_BASE_H_
diff --git a/src/tint/utils/containers/vector.h b/src/tint/utils/containers/vector.h index e5a7acf..0e40fda 100644 --- a/src/tint/utils/containers/vector.h +++ b/src/tint/utils/containers/vector.h
@@ -168,8 +168,6 @@ } // namespace internal -// VectorIterator intrinsically depends on pointer math, so will always cause UBU warnings -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// VectorIterator is a forward iterator of Vector elements. template <typename T, bool FORWARD = true> class VectorIterator { @@ -309,14 +307,20 @@ /// Increments the iterator (prefix) /// @returns this VectorIterator VectorIterator& operator++() { - this->ptr_ = FORWARD ? this->ptr_ + 1 : this->ptr_ - 1; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + this->ptr_ = DAWN_UNSAFE_BUFFERS(FORWARD ? this->ptr_ + 1 : this->ptr_ - 1); return *this; } /// Decrements the iterator (prefix) /// @returns this VectorIterator VectorIterator& operator--() { - this->ptr_ = FORWARD ? this->ptr_ - 1 : this->ptr_ + 1; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + this->ptr_ = DAWN_UNSAFE_BUFFERS(FORWARD ? this->ptr_ - 1 : this->ptr_ + 1); return *this; } @@ -324,7 +328,10 @@ /// @returns a VectorIterator that points to the element before the increment VectorIterator operator++(int) { VectorIterator res = *this; - this->ptr_ = FORWARD ? this->ptr_ + 1 : this->ptr_ - 1; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + this->ptr_ = DAWN_UNSAFE_BUFFERS(FORWARD ? this->ptr_ + 1 : this->ptr_ - 1); return res; } @@ -332,7 +339,10 @@ /// @returns a VectorIterator that points to the element before the decrement VectorIterator operator--(int) { VectorIterator res = *this; - this->ptr_ = FORWARD ? this->ptr_ - 1 : this->ptr_ + 1; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + this->ptr_ = DAWN_UNSAFE_BUFFERS(FORWARD ? this->ptr_ - 1 : this->ptr_ + 1); return res; } @@ -340,7 +350,10 @@ /// @param n the number of elements /// @returns this VectorIterator VectorIterator operator+=(std::ptrdiff_t n) { - this->ptr_ = FORWARD ? this->ptr_ + n : this->ptr_ - n; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + this->ptr_ = DAWN_UNSAFE_BUFFERS(FORWARD ? this->ptr_ + n : this->ptr_ - n); return *this; } @@ -348,7 +361,10 @@ /// @param n the number of elements /// @returns this VectorIterator VectorIterator operator-=(std::ptrdiff_t n) { - this->ptr_ = FORWARD ? this->ptr_ - n : this->ptr_ + n; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + this->ptr_ = DAWN_UNSAFE_BUFFERS(FORWARD ? this->ptr_ - n : this->ptr_ + n); return *this; } @@ -356,9 +372,15 @@ /// @returns a new VectorIterator progressed by @p n elements VectorIterator operator+(std::ptrdiff_t n) const { #if TINT_VECTOR_MUTATION_CHECKS_ENABLED - return VectorIterator{FORWARD ? ptr_ + n : ptr_ - n, iterator_count_}; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + return VectorIterator{DAWN_UNSAFE_BUFFERS(FORWARD ? ptr_ + n : ptr_ - n), iterator_count_}; #else - return VectorIterator{FORWARD ? ptr_ + n : ptr_ - n}; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + return VectorIterator{DAWN_UNSAFE_BUFFERS(FORWARD ? ptr_ + n : ptr_ - n)}; #endif } @@ -366,16 +388,25 @@ /// @returns a new VectorIterator regressed by @p n elements VectorIterator operator-(std::ptrdiff_t n) const { #if TINT_VECTOR_MUTATION_CHECKS_ENABLED - return VectorIterator{FORWARD ? ptr_ - n : ptr_ + n, iterator_count_}; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + return VectorIterator{DAWN_UNSAFE_BUFFERS(FORWARD ? ptr_ - n : ptr_ + n), iterator_count_}; #else - return VectorIterator{FORWARD ? ptr_ - n : ptr_ + n}; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller (i.e., not incrementing past end() or decrementing before + // begin()). + return VectorIterator{DAWN_UNSAFE_BUFFERS(FORWARD ? ptr_ - n : ptr_ + n)}; #endif } /// @param other the other iterator /// @returns the number of elements between this iterator and @p other std::ptrdiff_t operator-(const VectorIterator& other) const { - return FORWARD ? ptr_ - other.ptr_ : other.ptr_ - ptr_; + // SAFETY: Pointer arithmetic is bounds-safe assuming standard STL iteration preconditions + // are satisfied by the caller, both iterators refer to the same vector allocation. + return FORWARD ? DAWN_UNSAFE_BUFFERS(ptr_ - other.ptr_) + : DAWN_UNSAFE_BUFFERS(other.ptr_ - ptr_); } private: @@ -384,7 +415,6 @@ std::atomic<uint32_t>* iterator_count_ = nullptr; #endif }; -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @param out the stream to write to /// @param it the VectorIterator @@ -596,8 +626,6 @@ /// be made size_t Capacity() const { return impl_.slice.buffer.size(); } - // Iterating the buffers to move - TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// Reserves memory to hold at least `new_cap` elements /// @param new_cap the new vector capacity void Reserve(size_t new_cap) { @@ -607,14 +635,15 @@ size_t len = impl_.slice.len; impl_.Allocate(new_cap); for (size_t i = 0; i < len; i++) { - new (&impl_.slice.buffer[i]) T(std::move(old_data[i])); - old_data[i].~T(); + // SAFETY: old_data is a valid array of length `len` elements. + new (&impl_.slice.buffer[i]) T(std::move(DAWN_UNSAFE_BUFFERS(old_data[i]))); + // SAFETY: old_data is a valid array of length `len` elements. + DAWN_UNSAFE_BUFFERS(old_data[i]).~T(); } impl_.slice.len = len; impl_.Free(old_data); } } - TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// Resizes the vector to the given length, expanding capacity if necessary. /// New elements are zero-initialized @@ -841,30 +870,40 @@ return const_iterator{impl_.slice.buffer.data(), &iterator_count_}; } - // STL end()/rbegin() is beyond the end of the span, so requires unsafe pointer math - TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @returns a forward iterator to one-pass the last element of the vector iterator end() { - return iterator{impl_.slice.buffer.data() + impl_.slice.len, &iterator_count_}; + // SAFETY: Implements the STL behaviour of end() being one beyond the end of the allocation, + // so depends on caller following the same restrictions, i.e. not dereferencing. + return iterator{DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len), + &iterator_count_}; } /// @returns a forward iterator to one-pass the last element of the vector const const_iterator end() const { - return const_iterator{impl_.slice.buffer.data() + impl_.slice.len, &iterator_count_}; + // SAFETY: Implements the STL behaviour of end() being one beyond the end of the allocation, + // so depends on caller following the same restrictions, i.e. not dereferencing. + return const_iterator{DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len), + &iterator_count_}; } /// @returns a reverse iterator to the last element of the vector reverse_iterator rbegin() { - return reverse_iterator{impl_.slice.buffer.data() + impl_.slice.len, &iterator_count_} + 1; + // SAFETY: Implements the STL behaviour of rbegin() being one beyond the end of the + // allocation, so depends on caller following the same restrictions, i.e. not dereferencing. + return reverse_iterator{DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len), + &iterator_count_} + + 1; } /// @returns a reverse iterator to the last element of the vector const const_reverse_iterator rbegin() const { - return const_reverse_iterator{impl_.slice.buffer.data() + impl_.slice.len, - &iterator_count_} + + // SAFETY: Implements the STL behaviour of rbegin() being one beyond the end of the + // allocation, so depends on caller following the same restrictions, i.e. not dereferencing. + return const_reverse_iterator{ + DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len), + &iterator_count_} + 1; } - TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @returns a reverse iterator to one element before the first element of the vector reverse_iterator rend() { @@ -882,26 +921,36 @@ /// @returns a forward iterator to the first element of the vector const const_iterator begin() const { return const_iterator{impl_.slice.buffer.data()}; } - // STL end()/rbegin() is beyond the end of the span, so requires unsafe pointer math - TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @returns a forward iterator to one-pass the last element of the vector - iterator end() { return iterator{impl_.slice.buffer.data() + impl_.slice.len}; } + iterator end() { + // SAFETY: Implements the STL behaviour of end() being one beyond the end of the allocation, + // so depends on caller following the same restrictions, i.e. not dereferencing. + return iterator{DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len)}; + } /// @returns a forward iterator to one-pass the last element of the vector const const_iterator end() const { - return const_iterator{impl_.slice.buffer.data() + impl_.slice.len}; + // SAFETY: Implements the STL behaviour of end() being one beyond the end of the allocation, + // so depends on caller following the same restrictions, i.e. not dereferencing. + return const_iterator{DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len)}; } /// @returns a reverse iterator to the last element of the vector reverse_iterator rbegin() { - return reverse_iterator{impl_.slice.buffer.data() + impl_.slice.len} + 1; + // SAFETY: Implements the STL behaviour of rbegin() being one beyond the end of the + // allocation, so depends on caller following the same restrictions, i.e. not dereferencing. + return reverse_iterator{DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len)} + + 1; } /// @returns a reverse iterator to the last element of the vector const const_reverse_iterator rbegin() const { - return const_reverse_iterator{impl_.slice.buffer.data() + impl_.slice.len} + 1; + // SAFETY: Implements the STL behaviour of rbegin() being one beyond the end of the + // allocation, so depends on caller following the same restrictions, i.e. not dereferencing. + return const_reverse_iterator{ + DAWN_UNSAFE_BUFFERS(impl_.slice.buffer.data() + impl_.slice.len)} + + 1; } - TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @returns a reverse iterator to one element before the first element of the vector reverse_iterator rend() { return reverse_iterator{impl_.slice.buffer.data()} + 1; } @@ -1370,14 +1419,15 @@ /// @returns a pointer to the first element in the vector const T* begin() const { return slice_->buffer.data(); } - // STL end()/rbegin() is beyond the end of the span, so requires unsafe pointer math - TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @returns a pointer to one past the last element in the vector - const T* end() const { return slice_->buffer.data() + slice_->len; } + const T* end() const { + // SAFETY: Implements the STL behaviour of end() being one beyond the end of the allocation, + // so depends on caller following the same restrictions, i.e. not dereferencing. + return DAWN_UNSAFE_BUFFERS(slice_->buffer.data() + slice_->len); + } /// @returns a reverse iterator starting with the last element in the vector auto rbegin() const { return std::reverse_iterator<const T*>(end()); } - TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); /// @returns the end for a reverse iterator auto rend() const { return std::reverse_iterator<const T*>(begin()); }
diff --git a/src/tint/utils/containers/vector_test.cc b/src/tint/utils/containers/vector_test.cc index b3d6949..fdb2ddd 100644 --- a/src/tint/utils/containers/vector_test.cc +++ b/src/tint/utils/containers/vector_test.cc
@@ -41,8 +41,6 @@ TINT_BEGIN_DISABLE_WARNING(UNREACHABLE_CODE); // Some of these tests are inspecting the underlying pointers being used by iterators, so there is // no simple way to avoid unsafe buffer usage warnings. -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - namespace tint::test { class C0 : public Castable<C0> {}; @@ -1976,15 +1974,18 @@ static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.begin())>>); static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.end())>>); EXPECT_EQ(&*vec.begin(), &vec[0]); - EXPECT_EQ(&*vec.end(), &vec[0] + 3); + // SAFETY: The vector has 3 elements, so &vec[0] + 3 represents the valid end-of-range pointer. + EXPECT_EQ(&*vec.end(), DAWN_UNSAFE_BUFFERS(&vec[0] + 3)); } TEST(TintVectorTest, RbeginRend_NoSpill) { Vector<std::string, 3> vec{"front", "mid", "back"}; static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.rbegin())>>); static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.rend())>>); - EXPECT_EQ(&*vec.rbegin(), &vec[0] + 2); - EXPECT_EQ(&*vec.rend(), &vec[0] - 1); + // SAFETY: The vector has 3 elements, so &vec[0] + 2 and &vec[0] - 1 represent the valid rbegin + // and rend pointers. + EXPECT_EQ(&*vec.rbegin(), DAWN_UNSAFE_BUFFERS(&vec[0] + 2)); + EXPECT_EQ(&*vec.rend(), DAWN_UNSAFE_BUFFERS(&vec[0] - 1)); } TEST(TintVectorTest, BeginEnd_WithSpill) { @@ -1992,15 +1993,18 @@ static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.begin())>>); static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.end())>>); EXPECT_EQ(&*vec.begin(), &vec[0]); - EXPECT_EQ(&*vec.end(), &vec[0] + 3); + // SAFETY: The vector has 3 elements, so &vec[0] + 3 represents the valid end-of-range pointer. + EXPECT_EQ(&*vec.end(), DAWN_UNSAFE_BUFFERS(&vec[0] + 3)); } TEST(TintVectorTest, RbeginRend_WithSpill) { Vector<std::string, 2> vec{"front", "mid", "back"}; static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.rbegin())>>); static_assert(!std::is_const_v<std::remove_reference_t<decltype(*vec.rend())>>); - EXPECT_EQ(&*vec.rbegin(), &vec[0] + 2); - EXPECT_EQ(&*vec.rend(), &vec[0] - 1); + // SAFETY: The vector has 3 elements, so &vec[0] + 2 and &vec[0] - 1 represent the valid rbegin + // and rend pointers. + EXPECT_EQ(&*vec.rbegin(), DAWN_UNSAFE_BUFFERS(&vec[0] + 2)); + EXPECT_EQ(&*vec.rend(), DAWN_UNSAFE_BUFFERS(&vec[0] - 1)); } TEST(TintVectorTest, ConstBeginEnd_NoSpill) { @@ -2008,15 +2012,18 @@ static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.begin())>>); static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.end())>>); EXPECT_EQ(&*vec.begin(), &vec[0]); - EXPECT_EQ(&*vec.end(), &vec[0] + 3); + // SAFETY: The vector has 3 elements, so &vec[0] + 3 represents the valid end-of-range pointer. + EXPECT_EQ(&*vec.end(), DAWN_UNSAFE_BUFFERS(&vec[0] + 3)); } TEST(TintVectorTest, ConstRbeginRend_NoSpill) { const Vector<std::string, 3> vec{"front", "mid", "back"}; static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.rbegin())>>); static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.rend())>>); - EXPECT_EQ(&*vec.rbegin(), &vec[0] + 2); - EXPECT_EQ(&*vec.rend(), &vec[0] - 1); + // SAFETY: The vector has 3 elements, so &vec[0] + 2 and &vec[0] - 1 represent the valid rbegin + // and rend pointers. + EXPECT_EQ(&*vec.rbegin(), DAWN_UNSAFE_BUFFERS(&vec[0] + 2)); + EXPECT_EQ(&*vec.rend(), DAWN_UNSAFE_BUFFERS(&vec[0] - 1)); } TEST(TintVectorTest, ConstBeginEnd_WithSpill) { @@ -2024,15 +2031,18 @@ static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.begin())>>); static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.end())>>); EXPECT_EQ(&*vec.begin(), &vec[0]); - EXPECT_EQ(&*vec.end(), &vec[0] + 3); + // SAFETY: The vector has 3 elements, so &vec[0] + 3 represents the valid end-of-range pointer. + EXPECT_EQ(&*vec.end(), DAWN_UNSAFE_BUFFERS(&vec[0] + 3)); } TEST(TintVectorTest, ConstRbeginRend_WithSpill) { const Vector<std::string, 2> vec{"front", "mid", "back"}; static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.rbegin())>>); static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec.rend())>>); - EXPECT_EQ(&*vec.rbegin(), &vec[0] + 2); - EXPECT_EQ(&*vec.rend(), &vec[0] - 1); + // SAFETY: The vector has 3 elements, so &vec[0] + 2 and &vec[0] - 1 represent the valid rbegin + // and rend pointers. + EXPECT_EQ(&*vec.rbegin(), DAWN_UNSAFE_BUFFERS(&vec[0] + 2)); + EXPECT_EQ(&*vec.rend(), DAWN_UNSAFE_BUFFERS(&vec[0] - 1)); } TEST(TintVectorTest, Equality) { @@ -2428,14 +2438,18 @@ static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec_ref.begin())>>); static_assert(std::is_const_v<std::remove_reference_t<decltype(*vec_ref.end())>>); EXPECT_EQ(&*vec_ref.begin(), &vec[0]); - EXPECT_EQ(&*vec_ref.end(), &vec[0] + 3); + // SAFETY: The referenced vector has 3 elements, so &vec[0] + 3 represents the valid + // end-of-range pointer. + EXPECT_EQ(&*vec_ref.end(), DAWN_UNSAFE_BUFFERS(&vec[0] + 3)); } TEST(TintVectorRefTest, RbeginRend) { Vector<std::string, 3> vec{"front", "mid", "back"}; const VectorRef<std::string> vec_ref(vec); - EXPECT_EQ(&*vec_ref.rbegin(), &vec[0] + 2); - EXPECT_EQ(&*vec_ref.rend(), &vec[0] - 1); + // SAFETY: The referenced vector has 3 elements, so &vec[0] + 2 and &vec[0] - 1 represent the + // valid rbegin and rend pointers. + EXPECT_EQ(&*vec_ref.rbegin(), DAWN_UNSAFE_BUFFERS(&vec[0] + 2)); + EXPECT_EQ(&*vec_ref.rend(), DAWN_UNSAFE_BUFFERS(&vec[0] - 1)); } TEST(TintVectorRefTest, ostream) { @@ -2464,5 +2478,4 @@ TINT_INSTANTIATE_TYPEINFO(tint::test::C2a); TINT_INSTANTIATE_TYPEINFO(tint::test::C2b); -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); TINT_END_DISABLE_WARNING(UNREACHABLE_CODE);
diff --git a/src/tint/utils/math/crc32.h b/src/tint/utils/math/crc32.h index 3fd0f34..705beb8 100644 --- a/src/tint/utils/math/crc32.h +++ b/src/tint/utils/math/crc32.h
@@ -34,6 +34,7 @@ #include <cstddef> #include "src/tint/utils/macros/compiler.h" +#include "src/utils/compiler.h" // This implementation of CRC32 uses C idioms that trigger '-Wunsafe-buffer-usage', but by // inspecting the code one can see that they are not actually unsafe or an acceptable compromise in @@ -74,8 +75,6 @@ // // A replacement implementing would need to be comparable in performance, be usable in constexpr, // and have our confidence in its safety. -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - namespace tint { /// CRC32 immutable lookup table data. @@ -121,8 +120,12 @@ /// @see https://en.wikipedia.org/wiki/Cyclic_redundancy_check#CRC-32_algorithm constexpr uint32_t CRC32(const char* s) { uint32_t crc = 0xffffffff; - for (auto* p = s; *p != '\0'; ++p) { - crc = (crc >> 8) ^ kCRC32LUT[static_cast<uint8_t>(crc) ^ static_cast<uint8_t>(*p)]; + // SAFETY: s is a null-terminated C-style string, so iterating until '\0' is bounds-safe. + for (auto* p = s; DAWN_UNSAFE_BUFFERS(*p) != '\0'; DAWN_UNSAFE_BUFFERS(++p)) { + // SAFETY: p is in bounds, and kCRC32LUT has 256 elements. The index is a uint8_t which is + // always < 256. + crc = DAWN_UNSAFE_BUFFERS((crc >> 8) ^ + kCRC32LUT[static_cast<uint8_t>(crc) ^ static_cast<uint8_t>(*p)]); } return crc ^ 0xffffffff; } @@ -134,13 +137,15 @@ auto* p = static_cast<const uint8_t*>(ptr); uint32_t crc = 0xffffffff; while (size--) { - crc = (crc >> 8) ^ kCRC32LUT[static_cast<uint8_t>(crc) ^ *p++]; + // SAFETY: size represents the valid bounds of ptr. Indexing kCRC32LUT is bounds-safe as the + // index is uint8_t. + crc = DAWN_UNSAFE_BUFFERS((crc >> 8) ^ kCRC32LUT[static_cast<uint8_t>(crc) ^ *p]); + // SAFETY: Increment is safe if original ptr and size are valid. + DAWN_UNSAFE_BUFFERS(++p); } return crc ^ 0xffffffff; } } // namespace tint -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - #endif // SRC_TINT_UTILS_MATH_CRC32_H_
diff --git a/src/tint/utils/memory/block_allocator.h b/src/tint/utils/memory/block_allocator.h index 4e5481b..47d0134 100644 --- a/src/tint/utils/memory/block_allocator.h +++ b/src/tint/utils/memory/block_allocator.h
@@ -35,6 +35,7 @@ #include "src/tint/utils/macros/compiler.h" #include "src/tint/utils/math/math.h" #include "src/tint/utils/memory/bitcast.h" +#include "src/utils/compiler.h" // This file implements a custom allocator & iterator using C-style data access. It is not // unexpected that -Wunsafe-buffer-usage triggers in this code, since the type of dynamic access @@ -42,8 +43,6 @@ // simple ways to quiet these errors either a) negatively affects the performance by introducing // unneeded copes, or b) uses typing shenanigans to work around the warning that other // linters/analyses are unhappy with. -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - namespace tint { /// A container and allocator of objects of (or deriving from) the template type `T`. @@ -268,7 +267,8 @@ } auto* base = &block.current->data[0]; - auto* ptr = tint::Bitcast<TYPE*>(base + block.current_offset); + // SAFETY: current_offset is guaranteed to be within the allocated Block data bounds. + auto* ptr = tint::Bitcast<TYPE*>(DAWN_UNSAFE_BUFFERS(base + block.current_offset)); block.current_offset += sizeof(TYPE); return ptr; } @@ -325,6 +325,4 @@ } // namespace tint -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - #endif // SRC_TINT_UTILS_MEMORY_BLOCK_ALLOCATOR_H_
diff --git a/src/tint/utils/memory/bump_allocator.h b/src/tint/utils/memory/bump_allocator.h index 6cf31ac..fcc59c6 100644 --- a/src/tint/utils/memory/bump_allocator.h +++ b/src/tint/utils/memory/bump_allocator.h
@@ -38,6 +38,7 @@ #include "src/tint/utils/macros/compiler.h" #include "src/tint/utils/math/math.h" #include "src/tint/utils/memory/bitcast.h" +#include "src/utils/compiler.h" // This file implements a custom allocator & iterator using C-style data access. It is not // unexpected that -Wunsafe-buffer-usage triggers in this code, since the type of dynamic access @@ -45,8 +46,6 @@ // simple ways to quiet these errors either a) negatively affects the performance by introducing // unneeded copes, or b) uses typing shenanigans to work around the warning that other // linters/analyses are unhappy with. -TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - namespace tint { /// A allocator for chunks of memory. The memory is owned by the BumpAllocator. When the @@ -111,8 +110,10 @@ } } - auto* base = Bitcast<std::byte*>(data.current) + sizeof(BlockHeader); - auto* ptr = base + data.current_offset; + // SAFETY: Block allocations are sized appropriately to hold header and data offset. + auto* base = DAWN_UNSAFE_BUFFERS(Bitcast<std::byte*>(data.current) + sizeof(BlockHeader)); + // SAFETY: current_offset is guaranteed to be within the allocated block size. + auto* ptr = DAWN_UNSAFE_BUFFERS(base + data.current_offset); data.current_offset += size_in_bytes; data.count++; return ptr; @@ -153,6 +154,4 @@ } // namespace tint -TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); - #endif // SRC_TINT_UTILS_MEMORY_BUMP_ALLOCATOR_H_
diff --git a/src/tint/utils/system/terminal_posix.cc b/src/tint/utils/system/terminal_posix.cc index a1c7574..2687390 100644 --- a/src/tint/utils/system/terminal_posix.cc +++ b/src/tint/utils/system/terminal_posix.cc
@@ -94,18 +94,20 @@ // These macros introduce identifiers that start with `__` and use c-style memory access, // thus cause warnings. TINT_BEGIN_DISABLE_WARNING(RESERVED_IDENTIFIER); - TINT_BEGIN_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); fd_set rfds{}; - FD_ZERO(&rfds); - FD_SET(STDIN_FILENO, &rfds); + // SAFETY: fd_set macro expansions use unsafe buffer operations internally. + DAWN_UNSAFE_BUFFERS(FD_ZERO(&rfds)); + // SAFETY: fd_set macro expansions use unsafe buffer operations internally. + DAWN_UNSAFE_BUFFERS(FD_SET(STDIN_FILENO, &rfds)); timeval tv{}; tv.tv_sec = 0; tv.tv_usec = 100'000; int res = select(STDIN_FILENO + 1, &rfds, nullptr, nullptr, &tv); - return res > 0 && FD_ISSET(STDIN_FILENO, &rfds); - TINT_END_DISABLE_WARNING(UNSAFE_BUFFER_USAGE); + // SAFETY: fd_set macro expansions use unsafe buffer operations internally. + bool isset = DAWN_UNSAFE_BUFFERS(FD_ISSET(STDIN_FILENO, &rfds)); TINT_END_DISABLE_WARNING(RESERVED_IDENTIFIER); + return res > 0 && isset; }; // Helpers for parsing the response.