Skip to content

Commit 13cd606

Browse files
[XLA] Fix raw memory Segfault on array queries against default-constructed Buffer shapes
Problem: When a Buffer shape is instantiated via Shape(BUFFER) without setting its underlying buffer_shape, its internal shape is in InvalidState. If an array property accessor (such as dimensions()) is invoked on this object, the helper array_state_maybe_underneath_buffer() unconditionally dereferenced if_array_state(), causing a fatal Segmentation Fault. Solution: Refactored array_state_maybe_underneath_buffer() to call array_state() instead of if_array_state(). This ensures that attempting to read array properties from an uninitialized or broken Buffer shape triggers a graceful, programmer-friendly compiler CHECK failure with clear instructions, rather than executing a raw memory crash. Testing: Added unit test BufferShapeArrayAccessorTriggersInformativeCheckFailure in shape_test.cc. PiperOrigin-RevId: 932772958
1 parent a184a68 commit 13cd606

2 files changed

Lines changed: 48 additions & 4 deletions

File tree

third_party/xla/xla/shape.h

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -840,17 +840,31 @@ inline const Shape::ArrayState& Shape::array_state_maybe_underneath_buffer()
840840
return *array;
841841
}
842842
const BufferState* buffer = if_buffer_state();
843-
CHECK_NE(buffer, nullptr);
844-
return *buffer->buffer_shape->if_array_state();
843+
CHECK(buffer)
844+
<< "Expected an array or buffer shape. Got " << ToString()
845+
<< "\nThis is a programmer error. Please read the Shape object's "
846+
"array properties only when it is an array or buffer shape.";
847+
CHECK(buffer->buffer_shape->IsArrayExcludingBuffer())
848+
<< "Expected a fully initialized Buffer shape containing an "
849+
"underlying array, but got an uninitialized/empty Buffer shape: "
850+
<< ToString();
851+
return buffer->buffer_shape->array_state();
845852
}
846853

847854
inline Shape::ArrayState& Shape::array_state_maybe_underneath_buffer() {
848855
if (ArrayState* array = if_array_state(); ABSL_PREDICT_TRUE(array)) {
849856
return *array;
850857
}
851858
BufferState* buffer = if_buffer_state();
852-
CHECK_NE(buffer, nullptr);
853-
return *buffer->buffer_shape->if_array_state();
859+
CHECK(buffer)
860+
<< "Expected an array or buffer shape. Got " << ToString()
861+
<< "\nThis is a programmer error. Please read the Shape object's "
862+
"array properties only when it is an array or buffer shape.";
863+
CHECK(buffer->buffer_shape->IsArrayExcludingBuffer())
864+
<< "Expected a fully initialized Buffer shape containing an "
865+
"underlying array, but got an uninitialized/empty Buffer shape: "
866+
<< ToString();
867+
return buffer->buffer_shape->array_state();
854868
}
855869

856870
inline ABSL_ATTRIBUTE_ALWAYS_INLINE const Shape& Shape::tuple_shapes(

third_party/xla/xla/shape_test.cc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,5 +458,35 @@ void BM_TupleShapeEqual(::testing::benchmark::State& state) {
458458
}
459459
BENCHMARK(BM_TupleShapeEqual);
460460

461+
TEST(Shape, BufferShapeArrayAccessorTriggersInformativeCheckFailure) {
462+
Shape shape(BUFFER);
463+
// Verifies that attempting to read array properties from an uninitialized or
464+
// invalid Buffer shape triggers an explicit, programmer-friendly CHECK
465+
// failure rather than a raw memory fault.
466+
EXPECT_DEATH_IF_SUPPORTED(
467+
shape.dimensions(),
468+
"Expected a fully initialized Buffer shape containing an "
469+
"underlying array, but got an uninitialized/empty Buffer shape");
470+
EXPECT_DEATH_IF_SUPPORTED(
471+
shape.set_dynamic_dimension(0, true),
472+
"Expected a fully initialized Buffer shape containing an "
473+
"underlying array, but got an uninitialized/empty Buffer shape");
474+
}
475+
476+
TEST_F(ShapeTest,
477+
NonArrayNonBufferShapeAccessorTriggersInformativeCheckFailure) {
478+
EXPECT_DEATH_IF_SUPPORTED(tuple_.dimensions(),
479+
"Expected an array or buffer shape.");
480+
EXPECT_DEATH_IF_SUPPORTED(token_.dimensions(),
481+
"Expected an array or buffer shape.");
482+
483+
Shape mutable_tuple = tuple_;
484+
EXPECT_DEATH_IF_SUPPORTED(mutable_tuple.set_dynamic_dimension(0, true),
485+
"Expected an array or buffer shape.");
486+
Shape mutable_token = token_;
487+
EXPECT_DEATH_IF_SUPPORTED(mutable_token.set_dynamic_dimension(0, true),
488+
"Expected an array or buffer shape.");
489+
}
490+
461491
} // namespace
462492
} // namespace xla

0 commit comments

Comments
 (0)