[Bug fortran/65089] New: FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[Bug fortran/65089] New: FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

            Bug ID: 65089
           Summary: FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested
                    with -fsanitize=address
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dominiq at lps dot ens.fr

I have tested gfortran with

make -k check-gfortran
RUNTESTFLAGS="--target_board=unix'{-m32/-fsanitize=address,-m64/-fsanitize=address}'"

for 95 unexpected failures for -m32 or -m64 (well below what I anticipated!-).

I indeed see pr60576 (assumed_rank_7.f90), pr64921 (class_allocate_18.f90), and
pr64986 (class_to_type_4.f90). I also see 40+ failures associated with scan-*
which are probably expected: -fsanitize=address may change optimizations.
Finally (this PR) the tests gfortran.dg/io_real_boz(2|_[45]).f90 give outputs
of the kind

=================================================================
==24090==ERROR: AddressSanitizer: global-buffer-overflow on address
0x00010c37ce04 at pc 0x00010c3948c8 bp 0x7fff53882e40 sp 0x7fff538825f0
READ of size 3 at 0x00010c37ce04 thread T0
    #0 0x10c3948c7  (/opt/gcc/gcc4.10w/lib/libasan.2.dylib+0x138c7)
    #1 0x10d15f1c5  (/opt/gcc/gcc4.10w/lib/libgfortran.3.dylib+0xc01c5)
    #2 0x10c37cd9f 
(/Users/dominiq/Documents/Fortran/g95bench/win/f90/bug/a.out+0x100000d9f)

0x00010c37ce04 is located 60 bytes to the left of global variable '*LC4'
defined in '/opt/gcc/_clean/gcc/testsuite/gfortran.dg/io_real_boz_5.f90'
(0x10c37ce40) of size 1
0x00010c37ce04 is located 0 bytes to the right of global variable '*LC3'
defined in '/opt/gcc/_clean/gcc/testsuite/gfortran.dg/io_real_boz_5.f90'
(0x10c37ce00) of size 4
...

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[hp at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hp at gcc dot gnu.org

--- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Isn't this a dup of bug 64986?  Or bug 63205?

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-03
     Ever confirmed|0                           |1

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Isn't this a dup of bug 64986?  Or bug 63205?

I doubt it. This PR can be reproduced on x86_64-apple-darwin14 by compiling the
test with -std=f95 or -std=f2003 (not needed to reproduce pr64986).

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jvdelisle at gcc dot gnu.org

--- Comment #3 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
With the following patch, the fsanitize error goes away in the io_real_boz2
case. With -std=f2003 the case yields an expected runtime error.  It is the
formatting of this error where the -fsanitize does not like referencing a
pointer for some reason.

Index: format.c
===================================================================
--- format.c    (revision 221842)
+++ format.c    (working copy)
@@ -1117,9 +1117,9 @@
 void
 format_error (st_parameter_dt *dtp, const fnode *f, const char *message)
 {
-  int width, i, offset;
+  int width, i, offset, len;
 #define BUFLEN 300
-  char *p, buffer[BUFLEN];
+  char *p, *q, buffer[BUFLEN];
   format_data *fmt = dtp->u.p.fmt;

   if (f != NULL)
@@ -1132,9 +1132,12 @@
   else
     snprintf (buffer, BUFLEN, "%s\n", message);

+  /* Find the length of the format string.  */
+  for (q = p, len = 0; *q; len++, q++);
+
   /* Get the offset into the format string where the error occurred.  */
   offset = dtp->format_len - (fmt->reversion_ok ?
-                  (int) strlen(p) : fmt->format_string_len);
+                  len : fmt->format_string_len);

   width = dtp->format_len;

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #4 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
The patch eliminates the issue for all three of the BOZ cases. What I do not
understand is why passing the pointer p to strlen causes a problem.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #5 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Le 4 avr. 2015 à 00:25, jvdelisle at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> a écrit :
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089
> 
> --- Comment #4 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
> The patch eliminates the issue for all three of the BOZ cases. What I do not
> understand is why passing the pointer p to strlen causes a problem.
> 
> -- 
> You are receiving this mail because:
> You reported the bug.

PR61632 comment 16?
>From gcc-bugs-return-482695-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Apr 03 22:31:23 2015
Return-Path: <gcc-bugs-return-482695-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 8587 invoked by alias); 3 Apr 2015 22:31:22 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 8516 invoked by uid 48); 3 Apr 2015 22:31:19 -0000
From: "doko at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/65670] [5 Regression] missing libstdc++ symbols on powerpc64le-linux-gnu
Date: Fri, 03 Apr 2015 22:31:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: doko at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: cf_gcctarget
Message-ID: <bug-65670-4-d6AWpf5ysu@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-65670-4@http.gcc.gnu.org/bugzilla/>
References: <bug-65670-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-04/txt/msg00247.txt.bz2
Content-length: 548

