[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-03-15 Thread fxcoudert at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

--- Comment #8 from Francois-Xavier Coudert  
2011-03-15 15:36:16 UTC ---
(In reply to comment #7)
> - Yes, Fortran itself does not have unsigned integers, but the string length
> type is invisible to Fortran programs. The LEN intrinsic does return the 
> string
> length typecast to a signed integer kind depending on the optional KIND
> argument, or to a default kind integer.

My point is just that, if you're merely changing the size of the variable, you
are less likely to unearth new bugs than if you change both that and
signedness.

Plus, as you say, the difference between SIZE_MAX and SSIZE_MAX is a corner
case, and maybe a huge size_t value isn't usable in common code.

Finally, you'd loose some compatibility with what used to be done

> Also, consider that at the asm level, typecasting from an unsigned to a
> signed value of the same size is a no-op, so there is no efficiency loss.

And casts from signed to unsigned, with checks for positivity, when string
lengths are passed as function arguments.


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-03-15 Thread jb at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

--- Comment #7 from Janne Blomqvist  2011-03-15 15:26:05 
UTC ---
(In reply to comment #3)
> (In reply to comment #2)
> > If not before, perhaps something to fix when/if we change to use size_t for
> > string lengths, see http://gcc.gnu.org/wiki/LibgfortranAbiCleanup
> 
> Just as a remark: you're not going to use size_t, but the signed ssize_t,
> right?

I think the idea was to use the unsigned size_t. 

- size_t is the natural type for representing the size of a memory object in
bytes. There is no need for negative values, and SSIZE_MAX is smaller than the
largest possible object (=SIZE_MAX) (an obscure corner case, but still).

- size_t is what the C string.h functions use, which is used in the
implementation of various string handling functions in gfortran (in libgfortran
and code generated directly by the frontend). Using the same type might also
help mixed C/Fortran programs.

- size_t is what Intel Fortran uses

- Yes, Fortran itself does not have unsigned integers, but the string length
type is invisible to Fortran programs. The LEN intrinsic does return the string
length typecast to a signed integer kind depending on the optional KIND
argument, or to a default kind integer. Depending on whether the target
supports an integer kind > sizeof(size_t) it might be possible to represent all
possible string lengths, or then not. But IMHO this does not change the fact
that the correct type for the (Fortran invisible) string length is the unsigned
size_t. Also, consider that at the asm level, typecasting from an unsigned to a
signed value of the same size is a no-op, so there is no efficiency loss.


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-03-12 Thread fxcoudert at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

Francois-Xavier Coudert  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED
   Target Milestone|--- |4.6.0

--- Comment #6 from Francois-Xavier Coudert  
2011-03-12 10:35:59 UTC ---
Fixed on trunk. CTIME is a little used intrinsic, and we don't actually have
any example of real-world failure, so I don't plan on backporting the patch.


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-03-12 Thread fxcoudert at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

--- Comment #5 from Francois-Xavier Coudert  
2011-03-12 10:28:04 UTC ---
Author: fxcoudert
Date: Sat Mar 12 10:28:01 2011
New Revision: 170898

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=170898
Log:
PR fortran/47552
* trans-intrinsic.c (gfc_conv_intrinsic_ctime): Fix type of
the string length variable.

Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/trans-intrinsic.c


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-03-12 Thread burnus at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

--- Comment #4 from Tobias Burnus  2011-03-12 
08:03:37 UTC ---
(In reply to comment #3)
> (In reply to comment #2)
> Which is wrong. This is fixed by the patch:

The patch is OK with a changelog and CCing the patch to gcc-patches@/fortran@


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-03-11 Thread fxcoudert at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

Francois-Xavier Coudert  changed:

   What|Removed |Added

   Keywords||patch
 CC||fxcoudert at gcc dot
   ||gnu.org

--- Comment #3 from Francois-Xavier Coudert  
2011-03-11 21:32:33 UTC ---
(In reply to comment #2)
> This seems to be because the libgfortran implementation uses gfc_charlen_type
> for the length of the string,

Which is the correct thing the do!

> but the frontend passes the address of an
> integer(8) variable

Which is wrong. This is fixed by the patch:

Index: trans-intrinsic.c
===
--- trans-intrinsic.c(revision 170612)
+++ trans-intrinsic.c(working copy)
@@ -1501,7 +1501,7 @@
   args = XALLOCAVEC (tree, num_args);

   var = gfc_create_var (pchar_type_node, "pstr");
-  len = gfc_create_var (gfc_get_int_type (8), "len");
+  len = gfc_create_var (gfc_charlen_type_node, "len");

   gfc_conv_intrinsic_function_args (se, expr, &args[2], num_args - 2);
   args[0] = gfc_build_addr_expr (NULL_TREE, var);


> If not before, perhaps something to fix when/if we change to use size_t for
> string lengths, see http://gcc.gnu.org/wiki/LibgfortranAbiCleanup

Just as a remark: you're not going to use size_t, but the signed ssize_t,
right?


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-02-22 Thread jb at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

--- Comment #2 from Janne Blomqvist  2011-02-22 15:34:57 
UTC ---
This seems to be because the libgfortran implementation uses gfc_charlen_type
for the length of the string, but the frontend passes the address of an
integer(8) variable. As a quick and dirty test, changing the libgfortran
implementation to use "long" removes the valgrind errors on x86_64. Though the
correct fix is to change the frontend to create a variable of the correct type.

If not before, perhaps something to fix when/if we change to use size_t for
string lengths, see http://gcc.gnu.org/wiki/LibgfortranAbiCleanup


[Bug fortran/47552] CTIME: Valgrind warning "depends on uninitialised value"

2011-01-31 Thread jb at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47552

Janne Blomqvist  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2011.01.31 12:57:07
 Ever Confirmed|0   |1

--- Comment #1 from Janne Blomqvist  2011-01-31 12:57:07 
UTC ---
FWIW, I see the same problem with gfortran 4.4 @work, so probably not something
due to my recent ctime_r() patch.