Skip to content

Fix: CFAttributedString multi-run attribute storage - #151

Open
DTW-Thalion wants to merge 1 commit into
gnustep:masterfrom
DTW-Thalion:fix-cfattributedstring-mutable
Open

Fix: CFAttributedString multi-run attribute storage#151
DTW-Thalion wants to merge 1 commit into
gnustep:masterfrom
DTW-Thalion:fix-cfattributedstring-mutable

Conversation

@DTW-Thalion

@DTW-Thalion DTW-Thalion commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Setting an attribute across a mutable attributed string that holds two
or more runs crashed before this change. The root cause was an
off-by-one in GSBSearch, which set its upper bound to mid - 1 and so
returned -1 for any key below the first entry; the CFAttributedString
run lookups then indexed the run array with that -1.

Several further defects in the mutable core were exposed once the
lookup no longer crashed:

The mutable and immutable instance-size macros subtracted the struct
size from CFRuntimeClass rather than subtracting CFRuntimeBase from
the struct, so instances were allocated smaller than the struct and
_isEditing was never a defined value.

The attribute cache is a bag keyed on the attribute-set pointer.
Reusing an already cached set did not raise its reference count, so
releasing one run freed a set that another run still pointed at. This
adds GSHashTableAddValueCounted for the counted insert and routes
CFBagAddValue through it as well, so a bag now records multiplicity.

The run array grew and shrank with the wrong element count and left the
stored capacity stale, and the coalescing pass read one slot before the
start of the array.

CFAttributedStringGetAttributeAndLongestEffectiveRange and
CFAttributedStringGetAttributesAndLongestEffectiveRange returned NULL
and are now implemented.

Tests/CFAttributedString/multirun.m and longest_range.m cover the
multi-run accessors and the longest-effective-range walk. Both abort on
the current master and pass with this change. The full suite shows no
new failures against master.

Related

Part of the CFAttributedString series. #152 (substring, attribute removal and replacement) is stacked on this branch. Merge order for #152: #150 and #151 first, then #152.

Setting an attribute across a string that holds two or more runs
crashed.  GSBSearch set its upper bound to mid - 1, skipping index 0
and returning -1 for any key below the first entry, and the callers
indexed the run array with that value.  Restore the usual binary
search bound.

The mutable and immutable instance sizes subtracted the struct size
from CFRuntimeClass instead of subtracting CFRuntimeBase from the
struct.  Instances were allocated too small and _isEditing held
garbage.  Correct both size macros and initialise _isEditing.

The attribute cache is a bag keyed on the set pointer.  Reusing a
cached set left its count unchanged, so releasing one run freed a set
that another run still referenced.  Count cached references through a
new GSHashTableAddValueCounted, and give CFBag the same counting so a
repeated value raises its multiplicity.

Growing and shrinking the run array used the wrong element count and
left the capacity stale, and coalescing adjacent equal runs read past
the start of the array.  Correct the reallocation sizes and the
coalesce bounds.

Implement CFAttributedStringGetAttributeAndLongestEffectiveRange and
CFAttributedStringGetAttributesAndLongestEffectiveRange, which
returned NULL.
@DTW-Thalion
DTW-Thalion force-pushed the fix-cfattributedstring-mutable branch from c4201ef to 4c2a639 Compare July 24, 2026 15:22
@DTW-Thalion DTW-Thalion reopened this Jul 24, 2026
@DTW-Thalion DTW-Thalion reopened this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant