llvm / llvm/llvm-project

[flang] Prohibited allocatable/pointer component in derived-type I/O is silently accepted

Open
#213,324 0 comments 0 reactions 0 assignees View on GitHub
accepts-invalid flang
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

## Summary

Order-dependent `visited` set silently accepts a prohibited I/O component (PDTs).

Derived-type component walks in `flang/lib/Semantics/check-io.cpp` use a helper
set, `VisitedSymbolSet` (a `std::unordered_set`), that is
keyed on `derived.typeSymbol()` and **never erased as the recursion unwinds**.
The set is documented as a cycle breaker for illegal recursive types
(F2023 C749), which is a *path* property, but because entries are never removed
it actually behaves as a *global* "seen anywhere" set.

That global behavior is only sound if the result of walking a type depends
solely on its type symbol. It does not, because each walk consults

```cpp
HasDefinedIo(which, componentDerived, &scope)
```

which is decided **per instantiation**. Once any instantiation of a type symbol
is inserted, every later instantiation of that same symbol is pruned at the top
of the recursive call *before its subtree is examined*. The outcome depends on
traversal order (components are visited in the `memcmp` order of their names).
A prohibited item can be **silently accepted** if a "safe" instantiation is
visited first. This is observable today in `FindUnsafeIoDirectComponent`;
`FindInaccessibleComponent` has the same pattern but its order-dependence is
currently unreachable (see below).

## Affected functions

