luckymarmot/ThemeKit

Crash on CacheKey Hash in Common.swift

PaulSolt opened this issue · 2 comments

I'm randomly seeing crashes with the hash for ThemeKit. I don't know if this has been modified in a later version, as I'm still using an older version.

Thread 1: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)

The hash value appears to overflow in Common.swift

Crash hash

func CacheKey(selector: Selector, colorSpace: NSColorSpace?, theme: Theme?) -> NSNumber {
    let hashValue = selector.hashValue + (colorSpace == nil ? 0 : (colorSpace!.hashValue << 4)) + (theme == nil ? 0 : (theme!.hash << 8))
    return NSNumber(value: hashValue)
}

The problem for me happens when the selector.hashValue is added to the result of the (theme!.hash << 8). It overflows and randomly crashes.

Question: Is there a reasons to shift 8 for this hash value?

Here's some output from the debugger:

(lldb) po let $x = theme!.hash
(lldb) po let $y = selector.hashValue
(lldb) po $x + $y
9214902782392903245

(lldb) po ($x << 8) + $y
error: Execution was interrupted, reason: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0).
The process has been returned to the state before expression evaluation.
(lldb) po $x
105553140946048

(lldb) po $y
9214797229251957197

(lldb) po $x << 4
1688850255136768

(lldb) po $x << 8
27021604082188288

(lldb) po let $z = $x << 8
(lldb) po $z + $y
error: Execution was interrupted, reason: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0).
The process has been returned to the state before expression evaluation.
(lldb) po selector.hashValue + (theme == nil ? 0 : (theme!.hash << 8))
error: Execution was interrupted, reason: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0).
The process has been returned to the state before expression evaluation.
(lldb) 

Hi Paul! Yep, that has been fixed some time ago on 3eb14a8 . You can either update to the last version or wait for the next Swift 5 compatible conversion - I will upgrade soon ThemeKit...
Thanks for reporting the issue.

Previously fixed on 3eb14a8