Loading a specific PDB file crashes Avogadro

[fxcoudert]

Avogadro version:

• Avogadrolibs: 1.99.0
• Qt: 5.15.8

Desktop version: (please complete the following information):

• OS: macOS 14.3.1 (arm64)
• Version: 1.99.0

Describe the bug
Loading a specific PDB file crashes Avogadro

To Reproduce
Steps to reproduce the behavior:

1. Download PDB file at https://gist.github.com/fxcoudert/6ea151fc53e446572d990aaa69468558
2. Drag-and-drop it into Avogadro
3. Observe crash

Expected behavior
That PDB file loads fine with various software. At the very least, it should give an error but not crash.

[welcome]

Thanks for opening your first issue here! Please try to include example files and screenshots if possible. If you're looking for support, please post on our forum: https://discuss.avogadro.cc/

[fxcoudert]

If the crash happens during a drop event, the backtrace of the active thread is:

Thread 0::  Dispatch queue: com.apple.main-thread
0   CoreFoundation                	       0x18549a804 __CFStringHash + 884
1   CoreFoundation                	       0x1854c9d10 -[__NSDictionaryM setObject:forKey:] + 292
2   CoreFoundation                	       0x1854f72ec -[NSMutableDictionary addEntriesFromDictionary:] + 216
3   AppKit                        	       0x188cbcef8 -[NSAppearance _callCoreUIWithBlock:options:requireBezelTintColor:] + 412
4   AppKit                        	       0x188cf630c -[NSCompositeAppearance _callCoreUIWithBlock:options:requireBezelTintColor:] + 288
5   AppKit                        	       0x188d0bbb8 -[NSAppearance _createOrUpdateLayer:options:] + 100
6   AppKit                        	       0x1899097c0 -[NSTitlebarContainerView _configureLayer:forDividerStyle:widgetHeight:] + 288
7   AppKit                        	       0x189909dd4 -[NSTitlebarContainerView _updateDividerLayerForController:animated:] + 1012
8   AppKit                        	       0x188d0aa10 -[NSTitlebarContainerView observeValueForKeyPath:ofObject:change:context:] + 340
9   Foundation                    	       0x1865efcf4 NSKeyValueNotifyObserver + 252
10  Foundation                    	       0x18669fee0 NSKeyValueDidChange + 360
11  Foundation                    	       0x1865e2e68 -[NSObject(NSKeyValueObservingPrivate) _changeValueForKeys:count:maybeOldValuesDict:maybeNewValuesDict:usingBlock:] + 680
12  Foundation                    	       0x18660c458 -[NSObject(NSKeyValueObservingPrivate) _changeValueForKey:key:key:usingBlock:] + 64
13  Foundation                    	       0x18662d97c _NSSetRectValueAndNotify + 324
14  AppKit                        	       0x189aaab38 -[NSWindowSectionContentController _updateSectionState] + 336
15  AppKit                        	       0x18987c62c -[NSWindowSectionController setWindow:] + 164
16  AppKit                        	       0x1899082a4 -[NSTitlebarContainerView viewDidMoveToWindow] + 48
17  AppKit                        	       0x188d009d8 -[NSView _setWindow:] + 1788
18  AppKit                        	       0x188d080e4 -[NSView addSubview:] + 212
19  AppKit                        	       0x188d0dbbc -[NSFrameView addSubview:] + 52
20  AppKit                        	       0x188d0db70 -[NSThemeFrame addSubview:] + 456
21  AppKit                        	       0x188d0d590 -[NSView addSubview:positioned:relativeTo:] + 372
22  AppKit                        	       0x188d0d398 -[NSThemeFrame addSubview:positioned:relativeTo:] + 52
23  AppKit                        	       0x188d0d34c -[NSThemeFrame _addKnownSubview:positioned:relativeTo:] + 44
24  AppKit                        	       0x188d03ef0 __49-[NSThemeFrame _floatTitlebarAndToolbarFromInit:]_block_invoke + 324
25  AppKit                        	       0x188d03ccc +[NSAnimationContext runAnimationGroup:] + 56
26  AppKit                        	       0x188d03b78 -[NSThemeFrame _floatTitlebarAndToolbarFromInit:] + 120
27  AppKit                        	       0x188cffed4 -[NSThemeFrame initWithFrame:styleMask:owner:] + 212
28  AppKit                        	       0x188cfdf74 -[NSWindow _commonInitFrame:styleMask:backing:defer:] + 524
29  AppKit                        	       0x188cfda20 -[NSWindow _initContent:styleMask:backing:defer:contentView:] + 796
30  AppKit                        	       0x188e566b0 -[NSPanel _initContent:styleMask:backing:defer:contentView:] + 48
31  AppKit                        	       0x188cfd6f8 -[NSWindow initWithContentRect:styleMask:backing:defer:] + 48
32  AppKit                        	       0x188e56664 -[NSPanel initWithContentRect:styleMask:backing:defer:] + 48
33  AppKit                        	       0x188f57bb0 -[NSWindow initWithContentRect:styleMask:backing:defer:screen:] + 24
34  libqcocoa.dylib               	       0x100ff69f4 0x100fcc000 + 174580
35  Avogadro2                     	       0x100ccc938 Avogadro::MainWindow::dropEvent(QDropEvent*) + 360
36  libQt5Widgets.5.15.8.dylib    	       0x101fd5e48 QWidget::event(QEvent*) + 104
37  HIServices                    	       0x18b88da7c CallReceiveMessageCollectionWithMessage + 116
38  HIServices                    	       0x18b887b18 DoMultipartDropMessage + 104
39  HIServices                    	       0x18b8878d0 DoDropMessage + 56
40  HIServices                    	       0x18b88b664 CoreDragMessageHandler + 924
41  CoreFoundation                	       0x1855ac718 __CFMessagePortPerform + 596
42  CoreFoundation                	       0x18550ff6c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 60
43  CoreFoundation                	       0x18550fe8c __CFRunLoopDoSource1 + 520
44  CoreFoundation                	       0x18550e858 __CFRunLoopRun + 2244
45  CoreFoundation                	       0x18550d93c CFRunLoopRunSpecific + 608
46  HIToolbox                     	       0x18fad6448 RunCurrentEventLoopInMode + 292
47  HIToolbox                     	       0x18fad6284 ReceiveNextEventCommon + 648
48  HIToolbox                     	       0x18fad5fdc _BlockUntilNextEventMatchingListInModeWithFilter + 76
49  AppKit                        	       0x188ceced0 _DPSNextEvent + 660
50  AppKit                        	       0x1894d7eec -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 716
51  AppKit                        	       0x188ce037c -[NSApplication run] + 476
52  libqcocoa.dylib               	       0x100ff8a64 0x100fcc000 + 182884
53  dyld                          	       0x1850b10e0 start + 2360

