bugGNU Octave - Bugs: bug #50874, [octave forge] (image) bwdist call...

 
 

bug #50874: [octave forge] (image) bwdist call causes application crash

Submitter:  None
Submitted:  Tue 25 Apr 2017 01:38:13 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Erwin Nindl Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * other
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 13 Dec 2018 06:08:58 PM UTC, comment #15: 

Thank you for the fix. I have pushed your change
http://hg.code.sf.net/p/octave/image/rev/8445342519be

I did not kept the entry on the NEWS file because I thought it was not relevant enough, and just kept the function on the other functions changed section.

Carnë Draug <carandraug>
Group Member
Thu 13 Dec 2018 05:54:51 AM UTC, comment #14: 

Please review.

Avinoam Kalma <avinoam>
Group Member
Sun 02 Dec 2018 10:11:22 PM UTC, comment #13: 

Attached changeset that fix the case of 1D (and the indices)
Please review


(file #45566)

Avinoam Kalma <avinoam>
Group Member
Sun 02 Dec 2018 02:16:11 AM UTC, comment #12: 

These are actually two issues, one for the case of there being no foreground elements and another for the case of 1d input.

The patches suggested are not enough because while they fix the issue of the returned distances, the returned indices (second output argument) is incorrect.

For the case of the no foreground elements I just pushed a patch that seems to fix it.

For the case of 1d vectors, I'm not sure how to fix it.

Carnë Draug <carandraug>
Group Member
Sun 25 Nov 2018 10:32:08 AM UTC, comment #11: 

I have now also tested this patch (file #45484) on a Win7 machine with Octave 4.2.1:

  • the original bwdist gives me a segfault with the code input from comment #0 and comment #5.
  • after applying the patch, bwdist.cc compiles fine with mkoctfile :)
  • with the resulting patched function bwdist I get the right results for the code input from comment #0 and comment #5. No segfaults any more.


So from my side, I would say this patch does its job fine, also on a Windows system.

Hartmut <hardy>
Sun 25 Nov 2018 05:12:58 AM UTC, comment #10: 

@Hartmut: Thanks for the review. I will add a note in the NEWS file

@Carne, could you please review this patch?

Avinoam Kalma <avinoam>
Group Member
Sat 24 Nov 2018 09:25:08 PM UTC, comment #9: 

I tried to review this patch (file #45484) as good as I could. Here are my comments:

  • I have done my testing only on a linux machine. I did not see any crashes, neither on the code from commment #0 nor comment #5. But I have done NO TESTING on a WINDOWS machine, so I cannot tell of the segfault is gone now.
  • I am not so fluent in C. So I have NOT CHECKED the added functionality of this patch on a principle basis.
  • The tests all pass for me. The old tests as well as the newly added tests.
  • I get compatible results now for the examples in comment #6.
  • There is not yet a commit message, nor a text for the NEWS file in the patch, yet.


So, as far as I can tell now this patch seems to be fine.

It would be nice to get

  • (A) someone more fluent in C to have a look at the newly added code pieces on a principal basis. @Carne ?
  • (B) someone to test this on a Windows machine. I will try to do this once I get access to a Windows machine.
Hartmut <hardy>
Sun 18 Nov 2018 08:55:48 PM UTC, comment #8: 

Please review the attached patch (bwdist_V2.patch),
it (should) solve all the problems described in comment #6.

(file #45484)

Avinoam Kalma <avinoam>
Group Member
Thu 15 Nov 2018 08:51:25 PM UTC, comment #7: 

A few corrections:

If all the entries are zero, the result should be all Inf's.

On Windows, Octave crashes on row vector. On Linux, there is no
crash, but the results are wrong.
 
And my questions is:

How do I return an Inf matrix for the first case (has_ones == 0 in the code)?

Avinoam Kalma <avinoam>
Group Member
Thu 15 Nov 2018 08:29:45 PM UTC, comment #6: 

There are few problems with bwdist

  • If all the entries are zero, the result should be all NaNs:



Octave:
>> bwdist ([0 0; 0 0])
ans =

   4.5253e+04   4.5254e+04
   4.5254e+04   4.5255e+04
Matlab:
>> bwdist ([0 0; 0 0])

   Inf   Inf
   Inf   Inf


  • bwdist crashes on row vectors
  • bwdist gives wrong results on column vectors:



Octave:
>> bwdist([1 0 0]')
ans =

   0.0000e+00
   4.5255e+04
   4.5255e+04
Matlab:
ans =

     0
     1
     2


Please review the attached patch. It treats the 1D problem
(and I have to add some relevant test).
The method is to increase the distance after we meet a zero,
and do it again in the opposite direction.

How do I return a NaN matrix for the first case (has_ones == 0 in the code).


(file #45434)

Avinoam Kalma <avinoam>
Group Member
Thu 04 May 2017 10:37:02 AM UTC, comment #5: 

I run the command

bwdist ([1 0]);

Avinoam Kalma <avinoam>
Group Member
Thu 04 May 2017 10:33:46 AM UTC, comment #4: 

Checking again, I have found that the crash is in line 330,
disty[i]=newdisty, because i=-1.
From some reason, in line 329 distx[i]=newdistx does not crash.

i=-1 already in line 321
          olddist2 = (*func)(distx[i], disty[i]);
So olddist2 is actually garbage.

Avinoam Kalma <avinoam>
Group Member
Tue 02 May 2017 06:23:51 PM UTC, comment #3: 

I could reproduce this crash in Windows, 4.2.1 and 4.3.0+.
In 4.0.2 there is no crash.

I have tried to add some prints in order to locate the reason
to the crash.

Carnë: Could you please upload here a version with printouts, so we could compare the Linux results with the Windows results, and may be it will give you a hint to the bug solution?

Avinoam Kalma <avinoam>
Group Member
Tue 02 May 2017 04:48:30 PM UTC, comment #2: 

Works for me on Linux (Debian Jessie). I have tried Octave 4.0.0, 4.2.1, and 4.3.0+ against the last image package release and the last image development head.

If the problems only happens on Windows, I won't be able to reproduce it. I would still welcome any patch though. Or can anyone give me more details for reproducing it on Linux?

Carnë Draug <carandraug>
Group Member
Sun 30 Apr 2017 06:32:32 PM UTC, comment #1: 

Confirmed.
Adding package maintainer to CC.

Avinoam Kalma <avinoam>
Group Member
Tue 25 Apr 2017 01:38:13 PM UTC, original submission:  

A call to bwdist fails on Win7 amd64.
It works correctly with Octave v4.0.3.




GNU Octave Shell:
---------------------------------------------------------------

GNU Octave, version 4.2.1
Copyright (C) 2017 John W. Eaton and others.
This is free software; see the source code for copying conditions.
There is ABSOLUTELY NO WARRANTY; not even for MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE.  For details, type 'warranty'.

Octave was configured for "x86_64-w64-mingw32".

Additional information about Octave is available at http://www.octave.org.

Please contribute if you find this software useful.
For more information, visit http://www.octave.org/get-involved.html

Read http://www.octave.org/bugs.html to learn how to submit bug reports.
For information about changes from previous versions, type 'news'.

octave:1> pkg load image
octave:2> foo = [1, 1, 1, 1, 0, 0, 0];
octave:3> bar = bwdist(foo);

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45566:  bwdist_V3.cs added by avinoam (4KiB - application/octet-stream)
file #45484:  bwdist_V2.patch added by avinoam (3KiB - application/octet-stream)
file #45434:  bwdist.patch added by avinoam (1KiB - application/octet-stream)
file #40501:  gnu-octave-bug.png added by None (81KiB - image/png)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-12-13 carandraug StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2018-12-02 avinoam Attached File- Added bwdist_V3.cs, #45566
    2018-11-18 avinoam Attached File- Added bwdist_V2.patch, #45484
    2018-11-15 avinoam StatusNeed Info Patch Submitted
        Release4.2.1 other
    2018-11-15 avinoam Attached File- Added bwdist.patch, #45434
        Carbon-Copy- Added hardy
    2017-08-12 jwe Summarybwdist call causes application crash [octave forge] (image) bwdist call causes application crash
    2017-05-02 carandraug StatusConfirmed Need Info
    2017-04-30 avinoam CategoryOctave Function Octave Package
    2017-04-30 avinoam StatusNone Confirmed
        Carbon-Copy- Added -email is unavailable-
    2017-04-25 None Attached File- Added gnu-octave-bug.png, #40501

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code