bugGNU Octave - Bugs: bug #55440, clang's scan-build static analysis...

 
 

bug #55440: clang's scan-build static analysis report

Submitter:  Dmitri A. Sergatskov <dasergatskov>
Submitted:  Thu 10 Jan 2019 05:32:16 PM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 5.0.1 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 17 Jan 2019 04:35:07 PM UTC, comment #18: 

I cherrypicked a few of the "dead assignment" warnings that I could easily fix and did that here https://hg.savannah.gnu.org/hgweb/octave/rev/f8d380d04b01.

There may still be a few things in the clang report that could be fixed, but they are buried under thousands of false positives.  I think we have made enough progress on this report for the 5.0 release so I am going to close it and mark the task done at https://wiki.octave.org/5.0.0_Release_Checklist.

Rik <rik5>
Group administrator
Thu 17 Jan 2019 04:33:37 AM UTC, comment #17: 

I fixed the 3 warnings in dSparse.cc bsolve functions of type "Result of operation is garbage or undefined".  See https://hg.savannah.gnu.org/hgweb/octave/rev/50439c2e8bbf.

Rik <rik5>
Group administrator
Thu 17 Jan 2019 04:20:57 AM UTC, comment #16: 

Of the 11 warnings for "Result of operation is garbage or undefined", the 4 that are in the inverse function of liboctave/array/XXXMatrix.cc are all false positives.


Rik <rik5>
Group administrator
Wed 16 Jan 2019 08:26:59 PM UTC, comment #15: 

I agree that the code in oscanf and related functions in oct-stream.cc could probably be clearer, but I think those warnings are false positives.

John W. Eaton <jwe>
Group administrator
Wed 16 Jan 2019 07:31:06 PM UTC, comment #14: 

I just fixed an "unitialized argument value" in _glpk_.cc.  The same warning in urlwrite.cc is a false positive.  That still leaves 7 in oct-stream.cc which I won't go after since I find that code too confusing.

Rik <rik5>
Group administrator
Wed 16 Jan 2019 07:15:15 PM UTC, comment #13: 

Regarding static nil reps, there is no need to fix it.  We've discussed this issue a number of times and I think each time determined that the count for those objects will never go to zero.  The same thing is used by octave_value (for example) except that the static object is contained inside a function instead of being a static member variable.

John W. Eaton <jwe>
Group administrator
Wed 16 Jan 2019 07:04:39 PM UTC, comment #12: 

The two reports about "Branch condition evaluates to a garbage value" are false positives.

Rik <rik5>
Group administrator
Wed 16 Jan 2019 06:41:54 PM UTC, comment #11: 

I fixed the single "garbage return value" warning here https://hg.savannah.gnu.org/hgweb/octave/rev/1dcbdc61a518.

Rik <rik5>
Group administrator
Wed 16 Jan 2019 06:28:07 PM UTC, comment #10: 

The single report about "Assigned value is garbage or undefined" is a false positive.

Rik <rik5>
Group administrator
Wed 16 Jan 2019 06:18:58 PM UTC, comment #9: 

The "use of zero allocated" is in oct-tex-lexer.cc which is auto-generated code by lex/yacc which we will not bother to fix.  To summarize, these three categories have been cleared

1) Dead initialization
2) Use of zero allocated
3) Allocator sizeof operand mismatch

Rik <rik5>
Group administrator
Wed 16 Jan 2019 06:15:20 PM UTC, comment #8: 

The "division by zero" report is invalid.

Rik <rik5>
Group administrator
Wed 16 Jan 2019 06:14:19 PM UTC, comment #7: 

I fixed the dead initialization in Range.cc here (https://hg.savannah.gnu.org/hgweb/octave/rev/275a4d4ab56c).

Rik <rik5>
Group administrator
Wed 16 Jan 2019 05:34:06 PM UTC, comment #6: 

@Markus: jwe is probably the only one at the moment who understands whether your patch is correct.  However, I would say that unloading shared memory libraries is a fairly rare event so if we put in one extra comparison (rep != nilrep) there isn't going to be a performance penalty.  With that in mind, I would be tempted to commit your change, but put a FIXME note in front of the code explaining what you were trying to do.

Rik <rik5>
Group administrator
Wed 16 Jan 2019 06:27:24 AM UTC, comment #5: 

Changeset http://hg.savannah.gnu.org/hgweb/octave/rev/84e0e0b6493e fixes "Called C++ object pointer is null" logic error in main_window.

Torsten Lilge <ttl>
Group Member
Wed 16 Jan 2019 05:32:16 AM UTC, comment #4: 

Changeset http://hg.savannah.gnu.org/hgweb/octave/rev/8d2e8d5d617d fixes dead assignemnts in libgui.

Torsten Lilge <ttl>
Group Member
Sat 12 Jan 2019 04:33:46 PM UTC, comment #3: 

The analyser might have a point that there is something off with the "Bad free" in oct-shlib.h: "delete" might be called on the static nil_rep.
This might be a fix:

diff -r 8b0c9dc0799a liboctave/util/oct-shlib.h
--- a/liboctave/util/oct-shlib.h        Fri Jan 11 15:22:59 2019 -0500
+++ b/liboctave/util/oct-shlib.h        Sat Jan 12 16:33:32 2019 +0100
@@ -126,7 +126,7 @@

     ~dynamic_library (void)
     {
-      if (--rep->count == 0)
+      if (--rep->count == 0 && rep != nilrep)
         delete rep;
     }

@@ -140,7 +140,7 @@
     {
       if (rep != sl.rep)
         {
-          if (--rep->count == 0)
+          if (--rep->count == 0 && rep != nilrep)
             delete rep;

           rep = sl.rep;


But I am not sure whether I am following the code correctly. And I don't know whether we have to be more careful with the reference count of the nilrep. Could someone else please check?

Markus Mützel <mmuetzel>
Group administrator
Sat 12 Jan 2019 02:51:54 PM UTC, comment #2: 

I fixed one of the uncovered oddities in this changeset:
http://hg.savannah.gnu.org/hgweb/octave/rev/9a91f675fd5f

How should we keep track of what points have been worked on?

Markus Mützel <mmuetzel>
Group administrator
Fri 11 Jan 2019 04:39:41 PM UTC, comment #1: 

I took a look.  Lots, and lots of the identified issues seem to result from not understanding Octave's memory management system, rather than being actual bugs in the memory management system.

I'm not sure how to usefully debug this.  It seems like it could be hours and hours unless we simply ignore certain classes of bugs.

Rik <rik5>
Group administrator
Thu 10 Jan 2019 05:32:16 PM UTC, original submission:  

clang's scan-build static analysis report (clang 7.0.1) is available as:



https://drive.google.com/open?id=1_RQNGwQ77rfI3cIwyYJFVNflrRYqGgnS


(Compressed directory is 11M)
Look at index.html with you browser.


Info on scan-build:
 https://clang-analyzer.llvm.org/scan-build.html

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by dasergatskov (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-01-17 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2019-01-16 rik5 StatusNone In Progress
    2019-01-11 rik5 Carbon-Copy- Added jwe

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code