Skip to content

Commit b50fbed

Browse files
committed
[flang][runtime] Validate pointer DEALLOCATE
The standard requires a compiler to diagnose an incorrect use of a pointer in a DEALLOCATE statement. The pointer must be associated with an entire object that was allocated as a pointer (not allocatable) by an ALLOCATE statement. Implement by appending a validation footer to pointer allocations. This is an extra allocated word that encodes the base address of the allocation. If it is not found after the data payload when the pointer is deallocated, signal an error. There is a chance of a false positive result, but that should be vanishingly unlikely. This change requires all pointer allocations (not allocatables) to take place in the runtime in PointerAllocate(), which might be slower in cases that could otherwise be handled with a native memory allocation operation. I believe that memory allocation of pointers is less common than with allocatables, which are not affected. If this turns out to become a performance problem, we can inline the creation and initialization of the footer word. Fixes #78391.
1 parent 345c1ea commit b50fbed

File tree

8 files changed

+85
-24
lines changed

8 files changed

+85
-24
lines changed

flang/include/flang/Runtime/descriptor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ class Descriptor {
375375
// allocation. Does not allocate automatic components or
376376
// perform default component initialization.
377377
RT_API_ATTRS int Allocate();
378+
RT_API_ATTRS void SetByteStrides();
378379

379380
// Deallocates storage; does not call FINAL subroutines or
380381
// deallocate allocatable/automatic components.

flang/include/flang/Runtime/magic-numbers.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ same allocatable.
6363
#endif
6464
#define FORTRAN_RUNTIME_STAT_MOVE_ALLOC_SAME_ALLOCATABLE 109
6565

66+
#if 0
67+
Additional status code for a bad pointer DEALLOCATE.
68+
#endif
69+
#define FORTRAN_RUNTIME_STAT_BAD_POINTER_DEALLOCATION 110
70+
6671
#if 0
6772
ieee_class_type values
6873
The sequence is that of F18 Clause 17.2p3, but nothing depends on that.

flang/lib/Lower/Allocatable.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,9 @@ class AllocateStmtHelper {
454454
const fir::MutableBoxValue &box) {
455455
if (!box.isDerived() && !errorManager.hasStatSpec() &&
456456
!alloc.type.IsPolymorphic() && !alloc.hasCoarraySpec() &&
457-
!useAllocateRuntime) {
457+
!useAllocateRuntime && !box.isPointer()) {
458+
// Pointers must use PointerAllocate so that their deallocations
459+
// can be validated.
458460
genInlinedAllocation(alloc, box);
459461
return;
460462
}

flang/runtime/descriptor.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ RT_API_ATTRS int Descriptor::Allocate() {
162162
}
163163
// TODO: image synchronization
164164
raw_.base_addr = p;
165+
SetByteStrides();
166+
return 0;
167+
}
168+
169+
RT_API_ATTRS void Descriptor::SetByteStrides() {
165170
if (int dims{rank()}) {
166171
std::size_t stride{ElementBytes()};
167172
for (int j{0}; j < dims; ++j) {
@@ -170,7 +175,6 @@ RT_API_ATTRS int Descriptor::Allocate() {
170175
stride *= dimension.Extent();
171176
}
172177
}
173-
return 0;
174178
}
175179

176180
RT_API_ATTRS int Descriptor::Destroy(

flang/runtime/pointer.cpp

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,32 @@ int RTDEF(PointerAllocate)(Descriptor &pointer, bool hasStat,
129129
if (!pointer.IsPointer()) {
130130
return ReturnError(terminator, StatInvalidDescriptor, errMsg, hasStat);
131131
}
132-
int stat{ReturnError(terminator, pointer.Allocate(), errMsg, hasStat)};
133-
if (stat == StatOk) {
134-
if (const DescriptorAddendum * addendum{pointer.Addendum()}) {
135-
if (const auto *derived{addendum->derivedType()}) {
136-
if (!derived->noInitializationNeeded()) {
137-
stat = Initialize(pointer, *derived, terminator, hasStat, errMsg);
138-
}
132+
std::size_t byteSize{pointer.Elements() * pointer.ElementBytes()};
133+
// Add space for a footer to validate during DEALLOCATE.
134+
constexpr std::size_t align{sizeof(std::uintptr_t)};
135+
byteSize = ((byteSize + align - 1) / align) * align;
136+
std::size_t total{byteSize + sizeof(std::uintptr_t)};
137+
void *p{std::malloc(total)};
138+
if (!p) {
139+
return ReturnError(terminator, CFI_ERROR_MEM_ALLOCATION, errMsg, hasStat);
140+
}
141+
pointer.set_base_addr(p);
142+
pointer.SetByteStrides();
143+
// Fill the footer word with the XOR of the ones' complement of
144+
// the base address, which is a value that would be highly unlikely
145+
// to appear accidentally at the right spot.
146+
std::uintptr_t *footer{
147+
reinterpret_cast<std::uintptr_t *>(static_cast<char *>(p) + byteSize)};
148+
*footer = ~reinterpret_cast<std::uintptr_t>(p);
149+
int stat{StatOk};
150+
if (const DescriptorAddendum * addendum{pointer.Addendum()}) {
151+
if (const auto *derived{addendum->derivedType()}) {
152+
if (!derived->noInitializationNeeded()) {
153+
stat = Initialize(pointer, *derived, terminator, hasStat, errMsg);
139154
}
140155
}
141156
}
142-
return stat;
157+
return ReturnError(terminator, stat, errMsg, hasStat);
143158
}
144159

145160
int RTDEF(PointerAllocateSource)(Descriptor &pointer, const Descriptor &source,
@@ -163,6 +178,18 @@ int RTDEF(PointerDeallocate)(Descriptor &pointer, bool hasStat,
163178
if (!pointer.IsAllocated()) {
164179
return ReturnError(terminator, StatBaseNull, errMsg, hasStat);
165180
}
181+
// Validate the footer. This should fail if the pointer doesn't
182+
// span the entire object, or the object was not allocated as a
183+
// pointer.
184+
std::size_t byteSize{pointer.Elements() * pointer.ElementBytes()};
185+
constexpr std::size_t align{sizeof(std::uintptr_t)};
186+
byteSize = ((byteSize + align - 1) / align) * align;
187+
void *p{pointer.raw().base_addr};
188+
std::uintptr_t *footer{
189+
reinterpret_cast<std::uintptr_t *>(static_cast<char *>(p) + byteSize)};
190+
if (*footer != ~reinterpret_cast<std::uintptr_t>(p)) {
191+
return ReturnError(terminator, StatBadPointerDeallocation, errMsg, hasStat);
192+
}
166193
return ReturnError(terminator,
167194
pointer.Destroy(/*finalize=*/true, /*destroyPointers=*/true, &terminator),
168195
errMsg, hasStat);

flang/runtime/stat.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ RT_API_ATTRS const char *StatErrorString(int stat) {
6666
case StatMoveAllocSameAllocatable:
6767
return "MOVE_ALLOC passed the same address as to and from";
6868

69+
case StatBadPointerDeallocation:
70+
return "DEALLOCATE of a pointer that is not the whole content of a pointer "
71+
"ALLOCATE";
72+
6973
default:
7074
return nullptr;
7175
}

flang/runtime/stat.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ enum Stat {
5151
StatValueTooShort = FORTRAN_RUNTIME_STAT_VALUE_TOO_SHORT,
5252
StatMoveAllocSameAllocatable =
5353
FORTRAN_RUNTIME_STAT_MOVE_ALLOC_SAME_ALLOCATABLE,
54+
StatBadPointerDeallocation = FORTRAN_RUNTIME_STAT_BAD_POINTER_DEALLOCATION,
5455
};
5556

5657
RT_API_ATTRS const char *StatErrorString(int);

flang/test/Lower/Intrinsics/c_loc.f90

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -177,20 +177,37 @@ subroutine c_loc_arraysection()
177177
! CHECK: %[[VAL_2:.*]] = fir.zero_bits !fir.ptr<i32>
178178
! CHECK: fir.store %[[VAL_2]] to %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
179179
! CHECK: %[[VAL_3:.*]] = fir.alloca !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}> {bindc_name = "ptr", uniq_name = "_QFc_loc_non_save_pointer_scalarEptr"}
180-
! CHECK: %[[VAL_4:.*]] = fir.allocmem i32 {fir.must_be_heap = true, uniq_name = "_QFc_loc_non_save_pointer_scalarEi.alloc"}
181-
! CHECK: %[[VAL_5:.*]] = fir.convert %[[VAL_4]] : (!fir.heap<i32>) -> !fir.ptr<i32>
182-
! CHECK: fir.store %[[VAL_5]] to %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
183-
! CHECK: %[[VAL_6:.*]] = arith.constant 10 : i32
184-
! CHECK: %[[VAL_7:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
185-
! CHECK: fir.store %[[VAL_6]] to %[[VAL_7]] : !fir.ptr<i32>
186-
! CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
187-
! CHECK: %[[VAL_9:.*]] = fir.embox %[[VAL_8]] : (!fir.ptr<i32>) -> !fir.box<i32>
188-
! CHECK: %[[VAL_10:.*]] = fir.alloca !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
189-
! CHECK-DAG: %[[VAL_11:.*]] = fir.box_addr %[[VAL_9]] : (!fir.box<i32>) -> !fir.ref<i32>
190-
! CHECK-DAG: %[[VAL_12:.*]] = fir.convert %[[VAL_11]] : (!fir.ref<i32>) -> i64
191-
! CHECK-DAG: %[[VAL_13:.*]] = fir.field_index __address, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
192-
! CHECK-DAG: %[[VAL_14:.*]] = fir.coordinate_of %[[VAL_10]], %[[VAL_13]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
193-
! CHECK: fir.store %[[VAL_12]] to %[[VAL_14]] : !fir.ref<i64>
180+
! CHECK: %[[VAL_false:.*]] = arith.constant false
181+
! CHECK: %[[VAL_4:.*]] = fir.absent !fir.box<none>
182+
! CHECK: %[[VAL_5:.*]] = fir.address_of(@{{.*}}) : !fir.ref<!fir.char<1,{{.*}}>>
183+
! CHECK: %[[C_LN:.*]] = arith.constant {{.*}} : i32
184+
! CHECK: %[[VAL_6:.*]] = fir.zero_bits !fir.ptr<i32>
185+
! CHECK: %[[VAL_7:.*]] = fir.embox %[[VAL_6:.*]] : (!fir.ptr<i32>) -> !fir.box<!fir.ptr<i32>>
186+
! CHECK: fir.store %[[VAL_7:.*]] to %[[VAL_0:.*]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
187+
! CHECK: %[[VAL_8:.*]] = fir.convert %[[VAL_0:.*]] : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> !fir.ref<!fir.box<none>>
188+
! CHECK: %[[VAL_9:.*]] = fir.convert %[[VAL_5:.*]] : (!fir.ref<!fir.char<1,{{.*}}>>) -> !fir.ref<i8>
189+
! CHECK: %[[VAL_10:.*]] = fir.call @_FortranAPointerAllocate(%[[VAL_8:.*]], %[[VAL_false:.*]], %[[VAL_4:.*]], %[[VAL_9:.*]], %[[C_LN:.*]]) fastmath<contract> : (!fir.ref<!fir.box<none>>, i1, !fir.box<none>, !fir.ref<i8>, i32) -> i32
190+
! CHECK: %[[VAL_11:.*]] = fir.load %[[VAL_0:.*]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
191+
! CHECK: %[[VAL_12:.*]] = fir.box_addr %[[VAL_11:.*]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
192+
! CHECK: fir.store %[[VAL_12:.*]] to %[[VAL_1:.*]] : !fir.ref<!fir.ptr<i32>>
193+
! CHECK: %[[C_10:.*]] = arith.constant 10 : i32
194+
! CHECK: %[[VAL_13:.*]] = fir.load %[[VAL_1:.*]] : !fir.ref<!fir.ptr<i32>>
195+
! CHECK: fir.store %[[C_10]] to %[[VAL_13:.*]] : !fir.ptr<i32>
196+
! CHECK: %[[VAL_14:.*]] = fir.load %[[VAL_1:.*]] : !fir.ref<!fir.ptr<i32>>
197+
! CHECK: %[[VAL_15:.*]] = fir.embox %[[VAL_14:.*]] : (!fir.ptr<i32>) -> !fir.box<i32>
198+
! CHECK: %[[VAL_16:.*]] = fir.alloca !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
199+
! CHECK: %[[VAL_17:.*]] = fir.field_index __address, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
200+
! CHECK: %[[VAL_18:.*]] = fir.coordinate_of %[[VAL_16:.*]], %[[VAL_17:.*]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
201+
! CHECK: %[[VAL_19:.*]] = fir.box_addr %[[VAL_15:.*]] : (!fir.box<i32>) -> !fir.ref<i32>
202+
! CHECK: %[[VAL_20:.*]] = fir.convert %[[VAL_19:.*]] : (!fir.ref<i32>) -> i64
203+
! CHECK: fir.store %[[VAL_20:.*]] to %[[VAL_18:.*]] : !fir.ref<i64>
204+
! CHECK: %[[VAL_21:.*]] = fir.field_index __address, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
205+
! CHECK: %[[VAL_22:.*]] = fir.coordinate_of %[[VAL_16:.*]], %[[VAL_21:.*]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
206+
! CHECK: %[[VAL_23:.*]] = fir.field_index __address, !fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>
207+
! CHECK: %[[VAL_24:.*]] = fir.coordinate_of %[[VAL_3:.*]], %[[VAL_23:.*]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>, !fir.field) -> !fir.ref<i64>
208+
! CHECK: %[[VAL_25:.*]] = fir.load %[[VAL_22:.*]] : !fir.ref<i64>
209+
! CHECK: fir.store %[[VAL_25:.*]] to %[[VAL_24:.*]] : !fir.ref<i64>
210+
! CHECK: return
194211
! CHECK: }
195212

196213
subroutine c_loc_non_save_pointer_scalar()

0 commit comments

Comments
 (0)