813 default: segfault after start

[Beep6581]

Originally reported on Google Code with ID 463

Build c8cba476fffd 813 default tip segfaults just after starting RT. Gdb says the following:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb20c3b70 (LWP 19326)]
0x0825d8a5 in rtengine::CurveFactory::complexCurve(double, double, double, double,
double, double, double, double, double, bool, std::vector<double, std::allocator<double>
> const&, unsigned int*, float*, float*, int*, unsigned int*, int)
    ()

Ubuntu 10.10 32-bit. 

Reported by paul.matthijsse4 on 2011-01-06 11:31:08

[Beep6581]

Hello Paul (c'est curieux de parler en anglais ...)

I do not think this is my amendment that brings this bug. It only affects the variables
of files pp3 (for exposure before interpolation), more configuration variables (Rtgui)..

 Emil posted a few hours earlier another amendment (that night in France). It amends
the "curve".

On Windows, for once no problem ..

Reported by jdesmis on 2011-01-06 13:29:23

[Beep6581]

update: same story with branch_3.0 (03ea21dbd3c7 809 branch_3.0). Don't know where this
comes from, yesterday's builds were okay. 

Reported by paul.matthijsse4 on 2011-01-06 13:35:49

[Beep6581]

It does look like a result of the highlight patch.  Well, it was sitting in the issues
ticket for weeks, I guess nobody tested it on linux :(

Reported by ejm.60657 on 2011-01-06 14:57:26

[Beep6581]

Hello Emil, you're referring to issue 431? But you're right, I don't test patches that
are published here, I wait for them to be included in one of the branches. This is
to avoid that 'hg update' starts to complain tomorrow about merge conflicts (saw that
before and I'm not so familiar with mercurial to solve that quickly). 

Reported by paul.matthijsse4 on 2011-01-06 15:17:44

[Beep6581]

To test a patch, save it to the RT folder, then execute 'hg import --no-commit <patchname>',
and recompile (and if necessary re-cmake).  When done testing the patch, execute 'hg
update -C' and your directory will be restored to its state before the patch (C for
clear away the changes).  You should be able to 'hg pull' and 'hg update' without problems,
at least it works for me whereas before I would indeed periodically run into local
changes conflicting with updates from the Google repository.  The trick is always to
clear out your local changes before attempting an update from the official repo.

Reported by ejm.60657 on 2011-01-07 00:33:01

[Beep6581]

Hello Paul
I'm just updating the branch_3.0 with "exposure_before_interpolation" (from 441 and
default)

But, I confirm, on Windows in debug mode no error message with Emil' patch "curves
and highlights"

Reported by jdesmis on 2011-01-07 06:23:06

[Beep6581]

Confirmed crash:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffdffff710 (LWP 24915)]
0x00000000007c9417 in rtengine::CurveFactory::complexCurve (ecomp=2.6000000000000001,
black=0.003356984817273213, hlcompr=0, hlcomprthresh=0, shcompr=0, br=0, contr=42,

    defmul=0, gamma_=2.2000000000000002, igamma=true, curvePoints=..., histogram=0x1adce20,
hlCurve=0x2398de0, shCurve=0x2318db0, outCurve=0x2058d20, 
    outBeforeCCurveHistogram=0x0, skip=16) at /home/drslony/rawtherapee/rtengine/curves.cc:540
540                                     avg += dcurve[(int)shCurve[(int)hlCurve[i]*i]]
* histogram[i];
(gdb) bt
#0  0x00000000007c9417 in rtengine::CurveFactory::complexCurve (ecomp=2.6000000000000001,
black=0.003356984817273213, hlcompr=0, hlcomprthresh=0, shcompr=0, br=0, contr=42,

    defmul=0, gamma_=2.2000000000000002, igamma=true, curvePoints=..., histogram=0x1adce20,
hlCurve=0x2398de0, shCurve=0x2318db0, outCurve=0x2058d20, 
    outBeforeCCurveHistogram=0x0, skip=16) at /home/drslony/rawtherapee/rtengine/curves.cc:540
#1  0x00000000008482f2 in rtengine::Thumbnail::processImage (this=0x1602200, params=...,
rheight=146, interp=rtengine::TI_Bilinear, myscale=@0x7fffdfffec18)
    at /home/drslony/rawtherapee/rtengine/rtthumbnail.cc:704
#2  0x00000000007732e9 in Thumbnail::processThumbImage (this=0x110e400, pparams=...,
h=146, scale=@0x7fffdfffec18) at /home/drslony/rawtherapee/rtgui/thumbnail.cc:289
#3  0x0000000000690857 in ThumbImageUpdater::Impl::processNextJob (this=0x15d8310)
at /home/drslony/rawtherapee/rtgui/thumbimageupdater.cc:159
#4  0x00000000006916ed in sigc::bound_mem_functor0<void, ThumbImageUpdater::Impl>::operator()
(this=0x16b9468) at /usr/include/sigc++-2.0/sigc++/functors/mem_fun.h:1787
#5  0x000000000069161e in sigc::adaptor_functor<sigc::bound_mem_functor0<void, ThumbImageUpdater::Impl>
>::operator() (this=0x16b9460)
    at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:251
#6  0x000000000069149d in sigc::internal::slot_call0<sigc::bound_mem_functor0<void,
ThumbImageUpdater::Impl>, void>::call_it (rep=0x16b9430)
    at /usr/include/sigc++-2.0/sigc++/functors/slot.h:103
#7  0x00000031fe241ec2 in ?? () from /usr/lib/libglibmm-2.4.so.1
#8  0x000000321aa6dcac in ?? () from /usr/lib/libglib-2.0.so.0
#9  0x000000321aa6b5d4 in ?? () from /usr/lib/libglib-2.0.so.0
#10 0x0000003218606c1a in start_thread () from /lib/libpthread.so.0
#11 0x0000003217ad1cad in clone () from /lib/libc.so.6

Some more here:
http://pastebin.com/xZeh9Tw5

Reported by entertheyoni on 2011-01-08 01:56:50

• Status changed: Accepted
• Labels added: Priority-High, OpSys-Linux
• Labels removed: Priority-Low

[Beep6581]

ps. hg disect says:
The first bad revision is:                                                        

changeset:   815:e3481c271922
user:        Emil Martinec <ejm.60657@gmail.com>
date:        Wed Jan 05 20:11:32 2011 -0600
summary:     New highlight recovery tool, including highlight recovery threshold.

Reported by entertheyoni on 2011-01-08 04:58:31

[Beep6581]

Same here.

Reported by thx4uall on 2011-01-08 08:02:22

[Beep6581]

Issue 458 has been merged into this issue.

Reported by entertheyoni on 2011-01-08 12:26:28

[Beep6581]

Reported by entertheyoni on 2011-01-08 12:27:29

[Beep6581]

Looking at the code, the only thing that I can imagine might cause a different behavior
is that the C compiler on linux might not like the line continuation in the definition
of complexCurve at line 408 of curves.cc.

