Skip to content

Commit

Permalink
[flang] Fixed write past allocated descriptor in PointerAssociateRema…
Browse files Browse the repository at this point in the history
…pping. (#127000)

The pointer descriptor might be smaller than the target descriptor,
so `operator=` would write beyound the pointer descriptor.
  • Loading branch information
vzakhari authored Feb 13, 2025
1 parent 9a63a2c commit 660cdac
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 10 deletions.
17 changes: 7 additions & 10 deletions flang/runtime/pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,18 @@ void RTDEF(PointerAssociateLowerBounds)(Descriptor &pointer,
void RTDEF(PointerAssociateRemapping)(Descriptor &pointer,
const Descriptor &target, const Descriptor &bounds, const char *sourceFile,
int sourceLine) {
pointer = target;
pointer.raw().attribute = CFI_attribute_pointer;
Terminator terminator{sourceFile, sourceLine};
SubscriptValue byteStride{/*captured from first dimension*/};
std::size_t boundElementBytes{bounds.ElementBytes()};
std::size_t boundsRank{
static_cast<std::size_t>(bounds.GetDimension(1).Extent())};
pointer.raw().rank = boundsRank;
// We cannot just assign target into pointer descriptor, because
// the ranks may mismatch. Use target as a mold for initializing
// the pointer descriptor.
INTERNAL_CHECK(static_cast<std::size_t>(pointer.rank()) == boundsRank);
pointer.ApplyMold(target, boundsRank);
pointer.set_base_addr(target.raw().base_addr);
pointer.raw().attribute = CFI_attribute_pointer;
for (unsigned j{0}; j < boundsRank; ++j) {
auto &dim{pointer.GetDimension(j)};
dim.SetBounds(GetInt64(bounds.ZeroBasedIndexedElement<const char>(2 * j),
Expand All @@ -115,13 +119,6 @@ void RTDEF(PointerAssociateRemapping)(Descriptor &pointer,
"pointer (%zd > %zd)",
pointer.Elements(), target.Elements());
}
if (auto *pointerAddendum{pointer.Addendum()}) {
if (const auto *targetAddendum{target.Addendum()}) {
if (const auto *derived{targetAddendum->derivedType()}) {
pointerAddendum->set_derivedType(derived);
}
}
}
}

RT_API_ATTRS void *AllocateValidatedPointerPayload(std::size_t byteSize) {
Expand Down
44 changes: 44 additions & 0 deletions flang/unittests/Runtime/Pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,47 @@ TEST(Pointer, AllocateSourceZeroSize) {
EXPECT_EQ(p->GetDimension(0).UpperBound(), 0);
p->Destroy();
}

TEST(Pointer, PointerAssociateRemapping) {
using Fortran::common::TypeCategory;
// REAL(4), POINTER :: p(:)
StaticDescriptor<Fortran::common::maxRank, true> staticDesc;
auto p{staticDesc.descriptor()};
SubscriptValue extent[1]{1};
p.Establish(TypeCode{Fortran::common::TypeCategory::Real, 4}, 4, nullptr, 1,
extent, CFI_attribute_pointer);
std::size_t descSize{p.SizeInBytes()};
EXPECT_LE(descSize, staticDesc.byteSize);
// REAL(4), CONTIGUOUS, POINTER :: t(:,:,:)
auto t{Descriptor::Create(TypeCode{Fortran::common::TypeCategory::Real, 4}, 4,
nullptr, 3, nullptr, CFI_attribute_pointer)};
RTNAME(PointerSetBounds)(*t, 0, 1, 1);
RTNAME(PointerSetBounds)(*t, 1, 1, 1);
RTNAME(PointerSetBounds)(*t, 2, 1, 1);
RTNAME(PointerAllocate)(
*t, /*hasStat=*/false, /*errMsg=*/nullptr, __FILE__, __LINE__);
EXPECT_TRUE(RTNAME(PointerIsAssociated)(*t));
// INTEGER(4) :: b(2,1) = [[1,1]]
auto b{MakeArray<TypeCategory::Integer, 4>(
std::vector<int>{2, 1}, std::vector<std::int32_t>{1, 1})};
// p(1:1) => t
RTNAME(PointerAssociateRemapping)(p, *t, *b, __FILE__, __LINE__);
EXPECT_TRUE(RTNAME(PointerIsAssociated)(p));
EXPECT_EQ(p.rank(), 1);
EXPECT_EQ(p.Elements(), 1u);

// Verify that the memory past the p's descriptor is not affected.
const char *addr = reinterpret_cast<const char *>(&staticDesc);
const char *ptr = addr + descSize;
const char *end = addr + staticDesc.byteSize;
while (ptr != end) {
if (*ptr != '\0') {
std::fprintf(stderr, "byte %zd after pointer descriptor was written\n",
ptr - addr);
EXPECT_EQ(*ptr, '\0');
break;
}
++ptr;
}
p.Destroy();
}

0 comments on commit 660cdac

Please sign in to comment.