https://gcc.gnu.org/bugzilla/show_bug.cgi?ide670

Matthias Klose <doko at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|powerpc64le-linux-gnu       |powerpc64le-linux-gnu,
                   |                            |arm-linux-gnueabi

--- Comment #1 from Matthias Klose <doko at gcc dot gnu.org> ---
seen on arm-linux-gnueabi too (but not arm-linux-gnueabihf). and it's missing a
baseline symbols file too.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |kcc at gcc dot gnu.org

--- Comment #6 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
I confirme that the patch in comment 3 "fixes" the PR. I have added a line

printf("len=%d, strlen(p)=%d\n", len, strlen(p));

after the 'for' loop and AFAICT len == strlen(p), however runing the code
compiled with -fsanitize=address still gives the error.

So I am wondering if this PR is not a sanitizer false positive when using
strlen.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Sounds like either libgfortran bug, or fortran FE bug.
What asan_finish_file sees for .LC3 is:
 <string_cst 0x7ffff169ce40
    type <array_type 0x7ffff16c7d20
        type <integer_type 0x7ffff15083f0 character(kind=1) asm_written public
unsigned string-flag QI
            size <integer_cst 0x7ffff1504cc0 constant 8>
            unit size <integer_cst 0x7ffff1504cd8 constant 1>
            align 8 symtab -244602288 alias set -1 canonical type
0x7ffff15083f0 precision 8 min <integer_cst 0x7ffff1504cf0 0> max <integer_cst
0x7ffff1504c90 255>
            pointer_to_this <pointer_type 0x7ffff1527150>>
        string-flag SI
        size <integer_cst 0x7ffff1504e10 constant 32>
        unit size <integer_cst 0x7ffff1504e28 constant 4>
        align 8 symtab 0 alias set -1 canonical type 0x7ffff16c7d20
        domain <integer_type 0x7ffff16c7c78 type <integer_type 0x7ffff1508690
integer(kind=4)>
            SI size <integer_cst 0x7ffff1504e10 32> unit size <integer_cst
0x7ffff1504e28 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffff16c7c78
precision 32 min <integer_cst 0x7ffff1504f78 1> max <integer_cst 0x7ffff16c55b8
4>>>
    constant asm_written "(z0)">
i.e. a 4 bytes long string literal, which is not NUL terminated.
If you compile without -fsanitize=address, you can see that (z0) is directly
followed by unrelated strings:
 0000 696f5f72 65616c5f 626f7a5f 352e6639  io_real_boz_5.f9
 0010 3000287a 30295800 00000000 00000000  0.(z0)X.........
 0020 02010000 9b010000 00000000 00000000  ................
 0030 01000000 01000000 00000000 00000000  ................
 0040 1f000000 0000803f                    .......?        
so calling strlen on this is obviously undefined behavior.  Doesn't the FE pass
format_len which tells you how long the string is?  It really doesn't seem to
be NUL terminated unless by accident.
Isn't:
       character(len=32) :: str1
       character(len=4) :: str2
       str2 = '(z0)'
       x = 1.0_16 + 2.0_16**(-105)
       write (str1,str2) 'X'
       end
equivalent to that (again, with no NUL termination)?

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #8 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
True, fortran strings are not generally NULL terminated.  However, for internal
representation between Frontend and library we try to do so for safety. 
Evidently missed one or don't have it in this case.  Still odd that my manual,
so to speak, 'for' loop is finding a NULL and counting the correct length.  I
will do some more digging, probably something "obvious". Just not seeing it
yet.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jerry DeLisle from comment #8)
> True, fortran strings are not generally NULL terminated.  However, for
> internal representation between Frontend and library we try to do so for
> safety.  Evidently missed one or don't have it in this case.  Still odd that
> my manual, so to speak, 'for' loop is finding a NULL and counting the
> correct length.  I will do some more digging, probably something "obvious".
> Just not seeing it yet.