- `FindUnsafeIoDirectComponent` — **observable bug.** Misses an
allocatable/pointer ultimate component that is not shielded by defined I/O.
Its allocatable/pointer check fires for PDT components, so the order-dependent
pruning produces a real, reproducible missed diagnostic.
- `FindInaccessibleComponent` — **latent only (not observable).** It has the
identical never-erased `visited` pattern, but the order-dependence cannot be
triggered today: the inaccessible-component diagnostic does not fire for
parameterized-derived-type (PDT) components at all (see
[Why `FindInaccessibleComponent` is only latent](#why-findinaccessiblecomponent-is-only-latent)).
Since the bug requires PDTs (shared `typeSymbol`, per-instantiation shielding)
and the check never reports a witness for a PDT component regardless of order,
there is nothing to miss. The pattern should still be fixed for robustness,
but fixing it changes no observable behavior until the PDT-scoping limitation
is also addressed.

A third function with the identical pattern, `FindEnumerationTypeComponent`, was
already fixed by making its `visited` set path-scoped (insert on entry, erase on
unwind, keeping the `typeSymbol` key).

## Why it needs three levels

The shared type symbol that gets wrongly pruned must be *entered* (recursed
into) while walking the safe sibling, so that its symbol is inserted. The two
shielding code paths — `HasDefinedIo(...) -> return`/`continue` — do **not**
insert. Therefore the shield must sit one level *below* the shared symbol:

```
container
├─ a_safe : branch(1) ← visited first; entered, inserts `branch`
│ └─ item : leaf(1) ← shielded by per-instantiation defined I/O
└─ b_bad : branch(2) ← pruned: insert(`branch`) fails, subtree skipped
└─ item : leaf(2) ← NOT shielded; the prohibited item lives here
```

The component names `a_safe` / `b_bad` are chosen so the safe instantiation is
visited first (alphabetical `memcmp` order), which is what triggers the bug.

## Reproducer (observable bug: `FindUnsafeIoDirectComponent`)

Semantic-only (no lowering); compile with `flang -c repro.f90`. The file pairs a
`bug_case` with a structurally identical `control_case`:

- `control_case` — a lone unshielded `branch(2)` — is **correctly rejected**.
- `bug_case` — the same `leaf(2)` reached through a `container` whose
first-visited sibling `a_safe : branch(1)` is shielded — is **silently
accepted**, because walking `a_safe` inserts the shared `branch` symbol and
the never-erased `visited` set then prunes `b_bad : branch(2)` before its
subtree is examined.

Compiling emits **exactly one** error (for `control_case`); the absence of a
second error on `bug_case` is the bug.

```fortran
module m
type :: leaf(k)
integer, kind :: k = 2
real, allocatable :: a(:) ! the unsafe direct component
end type
interface write(unformatted)
module procedure wleaf1 ! matches leaf(1) only
end interface
type :: branch(k)
integer, kind :: k = 2
type(leaf(k)) :: item
end type
type :: container
type(branch(1)) :: a_safe ! visited first: leaf(1) is shielded
type(branch(2)) :: b_bad ! pruned: leaf(2)'s allocatable is missed
end type
contains
subroutine wleaf1(dtv, unit, iostat, iomsg)
class(leaf(1)), intent(in) :: dtv
integer, intent(in) :: unit
integer, intent(out) :: iostat
character(*), intent(in out) :: iomsg
iostat = 0
end subroutine
subroutine bug_case(u)
integer, intent(in) :: u
type(container) :: z
! BUG: no error today. leaf(2)'s allocatable is missed because branch was
! already visited while walking the shielded a_safe (branch(1)).
write(u) z
end subroutine
subroutine control_case(u)
integer, intent(in) :: u
type(branch(2)) :: y
! CONTROL: correctly rejected. Nothing shields leaf(2) here, so the
! allocatable direct component 'a' is flagged as expected.
write(u) y
end subroutine
end module
```

## Below is AI analysis of the remaining possible issue.
I have not verified the analysis of this issue below. The bug reported above and its
cause/test case have been validated. For what it's worth - the content below is left
to help whoever implements this issue fix.

## Why `FindInaccessibleComponent` is only latent

The same three-level PDT structure does **not** reproduce an order-dependent
missed diagnostic for `FindInaccessibleComponent`, because the
inaccessible-component check does not fire for PDT components at all — a lone,
unshielded `print *, leaf(2)` (where `leaf(2)` has a `PRIVATE` component and no
defined I/O) emits nothing. The check only reports a witness when
`FindModuleContaining(componentType->scope())` resolves to the defining module,
and for a PDT instantiation scope that lookup does not resolve to the module.
A non-parameterized type with the same private component **is** flagged.

So the never-erased `visited` set is latent here: even with the pruning removed,
the walk would still report nothing for a PDT. This is arguably a **separate
bug** (inaccessible-component I/O checks silently skipped for PDTs) worth its own
issue. The reproducer below documents the masking rather than an order bug:

```fortran
module defs
type :: leaf(k)
integer, kind :: k = 2
integer, private :: secret = 0 ! inaccessible outside this module
end type
type :: branch(k)
integer, kind :: k = 2
type(leaf(k)) :: item
end type
type :: plain
integer, private :: secret = 0 ! inaccessible outside this module
end type
end module

program p
use defs
type(leaf(2)) :: a
type(branch(2)) :: b
type(plain) :: q
! LIMITATION: no error today. The inaccessible-component check does not fire
! for PDT components, so neither of these is rejected.
print *, a
print *, b
! CONTROL: correctly rejected. A non-parameterized type with the same private
! component IS flagged, proving the diagnostic works for non-PDT types.
print *, q
end
```

## Suggested fix

Make each `visited` set **path-scoped**, exactly as was done for
`FindEnumerationTypeComponent`: keep the `typeSymbol()` key (which guarantees
termination — path length is bounded by the number of distinct type symbols),
but erase the symbol on unwind so it only prunes recursion when it names a true
ancestor on the current path (a genuine C749 recursive-type cycle):

```cpp
if (!visited.insert(&derived.typeSymbol()).second) {
return nullptr; // true ancestor on this path -> real cycle
}
// ... walk, capturing the result instead of returning early ...
visited.erase(&derived.typeSymbol()); // erase on unwind: path-scoped
return result;
```

Termination is preserved, and genuine recursive-type cycles still stop because
the ancestor is still present in the set when it is re-entered. The only cost is
that shared subtrees (diamond-shaped type graphs) may be re-walked; real Fortran
type nesting is shallow and narrow, so this is a non-issue in practice. If a
performance problem were ever observed, add a *separate* memo cache limited to
results that are provably instantiation-independent — do not overload the cycle
set.

Apply the same change to `FindUnsafeIoDirectComponent` (fixes the observable
bug) and to `FindInaccessibleComponent` (for robustness; no observable change
until the PDT-scoping limitation above is also fixed).

Assisted-by: AI

Contributor guide

Open the contributing guide

Research direction

Start in flang/lib/Semantics/check-io.cpp, focusing on FindUnsafeIoDirectComponent and FindInaccessibleComponent, and compare their visited handling with FindEnumerationTypeComponent. Compile the supplied Fortran reproducer with flang -c repro.f90; done means bug_case reports the prohibited allocatable component while control_case remains rejected, without order-dependent pruning.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, fortran
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.