Re: RFA: Sanitize deprecation messages (PR 84195)

[Martin Sebor <msebor at gmail dot com>]

On 02/05/2018 10:07 AM, Nick Clifton wrote:
> Hi Martin, Hi Dodji, Hi David,
>
>   PR 84195 makes the point that deprecation messages do not follow the
>   formatting guidelines set out by the -fmessage-length option, and that
>   they could even contain unwanted control characters:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84195
>
>   Below is my attempt at a patch to fix this.  It is not very clever,
>   just replacing the control characters with spaces, and doing it each
>   time that a deprecation warning is displayed.  But I figured that
>   such warnings are going to be rare and do not need to be efficiently
>   handled.  So I choose to keep it simple. :-)

Thanks for working on this!  I agree your patch is an improvement.

My only suggestions are to consider how control characters and
excessively messages are handled in other contexts and adopt
the same approach here.  In the tests I did, control characters
were mostly escaped (e.g., as \n or as \x0a) rather than replaced
with spaces.  I haven't thought too hard about how excessively
long messages should be handled, or about the interactions with
-fmessage-length= (i.e., if messages longer than the limit should
be broken up.)

Martin

>   No regressions with an x86_64-pc-linux-gnu toolchain.
>
>   OK to apply ?
>
> Cheers
>   Nick
>
> gcc/ChangeLog
> 2018-02-05  Nick Clifton  <nickc@redhat.com>
>
> 	PR 84195
> 	* tree.c (warn_deprecated_use): Sanitize deprecation messages.
>
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c	(revision 257389)
> +++ gcc/tree.c	(working copy)
> @@ -12436,6 +12436,39 @@
>    else
>      msg = NULL;
>
> +  char * new_msg = NULL;
> +  if (msg)
> +    {
> +      unsigned int i; /* FIXME: Do we need to check to see if msg is longer
> +			 than 2^32 ?  */
> +
> +      /* PR 84195: Sanitize the message:
> +	
> +	 If -fmessage-length is set to 0 then replace newline characters with
> +	 spaces.  Replace other non-printing ASCII characters with spaces too.
> +
> +	 We do this check here, rather than where the deprecated attribute is
> +	 recorded because the setting of -fmessage-length can be changed
> +	 between those two locations.  */
> +      for (i = 0; i < strlen (msg); i++)
> +	{
> +	  char c = msg[i];
> +	
> +	  if (! ISCNTRL (c))
> +	    continue;
> +
> +	  if (c != '\n' || ! pp_is_wrapping_line (global_dc->printer))
> +	    {
> +	      if (new_msg == NULL)
> +		new_msg = xstrdup (msg);
> +	      new_msg [i] = ' ';
> +	    }
> +	}
> +
> +      if (new_msg)
> +	msg = new_msg;
> +    }
> +
>    bool w;
>    if (DECL_P (node))
>      {
> @@ -12505,6 +12538,8 @@
>  	    }
>  	}
>      }
> +
> +  free (new_msg);
>  }
>
>  /* Return true if REF has a COMPONENT_REF with a bit-field field declaration