Rawtherapee: Crash when saving " Preferences -> Color Management -> Printer (soft-proofing) -> Color Profile"

[jtompkins66]

First time entering a bug report, hope I did everything right. :-)

Branch: master
Version: 4.2.1232
Changeset: 6a1ccb9b152e2feaa1ebac75a2c6cf6f8a673168
Compiler: cc 5.3.1
Processor: generic x86
System: Linux
Bit depth: 64 bits
Gtkmm: V2.24.4
Build type: debug
Build flags:  -std=gnu++11 -mtune=generic -Werror=unused-label -fopenmp -Werror=unknown-pragmas -g
Link flags:  -mtune=generic
OpenMP support: ON
MMAP support: ON

XUbuntu 16.04 (fully patched), 4 CPU cores, Intel(R) Core(TM) i5-2320 CPU @ 3.00GHz, 8 GB RAM

Downloaded via git and compiled locally, as I've been doing for years

Steps to reproduce crash:

1. Open RawTherapee
2. Open "Preferences", go to "Color Management" tab, under "Printer (soft-proofing)" select an ICC profile, click "OK" to save and exit preferences.
3. RawTherapee crashes on the "OK" above with "Segmentation fault" on STDOUT.

The ICC profile I pick, actually any of them I pick, all work fine with other applications.

Attached is the gdb stack trace

log.txt

[jtompkins66]

I was thinking, you may want the an actual profile. Attached is an Epson profile I got from Epson that's for a printer my local Photo store uses (where I have prints made). I had to put it in a ZIP file for the upload to work. Also if I hit "Cancel" to skip saving anything rather than "OK" it does not crash.

printer.zip

[Beep6581]

Cannot reproduce with your printer ICC or several from Adobe using 6a1ccb9b152e2feaa1ebac75a2c6cf6f8a673168 in Linux, tried with a relwithdebinfo build.

Branch: master
Version: 4.2.1232
Changeset: 6a1ccb9b152e2feaa1ebac75a2c6cf6f8a673168
Compiler: cc 4.9.3
Processor: Intel(R)\ Core(TM)\ m3-6Y30\ CPU\ @\ 0.90GHz
System: Linux
Bit depth: 64 bits
Gtkmm: V2.24.5
Build type: relwithdebinfo
Build flags: -std=c++11 -Wno-deprecated-declarations -Wno-unused-result -std=gnu++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -O2 -g -DNDEBUG
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

First time entering a bug report, hope I did everything right. :-)

Great report!

[jtompkins66]

Hummm, isn't that odd. I've tried wiping out my git download and git cloning everything from scratch then building (a completely fresh "clean" build). Still crashes. I do notice that any of the three setting under "Printer soft proofing", "Color profile", "Rendering intent" or "Black point compensation", if I change any of them and hit OK to save it crashes. But I can change anything else under Preferences and its fine (just "Printer soft proofing" crashes). I ran it under gdb again and when it crashed I saw this:

Thread 1 "rawtherapee" received signal SIGSEGV, Segmentation fault.
0x000000000083fe7c in EditorPanel::ColorManagementToolbar::updateSoftProofParameters (this=0x3561000, noEvent=false)
    at /usr/local/arc/rawtherapee/src/rtgui/editorpanel.cc:237
237                 processor->beginUpdateParams ();
(gdb) p processor
$2 = (rtengine::StagedImageProcessor * const&) @0x2c51ba0: 0x0

I'm a C programmer, not C++, but it appears to me that "processor" is a NULL pointer being dereferenced. Even tells me the source file and line which does seem to have something to do with saving the soft proofing. A NULL pointer dereference is still bad in C++ isn't it???

[Beep6581]

@Hombre57

[Floessie]

@jtompkins66 You're absolutely right! Although I cannot reproduce the problem here (debug and release build), I took a look, et voilà: processor can be nullptr and is guarded in every other place. So here is a preliminary patch:

diff --git a/rtgui/editorpanel.cc b/rtgui/editorpanel.cc
index 821bdf2d..9ad65261 100644
--- a/rtgui/editorpanel.cc
+++ b/rtgui/editorpanel.cc
@@ -233,12 +233,14 @@ private:
 #if !defined(__APPLE__) // monitor profile not supported on apple
         if (profileBox.get_active_row_number () > 0) {
 #endif
-            if (!noEvent) {
-                processor->beginUpdateParams ();
-            }
-            processor->setSoftProofing (softProof.get_sensitive() && softProof.get_active(), spGamutCheck.get_sensitive() && spGamutCheck.get_active());
-            if (!noEvent) {
-                processor->endUpdateParams (rtengine::EvMonitorTransform);
+            if (processor) {
+                if (!noEvent) {
+                    processor->beginUpdateParams ();
+                }
+                processor->setSoftProofing (softProof.get_sensitive() && softProof.get_active(), spGamutCheck.get_sensitive() && spGamutCheck.get_active());
+                if (!noEvent) {
+                    processor->endUpdateParams (rtengine::EvMonitorTransform);
+                }
             }
 #if !defined(__APPLE__) // monitor profile not supported on apple
         }

@Hombre57 Maybe there is an even better way (like an early return). I didn't investigate further.

HTH
Flössie

[Hombre57]

@Floessie Thanks for the patch, I'll look at it this evening, and sorry for this amateur's bug.

[Floessie]

@Hombre57 Call me an amateur then, because sometimes I let slip an unguarded deref through, too. :cry:

[Beep6581]

@jtompkins66 can you confirm the patch?

[jtompkins66]

I can when I get home after work, I am 6 hours west of GMT in the USA. I'm confident the patch will work. You guys are good! I've coded more than my share of NULL pointers in my life, it happens :-)

[jtompkins66]

Yes sir it worked. I also verified the changes are written to the config file and thus reloaded on program startup.

I notice the "tooltip" messages for the softproofing button talks about the output profile rather than the configured printer profile. Small thing. The tooltip for the "show in gray out of gamut colors" button also talks about output profile (which I suppose it should unless the softproofing printer profile is "active").

Thanks for the Labor of Love (at least I hope it is) in developing RawTherapee!

[Floessie]

Fixed on master.