From 033835ebd3652412dfac43bbda9b874396625814 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 7 Jul 2026 09:45:01 -0700 Subject: [PATCH] Add ABSL_ATTRIBUTE_LIFETIME_BOUND to Safe-bindings wrapper APIs. Annotate methods returning non-owning views (NodeView, string_view, Span) with ABSL_ATTRIBUTE_LIFETIME_BOUND to enable Clang -Wdangling warnings for use-after-free patterns. PiperOrigin-RevId: 943944506 --- kamadak_exif/exif_bridge.h | 4 ++-- saphyr/node.h | 24 ++++++++++++++++-------- zip/converters.h | 3 ++- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/kamadak_exif/exif_bridge.h b/kamadak_exif/exif_bridge.h index 624a40f..70a4e94 100644 --- a/kamadak_exif/exif_bridge.h +++ b/kamadak_exif/exif_bridge.h @@ -473,7 +473,7 @@ class Field { /** * The value of this field. */ - const Value& value() const { return value_; }; + const Value& value() const ABSL_ATTRIBUTE_LIFETIME_BOUND { return value_; }; private: friend class Exif; @@ -604,7 +604,7 @@ class ExifBytes final { /** * Returns a view to the Exif data. */ - absl::Span view() const { + absl::Span view() const ABSL_ATTRIBUTE_LIFETIME_BOUND { return absl::Span(vec_.as_ptr(), vec_.len()); } diff --git a/saphyr/node.h b/saphyr/node.h index a7af4c3..1e3e85d 100644 --- a/saphyr/node.h +++ b/saphyr/node.h @@ -9,6 +9,7 @@ #include "support/rs_std/str_ref.h" #include "crubit/rust.h" +#include "absl/base/attributes.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" @@ -42,6 +43,11 @@ class NodeView { // Returns a `NodeView` at `index` // Use `IsDefined()` to check if the returned node is defined to avoid // invalid pointer access. + // Note: NodeView methods deliberately omit ABSL_ATTRIBUTE_LIFETIME_BOUND. + // NodeView is a trivially-copyable pointer into the root Node's data, so + // chaining (e.g., view["a"]["b"]) is safe -- all returned NodeViews borrow + // from the same root Node, not from intermediate temporaries. The lifetime + // annotation is on Node's methods instead. NodeView operator[](size_t index) const; // Required to resolve overload ambiguity between size_t and const char* // for numeric literals like 0. It also checks for negative indices. @@ -205,25 +211,27 @@ class Node { // Accesses an element in a map by key. // Returns a `NodeView` for which `IsDefined()` is false if the node is not a // map or the key is not found. - NodeView operator[](rs_std::StrRef key) const; - NodeView operator[](const char* key) const { + NodeView operator[](rs_std::StrRef key) const ABSL_ATTRIBUTE_LIFETIME_BOUND; + NodeView operator[](const char* key) const ABSL_ATTRIBUTE_LIFETIME_BOUND { return operator[](rs_std::StrRef::FromUtf8Unchecked(key)); } - NodeView operator[](absl::string_view key) const { + NodeView operator[](absl::string_view key) const + ABSL_ATTRIBUTE_LIFETIME_BOUND { return operator[](rs_std::StrRef::FromUtf8Unchecked(key)); } // Accesses an element in a sequence by index. // Returns a `NodeView` for which `IsDefined()` is false if the node is not a // sequence or the index is out of bounds. - NodeView operator[](size_t index) const; - NodeView operator[](int index) const { + NodeView operator[](size_t index) const ABSL_ATTRIBUTE_LIFETIME_BOUND; + NodeView operator[](int index) const ABSL_ATTRIBUTE_LIFETIME_BOUND { return index < 0 ? NodeView() : operator[](static_cast(index)); } // A safer version of `operator[]` for sequence. - std::optional Get(size_t index) const; + std::optional Get(size_t index) const ABSL_ATTRIBUTE_LIFETIME_BOUND; // A safer version of `operator[]` for map. - std::optional Get(rs_std::StrRef key) const; + std::optional Get(rs_std::StrRef key) const + ABSL_ATTRIBUTE_LIFETIME_BOUND; // Sets the value at a given index in a sequence. // If `index` is within bounds, the existing value is replaced. @@ -237,7 +245,7 @@ class Node { std::optional as_optional() const; // Returns a view into the node. - NodeView as_view() const; + NodeView as_view() const ABSL_ATTRIBUTE_LIFETIME_BOUND; private: rust::NodeOwned node_; diff --git a/zip/converters.h b/zip/converters.h index 0855e40..f21704e 100644 --- a/zip/converters.h +++ b/zip/converters.h @@ -6,6 +6,7 @@ #include "crubit_helpers/string_conversions.h" #include "crubit/rust.h" +#include "absl/base/attributes.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" @@ -20,7 +21,7 @@ class RustVecU8Wrapper { explicit RustVecU8Wrapper(rust::VecU8 vec_u8) : vec_u8_(std::move(vec_u8)) {} - absl::string_view AsStringView() const { + absl::string_view AsStringView() const ABSL_ATTRIBUTE_LIFETIME_BOUND { return security::crubit_helpers::StringViewFromVecU8(vec_u8_); }