Could you see what happens if you put the arguments of that function back on one line
as it used to be in the code?  ie replace

    void CurveFactory::complexCurve (double ecomp, double black, double hlcompr, double
hlcomprthresh, \
                                     double shcompr, double br, double contr, double defmul, double gamma_, bool
igamma, \
                                     const std::vector<double>& curvePoints, unsigned int* histogram, \
                                     float* hlCurve, float* shCurve, int* outCurve, \
                                     unsigned int* outBeforeCCurveHistogram, int skip) {

by the previous version

    void CurveFactory::complexCurve (double ecomp, double black, double hlcompr, double
shcompr, double br, double contr, double defmul, double gamma_, bool igamma, const
std::vector<double>& curvePoints, unsigned int* histogram, float* hlCurve, float* shCurve,
int* outCurve, unsigned int* outBeforeCCurveHistogram, int skip) {

Reported by ejm.60657 on 2011-01-08 14:17:08

[Beep6581]

Emil the previous version you pasted differs by more than line continuation: "hlcomprthresh"
is missing. I tried the version you pasted, it wouldn't compile. I tried removing the
backslashes from the original, and it would segfault the same.

Reported by entertheyoni on 2011-01-08 16:03:02

[Beep6581]

Sorry, I should have included the extra variable:

void CurveFactory::complexCurve (double ecomp, double black, double hlcompr, double
hlcomprthresh, double shcompr, double br, double contr, double defmul, double gamma_,
bool igamma, const std::vector<double>& curvePoints, unsigned int* histogram, float*
hlCurve, float* shCurve, int* outCurve, unsigned int* outBeforeCCurveHistogram, int
skip) {

What I am trying to determine is whether the compiler doesn't like the line continuation.
 It is obviously working for both Mac and Win so the problem may be in differences
in the way the compiler is treating the code.  BTW removing the backslashes would make
things worse unless you deleted all the CR/LF as well

Reported by ejm.60657 on 2011-01-08 16:17:11

[Beep6581]

In issue 448 I modified FileCatalog constructor declaration into a more readable format
(line 196 of the patch) by adding line breaks in Eclipse. I wonder if this might cause
the same issue on linux. Could anyone test this?

Reported by michaelezra000 on 2011-01-08 16:31:38

[Beep6581]

Hello Emil, I did what you proposed but that doesn't work any better (and if that really
would be the problem, the compiler would complain during compilation). 

Found some more info. I can open/read in lots of directories with images from 2008
and 2009, open, edit and save them. In my 2010 directory, I can open (and edit, etc.)
all the folders until september without problems. The segfault occurs with raws in
my october folder and later. Moving all the pp3's in my december folder to a separate
directory does not help. Ditto with all the jpg's (so there are only raws left in this
directory, 115). 

Made a debug build and get the same backtrace as DrSlony. 

Very weird problem! 

Reported by paul.matthijsse4 on 2011-01-08 16:56:41

[Beep6581]

Edit: with "what you proposed" I meant taking away those line breaks. 
PS. Thanks for the how-to-test-patches!

Reported by paul.matthijsse4 on 2011-01-08 16:59:43

[Beep6581]

I can confirm that removing the line breaks (see comment #14) does not help the problem.
Ubuntu 10.04.1 64 bit.

Reported by gyurko.david@e-arc.hu on 2011-01-08 19:20:30

[Beep6581]

I've added some code to examine the details of the line in question.
                int histogramValue = histogram[i];
                float hlCurveValue = hlCurve[i];
                int shCurveIndex = (int)hlCurve[i]*i;
                int shCurveValue = (int)shCurve[(int)hlCurve[i]*i];
original line ->        avg += dcurve[(int)shCurve[(int)hlCurve[i]*i]] * histogram[i];

When the crash happens:
(gdb) print histogramValue
$1 = 0
(gdb) print hlCurveValue
$2 = 3.68618703
(gdb) print shCurveIndex
$3 = 65673                   ---- isn't this out of range?
(gdb) print shCurveValue
$4 = 45907276                ---- this looks wrong, it's used as an index to dcurve

Reported by kovacs.it on 2011-01-08 20:57:32

[Beep6581]

Ah, that explains it.  Try this patch:

Reported by ejm.60657 on 2011-01-08 22:13:50

---

• Attachment: highlight-bugfix.patch

[Beep6581]

The patch fixes the crash, thanks!

Reported by kovacs.it on 2011-01-08 22:40:17

[Beep6581]

Confirmed fixed, thank you!

Reported by entertheyoni on 2011-01-09 01:32:49

• Status changed: Verified

[Beep6581]

Works again, thanks! 
Ticket can be closed. 

Reported by paul.matthijsse4 on 2011-01-09 09:42:57

[Beep6581]

Works also for me.

Thank you!

Reported by thx4uall on 2011-01-09 10:17:47

[Beep6581]

Confirmed, works here as well. Emil, you may want to close the issue, thanks.

Reported by gyurko.david@e-arc.hu on 2011-01-09 14:25:45

[Beep6581]

Reported by ejm.60657 on 2011-01-09 15:11:54

• Status changed: Fixed