[flang-commits] [flang] [llvm] [flang-rt] Initialize I/O unit storage read by short-circuit predicates (PR #221126)

[Eugene Epshteyn via flang-commits]

eugeneepshteyn wrote:

Details:

`ConnectionState` and `OpenFile` hold `common::optional` members that are read through predicates of the form `opt && x < *opt`. Those predicates never use an indeterminate value in the abstract machine, but compilers routinely if-convert the short-circuit `&&` into a branchless compare and select, which speculates the payload load. Because these objects are placement-new'd into `malloc`'d storage by `UnitMap::Create()`, a memory checker sees the payload bytes as never written and reports a conditional branch that depends on uninitialized memory.

This is a false positive — the machine result is provably correct, since the engaged flag is 0 and both arms of the select are 0 — but it fires for *every* Fortran program that writes a record on AArch64, so the noise is unavoidable for anyone running Valgrind on Fortran code.

### Evidence

Same program, same Valgrind 3.27.1:

| | |
|---|---|
| AArch64, flang built from `7a1f6ad89e57` | `ERROR SUMMARY: 2 errors from 2 contexts`, both in `AdvanceRecord`, origin `UnitMap::Create` |
| x86-64, flang built from `dd7236de4812` | `ERROR SUMMARY: 0 errors from 0 contexts` |
| `libgfortran`, same program, same AArch64 host | `ERROR SUMMARY: 0 errors from 0 contexts` |

Disassembling the AArch64 `libflang_rt.runtime.so` — the two addresses Memcheck reports match these by page offset:

```asm
; site 1 — IsAfterEndfile()
ldr  x11, [x0, #40]      ; currentRecordNumber
ldr  x10, [x0, #80]      ; *endfileRecordNumber  <-- undefined payload
ldrb w9,  [x0, #88]      ; engaged flag (defined, 0)
cmp  x11, x10            ; NZCV now undefined
csel w12, wzr, w9, le
tbz  w12, #0, ...        ; <-- report #1
```

```asm
; site 2 — IsAtEOF(), reusing the same compare
cmp  x11, x10
csel w9, wzr, w9, lt
cbz  w9, ...             ; <-- report #2
strb wzr, [x0, #88]      ; endfileRecordNumber.reset()
```

The threshold is `-O1` in the runtime's own build: 0 errors at `-O0`, 2 at `-O1`/`-O2`/`-O3` on AArch64, 0 at every level on x86-64.

### What this changes

`common::ResetWithDefinedPayload()` leaves an optional disengaged but writes its payload storage; it is called at construction for the optionals in `ConnectionAttributes`, `ConnectionState` and `OpenFile`. This changes no observable behavior — it only makes the speculated read defined.

The helper lives in `flang/Common/optional.h` because both `connection.h` and `file.h` already include it directly, so no new include edges are needed. Happy to move it to a runtime-local header if you would rather it not sit in `Common`.

`OpenFile::pathLength_` also gets a plain initializer: it is read through the same if-convertible shape (`path() && pathLength() == n`, in `external-unit.cpp` and `unit-map.cpp`), and a unit created without `OPEN` or `Predefine()` leaves it indeterminate. Note `OpenStatementState::pathLength_` in `io-stmt.h` is already initialized this way.

`OpenFile::nextId_` was initialized here in the first revision of this PR; it has been **split out into #221187**, since it is a latent uninitialized member rather than a memory-checker false positive and does not belong with this change.

### Testing

`check-flang-rt` passes 365/365.

No new test is added, and I do not think one is possible in tree today: the difference is in shadow-memory definedness, which neither a LIT test nor a `flang-rt` unit test can observe, and there is no MSan or Valgrind CI for `flang-rt`. There is precedent for landing this kind of fix untested — 01f2f81f2b9 ("[flang-rt] Fixed uninitialized class member variable") is a one-line header change with no test.

To keep the claim honest, the fix was verified by compiling the **real, patched headers** into a standalone probe that reproduces `UnitMap::Create()`'s malloc + placement-new shape and calls the two predicates in `AdvanceRecord()`'s order:

```
arch=aarch64
unpatched: ok=1 currentRecordNumber=2 hasEndfile=0 | ERROR SUMMARY: 2 errors from 2 contexts
patched:   ok=1 currentRecordNumber=2 hasEndfile=0 | ERROR SUMMARY: 0 errors from 0 contexts
```

Identical program output, reports gone. The same probe on x86-64 is clean both ways, as expected.

Not covered: architectures other than x86-64 and AArch64, Windows, and device/offload builds where `common::optional` is flang's own implementation rather than `std::optional` (the helper should behave identically there, but it was not exercised).


https://github.com/llvm/llvm-project/pull/221126