Skip to content

UPSTREAM PR #27100: Guard against null stream_ in EpsCopyOutputStream::EnableAliasing#156

Open
loci-dev wants to merge 1 commit into
mainfrom
loci/pr-27100-fix_null_pointer_in_EnableAliasing
Open

UPSTREAM PR #27100: Guard against null stream_ in EpsCopyOutputStream::EnableAliasing#156
loci-dev wants to merge 1 commit into
mainfrom
loci/pr-27100-fix_null_pointer_in_EnableAliasing

Conversation

@loci-dev
Copy link
Copy Markdown

Note

Source pull request: protocolbuffers/protobuf#27100

Fix the issue described in protocolbuffers/protobuf#27099.

EpsCopyOutputStream can be constructed in "array-only" mode via EpsCopyOutputStream(void*, int, bool), which intentionally sets stream_ to nullptr (no backing ZeroCopyOutputStream). Calling EnableAliasing() in this state unconditionally dereferenced stream_ to call the virtual AllowsAliasing(), causing a null pointer dereference (UBSan: member call on null pointer) and immediate SIGSEGV.

The fix adds a null check before the virtual dispatch. When stream_ is null, AllowsAliasing() is meaningless and the result correctly evaluates to false, leaving aliasing_enabled_ unset.

@loci-review
Copy link
Copy Markdown

loci-review Bot commented Apr 24, 2026

Overview

Analysis of 10,164 functions (1 modified, 0 new, 0 removed, 10,163 unchanged) across build.protoc-stable shows minimal performance impact from a null-safety fix.

Power Consumption:

  • build.protoc-stable: +0.0% (588,876.33 nJ → 588,877.29 nJ)

Function Analysis

google::protobuf::io::EpsCopyOutputStream::EnableAliasing (build.protoc-stable)

  • Response Time: 50.90ns → 58.84ns (+7.94ns, +15.6%)
  • Throughput Time: 50.90ns → 58.84ns (+7.94ns, +15.6%)

Commit cfb66d6 added null pointer check (stream_ != nullptr) before dereferencing in the aliasing configuration logic. This prevents crashes when EnableAliasing() is called on uninitialized streams. The 8ns overhead occurs during stream initialization, not in parsing/serialization hot paths (TcParser::ParseLoop, MessageLite::SerializeWithCachedSizesToArray), making the impact negligible for overall protobuf operations. The control flow graph shows an added basic block for the null check, increasing paths from 2 to 3. This is a justified correctness improvement with acceptable performance cost.

💬 Questions? Tag @loci-dev

@loci-dev loci-dev force-pushed the main branch 9 times, most recently from f292971 to 1fdfb93 Compare April 29, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants