FYI: fix PR 11345

FYI: fix PR 11345

[Tom Tromey]

I'm checking this in.

This fixes PR 11345.  The bug is that "%%" doesn't work if it appears
after the last format in a printf:

(gdb) printf "%%\n"
%%

The fix is to use printf, not puts, to print the rest of the format
string.  This is always ok because we know that there are no format
specifiers other than "%%" in the string.

The patch comes from the PR; it is small enough not to need papers.  I
wrote the test case and updated the comment in printf_command.

Built and regression tested on x86-64 (compile farm).

Tom

2010-03-03  Dainis Jonitis  <jonitis@gmail.com>

	PR gdb/11345:
	* printcmd.c (printf_command): Print end of format string using
	printf_filtered.

2010-03-03  Tom Tromey  <tromey@redhat.com>

	PR gdb/11345:
	* gdb.base/printcmds.exp (test_printf): Add test.

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index c8cb35c..5e5ef8e 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2642,8 +2642,11 @@ printf_command (char *arg, int from_tty)
 	/* Skip to the next substring.  */
 	current_substring += strlen (current_substring) + 1;
       }
-    /* Print the portion of the format string after the last argument.  */
-    puts_filtered (last_arg);
+    /* Print the portion of the format string after the last argument.
+       Note that this will not include any ordinary %-specs, but it
+       might include "%%".  That is why we use printf_filtered and not
+       puts_filtered here.  */
+    printf_filtered (last_arg);
   }
   do_cleanups (old_cleanups);
 }
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 5598bde..9c2cd6b 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -686,6 +686,10 @@ proc test_printf {} {
 
     # Regression test for C lexer bug.
     gdb_test "printf \"%c\\n\", \"x\"\[1,0\]" "x"
+
+    # Regression test for "%% at end of format string.
+    # See http://sourceware.org/bugzilla/show_bug.cgi?id=11345
+    gdb_test "printf \"%%%d%%\\n\", 5" "%5%"
 }
 
 #Test printing DFP values with printf

Re: FYI: fix PR 11345

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> I'm checking this in.
Tom> This fixes PR 11345.  The bug is that "%%" doesn't work if it appears
Tom> after the last format in a printf:

While closing the PR I realized that this is small and safe enough for
7.1.  So, I am also committing it to the release branch.

Tom

Re: FYI: fix PR 11345

[Pedro Alves]

On Wednesday 03 March 2010 18:04:44, Tom Tromey wrote:
> -    /* Print the portion of the format string after the last argument.  */
> -    puts_filtered (last_arg);
> +    /* Print the portion of the format string after the last argument.
> +       Note that this will not include any ordinary %-specs, but it
> +       might include "%%".  That is why we use printf_filtered and not
> +       puts_filtered here.  */
> +    printf_filtered (last_arg);

This causes:

make[1]: Leaving directory `/home/pedro/gdb/baseline/build/gdb'
gcc -g3 -O0   -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber  -I../../src/gdb/gnulib -Ignulib  -DMI_OUT=1 -DTUI=1   `echo " -Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts " | sed "s/ -Wformat-nonliteral / /g"` \
                -Werror -c -o printcmd.o -MT printcmd.o -MMD -MP -MF .deps/printcmd.Tpo ../../src/gdb/printcmd.c
cc1: warnings being treated as errors
../../src/gdb/printcmd.c: In function 'printf_command':
../../src/gdb/printcmd.c:2649: error: format not a string literal and no format arguments
make: *** [printcmd.o] Error 1

Guess the original bug needs fixing in a different way.  :-/

-- 
Pedro Alves

Re: FYI: fix PR 11345

[Doug Evans]

On Wed, Mar 3, 2010 at 4:47 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 03 March 2010 18:04:44, Tom Tromey wrote:
>> -    /* Print the portion of the format string after the last argument.  */
>> -    puts_filtered (last_arg);
>> +    /* Print the portion of the format string after the last argument.
>> +       Note that this will not include any ordinary %-specs, but it
>> +       might include "%%".  That is why we use printf_filtered and not
>> +       puts_filtered here.  */
>> +    printf_filtered (last_arg);
>
> This causes:
>
> make[1]: Leaving directory `/home/pedro/gdb/baseline/build/gdb'
> gcc -g3 -O0   -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber  -I../../src/gdb/gnulib -Ignulib  -DMI_OUT=1 -DTUI=1   `echo " -Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts " | sed "s/ -Wformat-nonliteral / /g"` \
>                -Werror -c -o printcmd.o -MT printcmd.o -MMD -MP -MF .deps/printcmd.Tpo ../../src/gdb/printcmd.c
> cc1: warnings being treated as errors
> ../../src/gdb/printcmd.c: In function 'printf_command':
> ../../src/gdb/printcmd.c:2649: error: format not a string literal and no format arguments
> make: *** [printcmd.o] Error 1
>
> Guess the original bug needs fixing in a different way.  :-/

I'd be ok with just adding a dummy format arg (and a comment
explaining why it's there of course).

Re: FYI: fix PR 11345

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> ../../src/gdb/printcmd.c:2649: error: format not a string literal and no format arguments
Pedro> make: *** [printcmd.o] Error 1

Sorry about that.  I will fix it tomorrow.

Tom

Re: FYI: fix PR 11345

[Pedro Alves]

On Thursday 04 March 2010 02:35:44, Doug Evans wrote:
> I'd be ok with just adding a dummy format arg (and a comment
> explaining why it's there of course).

something like printf("%s", string) instead of printf(string)?
That doesn't work in this case, because printf("%s", "%%")
isn't the same as printf("%%"), so you're back to the
original bug.

-- 
Pedro Alves

Re: FYI: fix PR 11345

[Doug Evans]

On Wed, Mar 3, 2010 at 6:43 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 04 March 2010 02:35:44, Doug Evans wrote:
>> I'd be ok with just adding a dummy format arg (and a comment
>> explaining why it's there of course).
>
> something like printf("%s", string) instead of printf(string)?
> That doesn't work in this case, because printf("%s", "%%")
> isn't the same as printf("%%"), so you're back to the
> original bug.

printf (string, 0) is what I had in mind.
obviously printf ("%s", "%%") won't fix anything.

... but having tried it I'm seeing "... argument types not checked".
Blech.

Re: FYI: fix PR 11345

[Pedro Alves]

On Thursday 04 March 2010 02:56:39, Doug Evans wrote:
> printf (string, 0) is what I had in mind.
> obviously printf ("%s", "%%") won't fix anything.

Ah.  That wouldn't work because you'd still get a warning
because `string' isn't a string literal.

> 
> ... but having tried it I'm seeing "... argument types not checked".

Right, something like that.  :-)

-- 
Pedro Alves

Re: FYI: fix PR 11345

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> gcc -g3 -O0   -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber  -I../../src/gdb/gnulib -Ignulib  -DMI_OUT=1 -DTUI=1   `echo " -Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts " | sed "s/ -Wformat-nonliteral / /g"` \
Pedro>                 -Werror -c -o printcmd.o -MT printcmd.o -MMD -MP -MF .deps/printcmd.Tpo ../../src/gdb/printcmd.c
Pedro> cc1: warnings being treated as errors
Pedro> ../../src/gdb/printcmd.c: In function 'printf_command':
Pedro> ../../src/gdb/printcmd.c:2649: error: format not a string literal and no format arguments

BTW, what version of gcc is this?  I couldn't reproduce it.

My gcc (F11 system) doesn't warn, and svn trunk gcc has:

	  if (params == 0 && warn_format_security)
	    warning (OPT_Wformat_security,
		     "format not a string literal and no format arguments");
	  else if (params == 0 && warn_format_nonliteral)
	    warning (OPT_Wformat_nonliteral,
		     "format not a string literal and no format arguments");

We explicitly don't pass -Wformat-nonliteral for this file, and I don't
see -Wformat-security in there...

Could you try Doug's suggestion?

    printf_filtered (last_arg, 0);

If that doesn't work I will write a loop using strstr.

Tom

Re: FYI: fix PR 11345

[Pedro Alves]

On Thursday 04 March 2010 17:00:37, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> gcc -g3 -O0   -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber  -I../../src/gdb/gnulib -Ignulib  -DMI_OUT=1 -DTUI=1   `echo " -Wall -Wdeclaration-after-statement -Wpointer-arith -Wformat-nonliteral -Wno-pointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts " | sed "s/ -Wformat-nonliteral / /g"` \
> Pedro>                 -Werror -c -o printcmd.o -MT printcmd.o -MMD -MP -MF .deps/printcmd.Tpo ../../src/gdb/printcmd.c
> Pedro> cc1: warnings being treated as errors
> Pedro> ../../src/gdb/printcmd.c: In function 'printf_command':
> Pedro> ../../src/gdb/printcmd.c:2649: error: format not a string literal and no format arguments
> 
> BTW, what version of gcc is this?  I couldn't reproduce it.

kubuntu 9.10
Target: x86_64-linux-gnu
gcc version 4.4.1 (Ubuntu 4.4.1-4ubuntu9)

> 
> My gcc (F11 system) doesn't warn, and svn trunk gcc has:
> 
> 	  if (params == 0 && warn_format_security)
> 	    warning (OPT_Wformat_security,
> 		     "format not a string literal and no format arguments");
> 	  else if (params == 0 && warn_format_nonliteral)
> 	    warning (OPT_Wformat_nonliteral,
> 		     "format not a string literal and no format arguments");
> 

Ah.  It's probably an ubuntu local gcc change.

> We explicitly don't pass -Wformat-nonliteral for this file, and I don't
> see -Wformat-security in there...

Eh, I didn't realize that -Wformat-nonliteral should be of
for this file.  I now see the sneaky sed above.  That changes
perpective.

> 
> Could you try Doug's suggestion?
> 
>     printf_filtered (last_arg, 0);
> 
> If that doesn't work I will write a loop using strstr.

It works for me, indeed.  But didn't Doug say it didn't
for him?

On Thursday 04 March 2010 02:56:39, Doug Evans wrote:
> ... but having tried it I'm seeing "... argument types not checked".
> Blech.

-- 
Pedro Alves

Re: FYI: fix PR 11345

[Doug Evans]

On Thu, Mar 4, 2010 at 9:20 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> Eh, I didn't realize that -Wformat-nonliteral should be of
> for this file.  I now see the sneaky sed above.  That changes
> perpective.
>
>>
>> Could you try Doug's suggestion?
>>
>>     printf_filtered (last_arg, 0);
>>
>> If that doesn't work I will write a loop using strstr.
>
> It works for me, indeed.  But didn't Doug say it didn't
> for him?
>
> On Thursday 04 March 2010 02:56:39, Doug Evans wrote:
>> ... but having tried it I'm seeing "... argument types not checked".
>> Blech.

I was explicitly turning on the warning.
If we adopt the rule that this file can't be compiled with that
warning, that's different.

Re: FYI: fix PR 11345

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> I was explicitly turning on the warning.

On irc, Doug clarified that this was -Wformat=2.

Doug> If we adopt the rule that this file can't be compiled with that
Doug> warning, that's different.

Yeah, this file can't be compiled with -Wformat-nonliteral or anything
that implies it, like -Wformat=2.

I am checking in the appended to the trunk and the 7.1 branch.
Pedro verified that it works for him, and I also verified it locally
by passing -Wformat-security explicitly.

Tom

2010-03-04  Tom Tromey  <tromey@redhat.com>

	* printcmd.c (printf_command): Pass dummy argument to
	printf_filtered.

Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.171
diff -u -r1.171 printcmd.c
--- printcmd.c	3 Mar 2010 18:05:04 -0000	1.171
+++ printcmd.c	4 Mar 2010 18:16:15 -0000
@@ -2645,8 +2645,10 @@
     /* Print the portion of the format string after the last argument.
        Note that this will not include any ordinary %-specs, but it
        might include "%%".  That is why we use printf_filtered and not
-       puts_filtered here.  */
-    printf_filtered (last_arg);
+       puts_filtered here.  Also, we pass a dummy argument because
+       some platforms have modified GCC to include -Wformat-security
+       by default, which will warn here if there is no argument.  */
+    printf_filtered (last_arg, 0);
   }
   do_cleanups (old_cleanups);
 }