bugGNU Octave - Bugs: bug #63416, Messages from building with...

 
 

bug #63416: Messages from building with -fsanitize=undefined

Submitter:  Arun Giridhar <arungiridhar>
Submitted:  Fri 25 Nov 2022 10:29:27 AM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * stable Operating System:  * GNU/Linux
Fixed Release:  8.1.0 Planned Release:  8.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 28 Nov 2022 08:24:08 AM UTC, comment #16: 

Thank you for testing!

Markus Mützel <mmuetzel>
Group administrator
Sun 27 Nov 2022 08:16:41 PM UTC, comment #15: 

Yes, now it builds without "--disable-lib-visibility-flags" and passes "make check" without any runtime warnings.

CC="gcc" CXX="g++" F77="gfortran" CFLAGS="-pipe -march=native -O0 -ggdb3 -fsanitize=undefined" CXXFLAGS="$CFLAGS" FFLAGS="$CFLAGS" ../configure --enable-64 --enable-openmp --enable-java --disable-docs



  GEN      src/octave-cli-8.0.1
  CXXLD    src/octave-gui
  GEN      src/octave-gui-8.0.1
  GEN      doc/interpreter/doc-cache

Octave successfully built.  Now choose from the following:

   ./run-octave    - to run in place to test before installing
   make check      - to run the tests
   make install    - to install (PREFIX=/usr/local)

   HG ID for this build is "9978ef12aea3"



  PASS                            17648
  FAIL                                0
  XFAIL (reported bug)               46
  SKIP (missing feature)              4
  SKIP (run-time condition)          27


Arun Giridhar <arungiridhar>
Group Member
Sun 27 Nov 2022 06:35:30 PM UTC, comment #14: 

OT: Could you please check if it is working without `--disable-lib-visibility-flags` now? Do the checks still pass?


Markus Mützel <mmuetzel>
Group administrator
Sun 27 Nov 2022 11:36:42 AM UTC, comment #13: 

Thanks, jwe. It passes make check for me with no messages now. Closing as fixed.

Arun Giridhar <arungiridhar>
Group Member
Sat 26 Nov 2022 10:20:04 PM UTC, comment #12: 

I pushed the following changesets on stable and merged with default:

http://hg.savannah.gnu.org/hgweb/octave/rev/9d0d37acafbe
http://hg.savannah.gnu.org/hgweb/octave/rev/9978ef12aea3

Marking as ready for test.

John W. Eaton <jwe>
Group administrator
Sat 26 Nov 2022 08:27:32 PM UTC, comment #11: 

RE comment #10:  Yes, there is no wider type to use for signed int64 multiplication and abs(min_val) will overflow so we need to detect that case and avoid it.

John W. Eaton <jwe>
Group administrator
Sat 26 Nov 2022 05:21:41 PM UTC, comment #10: 

The patch in comment #7 indeed stops that integer abs() message from showing up.

To make sure I understand correctly: It only affected signed int64, because all smaller int types could be represented as a bigger type internally for multiplication, and unsigned types inherently don't have this problem, so your patch to signed int64 prevents that case from happening everywhere. Have I understood correctly?

Arun Giridhar <arungiridhar>
Group Member
Sat 26 Nov 2022 05:08:08 PM UTC, comment #9: 

Oops, I posted comment #8 to the wrong bug report.

John W. Eaton <jwe>
Group administrator
Sat 26 Nov 2022 04:42:47 PM UTC, comment #8: 

If you are wondering why


  warning('off','MATLAB:rmpath:DirNotFound');


doesn't suppress warnings in Octave, it's because Octave doesn't currently handle warning IDs that begin with "MATLAB:".

John W. Eaton <jwe>
Group administrator
Sat 26 Nov 2022 03:00:13 PM UTC, comment #7: 

The attached patch should avoid the problem with abs overflow.  Does it work for you?



