From 4c2a639891c56954c68becc071072aa90783c7f6 Mon Sep 17 00:00:00 2001 From: Todd White Date: Wed, 22 Jul 2026 17:38:59 -0400 Subject: [PATCH] Fix multi-run attribute storage in CFAttributedString 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. --- Source/CFAttributedString.c | 150 ++++++++++++++++++----- Source/CFBag.c | 2 +- Source/GSFunctions.c | 2 +- Source/GSHashTable.c | 26 ++++ Source/GSHashTable.h | 4 + Tests/CFAttributedString/longest_range.m | 52 ++++++++ Tests/CFAttributedString/multirun.m | 61 +++++++++ 7 files changed, 265 insertions(+), 32 deletions(-) create mode 100755 Tests/CFAttributedString/longest_range.m create mode 100644 Tests/CFAttributedString/multirun.m diff --git a/Source/CFAttributedString.c b/Source/CFAttributedString.c index 699b334..0431be5 100644 --- a/Source/CFAttributedString.c +++ b/Source/CFAttributedString.c @@ -125,7 +125,14 @@ CFAttributedStringCacheAttribute (CFDictionaryRef attribs) cachedAttr = insert; CFRelease (insert); } - + else + { + /* Another run now references this cached attribute set; bump its bag + * count so it survives until every referencing run is uncached. + */ + CFBagAddValue (_kCFAttributedStringCache, cachedAttr); + } + GSMutexUnlock (&_kCFAttributedStringCacheLock); return cachedAttr; @@ -284,8 +291,8 @@ CFAttributedStringGetTypeID (void) return _kCFAttributedStringTypeID; } -#define CFATTRIBUTESTRING_SIZE sizeof(CFRuntimeClass) \ - - sizeof(struct __CFAttributedString) +#define CFATTRIBUTESTRING_SIZE (sizeof(struct __CFAttributedString) \ + - sizeof(CFRuntimeBase)) static CFAttributedStringRef CFAttributedStringCreateInlined (CFAllocatorRef alloc, CFStringRef str, @@ -423,7 +430,48 @@ CFAttributedStringGetAttributeAndLongestEffectiveRange ( CFAttributedStringRef str, CFIndex loc, CFStringRef attrName, CFRange inRange, CFRange *longestEffRange) { - return NULL; /* FIXME */ + CFRange r; + CFTypeRef value; + CFIndex start; + CFIndex end; + CFIndex inEnd; + + value = CFAttributedStringGetAttribute (str, loc, attrName, &r); + start = r.location; + end = r.location + r.length; + inEnd = inRange.location + inRange.length; + + while (start > inRange.location) + { + CFRange pr; + CFTypeRef pv = CFAttributedStringGetAttribute (str, start - 1, attrName, + &pr); + if ((pv == NULL) != (value == NULL) + || (pv != NULL && !CFEqual (pv, value))) + break; + start = pr.location; + } + while (end < inEnd) + { + CFRange nr; + CFTypeRef nv = CFAttributedStringGetAttribute (str, end, attrName, &nr); + if ((nv == NULL) != (value == NULL) + || (nv != NULL && !CFEqual (nv, value))) + break; + end = nr.location + nr.length; + } + + if (start < inRange.location) + start = inRange.location; + if (end > inEnd) + end = inEnd; + if (longestEffRange) + { + longestEffRange->location = start; + longestEffRange->length = end - start; + } + + return value; } CFDictionaryRef @@ -431,7 +479,46 @@ CFAttributedStringGetAttributesAndLongestEffectiveRange ( CFAttributedStringRef str, CFIndex loc, CFRange inRange, CFRange *longestEffRange) { - return NULL; /* FIXME */ + CFRange r; + CFDictionaryRef attribs; + CFIndex start; + CFIndex end; + CFIndex inEnd; + + attribs = CFAttributedStringGetAttributes (str, loc, &r); + start = r.location; + end = r.location + r.length; + inEnd = inRange.location + inRange.length; + + while (start > inRange.location) + { + CFRange pr; + CFDictionaryRef pa = CFAttributedStringGetAttributes (str, start - 1, + &pr); + if (!CFEqual (pa, attribs)) + break; + start = pr.location; + } + while (end < inEnd) + { + CFRange nr; + CFDictionaryRef na = CFAttributedStringGetAttributes (str, end, &nr); + if (!CFEqual (na, attribs)) + break; + end = nr.location + nr.length; + } + + if (start < inRange.location) + start = inRange.location; + if (end > inEnd) + end = inEnd; + if (longestEffRange) + { + longestEffRange->location = start; + longestEffRange->length = end - start; + } + + return attribs; } @@ -451,12 +538,13 @@ InsertAttributesAtIndex (CFMutableAttributedStringRef str, CFIndex idx, if (working->_attribCount == working->_attribCap) { /* Grow */ + working->_attribCap <<= 1; working->_attribs = CFAllocatorReallocate (alloc, working->_attribs, - (working->_attribCap << 1), + sizeof(Attr) * working->_attribCap, 0); } - + /* Move things to the right */ stop = &working->_attribs[idx]; cur = &working->_attribs[working->_attribCount]; @@ -528,18 +616,19 @@ RemoveAttributesAtIndex (CFMutableAttributedStringRef str, CFRange range) cur = &working->_attribs[range.location]; next = cur + range.length; - stop = cur + (working->_attribCount - (range.location + range.length) - 1); + stop = cur + (working->_attribCount - (range.location + range.length)); while (cur < stop) *cur++ = *next++; working->_attribCount -= range.length; - + if (working->_attribCount < (working->_attribCap >> 2) && working->_attribCount > 9) { /* Shrink */ + working->_attribCap >>= 1; working->_attribs = CFAllocatorReallocate (alloc, working->_attribs, - (working->_attribCap >> 1), + sizeof(Attr) * working->_attribCap, 0); } } @@ -556,35 +645,35 @@ CFAttributedStringCoalesce (CFMutableAttributedStringRef str, CFRange range) { CFIndex cur; CFIndex end; - Attr *array; - - array = working->_attribs; - if (range.location > 0) - { - if (array[range.location - 1].attrib == array[range.location].attrib) - { - RemoveAttributesAtIndex (str, CFRangeMake (range.location, 1)); - range.length -= 1; - } - } - - cur = range.location; - end = range.location + range.length; - + Attr *array = working->_attribs; + + /* Run 0 has no predecessor to merge into, so start at 1. Also fold in + * the run that immediately follows the affected range, since removing + * attributes can make it equal to its new predecessor. + */ + cur = range.location < 1 ? 1 : range.location; + end = range.location + range.length + 1; + if (end > working->_attribCount) + end = working->_attribCount; + while (cur < end) { - if (array[cur - 1].attrib == array[cur].attrib) + if (array[cur].attrib == array[cur - 1].attrib) { + /* This run repeats its predecessor: drop it (the predecessor's + * range absorbs it) and re-check the run now shifted into cur. + */ RemoveAttributesAtIndex (str, CFRangeMake (cur, 1)); end -= 1; } - cur++; + else + cur += 1; } } } -#define CFMUTABLEATTRIBUTESTRING_SIZE sizeof(CFRuntimeClass) \ - - sizeof(struct __CFMutableAttributedString) +#define CFMUTABLEATTRIBUTESTRING_SIZE (sizeof(struct __CFMutableAttributedString) \ + - sizeof(CFRuntimeBase)) CFMutableAttributedStringRef CFAttributedStringCreateMutable (CFAllocatorRef alloc, CFIndex maxLength) @@ -602,7 +691,8 @@ CFAttributedStringCreateMutable (CFAllocatorRef alloc, CFIndex maxLength) new->_attribCount = 1; new->_attribs[0].index = 0; new->_attribs[0].attrib = CFAttributedStringGetBlankAttribute (); - + new->_isEditing = 0; + CFAttributedStringSetMutable ((CFAttributedStringRef)new); } diff --git a/Source/CFBag.c b/Source/CFBag.c index d94da9b..3e855b6 100644 --- a/Source/CFBag.c +++ b/Source/CFBag.c @@ -198,7 +198,7 @@ CFBagCreateMutableCopy (CFAllocatorRef allocator, CFIndex capacity, void CFBagAddValue (CFMutableBagRef bag, const void *value) { - GSHashTableAddValue ((GSHashTableRef)bag, value, value); + GSHashTableAddValueCounted ((GSHashTableRef)bag, value, value); } void diff --git a/Source/GSFunctions.c b/Source/GSFunctions.c index b2a23aa..80b1e49 100644 --- a/Source/GSFunctions.c +++ b/Source/GSFunctions.c @@ -47,7 +47,7 @@ GSBSearch (const void *array, const void *key, CFRange range, CFIndex size, r = comp (key, cur, ctxt); if (r == kCFCompareLessThan) { - max = mid - 1; + max = mid; } else if (r == kCFCompareGreaterThan) { diff --git a/Source/GSHashTable.c b/Source/GSHashTable.c index 6dc1802..d989c3f 100644 --- a/Source/GSHashTable.c +++ b/Source/GSHashTable.c @@ -655,6 +655,32 @@ GSHashTableAddValue (GSHashTableRef table, const void *key, const void *value) } } +void +GSHashTableAddValueCounted (GSHashTableRef table, const void *key, + const void *value) +{ + GSHashTableBucket *bucket; + + GSHashTableGrowIfNeeded (table); + + bucket = GSHashTableFindBucket (table, key, _kGSHashTableRetrieve); + if (!bucket) + bucket = GSHashTableFindBucket (table, key, _kGSHashTableInsert); + + if (bucket->count <= 0) + { + GSHashTableAddKeyValuePair (table, bucket, key, value); + table->_count += 1; + } + else + { + /* Increment the multiplicity so a bag counts repeated additions; + * GSHashTableRemoveValue decrements it symmetrically. + */ + bucket->count += 1; + } +} + void GSHashTableReplaceValue (GSHashTableRef table, const void *key, const void *value) diff --git a/Source/GSHashTable.h b/Source/GSHashTable.h index a6fb057..e4bb2b1 100644 --- a/Source/GSHashTable.h +++ b/Source/GSHashTable.h @@ -140,6 +140,10 @@ GSHashTableCreateMutableCopy (CFAllocatorRef alloc, GSHashTableRef table, GS_PRIVATE void GSHashTableAddValue (GSHashTableRef table, const void *key, const void *value); +GS_PRIVATE void +GSHashTableAddValueCounted (GSHashTableRef table, const void *key, + const void *value); + GS_PRIVATE void GSHashTableReplaceValue (GSHashTableRef table, const void *key, const void *value); diff --git a/Tests/CFAttributedString/longest_range.m b/Tests/CFAttributedString/longest_range.m new file mode 100755 index 0000000..d9457cb --- /dev/null +++ b/Tests/CFAttributedString/longest_range.m @@ -0,0 +1,52 @@ +#include "CoreFoundation/CFAttributedString.h" +#include "CoreFoundation/CFString.h" +#include "CoreFoundation/CFDictionary.h" + +#include "../CFTesting.h" + +int +main (void) +{ + CFRange lr; + CFTypeRef v; + CFDictionaryRef d; + CFMutableAttributedStringRef m; + + m = CFAttributedStringCreateMutable (NULL, 0); + CFAttributedStringReplaceString (m, CFRangeMake (0, 0), CFSTR ("Hello World")); + /* (0,5) = {color, shared}, (5,11) = {weight, shared}. */ + CFAttributedStringSetAttribute (m, CFRangeMake (0, 5), CFSTR ("color"), + CFSTR ("red")); + CFAttributedStringSetAttribute (m, CFRangeMake (0, 11), CFSTR ("shared"), + CFSTR ("x")); + CFAttributedStringSetAttribute (m, CFRangeMake (5, 6), CFSTR ("weight"), + CFSTR ("bold")); + + v = CFAttributedStringGetAttributeAndLongestEffectiveRange ( + m, 2, CFSTR ("shared"), CFRangeMake (0, 11), &lr); + PASS_CFEQ(v, CFSTR ("x"), "The longest-range shared value is returned."); + PASS_CF(lr.location == 0 && lr.length == 11, + "shared has the same value across both runs."); + + v = CFAttributedStringGetAttributeAndLongestEffectiveRange ( + m, 2, CFSTR ("color"), CFRangeMake (0, 11), &lr); + PASS_CFEQ(v, CFSTR ("red"), "The longest-range color value is returned."); + PASS_CF(lr.location == 0 && lr.length == 5, + "color has the same value only over (0,5)."); + + d = CFAttributedStringGetAttributesAndLongestEffectiveRange ( + m, 8, CFRangeMake (0, 11), &lr); + PASS_CF(d != NULL && CFDictionaryGetCount (d) == 2, + "The longest-range attributes at 8 are the second run's two."); + PASS_CF(lr.location == 5 && lr.length == 6, + "The full attribute set stays equal only over (5,6)."); + + v = CFAttributedStringGetAttributeAndLongestEffectiveRange ( + m, 2, CFSTR ("shared"), CFRangeMake (0, 3), &lr); + PASS_CF(lr.location == 0 && lr.length == 3, + "The longest range is clamped to the search range."); + + CFRelease (m); + + return 0; +} diff --git a/Tests/CFAttributedString/multirun.m b/Tests/CFAttributedString/multirun.m new file mode 100644 index 0000000..9054a89 --- /dev/null +++ b/Tests/CFAttributedString/multirun.m @@ -0,0 +1,61 @@ +#include "CoreFoundation/CFAttributedString.h" +#include "CoreFoundation/CFString.h" +#include "CoreFoundation/CFDictionary.h" + +#include "../CFTesting.h" + +int +main (void) +{ + CFRange er; + CFDictionaryRef d; + CFMutableAttributedStringRef m; + + m = CFAttributedStringCreateMutable (NULL, 0); + CFAttributedStringReplaceString (m, CFRangeMake (0, 0), CFSTR ("Hello World")); + + /* Two runs: (0,5) = {color}, (5,11) = {weight}. */ + CFAttributedStringSetAttribute (m, CFRangeMake (0, 5), CFSTR ("color"), + CFSTR ("red")); + CFAttributedStringSetAttribute (m, CFRangeMake (5, 6), CFSTR ("weight"), + CFSTR ("bold")); + + d = CFAttributedStringGetAttributes (m, 2, &er); + PASS_CF(d != NULL && CFDictionaryGetCount (d) == 1 + && er.location == 0 && er.length == 5, + "The first run's attributes cover (0,5)."); + PASS_CFEQ(CFAttributedStringGetAttribute (m, 2, CFSTR ("color"), NULL), + CFSTR ("red"), "The first run has the color attribute."); + d = CFAttributedStringGetAttributes (m, 8, &er); + PASS_CF(d != NULL && CFDictionaryGetCount (d) == 1 + && er.location == 5 && er.length == 6, + "The second run's attributes cover (5,6)."); + PASS_CFEQ(CFAttributedStringGetAttribute (m, 8, CFSTR ("weight"), NULL), + CFSTR ("bold"), "The second run has the weight attribute."); + PASS_CF(CFAttributedStringGetAttribute (m, 2, CFSTR ("weight"), NULL) == NULL, + "weight is absent from the first run."); + PASS_CF(CFAttributedStringGetAttribute (m, 8, CFSTR ("color"), NULL) == NULL, + "color is absent from the second run."); + d = CFAttributedStringGetAttributes (m, 5, &er); + PASS_CF(er.location == 5 && er.length == 6, + "A boundary index resolves to the second run."); + + /* Setting an attribute over both runs keeps each run's own attributes. */ + CFAttributedStringSetAttribute (m, CFRangeMake (0, 11), CFSTR ("lang"), + CFSTR ("en")); + d = CFAttributedStringGetAttributes (m, 2, &er); + PASS_CF(CFDictionaryGetCount (d) == 2 + && CFDictionaryGetValue (d, CFSTR ("color")) != NULL + && CFDictionaryGetValue (d, CFSTR ("lang")) != NULL + && er.location == 0 && er.length == 5, + "A spanning attribute merges into the first run."); + d = CFAttributedStringGetAttributes (m, 8, &er); + PASS_CF(CFDictionaryGetCount (d) == 2 + && CFDictionaryGetValue (d, CFSTR ("weight")) != NULL + && CFDictionaryGetValue (d, CFSTR ("lang")) != NULL + && er.location == 5 && er.length == 6, + "A spanning attribute merges into the second run."); + CFRelease (m); + + return 0; +}