[classic / 8.1.0] Use-after-free after broadcast `ckDestroy()`: `attemptDelivery()` ignores failed delivery

Read src/ck-core/ckarray.C, especially CkArrayBroadcaster::attemptDelivery(), and trace how its delivery result is used before bringUpToDate(el). Build the classic runtime and run the supplied destroy.ci / destroy.C reproduction under Valgrind; the issue also requests regression coverage for a dynamically inserted array whose broadcast entry deletes its elements. Done means both deletions are acknowledged exactly once and Memcheck reports zero errors.

This bug report was prepared in collaboration with an LLM. It was the result of an agentic backtrace of a crash using 8.1.0, and it seems to have been corrected in the latest dev branch of recharm.

Summary

Broadcasting ckDestroy() to a dynamically inserted array can make CkArrayBroadcaster::attemptDelivery() call bringUpToDate(el) after the delivered entry has deleted el. Valgrind reproduces the invalid read with two small array elements on a single PE. The same program completes with zero Valgrind errors after adding a failed-delivery check.

This was investigated after an NRPy/superB application crashed in CkLocRec::invokeEntry() while delivering ckDestroy() during temporary-array cleanup.

The equivalent deletion check is already present on the recharm development branch through PR #4093. This report requests a corresponding fix for the classic Converse implementation.

Affected source and upstream status

Source	Failed-delivery check in CkArrayBroadcaster::attemptDelivery()
Released v8.1.0	Missing before bringUpToDate(el) when stableLocations is false.
classic maintenance branch	The inspected source has the same missing check.
recharm, after PR #4093	Returns immediately when performDelivery() returns false.

The development branch also changes broadcast ordering and storage. The proposed classic fix below preserves the existing stableLocations behavior and adds the deletion check only.

Build and execution environment

Item	Original application	Standalone reproduction
Platform	Linux, x86-64	Linux, x86-64
Runtime	Charm++ 8.1.0, classic Converse; startup reports v80100	Runtime headers and archives copied from the same application build.
Runtime build target	multicore-linux-x86_64 gcc	Same target and runtime configuration.
Compiler	GCC/G++ 12.3.0	G++ 13.3.0 for the example and controlled recompilation of ckarray.C; other runtime objects retained from the captured build.
Runtime options	Production build, Fortran disabled	Same captured runtime configuration; the controlled ckarray.C comparisons both use -g -O2.
Application compilation	Generated C/C++ superB executable	charmc -g -O0 -no-pie destroy.C -o destroy
Worker count	96 PEs on 48 physical cores	One PE for the completed before/after checks.
Memory checker	Core inspected with GDB	Valgrind 3.22.0, Memcheck.

The captured ckarray.C was compared with the official v8.1.0 file and matched after ignoring trailing whitespace. This is not a claim that every file in the captured runtime was independently compared with the release.

Minimal example

The example dynamically inserts two nonmigrating elements, waits for insertion to finish through quiescence detection, broadcasts ckDestroy(), and waits for deletion notifications. It checks that each element is destroyed exactly once.

The completed reproduction uses one PE.

destroy.ci

mainmodule destroy {
  readonly CProxy_Main mainProxy;

  mainchare Main {
    entry Main(CkArgMsg *msg);
    entry void destroyArray();
    entry void elementDeleted(int index);
    entry void done();
  };

  array [1D] Worker {
    entry Worker();
  };
};

destroy.C

#include "charm++.h"
#include "destroy.decl.h"

CProxy_Main mainProxy;

class Main : public CBase_Main {
  CProxy_Worker workers;
  bool deleted[2] = {false, false};
  int deletedCount = 0;

public:
  Main(CkArgMsg *msg) {
    delete msg;
    mainProxy = thisProxy;
    workers = CProxy_Worker::ckNew();
    for (int index = 0; index < 2; ++index)
      workers[index].insert(index % CkNumPes());
    workers.doneInserting();
    CkStartQD(CkCallback(CkIndex_Main::destroyArray(), thisProxy));
  }

  void destroyArray() {
    CkPrintf("Broadcasting ckDestroy() to two array elements\n");
    workers.ckDestroy();
    CkStartQD(CkCallback(CkIndex_Main::done(), thisProxy));
  }

  void elementDeleted(int index) {
    if (index < 0 || index >= 2 || deleted[index])
      CkAbort("Unexpected or duplicate element destruction");
    deleted[index] = true;
    ++deletedCount;
  }

  void done() {
    if (deletedCount != 2)
      CkAbort("An array element was not destroyed");
    CkPrintf("PASS: both array elements destroyed exactly once\n");
    CkExit();
  }
};

class Worker : public CBase_Worker {
public:
  Worker() { setMigratable(false); }
  Worker(CkMigrateMessage *msg) : CBase_Worker(msg) {}
  ~Worker() { mainProxy.elementDeleted(thisIndex); }
};

#include "destroy.def.h"

Steps to reproduce

Build the released classic runtime. Run these commands in a new working directory; the two example files above should be saved in that directory.

git clone --depth 1 --branch v8.1.0 https://github.com/charmplusplus/charm.git charm-8.1.0
(
  cd charm-8.1.0
  ./build charm++ multicore-linux-x86_64 gcc --with-production --disable-fortran -j4
)

charmc_path="$PWD/charm-8.1.0/multicore-linux-x86_64-gcc/bin/charmc"
"$charmc_path" destroy.ci
"$charmc_path" -g -O0 -no-pie destroy.C -o destroy

valgrind --tool=memcheck --error-exitcode=99 --leak-check=no ./destroy +p1

-no-pie allows linking a non-PIE runtime archive on a host whose compiler defaults to PIE. The runtime build flags above match the affected application's configuration, with a smaller compilation job limit. The executed local checks used the captured runtime archives and the controlled object replacements described below, rather than performing a second complete clone/build.

Expected behavior

Both array elements are destroyed exactly once, execution reaches the final quiescence callback, and Memcheck reports zero errors. No code reads the element after the deletion entry returns.

Observed behavior before the fix

The example prints normal completion:

Broadcasting ckDestroy() to two array elements
PASS: both array elements destroyed exactly once

However, Memcheck reports an invalid read from the freed array element, and the command exits with status 99 because of --error-exitcode=99. A normal application exit alone does not detect this defect.

The following is trimmed output from the controlled unmodified-source run. Process identifiers, instruction addresses, and executable paths are omitted; function names and source locations are retained.

Invalid read of size 4
   at CkArrayBroadcaster::bringUpToDate(ArrayElement*) (upstream-ckarray.C:1379)
   by CkArray::recvBroadcast(CkMessage*) (upstream-ckarray.C:1735)
   by CkDeliverMessageFree
   by _processHandler(void*, CkCoreState*)

Address ... is 116 bytes inside a block of size 160 free'd
   at free
   by Chare::operator delete(void*) (charm++.h:259)
   by Worker::~Worker() (destroy.C:47)
   by deleteElt (ckarray.h:795)
   by ArrayElement::ckDestroy() (upstream-ckarray.C:387)
   by CkIndex_ArrayElement::_call_ckDestroy_void(void*, void*) (CkArray.def.h:2654)
   by CkDeliverMessageReadonly
   by CkLocRec::invokeEntry(CkMigratable*, void*, int, bool)
   by ckInvokeEntry (ckmigratable.h:68)
   by CkArrayBroadcaster::performDelivery(CkArrayMessage*, ArrayElement*, bool)
   by CkArrayBroadcaster::attemptDelivery(CkArrayMessage*, ArrayElement*, bool)
   by CkArray::recvBroadcast(CkMessage*)

ERROR SUMMARY: 2 errors from 1 contexts (suppressed: 0 from 0)

upstream-ckarray.C is the temporary filename used for the unmodified tagged source during the controlled object recompilation. The original captured runtime produces the same invalid-read finding.

Source analysis

1. CkArray::recvBroadcast() delivers the destruction broadcast to each local element through CkArrayBroadcaster::attemptDelivery().
2. performDelivery() invokes the entry. ArrayElement::ckDestroy() deletes the element. CkLocRec::invokeEntry() detects deletion through its existing deletion marker and returns false.
3. In the affected implementation, attemptDelivery() returns that result only when stableLocations is true. A dynamically inserted array takes the other path and calls bringUpToDate(el) despite the failed delivery.
4. bringUpToDate() reads the element's broadcast epoch through its listener data. That element has already been freed.

The loop inside bringUpToDate() already stops when a subsequent delivery returns false. The missing check is after the first delivery in attemptDelivery(). See the tagged broadcast implementation and deletion detection in CkLocRec::invokeEntry().

Proposed classic fix

Preserve the stable-location return and also return when the element was deleted during delivery:

--- a/src/ck-core/ckarray.C
+++ b/src/ck-core/ckarray.C
@@ -1362,6 +1362,7 @@
   const bool deliveryStatus = performDelivery(bcast, el, doFree);
-  // If locations are stable, we deliver msgs as they come in, so only deliver one
-  if (stableLocations)
+  // A deleted or migrated element cannot receive later broadcasts here.
+  // Stable locations also require only this one delivery.
+  if (!deliveryStatus || stableLocations)
   {
     return deliveryStatus;
   }

The equivalent early return is present in the upstream PR #4093 change. The source comparison establishes equivalent deletion protection; it does not establish the authorship of the local classic patch used for these checks.

Rebuild the runtime after applying the change, then relink destroy and repeat the same Memcheck command. Relinking is required because the affected runtime code is in the static runtime archive.

Completed validation

Runtime used by the standalone example	PEs	Memcheck result	Exit status	Both deletions acknowledged exactly once
Original captured 8.1.0 runtime	1	Two invalid reads, one error context.	99	Yes
Same runtime with unmodified tagged ckarray.C recompiled using G++ 13.3.0, -g -O2	1	Two invalid reads, one error context.	99	Yes
Same runtime with only the classic guard and its explanatory comments changed; ckarray.C recompiled using G++ 13.3.0, -g -O2	1	Zero errors, zero error contexts.	0	Yes

The last two rows use matching compiler flags for ckarray.C; the other runtime objects and the example source are unchanged. No Valgrind suppression file was supplied. Leak checking was disabled because this comparison concerns invalid reads, not process-exit allocations.

An additional two-PE Memcheck attempt with the original captured runtime timed out during startup before the destruction broadcast. It supplies no two-PE result. No multi-PE success, general migration correctness, or complete Charm++ test-suite result is claimed here.

Requested resolution

Please consider backporting the failed-delivery check to classic and a future classic bug-fix release, together with regression coverage for a dynamically inserted array whose broadcast entry deletes its elements. The example above provides both deletion-count checks and a Memcheck failure on the affected code.