llvm / llvm/llvm-project

[clang] ConstantExpr serialises an uninitialised tail-allocated uint64_t into module files

Open
#221,998 5 comments 0 reactions 0 assignees View on GitHub
clang:modules
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

**Read this first, or the reproducer below will look narrower than it is.**

`-D_GLIBCXX_ASSERTIONS` makes the bug *visible*; it is not what causes it. The malformed records are emitted in every configuration tested, including ones whose module files are byte-identical across runs. Checking "does it still reproduce without that flag" answers a different question and comes back clean.

The quickest way to see it for yourself is `MALLOC_PERTURB_` on the four-line reproducer below — the serialised operand comes out as the allocator's fill byte.

Likewise, read the record diff rather than the byte diff: one operand is VBR-encoded, so a wider value shifts every later bit offset, and byte diffs of two module files give either ~26 bytes or ~20 million depending on which two runs you catch.

### Summary

`ASTStmtWriter::VisitConstantExpr` serialises `E->Int64Result()` whenever the storage kind is `Int64`, without checking whether a result was ever stored. `ConstantExpr`'s constructor placement-news the trailing `APValue` but leaves the trailing `uint64_t` uninitialised. A `ConstantExpr` created with `Int64` storage — chosen from the expression's *type*, before any result exists — whose result is never set therefore writes uninitialised allocator memory into the module file.

It is an uninitialised read (UB), and the bytes it writes are whatever the allocator's memory held: zeros most of the time, and on this machine also stale heap addresses and a stack address. When they are not zeros, the module file stops being reproducible.

### Minimal reproducer — four lines, no flags, no standard library

A `consteval` call nested inside another one. The inner candidate is removed from the immediate-invocation set and never has its result set:

```cpp
export module nest;
consteval int inner() { return 1; }
consteval int outer(int x) { return x + 1; }
export int v = outer(inner());
```

```
clang++ -std=c++23 -x c++-module --precompile nest.cppm -o nest.pcm
llvm-bcanalyzer --dump nest.pcm | grep UnknownCode171
```

```

```

Two records with `ResultKind = Int64` (`op2=1`), `APValueKind = None` (`op3=0`) and `BitWidth = 0` (`op5=0`) — storage for a 64-bit integer result, and no result ever set. `op7` is the uninitialised read. It prints `0` here because that memory happens to be fresh; MSan on the writer should flag it directly. **The shape is the bug; the value is the weather.**

### Proof that `op7` is uninitialised heap memory

`MALLOC_PERTURB_` fills newly allocated memory with `value ^ 0xff`. Run the four-line reproducer under it and the operand comes out as the allocator's fill pattern:

```
$ for p in 0 205 170 17; do
MALLOC_PERTURB_=$p clang++ -std=c++23 -x c++-module --precompile nest.cppm -o p$p.pcm
llvm-bcanalyzer --dump p$p.pcm | grep -o 'UnknownCode171.*op7=-\?[0-9]*'
done
```

| `MALLOC_PERTURB_` | `op7` | as hex | fill byte |
|---|---|---|---|
| unset / 0 | `0` | `0x0000000000000000` | — |
| 205 | `3617008641903833650` | `0x3232323232323232` | `205 ^ 0xff = 0x32` |
| 170 | `6148914691236517205` | `0x5555555555555555` | `170 ^ 0xff = 0x55` |
| 17 | `-1229782938247303442` | `0xEEEEEEEEEEEEEEEE` | `17 ^ 0xff = 0xEE` |

The serialised operand *is* the malloc fill byte, repeated eight times, in all three cases. No debugger, no MSan, no bitcode archaeology needed to see it.

The same run over `std.cc` — in the `-O2` build **without** assertions, the one that is byte-identical across runs — lights up **all five** records:

```
MALLOC_PERTURB_=205, -O2, no assertions:
5 records, every one op7 = 3617008641903833650 (0x3232323232323232)
```

So all five are uninitialised reads, including the three that read `0` in every ordinary run and including every record in the configuration that looks reproducible.

### Where it becomes visible: a non-reproducible module file

```
clang++ -x c++ -std=c++23 -O2 -Wall -Wextra -Wpedantic -Werror \
-D_GLIBCXX_ASSERTIONS -Wno-reserved-module-identifier \
--precompile -x c++-module /usr/include/c++/16/bits/std.cc -o std.pcm
```

Run it three times (keep the `-o` path the same LENGTH each time — the path length is itself a variable, see below). Three different `sha256`. Drop `-D_GLIBCXX_ASSERTIONS`, keep everything else: three runs, one hash, byte-identical.

**That flag is not the trigger.** Both builds emit the same five malformed records. In the no-assertions build all five uninitialised reads land on zeroed memory, so the file comes out identical every time; assertions shift the allocation sequence enough to move one of them onto live memory. Every build of `std.cc` tested carries the same five:

| what was compiled | assertions | malformed records | reproducible across runs |
|---|---|---|---|
| `std.cc`, `-O2` | no | 5 | yes, byte-identical over 3 runs |
| `std.cc`, `-O2` | yes | 5 | no, 3 hashes over 3 runs |
| `std.cc`, `-O1 -g -fsanitize=address,undefined` | yes | 5 | no |
| the four-line module above | — | 2 | yes |

