Patchwork [libgfortran] PR43899 Wrong unused-variable warning with NAMELISTs

login
register
mail settings
Submitter Jerry DeLisle
Date Nov. 3, 2010, 5:39 a.m.
Message ID <4CD0F58C.5040806@frontier.com>
Download mbox | patch
Permalink /patch/69944/
State New
Headers show

Comments

Jerry DeLisle - Nov. 3, 2010, 5:39 a.m.
This PR is about two issues.  The first is a bogus warning about the variable 
being unused when -Wall is being used.  The second issue has to do with warning 
a user when a namelist read of a character object is truncated. Only the second 
issue is addressed here.

As Tobias points out, the test case is legal.  The attached patch adds a new 
runtime warning function to libgfortran and uses it in list_read.c if the test 
case has been compiled with -fbounds-check.  The warning is issued to stderr, 
and execution proceeds normally.

This will give users an opportunity to debug code that may unintentionally have 
an issue.

The runtime warning will also be useful down the road.

Regression tested on x86-64-linux-gnu.  OK for trunk.

Regards,

Jerry

2010-11-02  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/43899
	* runtime/error.c (generate_error): New function to generate a run
	time warning message. Fix some whitespace.
	* gfortran.map: Add symbol for new function.
	* libgfortran.h: Add prototype for new function.
	* io/list_read.c (nml_read_obj): Use new function to warn when a
	character namelist object is truncated.  Only warn if compiled
	with -fbounds-check.
! { dg-do run }
! { dg-options "-fbounds-check" }

  character(35) :: nml_contents = "&NMLIST NML_STRING='123456789' /"
  character(4)  :: nml_string
  namelist /nmlist/ nml_string
  nml_string = "abcd"
  read(nml_contents,nml=nmlist)
end program 
! { dg-output "Fortran runtime warning: Namelist object 'nml_string' truncated on read." }
Janne Blomqvist - Nov. 3, 2010, 10:44 a.m.
On Wed, Nov 3, 2010 at 07:39, Jerry DeLisle <jvdelisle@frontier.com> wrote:
> 2010-11-02  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
>
>        PR libgfortran/43899
>        * runtime/error.c (generate_error): New function to generate a run
>        time warning message. Fix some whitespace.

generate_warning, not generate_error. The comment above the function
in the patch also suffers from the same problem. Also, the iexport()
line should be removed (see below).

>        * gfortran.map: Add symbol for new function.

Please remove this. We don't need to export generate_warning, as it's
never called from outside the library.

If at some point in the future we need to do that, it can be exported then.

>        * libgfortran.h: Add prototype for new function.

Due to the above, should be marked with internal_proto() instead of
iexport_proto().


Otherwise the patch looks good. Ok for trunk with the above changes.
Jerry DeLisle - Nov. 3, 2010, 2:59 p.m.
On 11/03/2010 03:44 AM, Janne Blomqvist wrote:
> On Wed, Nov 3, 2010 at 07:39, Jerry DeLisle<jvdelisle@frontier.com>  wrote:
>> 2010-11-02  Jerry DeLisle<jvdelisle@gcc.gnu.org>
>>
>>         PR libgfortran/43899
>>         * runtime/error.c (generate_error): New function to generate a run
>>         time warning message. Fix some whitespace.

OK, Copy/Paste fat fingers, thanks.
>
> generate_warning, not generate_error. The comment above the function
> in the patch also suffers from the same problem. Also, the iexport()
> line should be removed (see below).
>
>>         * gfortran.map: Add symbol for new function.
>
> Please remove this. We don't need to export generate_warning, as it's
> never called from outside the library.

OK

>
> If at some point in the future we need to do that, it can be exported then.
>
>>         * libgfortran.h: Add prototype for new function.
>
> Due to the above, should be marked with internal_proto() instead of
> iexport_proto().
>

OK and thanks for review.
>
> Otherwise the patch looks good. Ok for trunk with the above changes.
>

Will fix before commit.

Jerry

Patch

Index: runtime/error.c
===================================================================
--- runtime/error.c	(revision 166182)
+++ runtime/error.c	(working copy)
@@ -443,6 +443,22 @@  generate_error (st_parameter_common *cmp, int fami
 }
 iexport(generate_error);
 
+
+/* generate_error()-- Similar to generate_error but just give a warning.  */
+
+void
+generate_warning (st_parameter_common *cmp, const char *message)
+{
+
+  if (message == NULL)
+    message = " ";
+
+  show_locus (cmp);
+  st_printf ("Fortran runtime warning: %s\n", message);
+}
+iexport(generate_warning);
+
+
 /* Whether, for a feature included in a given standard set (GFC_STD_*),
    we should issue an error or a warning, or be quiet.  */
 
@@ -462,7 +478,6 @@  notification_std (int std)
 }
 
 
-
 /* Possibly issue a warning/error about use of a nonstandard (or deleted)
    feature.  An error/warning will be issued if the currently selected
    standard does not contain the requested bits.  */
Index: gfortran.map
===================================================================
--- gfortran.map	(revision 166182)
+++ gfortran.map	(working copy)
@@ -1116,6 +1116,7 @@  GFORTRAN_1.4 {
     _gfortran_bessel_yn_r10;
     _gfortran_bessel_yn_r16;
     _gfortran_error_stop_numeric;
+    _gfortran_generate_warning;
     _gfortran_iall_i1;
     _gfortran_iall_i2;
     _gfortran_iall_i4;
Index: libgfortran.h
===================================================================
--- libgfortran.h	(revision 166182)
+++ libgfortran.h	(working copy)
@@ -733,6 +733,9 @@  internal_proto(translate_error);
 extern void generate_error (st_parameter_common *, int, const char *);
 iexport_proto(generate_error);
 
+extern void generate_warning (st_parameter_common *, const char *);
+iexport_proto(generate_warning);
+
 extern try notify_std (st_parameter_common *, int, const char *);
 internal_proto(notify_std);
 
Index: io/list_read.c
===================================================================
--- io/list_read.c	(revision 166182)
+++ io/list_read.c	(working copy)
@@ -2586,7 +2586,19 @@  nml_read_obj (st_parameter_dt *dtp, namelist_info
 	  break;
 
 	case BT_CHARACTER:
-	  m = (dlen < dtp->u.p.saved_used) ? dlen : dtp->u.p.saved_used;
+	  if (dlen < dtp->u.p.saved_used)
+	    {
+	      if (compile_options.bounds_check)
+		{
+		  snprintf (nml_err_msg, nml_err_msg_size,
+			    "Namelist object '%s' truncated on read.",
+			    nl->var_name);
+		  generate_warning (&dtp->common, nml_err_msg);
+		}
+	      m = dlen;
+	    }
+	  else
+	    m = dtp->u.p.saved_used;
 	  pdata = (void*)( pdata + clow - 1 );
 	  memcpy (pdata, dtp->u.p.saved_string, m);
 	  if (m < dlen)