(file #54023)

John W. Eaton <jwe>
Group administrator
Sat 26 Nov 2022 01:24:29 PM UTC, comment #6: 

"What was the reason for the message and for the fix?"

Data members are constructed in the order they appear in the class declaration and destroyed in the reverse order.  The problem was happening because the output system was using the evaluator after the evaluator was destroyed.  I would guess that most of the time this error was unlikely to cause a crash because the memory used by the evaluator wasn't reused or overwritten by something else between the time the evaluator destructor was called and the last time it was used.  But we shouldn't rely on that.  Reordering the objects eliminated that problem.

I'll check in a proper changeset soon.

John W. Eaton <jwe>
Group administrator
Fri 25 Nov 2022 08:40:33 PM UTC, comment #5: 

No messages now from make check!

What was the reason for the message and for the fix? Was it an internal layout difference?

Arun Giridhar <arungiridhar>
Group Member
Fri 25 Nov 2022 07:55:22 PM UTC, comment #4: 

Made it apply. The second was to interpreter.h not interpreter.cc. It's building now.

Updated patch attached.


(file #54020)

Arun Giridhar <arungiridhar>
Group Member
Fri 25 Nov 2022 07:47:47 PM UTC, comment #3: 

The patch does not apply. It gives this error:

applying eval-diffs.txt
patching file libinterp/corefcn/interpreter.cc
Hunk #3 FAILED at 585
Hunk #4 FAILED at 603
2 out of 4 hunks FAILED -- saving rejects to file libinterp/corefcn/interpreter.cc.rej
abort: patch failed to apply


Are they both supposed to go to the same file or is the second one supposed to go to pt-eval.h?

Arun Giridhar <arungiridhar>
Group Member
Fri 25 Nov 2022 06:56:46 PM UTC, comment #2: 

Does the attached change eliminate the message about the tree_evaluator for you?




(file #54019)

John W. Eaton <jwe>
Group administrator
Fri 25 Nov 2022 03:03:06 PM UTC, comment #1: 

The messages from comment #0 about the interpreter show up even without make check. Just running "./run-octave" and exiting without any commands is enough to give those messages.

It doesn't get in the way of anything else though.

Arun Giridhar <arungiridhar>
Group Member
Fri 25 Nov 2022 10:29:27 AM UTC, original submission:  

This happens on both fa254b08f949 (stable) and on ce5b4a00b022 (default).

Build flags:

CC="gcc" CXX="g++" F77="gfortran" CFLAGS="-pipe -march=native -O0 -ggdb3 -fsanitize=undefined" CXXFLAGS="$CFLAGS" FFLAGS="$CFLAGS" ../configure --enable-64 --enable-openmp --disable-java --disable-docs --disable-lib-visibility-flags


(The "--disable-lib-visibility-flags" was necessary with "-fsanitize=undefined" otherwise it gave linker errors about undefined symbols.)

Then "make check" proceeds normally and returns 0 except for three messages.

The first message about absolute values is definitely a false positive, because a standalone C++ test with std::abs also had the same behavior:

  integer.tst ....................................................../liboctave/util/oct-inttypes.h:69:26: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself
 pass   77/77


The other two messages are these:

../libinterp/corefcn/interpreter.h:169:68: runtime error: member call on address 0x7ff3e00058d8 which does not point to an object of type 'tree_evaluator'
0x7ff3e00058d8: note: object is of type 'octave::tree_walker'
 00 00 00 00  c8 96 b4 31 f4 7f 00 00  80 4a 00 e0 f3 7f 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'octave::tree_walker'

../libinterp/parse-tree/pt-eval.h:187:44: runtime error: member access within address 0x7ff3e00058d8 which does not point to an object of type 'tree_evaluator'
0x7ff3e00058d8: note: object is of type 'octave::tree_walker'
 00 00 00 00  c8 96 b4 31 f4 7f 00 00  80 4a 00 e0 f3 7f 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'octave::tree_walker'


Are they false positves too?

The result of "make check" was:

  PASS                            17615
  FAIL                                0
  XFAIL (reported bug)               46
  SKIP (missing feature)             45
  SKIP (run-time condition)          26


Arun Giridhar <arungiridhar>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54023:  abs-diffs.txt added by jwe (682B - text/plain)
file #54020:  new.patch added by arungiridhar (1KiB - text/x-patch)
file #54019:  eval-diffs.txt added by jwe (1KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by arungiridhar (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-11-27 arungiridhar StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-11-26 jwe StatusNone Ready For Test
        Fixed ReleaseNone 8.1.0
        Planned ReleaseNone 8.1.0
    2022-11-26 jwe Attached File- Added abs-diffs.txt, #54023
    2022-11-25 arungiridhar Attached File- Added new.patch, #54020
    2022-11-25 jwe Attached File- Added eval-diffs.txt, #54019

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code