**A `yes` in the last column means no bad read landed on anything, not that the build is clean.** All five records are present in every row; the four-line module is the same point at the small end, with two malformed records in a module file that is byte-identical every time.

- clang 22.1.8 (Arch `clang 22.1.8-1`), libstdc++ 16, x86-64 Linux.
- Under `setarch -R` the output is byte-identical across runs (the offending operand becomes a fixed non-zero address, not zero).
- All readings are from one install. Nobody involved has a second machine to confirm on.

### The record

`llvm-bcanalyzer --dump` on two equal-sized runs differs in one record — one of five in this file with the shape below, and one of two that vary — in `AST_BLOCK > DECLTYPES_BLOCK`. `llvm-bcanalyzer` prints it as `UnknownCode171` because it has no name for the code; 171 is `EXPR_CONSTANT`.

```

```

`0x5566B2128C90` and `0x55FE9B140C90` — both in the PIE range, both at the same page offset.

The operand map, against `ASTStmtWriter::VisitConstantExpr`, confirmed by building a module whose constant expressions are known:

```
// S<7>
// enum { A = 5 }
// enum { B = 300 }
```

So `op2` = `ResultKind`, `op3` = `APValueKind`, `op4` = `IsUnsigned`, `op5` = `BitWidth`, `op6` = `IsImmediateInvocation`, `op7` = `Int64Result()`.

The offending record reads `op2=1` (`Int64` storage) with `op3=0` (`APValue::None`) and `op5=0`. That is a node whose storage says "a 64-bit integer result is tail-allocated" and whose own bits say **no result was ever set** — so `op7` is a read of memory nobody wrote.

### Where the uninitialised object comes from

`ConstantExpr::getStorageKind(const Type*, const ASTContext&)` picks `Int64` from the type before any result exists. The constructor:

```cpp
ConstantExprBits.APValueKind = APValue::None;
ConstantExprBits.IsUnsigned = false;
ConstantExprBits.BitWidth = 0;
...
if (StorageKind == ConstantResultStorageKind::APValue)
::new (getTrailingObjects()) APValue();
```

The `APValue` trailing object is constructed; the `uint64_t` one is not. `Int64Result()` is only ever written by `MoveIntoResult`, which is not reached when no result is set.

Classifying all 18,666 `ConstantExpr` records in this module file by that map:

| `ResultKind` | `APValueKind` | `BitWidth` | count |
|---|---|---|---|
| `Int64` | set | > 0 | 18,586 |
| `APValue` | set | 0 | 75 |
| `Int64` | `None` | 0 | **5** |

All five are `IsImmediateInvocation`. Only two of them ever vary between runs; the other three land on bytes that happen to read `0` in every run taken so far. Distinguishing the two by their type operand:

| | site A (`op1=8064`) | site B (`op1=104`) |
|---|---|---|
| six ASLR-on runs | `0`, `4`, `4`, `6`, `0x2000000000`, `0x7FFC713A9900` | six distinct `0x55…`, all at one page offset |
| `setarch -R` | fixed per argv | fixed per argv |

Site B's stability — always the PIE range, always the same page offset for a given command line — is what an uninitialised read looks like when the allocation sequence is deterministic: the same slot, holding the same stale pointer, into an arena whose base moves with ASLR. Site A lands on assorted garbage including `0x7FFC713A9900`, a stack address, **and its value moves with the length of the `-o` path**, since argv sits on the stack. A field that tracks argv length is not one anything wrote on purpose.

### The count of differing records is not the count of uninitialised reads

Worth stating because it decides how a fix gets validated. There are five bad reads; between one and two are visible in any given pair of runs. Checking a fix by "two module files now agree" checks a property that is already true of three of the five — and of all five in the no-assertions build.

The sharpest case: two runs under `setarch -R` are byte-identical, and site B holds `0x55555563EC90` in both. Reproducibility fully restored, defect fully present.

The stable predicate is the shape, not the disagreement: `ResultKind == Int64` together with `APValueKind == None` should be unreachable. That is assertable in the writer and needs no second run — and it is also the right condition for a breakpoint if someone wants to find which declaration produces these, since it stops five times instead of 18,666.

### Suggested fix

Initialise the trailing `uint64_t` in the constructor:

```cpp
if (StorageKind == ConstantResultStorageKind::APValue)
::new (getTrailingObjects()) APValue();
else if (StorageKind == ConstantResultStorageKind::Int64)
Int64Result() = 0;
```

This keeps the record format unchanged, which matters: `ASTStmtReader::VisitConstantExpr` reads the operand back unconditionally, so making the writer conditional would need a matching reader change and a format bump.

Alternatively (or additionally) the writer could refuse to serialise a result that was never set, and an assertion there would catch the class rather than this instance.

Contributor guide

Open the contributing guide

Research direction

Start with ConstantExpr's constructor and getStorageKind, then inspect ASTStmtWriter::VisitConstantExpr and the matching ASTStmtReader::VisitConstantExpr. Use the four-line module reproducer with llvm-bcanalyzer --dump to identify Int64 records with APValueKind None. Done means the trailing uint64_t is initialized without changing the record format, and those malformed records no longer appear.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.