[patch+8.0] release branch: Fix: --enable-werror

Message ID 20170421085204.GA5501@host1.jankratochvil.net
State New, archived
Headers

Commit Message

Jan Kratochvil April 21, 2017, 8:52 a.m. UTC
  Hi,

gdb-8.0-branch
./configure --enable-werror --enable-targets=all
aarch64-tdep.c:3045:13: error: ‘void selftests::aarch64_process_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
arm-tdep.c:9601:13: error: ‘void selftests::arm_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]

OK for check-in?

Please do not tell me release branches do not support --enable-werror as this
is due to a sourcebase bug, not due to some unexpected compiler
incompatibility.

Sergio: Shouldn't it be also buildbot-tested?


Jan
gdb/ChangeLog
2017-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* aarch64-tdep.c (selftests::aarch64_process_record_test): Make it #if
	GDB_SELF_TEST.
	* arm-tdep.c (selftests::arm_record_test): Likewise.
  

Comments

Pedro Alves April 21, 2017, 2:03 p.m. UTC | #1
On 04/21/2017 09:52 AM, Jan Kratochvil wrote:
> Hi,
> 
> gdb-8.0-branch
> ./configure --enable-werror --enable-targets=all
> aarch64-tdep.c:3045:13: error: ‘void selftests::aarch64_process_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
> arm-tdep.c:9601:13: error: ‘void selftests::arm_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
> 
> OK for check-in?

OK.

> 
> Please do not tell me release branches do not support --enable-werror as this
> is due to a sourcebase bug, not due to some unexpected compiler
> incompatibility.

Certainly.

> Sergio: Shouldn't it be also buildbot-tested?

Sounds like a good idea.

Thanks,
Pedro Alves
  
Jan Kratochvil April 21, 2017, 2:18 p.m. UTC | #2
On Fri, 21 Apr 2017 16:03:50 +0200, Pedro Alves wrote:
> OK.

master:
	0d4c07afb1e5e37e8e296858f8b14a3b4adf950b
8.0:
	b2e445cf19788caa3640e8cfacab82995d2b1d13


Thanks,
Jan
  
Sergio Durigan Junior April 22, 2017, 4:20 p.m. UTC | #3
On Friday, April 21 2017, Jan Kratochvil wrote:

> Hi,
>
> gdb-8.0-branch
> ./configure --enable-werror --enable-targets=all
> aarch64-tdep.c:3045:13: error: ‘void selftests::aarch64_process_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
> arm-tdep.c:9601:13: error: ‘void selftests::arm_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
>
> OK for check-in?
>
> Please do not tell me release branches do not support --enable-werror as this
> is due to a sourcebase bug, not due to some unexpected compiler
> incompatibility.
>
> Sergio: Shouldn't it be also buildbot-tested?

Yes, it should.  We have an Arch64 builder managed by Yao.  It should
be possible to set up builders for the 8.0 branch.

> Jan
>
> gdb/ChangeLog
> 2017-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
> 	* aarch64-tdep.c (selftests::aarch64_process_record_test): Make it #if
> 	GDB_SELF_TEST.
> 	* arm-tdep.c (selftests::arm_record_test): Likewise.
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 28c2573..6113621 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -3040,10 +3040,12 @@ aarch64_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
>  		      paddress (gdbarch, tdep->lowest_pc));
>  }
>  
> +#if GDB_SELF_TEST
>  namespace selftests
>  {
>  static void aarch64_process_record_test (void);
>  }
> +#endif
>  
>  /* Suppress warning from -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_aarch64_tdep;
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index c8fabfb..9ac667f 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -9596,10 +9596,12 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
>  		      (unsigned long) tdep->lowest_pc);
>  }
>  
> +#if GDB_SELF_TEST
>  namespace selftests
>  {
>  static void arm_record_test (void);
>  }
> +#endif
>  
>  extern initialize_file_ftype _initialize_arm_tdep; /* -Wmissing-prototypes */
>  
>
  
Jan Kratochvil April 22, 2017, 4:26 p.m. UTC | #4
On Sat, 22 Apr 2017 18:20:06 +0200, Sergio Durigan Junior wrote:
> On Friday, April 21 2017, Jan Kratochvil wrote:
> > gdb-8.0-branch
> > ./configure --enable-werror --enable-targets=all
> > aarch64-tdep.c:3045:13: error: ‘void selftests::aarch64_process_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
> > arm-tdep.c:9601:13: error: ‘void selftests::arm_record_test()’ declared ‘static’ but never defined [-Werror=unused-function]
...
> > Sergio: Shouldn't it be also buildbot-tested?
> 
> Yes, it should.  We have an Arch64 builder managed by Yao.  It should
> be possible to set up builders for the 8.0 branch.

FYI no specific arch is required for the builder, it even happened for me on
my x86_64.  These were *-tdep.c files so --enable-targets=all is enough to
trigger it.

Sure more archs would make better coverage even for nat/* files.


Jan
  

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 28c2573..6113621 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3040,10 +3040,12 @@  aarch64_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
 		      paddress (gdbarch, tdep->lowest_pc));
 }
 
+#if GDB_SELF_TEST
 namespace selftests
 {
 static void aarch64_process_record_test (void);
 }
+#endif
 
 /* Suppress warning from -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_aarch64_tdep;
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index c8fabfb..9ac667f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9596,10 +9596,12 @@  arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
 		      (unsigned long) tdep->lowest_pc);
 }
 
+#if GDB_SELF_TEST
 namespace selftests
 {
 static void arm_record_test (void);
 }
+#endif
 
 extern initialize_file_ftype _initialize_arm_tdep; /* -Wmissing-prototypes */