-
Notifications
You must be signed in to change notification settings - Fork 256
fix(SEC-2): replace input-sized C VLAs with heap allocation at three sites #226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,7 +141,8 @@ + (MTMathList *)mathListForCharacters:(NSString *)chars | |
| { | ||
| NSParameterAssert(chars); | ||
| NSInteger len = chars.length; | ||
| unichar buff[len]; | ||
| unichar *buff = malloc(sizeof(unichar) * (size_t)len); | ||
| NSAssert(len == 0 || buff != NULL, @"Failed to allocate buff"); | ||
| [chars getCharacters:buff range:NSMakeRange(0, len)]; | ||
| MTMathList* list = [[MTMathList alloc] init]; | ||
| for (NSInteger i = 0; i < len; i++) { | ||
|
|
@@ -150,6 +151,7 @@ + (MTMathList *)mathListForCharacters:(NSString *)chars | |
| [list addAtom:atom]; | ||
| } | ||
| } | ||
| free(buff); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| return list; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -424,15 +424,31 @@ static UTF32Char styleCharacter(unichar ch, MTFontStyle fontStyle) | |
| } | ||
|
|
||
| static NSString* changeFont(NSString* str, MTFontStyle fontStyle) { | ||
| NSMutableString* retval = [NSMutableString stringWithCapacity:str.length]; | ||
| unichar charBuffer[str.length]; | ||
| [str getCharacters:charBuffer range:NSMakeRange(0, str.length)]; | ||
| for (int i = 0; i < str.length; ++i) { | ||
| unichar ch = charBuffer[i]; | ||
| UTF32Char unicode = styleCharacter(ch, fontStyle); | ||
| unicode = NSSwapHostIntToLittle(unicode); | ||
| NSString* charStr = [[NSString alloc] initWithBytes:&unicode length:sizeof(unicode) encoding:NSUTF32LittleEndianStringEncoding]; | ||
| [retval appendString:charStr]; | ||
| NSUInteger length = str.length; | ||
| NSMutableString* retval = [NSMutableString stringWithCapacity:length]; | ||
| // Hot path: almost every nucleus is a single character (length == 1). | ||
| // Use a fixed-size stack buffer for the common small case to avoid a | ||
| // malloc/free per call. Inputs longer than 256 unichars still fall back | ||
| // to the heap so the SEC-2 fix (no unbounded VLA) remains intact. | ||
| unichar stackBuf[256]; | ||
| unichar *charBuffer = (length <= 256) ? stackBuf : malloc(sizeof(unichar) * (size_t)length); | ||
| NSCAssert(length == 0 || charBuffer != NULL, @"Failed to allocate charBuffer"); | ||
| // Wrap in @try/@finally so charBuffer is released on all exit paths, | ||
| // including the IllegalCharacter / Invalid style exceptions that | ||
| // styleCharacter can @throw from inside the loop. | ||
| @try { | ||
| [str getCharacters:charBuffer range:NSMakeRange(0, length)]; | ||
| for (NSUInteger i = 0; i < length; ++i) { | ||
| unichar ch = charBuffer[i]; | ||
| UTF32Char unicode = styleCharacter(ch, fontStyle); | ||
| unicode = NSSwapHostIntToLittle(unicode); | ||
| NSString* charStr = [[NSString alloc] initWithBytes:&unicode length:sizeof(unicode) encoding:NSUTF32LittleEndianStringEncoding]; | ||
| [retval appendString:charStr]; | ||
| } | ||
| } @finally { | ||
| if (charBuffer != stackBuf) { | ||
| free(charBuffer); | ||
| } | ||
| } | ||
| return retval; | ||
| } | ||
|
|
@@ -2127,24 +2143,29 @@ - (MTDisplay*) makeTable:(MTMathTable*) table | |
| return [[MTMathListDisplay alloc] initWithDisplays:[NSArray array] range:table.indexRange]; | ||
| } | ||
|
|
||
| CGFloat columnWidths[numColumns]; | ||
| // NOTE: Using memset to initialize columnWidths array avoids | ||
| // Xcode Analyze "Assigned value is garbage or undefined". | ||
| // https://stackoverflow.com/questions/21191194/analyzer-warning-assigned-value-is-garbage-or-undefined | ||
| memset(columnWidths, 0, sizeof(columnWidths)); | ||
| NSArray<NSArray<MTDisplay*>*>* displays = [self typesetCells:table columnWidths:columnWidths]; | ||
|
|
||
| // Position all the columns in each row | ||
| NSMutableArray<MTDisplay*>* rowDisplays = [NSMutableArray arrayWithCapacity:table.cells.count]; | ||
| for (NSArray<MTDisplay*>* row in displays) { | ||
| MTMathListDisplay* rowDisplay = [self makeRowWithColumns:row forTable:table columnWidths:columnWidths]; | ||
| [rowDisplays addObject:rowDisplay]; | ||
| CGFloat *columnWidths = calloc(numColumns, sizeof(CGFloat)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most math tables have a very small number of columns (typically under 10). Unconditionally allocating the Using a hybrid stack/heap allocation pattern avoids heap allocation for typical tables while safely falling back to CGFloat stackColumnWidths[32];
CGFloat *columnWidths = stackColumnWidths;
if (numColumns > 32) {
columnWidths = calloc(numColumns, sizeof(CGFloat));
if (!columnWidths) {
return [[MTMathListDisplay alloc] initWithDisplays:[NSArray array] range:table.indexRange];
}
} else {
memset(stackColumnWidths, 0, sizeof(stackColumnWidths));
} |
||
| NSAssert(columnWidths != NULL, @"Failed to allocate columnWidths"); | ||
| // Wrap in @try/@finally so columnWidths is released on all exit paths. | ||
| // typesetCells:/makeRowWithColumns: eventually call changeFont, which can | ||
| // @throw IllegalCharacter / Invalid style; those would otherwise leak the buffer. | ||
| MTMathListDisplay* tableDisplay = nil; | ||
| @try { | ||
| NSArray<NSArray<MTDisplay*>*>* displays = [self typesetCells:table columnWidths:columnWidths]; | ||
|
|
||
| // Position all the columns in each row | ||
| NSMutableArray<MTDisplay*>* rowDisplays = [NSMutableArray arrayWithCapacity:table.cells.count]; | ||
| for (NSArray<MTDisplay*>* row in displays) { | ||
| MTMathListDisplay* rowDisplay = [self makeRowWithColumns:row forTable:table columnWidths:columnWidths]; | ||
| [rowDisplays addObject:rowDisplay]; | ||
| } | ||
|
|
||
| // Position all the rows | ||
| [self positionRows:rowDisplays forTable:table]; | ||
| tableDisplay = [[MTMathListDisplay alloc] initWithDisplays:rowDisplays range:table.indexRange]; | ||
| tableDisplay.position = _currentPosition; | ||
| } @finally { | ||
| free(columnWidths); | ||
| } | ||
|
|
||
| // Position all the rows | ||
| [self positionRows:rowDisplays forTable:table]; | ||
| MTMathListDisplay* tableDisplay = [[MTMathListDisplay alloc] initWithDisplays:rowDisplays range:table.indexRange]; | ||
| tableDisplay.position = _currentPosition; | ||
| return tableDisplay; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unconditional heap allocation via
mallocfor every call tomathListForCharacters:introduces unnecessary performance overhead, especially for typical short math strings (which are usually well under 100 characters). Additionally, ifmallocfails (returningNULL), passingNULLtogetCharacters:range:will cause a crash.Using a hybrid stack/heap allocation pattern (using a small stack buffer for common short lengths and only falling back to
mallocfor exceptionally large inputs) avoids heap allocation overhead in the vast majority of cases while safely handling large inputs and potential allocation failures.