Has libgfortran been compiled with -fsanitize=address, or just the testcase?
If the for loop is in the library that has not been instrumented, it is obvious
why it doesn't trigger, while if you call strlen there and it is emitted as a
library call, then it is intercepted by libasan and thus is instrumented.  And
the global variable (string literal) strlen is called on resides in the
executable and is instrumented there.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #10 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Has libgfortran been compiled with -fsanitize=address, or just the testcase?

Only the testcase for me.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #11 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #9)
> (In reply to Jerry DeLisle from comment #8)
> > True, fortran strings are not generally NULL terminated.  However, for
> > internal representation between Frontend and library we try to do so for
> > safety.  Evidently missed one or don't have it in this case.  Still odd that
> > my manual, so to speak, 'for' loop is finding a NULL and counting the
> > correct length.  I will do some more digging, probably something "obvious".
> > Just not seeing it yet.
> 
> Has libgfortran been compiled with -fsanitize=address, or just the testcase?
> If the for loop is in the library that has not been instrumented, it is
> obvious why it doesn't trigger, while if you call strlen there and it is
> emitted as a library call, then it is intercepted by libasan and thus is
> instrumented.  And the global variable (string literal) strlen is called on
> resides in the executable and is instrumented there.

Thanks for this explanation, it makes perfect sense.  Also, I have located the
problem. When we do internal string I/O (using a string instead of an actual
file) we bypass format string caching and skip creation of the null terminated
version of the format string. I am working on a patch.

  format_cache_ok = !is_internal_unit (dtp);

--- snip ---

  if (format_cache_ok)
    {
      char *fmt_string = xmalloc (dtp->format_len + 1);
      memcpy (fmt_string, dtp->format, dtp->format_len);
      dtp->format = fmt_string;
      dtp->format[dtp->format_len] = '\0';
    }

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #12 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Created attachment 35302
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35302&action=edit
Preliminary patch - needs testing

This patch resolves the -fsanitize=address issue and also does some memory
cleanup on formatted I/O errors.  I have regression tested and all is OK, but
have not tried all the variations with -m32 or -fsanitize=address on the
testsuite.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #13 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> This patch resolves the -fsanitize=address issue and also does some memory
> cleanup on formatted I/O errors.  I have regression tested and all is OK, but
> have not tried all the variations with -m32 or -fsanitize=address on the
> testsuite.

The patch looks good (even for my limited understanding of the code). I have
run the fortran test suite with

check-gfortran
RUNTESTFLAGS="--target_board=unix'{-m32,-m64,-m32/-fsanitize=address,-m64/-fsanitize=address}'"

The io_real_boz* problem is fixed without new regression. I have also
"sanitized" the NIST test suite which runs without error for both -m32 and
-m64.

Thanks for working on this issue.

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #14 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Author: jvdelisle
Date: Wed Apr 15 01:27:03 2015
New Revision: 222111

URL: https://gcc.gnu.org/viewcvs?rev=222111&root=gcc&view=rev
Log:
2015-04-14 Jerry DeLisle  <jvdelisle@gcc.gnu.org>

    PR libgfortran/65089
    * io/format.h (free_format): New function to free memory
    allocated for building format error messages.
    * io/format.c (format_error): Add checks before freeing memory
    to avoid potential segfaults and free formatting data when
    needed on error conditions. Always allocate and NULL terminate
    the string.
    * io/transfer.c (st_read_done, st_write_done): Use new
    free_format function to clean up memory allocations when done.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/format.c
    trunk/libgfortran/io/format.h
    trunk/libgfortran/io/transfer.c

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #15 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Fixed on trunk.  It would be good to backport to 5.2 and probably 4.9.x

[Bug fortran/65089] FAIL: gfortran.dg/io_real_boz(2|_[45]).f90 when tested with -fsanitize=address

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65089

--- Comment #16 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Fixed on trunk.  It would be good to backport to 5.2 and probably 4.9.x

IMO 4.9 is almost EOL and is not worth the trouble. What is the plan for 5.3?