bugGNU Octave - Bugs: bug #63712, address-sanitizer-flags should be...

 
 

bug #63712: address-sanitizer-flags should be in CFLAGS/CXXFLAGS

Submitter:  Dmitri A. Sergatskov <dasergatskov>
Submitted:  Thu 26 Jan 2023 07:25:01 PM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 8.0.90 Operating System:  * GNU/Linux
Fixed Release:  9.1.0 Planned Release:  9.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 02 Feb 2023 07:25:55 PM UTC, comment #4: 

Looks good! Now I see:

octave:2> __octave_config_info__ ("WARN_CXXFLAGS")
ans =  -Wall -W -Wshadow -Woverloaded-virtual -Wold-style-cast -Wformat -Wpointer-arith -Wwrite-strings -Wcast-align -Wcast-qual
octave:3> __octave_config_info__ ("XTRA_CXXFLAGS")
ans =  -pthread -fopenmp -fsanitize=address -fno-omit-frame-pointer


and the flags are exported through mkoctfile:

octave:1> pkg install statistics -forge -verbose
downloading tarball(s) from:
- https://packages.octave.org/download/statistics-1.5.3.tar.gz
mkdir (/tmp/oct-aWezt4)
untar (/tmp/statistics-1.5.3-EYwOe7.tar.gz, /tmp/oct-aWezt4)
make: Entering directory '/tmp/oct-aWezt4/statistics-release-1.5.3/src'
/usr/local/bin/mkoctfile-9.0.0 --verbose       libsvmread.cc
g++ -c  -fPIC -I/usr/local/include/octave-9.0.0/octave/.. -I/usr/local/include/octave-9.0.0/octave -I/usr/local/include  -pthread -fopenmp -fsanitize=address -fno-omit-frame-pointer -g -ggdb3 -O0    libsvmread.cc -o /tmp/oct-FJE30I.o


Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 02 Feb 2023 06:49:09 PM UTC, comment #3: 

No comments so I committed the patch here: http://hg.savannah.gnu.org/hgweb/octave/rev/3641b742d8aa.

Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Sat 28 Jan 2023 11:00:26 PM UTC, comment #2: 

This is guidance from Gentoo (https://devmanual.gentoo.org/general-concepts/autotools/index.html)


Sometimes, badly behaved Makefile.am files will override user variables such as CFLAGS. This must not be allowed — see Not Filtering Variables. There are separate special variables which should be used in these situations — for setting CFLAGS. for example, a Makefile.am should use AM_CFLAGS so that user preferences are not ignored.

So, if a Makefile.am contains, say:

CFLAGS="-Wall"
You should sed or patch it to use:

AM_CFLAGS="-Wall"


As a separate issue, I see that configure.ac is changing CFLAGS occasionally before doing certain compilation tests.  Maybe that should be looked at.

But, I also agree that WARN_XXX is a weird place to put the address-sanitizer flags.  Attached is a changeset for review which moves these flags into XTRA_XXXFLAGS.

(file #54282)

Rik <rik5>
Group administrator
Fri 27 Jan 2023 11:31:10 AM UTC, comment #1: 

Maybe the guidelines have changed, but I remember advice in the past from the autotools docs or the GNU coding standards that variables like CFLAGS and CXXFLAGS should be left for the user to define and traditionally had the default value of "-O2 -g".

However, I'm not sure why the address sanitizer flags ended up in the WARN_* flags instead of the XTRA_* flags that are used in the Octave Makefile.

John W. Eaton <jwe>
Group administrator
Thu 26 Jan 2023 07:25:01 PM UTC, original submission:  

In octave 8.0.90 address-sanitizer-flags (-fsanitize=address -fno-omit-frame-pointer) are put into WARN_CFLAGS and WARN_CXXFLAGS.

That looks wrong. WARN_* flags should not change the binary.

Also as of an immediate effect -- the packages do not inherit those flags from octave.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54282:  xtra_flags.cset added by rik5 (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2023-02-02 rik5 Open/ClosedOpen Closed
    2023-02-02 rik5 Item GroupNone Other
        StatusNone Fixed
        Fixed ReleaseNone 9.1.0
        Planned ReleaseNone 9.1.0
    2023-01-28 rik5 Attached File- Added xtra_flags.cset, #54282

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code