Skip to content
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

Crash in rtengine::Color::rgb2lab01, "wprof" nullptr deref of rtengine::ICCStore::getInstance()->workingSpaceMatrix(profileCalc); #6357

Closed
mandree opened this issue Sep 11, 2021 · 8 comments · Fixed by #6889
Milestone

Comments

@mandree
Copy link

mandree commented Sep 11, 2021

A SIGSEGV crash (nullptr) dereference has been reported against FreeBSD's port of RawTherapee 5.8 as FreeBSD's Bugzilla #257255.
Looking at color.cc and iccstore.cc git logs, I don't see obvious related code changes since release 5.8. (I am the maintainer of FreeBSD's port and package of Rawtherapee.)

I have not fully established how the reporter triggers this (he claims zooming in, possibly moving the frame) and asked for more information, however what happens in the code is in this rtengine/color.cc fragment:

   const TMatrix wprof = rtengine::ICCStore::getInstance()->workingSpaceMatrix(profileCalc);

    const float xyz_rgb[3][3] = { {static_cast<float>(wprof[0][0]), static_cast<float>(wprof[0][1]), static_cast<float>(wprof[0][2])},
                                  {static_cast<float>(wprof[1][0]), static_cast<float>(wprof[1][1]), static_cast<float>(wprof[1][2])},
                                  {static_cast<float>(wprof[2][0]), static_cast<float>(wprof[2][1]), static_cast<float>(wprof[2][2])}
                                };

These are the arguments and locals of the relevant stack frame.
Note wprof is 0, and the crash is on the xyz_rgb[3][3] initialization.
If I understand the code correctly, this means that in the wMatrices, either sRGB is not found, or its second member itself is null.

#0  0x0000000000ddb930 in rtengine::Color::rgb2lab01(Glib::ustring const&, Glib::ustring const&, float, float, float, float&, float&, float&, bool)
    (profile=..., profileW=..., r=1, g=1, b=0, LAB_l=@0x7fffffffbe28: -nan(0x7fcab0), LAB_a=@0x7fffffffbe30: -nan(0x7fcab0), LAB_b=@0x7fffffffbe2c: 4.59163468e-41, workingSpace=false)
    at /usr/ports/graphics/rawtherapee/work/rawtherapee-5.8/rtengine/color.cc:483
        profileCalc = {static npos = 18446744073709551615, string_ = "sRGB"}
        wprof = 0x0
        xyz_rgb = 
            {{-nan(0x7fbde0), 4.59163468e-41, 1.52135275e-37}, {1.12103877e-44, -1.06849469e-12, -1.78599382e+37}, {3.93009116e-29, 1.12103877e-44, 1.40129846e-45}}
        var_X = 3.98482128e-29
        var_Y = 1.12103877e-44
        var_Z = 3.98482609e-29
        varxx = 1.12103877e-44
        varyy = -nan(0x7fbd90)
        varzz = 4.59163468e-41
@mandree
Copy link
Author

mandree commented Sep 11, 2021

related rtengine/iccstore.cc code excerpts:

    using ProfileMap = std::map<Glib::ustring, cmsHPROFILE>;
...
    MatrixMap wMatrices;
...
    TMatrix workingSpaceMatrix(const Glib::ustring& name) const
    {
        const MatrixMap::const_iterator r = wMatrices.find(name);

        return
            r != wMatrices.end()
            ? r->second
            : wMatrices.find("sRGB")->second;
    }

@Thanatomanic
Copy link
Contributor

@mandree Thanks for the report. Are you able to test the situation in a build of the current dev branch? Otherwise, please share the affected raw + pp3.

@mandree
Copy link
Author

mandree commented Oct 5, 2021

I personally cannot reproduce the crash at all, and haven't heard back from the reporter. I've closed the FreeBSD bug report with "Unable to reproduce". Let's see if that wakes the bug reporter.

@mandree
Copy link
Author

mandree commented Nov 13, 2021

So I've received a report update on FreeBSD's end and the problem's submitter surmises that the drm-kmod (FreeBSD's DRM direct rendering kernel module) amdgpu driver might be the culprit. Reporter observes that replacing his AMD graphics card for an nvidia card and swapping out drivers correspondingly, fixes the crash. Also, if use uses the AMD VGA with the vesa driver, the crash does not happen.

So, where any color-profile related safeguards vs. nullptr added since 5.8 release such that it is worthwhile testing the development version? Or should one be added to dev and we backport it to 5.8?

@mandree
Copy link
Author

mandree commented Nov 13, 2021