If the crash occurs with the File > Open menu, the backtrace is the following:

Thread 0::  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	       0x1853f1808 semaphore_timedwait_trap + 8
1   SkyLight                      	       0x18aead57c SLSTransactionWait + 224
2   SkyLight                      	       0x18ae08e78 SLSConnectionSynchronizeSLSCATransaction + 48
3   SkyLight                      	       0x18ac03b40 SLSWindowIsOrderedIn + 48
4   AppKit                        	       0x188ee44a4 -[_NSCGSWindowOrdering isWindowOrderedIn:] + 108
5   AppKit                        	       0x188f57c00 -[NSWindow _fromScreenCommonCode:] + 52
6   AppKit                        	       0x188f57bbc -[NSWindow initWithContentRect:styleMask:backing:defer:screen:] + 36
7   libqcocoa.dylib               	       0x1024369f4 0x10240c000 + 174580
8   Avogadro2                     	       0x10210fb64 Avogadro::MainWindow::importFile() + 616
9   libQt5Core.5.15.8.dylib       	       0x104179424 0x104034000 + 1332260
10  CoreFoundation                	       0x18550f970 __CFRunLoopDoSource0 + 176
11  CoreFoundation                	       0x18550f740 __CFRunLoopDoSources0 + 340
12  CoreFoundation                	       0x18550e2d0 __CFRunLoopRun + 828
13  CoreFoundation                	       0x18550d93c CFRunLoopRunSpecific + 608
14  HIToolbox                     	       0x18fad6448 RunCurrentEventLoopInMode + 292
15  HIToolbox                     	       0x18fad6284 ReceiveNextEventCommon + 648
16  HIToolbox                     	       0x18fad5fdc _BlockUntilNextEventMatchingListInModeWithFilter + 76
17  AppKit                        	       0x188ceced0 _DPSNextEvent + 660
18  AppKit                        	       0x1894d7eec -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 716
19  AppKit                        	       0x188ce037c -[NSApplication run] + 476
20  libqcocoa.dylib               	       0x102438a64 0x10240c000 + 182884
21  dyld                          	       0x1850b10e0 start + 2360

[ghutchis]

Because we do file loading in a separate thread, backtraces are unfortunately not very useful.

I have a better trace from the command-line. Since we're using our own code, the PDB reader isn't quite as battle-tested as the Open Babel code.

[fxcoudert]

the PDB reader isn't quite as battle-tested as the Open Babel code

I had a look and there's at least one weird thing: it assumes all SE atoms are sulfur, which means it's mistyping selenium atoms.

avogadrolibs/avogadro/io/pdbformat.cpp

Line 147 in 7061aac

if (element == "SE") // For Sulphur

I tested with a simple PDB file with a selenium atom, which VMD and Mercury read fine, but Avogadro transforms the selenium into sulfur.

$ cat test.pdb      
HETATM    1  SE  HOH     1       0.013   0.831  -0.083  1.00  0.00           SE 
HETATM    2  H   HOH     0       0.941   0.844   0.163  1.00  0.00           H  
HETATM    3  H   HOH     0      -0.250  -0.068  -0.293  1.00  0.00           H  
END

I've also looked at openbabel and it definitely trusts the element field:
https://github.com/openbabel/openbabel/blob/32cf131444c1555c749b356dab44fb9fe275271f/src/formats/pdbformat.cpp#L932

[ghutchis]

It's one thing if the SE is in the atom name, but SE in the element column is reliable.

#1643

[github-actions]

Here are the build results
macOS.dmg
Avogadro2.AppImage
Win64.exe
Artifacts will only be retained for 90 days.

[github-actions]

Here are the build results
Avogadro2.AppImage
macOS.dmg
Win64.exe
Artifacts will only be retained for 90 days.