bryancall opened a new issue, #13722:
URL: https://github.com/apache/trafficserver/issues/13722
### Summary
On Apple clang, `Base_Binary_Cmp::TYPES` in
`plugins/experimental/txn_box/plugin/src/Comparison.cc` initializes to an
all-zero mask. Every `eq`, `ne`, `lt`, `le`, `gt` and `ge` comparison is then
rejected at configuration load, so a `txn_box` configuration using any of them
fails to parse. It does not reproduce under GCC, which is why CI has never seen
it.
### Symptom
Any `txn_box` configuration containing a binary comparison fails to load
with a "not valid for active feature" error, pointing at the configuration
rather than at the compiler. That misdirection is probably why this has gone
unreported: it reads as a user error.
### Mechanism
`Comparison.cc:1062`:
```cpp
static inline const ActiveType TYPES = Cmp_Types::apply<ValueMaskFor>::value;
```
`Comparison::load()` runs `if (!cfg.active_type().can_satisfy(types))` for
every one of those keys, where `types` is that mask. `can_satisfy(ValueMask
vmask)` is `(_base_type & vmask).any()`. An all-zero `vmask` makes the
conjunction zero for any real active type, so the check fails unconditionally
and `load()` always returns the error.
The initializer depends on dynamic initialization of the templated
`apply<ValueMaskFor>::value` across translation units, which is where the
ordering goes wrong.
### Evidence
The pattern was isolated into a 45-line standalone file with no ATS or
libswoc dependency, mirroring `Base_Binary_Cmp`, `Cmp_Types` and `ValueMaskFor`
with toy types, and compiled four ways on one machine:
| compiler | `-O0` | `-O2 -DNDEBUG` |
|---|---|---|
| Apple clang 21.0.0 (clang-2100.3.34.2) | `TYPES` all-zero | `TYPES`
all-zero |
| GCC 16.2.0 | `TYPES` correct, bits set | `TYPES` correct, bits set |
Deterministic across repeats on both, so optimization level is not the
variable.
Causality was confirmed by a one-line controlled change in a full build:
replacing only that initializer made the configuration parse errors disappear
and the affected tests pass. Nothing else differed.
### Why CI does not catch it
CI builds with GCC, where the mask initializes correctly. The AuTest shard
containing the `txn_box` tests is green, which is consistent with the bug being
specific to Apple clang rather than evidence that the code is correct
everywhere.
### Suggested fix
Initialize the mask directly rather than through the templated static. The
file already does this in four places, including one with identical `static
inline` storage at line 909:
```cpp
static inline const ValueMask TYPES{MaskFor(NIL, STRING, BOOLEAN, INTEGER)};
```
`Cmp_Types` is declared immediately above as `INTEGER`, `BOOLEAN`,
`IP_ADDR`, `DURATION`, so the direct form is a faithful rewrite rather than a
behavioural change. GCC already produces the correct mask, so the change cannot
regress the platform CI covers.
### What is not established
Whether this is Apple clang violating the ordered-dynamic-initialization
guarantees of `[basic.start.dynamic]`, or unspecified ordering that the two
implementations are each permitted to resolve differently. That affects how the
fix should be described, not whether it is correct.
The compiler comparison above used a GCC installed on macOS rather than the
GCC in `ci.trafficserver.apache.org/ats/fedora:43`. Building the same
reproduction on that image would close the last gap, though the CI evidence
already points the same way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