If it helps any, the "competition" (darktable) ships with a CMS test program, and that shows something that I get on NVidia too, so may be unrelated, but still I am reporting the submitter's output of darktable-cmstest (but it may well be a red herring):


darktable-cmstest version 3.6.1
this executable was built with colord support enabled
darktable itself was built with colord support enabled

couldn't locate primary CRTC!
CRTC for screen 0 CRTC 1 has no mode or no output, skipping
CRTC for screen 0 CRTC 2 has no mode or no output, skipping
CRTC for screen 0 CRTC 3 has no mode or no output, skipping
CRTC for screen 0 CRTC 4 has no mode or no output, skipping
CRTC for screen 0 CRTC 5 has no mode or no output, skipping

DVI-I-1 the X atom and colord returned different profiles
        X atom: _ICC_PROFILE (0 bytes)
                description: (none)
        colord: "/usr/local/etc/gdm/home/.local/share/icc/edid-e457da63e7c1ee2b2f35eaf330fbd2be.icc"
                description: SyncMaster

@mandree
Copy link
Author

mandree commented Apr 19, 2023

I have received word today by a personal message that one FreeBSD 13 user found Rawtherapee to crash like this when launching his desktop session through lightdm and startxfce4 through the .x scripts, but not when launching Rawtherapee from a GNOME, Cinnamon, or XFCE4 desktop that is set up by gdm as display/login manager instead.

@tankist02
Copy link

This is the guy who wrote Andree. When I run RT when it crashes from terminal I see this output. There is no line about colorreload-gtk-module when RT is run under gdm and works fine.

$ rawtherapee
Gtk-Message: 15:35:53.272: Failed to load module "colorreload-gtk-module"

(rawtherapee:1481): GLib-GObject-CRITICAL **: 15:35:58.682: g_object_unref: assertion 'old_ref > 0' failed

(rawtherapee:1481): GLib-GObject-CRITICAL **: 15:35:58.682: g_object_unref: assertion 'old_ref > 0' failed

(rawtherapee:1481): GLib-GObject-CRITICAL **: 15:35:58.682: g_object_unref: assertion 'old_ref > 0' failed

(rawtherapee:1481): GLib-GObject-CRITICAL **: 15:35:58.682: g_object_unref: assertion 'old_ref > 0' failed

(rawtherapee:1481): GLib-GObject-CRITICAL **: 15:35:58.683: g_object_unref: assertion 'old_ref > 0' failed

(rawtherapee:1481): GLib-GObject-CRITICAL **: 15:35:58.683: g_object_unref: assertion 'old_ref > 0' failed
Segmentation fault

@Lawrence37
Copy link
Collaborator

#6889 should fix the crash.

@Lawrence37 Lawrence37 added this to the v5.10 milestone Jan 28, 2024
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Feb 23, 2024
ChangeLog:	https://rawtherapee.com/downloads/5.10/#new-features

We need to stick to GCC because LLVM-compiled code may take more than
twice as much processing time as GCC-compiled does, for my test set
comparing a few images with denoising and stuff on FreeBSD-14.0-RELEASE
amd64 comparing GCC 12.3 to clang 16.0, and similar values on
FreeBSD-13.2-RELEASE.

Pin GCC to 12 and override -stdlib accordingly, to fix
PR:		273682

Clean up Makefile a bit, but we cannot let go of the CCACHE
workarounds yet, which we need when enforcing GCC compile.
Convert some .if branches to options helpers.

Make LTO an option that defaults to on. For some strange reason,
massively-parallel compilation WITHOUT LTO appears to trigger
OOM kills much more than an LTO-enabled build.  Upstream states
that LTO build should run faster.

For one self-test that fails frequently with SIGPIPE (Exit code 141 is
128 for core dump + 13 for SIGPIPE), pipe through dd with bigger input
buffer to avoid SIGPIPE/exit code 141 test failures.

Replace echo by ${ECHO_CMD} in self-tests to appease portlint.
Portlint misdetects "file system" as bare use of file though and
suggests ${FILE}, which is wrong.

Revise warnings around the CCACHE hacks because
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277204
strives to move ports to CCACHE_ENABLED, which does not work for this
port.

Also, 5.10 should fix the profile/locale related crash
PR:		257255
by switching the std::map variable to use std::string as index,
rather than Glib::ustring, which caused inconsistencies with locales.
Upstream references (two bug reports, and the fix, in order):
Beep6581/RawTherapee#6357
Beep6581/RawTherapee#6876
Beep6581/RawTherapee@a95a58a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants