[v3,5/9] compile: Use -Wall, not -w

  Hi,

for a reason unknown to me GDB was using -w instead of -Wall for 'compile code'.
The problem is later patch for 'compile printf' really needs some warnings to
be able to catch for example missing format string parameters:
	(gdb) compile printf "%d\n"
GCC does not seem to be able to cancel -w (there is nothing like -no-w).

Besides that I think even 'compile code' can benefit from -Wall.

That #ifndef hack in print_one_macro() is not nice but while GCC does not warn
for redefinitions like
	#define MACRO val
	#define MACRO val
together with the GCC build-in macros I haven't found any other way how to
prevent the macro-redefinition warnings (when -w is no longer in effect).

That new testsuite XFAIL is there as if one changes the struct definition to be
compliant with cv-qualifiers (to prevent the warnings):
struct struct_type {
-  struct struct_type *selffield;
+  volatile struct struct_type *selffield;
only then GCC/GDB will hit the crash, described in that GDB PR 18202.


Thanks,
Jan


gdb/ChangeLog
2015-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* compile/compile-c-support.c (print_one_macro): Use #ifndef.
	(generate_register_struct): Use __gdb_uintptr for TYPE_CODE_PTR.
	(c_compute_program): Call generate_register_struct after typedefs.
	* compile/compile-loc2c.c (push, pushf_register_address)
	(pushf_register): Cast to GCC_UINTPTR.
	(do_compile_dwarf_expr_to_c): Use unused attribute.  Add space after
	type.  Use GCC_UINTPTR instead of void *.  Remove excessive cast.
	(compile_dwarf_expr_to_c): Use GCC_UINTPTR instead of void *.
	* compile/compile.c (_initialize_compile): Enable warnings for
	COMPILE_ARGS.

gdb/testsuite/ChangeLog
2015-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.compile/compile-ops.exp: Cast param to void.
	* gdb.compile/compile.exp: Complete type for _gdb_expr.
	(compile code struct_object.selffield = &struct_object): Add xfail.
---
 0 files changed
  

On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> Hi,
> 
> for a reason unknown to me GDB was using -w instead of -Wall for 'compile code'.
> The problem is later patch for 'compile printf' really needs some warnings to
> be able to catch for example missing format string parameters:
> 	(gdb) compile printf "%d\n"
> GCC does not seem to be able to cancel -w (there is nothing like -no-w).
> 
> Besides that I think even 'compile code' can benefit from -Wall.
> 
> That #ifndef hack in print_one_macro() is not nice but while GCC does not warn
> for redefinitions like
> 	#define MACRO val
> 	#define MACRO val
> together with the GCC build-in macros I haven't found any other way how to
> prevent the macro-redefinition warnings (when -w is no longer in effect).

I think GCC also knows how to suppress such warnings if the redefinitions
are in system includes.  So I guess GCC already has the smarts
to suppress those. '#pragma GCC system_header' might be close, though it may
be ignored if not done on a header.

Note we have #pragma GCC user_expression, which is a pragma handled by
the GCC plugin:

 static void
 plugin_init_extra_pragmas (void *, void *)
 {
   c_register_pragma ("GCC", "user_expression", plugin_pragma_user_expression);
 }

So if we need to, we can easily add another gdb-specific pragma that
enables whatever mode in gcc we need, and wrap the macros with that.

OTOH, if it's the inferior's version of the macro that is always wanted,
then #ifndef should be fine.  If it's gdb's version that is wanted though,
then that could be handled by an #undef before the #define.

But then again, I'm not exactly sure on what you mean by build-in
macros here.  Can you give an example?

>  
> -gdb_test_no_output "compile code struct_object.selffield = &struct_object"
> +set test "compile code struct_object.selffield = &struct_object"
> +gdb_test_multiple $test $test {
> +    -re "gdb command line:1:25: warning: assignment discards 'volatile' qualifier from pointer target type \\\[-Wdiscarded-qualifiers\\\]\r\n$gdb_prompt $" {
> +	xfail "$test (PR compile/18202)"
> +    }
> +}

Please leave a PASS path in place.  I think this would work:

gdb_test_multiple $test $test {
    -re "^$test\r\n$gdb_prompt $ $" {
	pass "$test"
    }
    -re "gdb command line:1:25: warning: assignment discards 'volatile' qualifier from pointer target type \\\[-Wdiscarded-qualifiers\\\]\r\n$gdb_prompt $" {
	xfail "$test (PR compile/18202)"
    }
}

Other than resolving/clarifying the #ifdef issue, this looks
good to me.

Thanks,
Pedro Alves
  

On Wed, 29 Apr 2015 17:47:08 +0200, Pedro Alves wrote:
> On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> I think GCC also knows how to suppress such warnings if the redefinitions
> are in system includes.  So I guess GCC already has the smarts
> to suppress those. '#pragma GCC system_header' might be close, though it may
> be ignored if not done on a header.

Another problem is that (currently) it would not be possible to turn it off in
the same file.  Therefore the user expression without not get any warnings
which would nullify the goal of this patch.


> OTOH, if it's the inferior's version of the macro that is always wanted,
> then #ifndef should be fine.  If it's gdb's version that is wanted though,
> then that could be handled by an #undef before the #define.

Only some GCC's internal macros are affected and I guess it is better to leave
them alone, therefore #ifndef is better (contrary to forced re-#define).


> But then again, I'm not exactly sure on what you mean by build-in
> macros here.  Can you give an example?

Without that #ifndef/#endif one gets:
	compile -r -- void _gdb_expr(){int i = 5;}^M
	/tmp/gdbobj-xpU1yB/out4.c:4:0: warning: "__FILE__" redefined [-Wbuiltin-macro-redefined]^M
	/tmp/gdbobj-xpU1yB/out4.c:5:0: warning: "__LINE__" redefined^M
	/tmp/gdbobj-xpU1yB/out4.c:6:0: warning: "__STDC_IEC_559_COMPLEX__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:46:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-xpU1yB/out4.c:7:0: warning: "__STDC_IEC_559__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:38:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-xpU1yB/out4.c:8:0: warning: "__STDC_ISO_10646__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:54:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-xpU1yB/out4.c:9:0: warning: "__STDC_NO_THREADS__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:57:0: note: this is the location of the previous definition^M
	(gdb) FAIL: gdb.compile/compile.exp: Test delimiter with -r

Without that #ifndef/#endif and with -Wno-builtin-macro-redefined one
expectedly gets mostly the same:
	compile -r -- void _gdb_expr(){int i = 5;}^M
	/tmp/gdbobj-sJlZmG/out4.c:5:0: warning: "__LINE__" redefined^M
	/tmp/gdbobj-sJlZmG/out4.c:6:0: warning: "__STDC_IEC_559_COMPLEX__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:46:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-sJlZmG/out4.c:7:0: warning: "__STDC_IEC_559__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:38:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-sJlZmG/out4.c:8:0: warning: "__STDC_ISO_10646__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:54:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-sJlZmG/out4.c:9:0: warning: "__STDC_NO_THREADS__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:57:0: note: this is the location of the previous definition^M
	(gdb) FAIL: gdb.compile/compile.exp: Test delimiter with -r

Using #undef before the #define one gets:
	compile -r -- void _gdb_expr(){int i = 5;}^M
	/tmp/gdbobj-vCA0XG/out4.c:7:0: warning: undefining "__FILE__" [-Wbuiltin-macro-redefined]^M
	/tmp/gdbobj-vCA0XG/out4.c:9:8: warning: undefining "__LINE__"^M
	/tmp/gdbobj-vCA0XG/out4.c:11:8: warning: undefining "__STDC_IEC_559_COMPLEX__"^M
	/tmp/gdbobj-vCA0XG/out4.c:13:8: warning: undefining "__STDC_IEC_559__"^M
	/tmp/gdbobj-vCA0XG/out4.c:15:8: warning: undefining "__STDC_ISO_10646__"^M
	/tmp/gdbobj-vCA0XG/out4.c:17:8: warning: undefining "__STDC_NO_THREADS__"^M
	(gdb) FAIL: gdb.compile/compile.exp: Test delimiter with -r

In the end the #ifndef+#define is not such a problem I think.


> Please leave a PASS path in place.  I think this would work:

Yes, almost:
> 
> gdb_test_multiple $test $test {
>     -re "^$test\r\n$gdb_prompt $ $" {
      -re "^$test\r\n$gdb_prompt $" {
> 	pass "$test"
>     }


Thanks,
Jan
  

On 05/03/2015 03:05 PM, Jan Kratochvil wrote:

> In the end the #ifndef+#define is not such a problem I think.

OK, let's keep that route then.

>> Please leave a PASS path in place.  I think this would work:
> 
> Yes, almost:
>>
>> gdb_test_multiple $test $test {
>>     -re "^$test\r\n$gdb_prompt $ $" {
>       -re "^$test\r\n$gdb_prompt $" {
>> 	pass "$test"
>>     }

Yeah, that double $ was a typo.

Thanks,
Pedro Alves