[PATCH] PR other/63613: Add fixincludes for dejagnu.h

[PATCH] PR other/63613: Add fixincludes for dejagnu.h

[David Malcolm]

<dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html

Remove the workaround from jit.exp that used -fgnu89-inline
in favor of a fixincludes to dejagnu.h that applies the upstream fix
to a local copy.

This should make it easier to support C++ testcases from jit.exp.

(I also needed to fix up the jit.dg/test-threads.c due to the
preprocessor tricks that that test plays in order to make
dejagnu.h be threadsafe).

This is the first time I've touched the "fixincludes" directory;
is this the correct way to make a change here?

Successfully bootstrapped & regrtested on x86_64-unknown-linux-gnu
(Fedora 20).

OK for trunk?

fixincludes/ChangeLog:
	PR other/63613
	* inclhack.def (dejagnu_h_make_inline_functions_static): New fix.
	* fixincl.x: Regenerate.
	* tests/base/dejagnu.h: New.

gcc/testsuite/ChangeLog:
	PR other/63613
	* jit.dg/jit.exp (DEFAULT_CFLAGS): Remove the -fgnu89-inline
	workaround for older versions of dejagnu.h in favor of using
	fixincludes.
	* jit.dg/test-threads.c (dejagnu_pass): Make static.
	(dejagnu_fail): Likewise.
	(dejagnu_note): Likewise.
	(pass): Likewise.
	(fail): Likewise.
	(note): Likewise.
---
 fixincludes/fixincl.x               | 48 ++++++++++++++++++++++++++++++-------
 fixincludes/inclhack.def            | 21 ++++++++++++++++
 fixincludes/tests/base/dejagnu.h    | 17 +++++++++++++
 gcc/testsuite/jit.dg/jit.exp        |  5 ----
 gcc/testsuite/jit.dg/test-threads.c | 12 +++++-----
 5 files changed, 84 insertions(+), 19 deletions(-)
 create mode 100644 fixincludes/tests/base/dejagnu.h

diff --git a/fixincludes/fixincl.x b/fixincludes/fixincl.x
index eb98f44..980b0d3 100644
--- a/fixincludes/fixincl.x
+++ b/fixincludes/fixincl.x
@@ -1,12 +1,12 @@
 /*  -*- buffer-read-only: t -*- vi: set ro:
- * 
+ *
  * DO NOT EDIT THIS FILE   (fixincl.x)
- * 
- * It has been AutoGen-ed  October 21, 2014 at 10:18:16 AM by AutoGen 5.16.2
+ *
+ * It has been AutoGen-ed  December  3, 2014 at 04:00:25 PM by AutoGen 5.18
  * From the definitions    inclhack.def
  * and the template file   fixincl
  */
-/* DO NOT SVN-MERGE THIS FILE, EITHER Tue Oct 21 10:18:17 CEST 2014
+/* DO NOT SVN-MERGE THIS FILE, EITHER Wed Dec  3 16:00:25 EST 2014
  *
  * You must regenerate it.  Use the ./genfixes script.
  *
@@ -15,7 +15,7 @@
  * certain ANSI-incompatible system header files which are fixed to work
  * correctly with ANSI C and placed in a directory that GNU C will search.
  *
- * This file contains 223 fixup descriptions.
+ * This file contains 224 fixup descriptions.
  *
  * See README for more information.
  *
@@ -9147,6 +9147,32 @@ static const char* apzX11_SprintfPatch[] = {
 #endif /* !defined __STDC__ */",
     (char*)NULL };
 
+/* * * * * * * * * * * * * * * * * * * * * * * * * *
+ *
+ *  Description of Dejagnu_H_Make_Inline_Functions_Static fix
+ */
+tSCC zDejagnu_H_Make_Inline_Functions_StaticName[] =
+     "dejagnu_h_make_inline_functions_static";
+
+/*
+ *  File name selection pattern
+ */
+tSCC zDejagnu_H_Make_Inline_Functions_StaticList[] =
+  "dejagnu.h\0";
+/*
+ *  Machine/OS name selection pattern
+ */
+#define apzDejagnu_H_Make_Inline_Functions_StaticMachs (const char**)NULL
+#define DEJAGNU_H_MAKE_INLINE_FUNCTIONS_STATIC_TEST_CT  0
+#define aDejagnu_H_Make_Inline_Functions_StaticTests   (tTestDesc*)NULL
+
+/*
+ *  Fix Command Arguments for Dejagnu_H_Make_Inline_Functions_Static
+ */
+static const char* apzDejagnu_H_Make_Inline_Functions_StaticPatch[] = { sed_cmd_z,
+    "-e", "s@^inline void$@static inline void@",
+    (char*)NULL };
+
 
 /* * * * * * * * * * * * * * * * * * * * * * * * * *
  *
@@ -9154,7 +9180,7 @@ static const char* apzX11_SprintfPatch[] = {
  */
 #define REGEX_COUNT          260
 #define MACH_LIST_SIZE_LIMIT 187
-#define FIX_COUNT            223
+#define FIX_COUNT            224
 
 /*
  *  Enumerate the fixes
@@ -9382,7 +9408,8 @@ typedef enum {
     X11_CLASS_FIXIDX,
     X11_CLASS_USAGE_FIXIDX,
     X11_NEW_FIXIDX,
-    X11_SPRINTF_FIXIDX
+    X11_SPRINTF_FIXIDX,
+    DEJAGNU_H_MAKE_INLINE_FUNCTIONS_STATIC_FIXIDX
 } t_fixinc_idx;
 
 tFixDesc fixDescList[ FIX_COUNT ] = {
@@ -10499,5 +10526,10 @@ tFixDesc fixDescList[ FIX_COUNT ] = {
   {  zX11_SprintfName,    zX11_SprintfList,
      apzX11_SprintfMachs,
      X11_SPRINTF_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
-     aX11_SprintfTests,   apzX11_SprintfPatch, 0 }
+     aX11_SprintfTests,   apzX11_SprintfPatch, 0 },
+
+  {  zDejagnu_H_Make_Inline_Functions_StaticName,    zDejagnu_H_Make_Inline_Functions_StaticList,
+     apzDejagnu_H_Make_Inline_Functions_StaticMachs,
+     DEJAGNU_H_MAKE_INLINE_FUNCTIONS_STATIC_TEST_CT, FD_MACH_ONLY,
+     aDejagnu_H_Make_Inline_Functions_StaticTests,   apzDejagnu_H_Make_Inline_Functions_StaticPatch, 0 }
 };
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index b27f163..66821ef 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -4738,4 +4738,25 @@ fix = {
 
     test_text = "extern char *\tsprintf();";
 };
+
+/*
+ * PR other/63613
+ * Old versions of dejagnu.h relied on gnu89 inline semantics.
+ * DejaGnu fixed this in ad36659ffa984a0541cfc2bd27f393e0d7d173a7
+ * by making the various "inline void" functions be "static inline void".
+ * Replicate that fix locally, if needed.
+ */
+fix = {
+    hackname = dejagnu_h_make_inline_functions_static;
+    files = dejagnu.h;
+
+    sed = 's@^inline void$'
+           '@static inline void@';
+
+    test_text =
+    "inline void\n"
+    "pass (const char* fmt, ...)\n"
+    "{ /* snip */ }\n";
+
+};
 /*EOF*/
diff --git a/fixincludes/tests/base/dejagnu.h b/fixincludes/tests/base/dejagnu.h
new file mode 100644
index 0000000..7b7a389
--- /dev/null
+++ b/fixincludes/tests/base/dejagnu.h
@@ -0,0 +1,17 @@
+/*  DO NOT EDIT THIS FILE.
+
+    It has been auto-edited by fixincludes from:
+
+	"fixinc/tests/inc/dejagnu.h"
+
+    This had to be done to correct non-standard usages in the
+    original, manufacturer supplied header file.  */
+
+
+
+#if defined( DEJAGNU_H_MAKE_INLINE_FUNCTIONS_STATIC_CHECK )
+static inline void
+pass (const char* fmt, ...)
+{ /* snip */ }
+
+#endif  /* DEJAGNU_H_MAKE_INLINE_FUNCTIONS_STATIC_CHECK */
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 438aabd..e342ba0 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -393,11 +393,6 @@ proc jit-dg-test { prog do_what extra_tool_flags } {
 # so that the JIT-built code can call into functions from the main program.
 set DEFAULT_CFLAGS "-I$srcdir/../jit -lgccjit -g -Wall -Werror -Wl,--export-dynamic"
 
-# <dejagnu.h> assumes -fgnu89-inline
-# See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63613
-# and http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
-append DEFAULT_CFLAGS " -fgnu89-inline"
-
 # Main loop.  This will invoke jig-dg-test on each test-*.c file.
 dg-runtest $tests "" $DEFAULT_CFLAGS
 
diff --git a/gcc/testsuite/jit.dg/test-threads.c b/gcc/testsuite/jit.dg/test-threads.c
index 7c248cc..09d5b53 100644
--- a/gcc/testsuite/jit.dg/test-threads.c
+++ b/gcc/testsuite/jit.dg/test-threads.c
@@ -20,14 +20,14 @@ static pthread_mutex_t dg_mutex = PTHREAD_MUTEX_INITIALIZER;
    harness.h injects macros before including <dejagnu.h> so that the
    pass/fail functions become "dejagnu_pass"/"dejagnu_fail" etc.  */
 
-void dejagnu_pass (const char* fmt, ...);
-void dejagnu_fail (const char* fmt, ...);
-void dejagnu_note (const char* fmt, ...);
+static void dejagnu_pass (const char* fmt, ...);
+static void dejagnu_fail (const char* fmt, ...);
+static void dejagnu_note (const char* fmt, ...);
 
 /* We now provide our own implementations of "pass"/"fail"/"note", which
    call the underlying dejagnu implementations, but with a mutex.  */
 
-inline void
+static inline void
 pass (const char* fmt, ...)
 {
   va_list ap;
@@ -42,7 +42,7 @@ pass (const char* fmt, ...)
   pthread_mutex_unlock (&dg_mutex);
 }
 
-inline void
+static inline void
 fail (const char* fmt, ...)
 {
   va_list ap;
@@ -57,7 +57,7 @@ fail (const char* fmt, ...)
   pthread_mutex_unlock (&dg_mutex);
 }
 
-inline void
+static inline void
 note (const char* fmt, ...)
 {
   va_list ap;
-- 
1.8.5.3

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Rainer Orth]

David Malcolm <dmalcolm@redhat.com> writes:

> <dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
> see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
>
> Remove the workaround from jit.exp that used -fgnu89-inline
> in favor of a fixincludes to dejagnu.h that applies the upstream fix
> to a local copy.
>
> This should make it easier to support C++ testcases from jit.exp.

I wonder how this would work if dejagnu.h doesn't live in a system
include dir (e.g. a self-compiled version)?  fixincludes won't touch
those AFAIU.  The previous version with -fgnu89-inline would still work
in that case provided dejagnu.h is found at all.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Jeff Law]

On 12/04/14 15:42, Rainer Orth wrote:
> David Malcolm <dmalcolm@redhat.com> writes:
>
>> <dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
>> see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
>>
>> Remove the workaround from jit.exp that used -fgnu89-inline
>> in favor of a fixincludes to dejagnu.h that applies the upstream fix
>> to a local copy.
>>
>> This should make it easier to support C++ testcases from jit.exp.
>
> I wonder how this would work if dejagnu.h doesn't live in a system
> include dir (e.g. a self-compiled version)?  fixincludes won't touch
> those AFAIU.  The previous version with -fgnu89-inline would still work
> in that case provided dejagnu.h is found at all.
Presumably in that case the answer is upgrade dejagnu? :-)

jeff

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Jeff Law]

On 12/04/14 15:19, David Malcolm wrote:
> <dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
> see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
>
> Remove the workaround from jit.exp that used -fgnu89-inline
> in favor of a fixincludes to dejagnu.h that applies the upstream fix
> to a local copy.
>
> This should make it easier to support C++ testcases from jit.exp.
>
> (I also needed to fix up the jit.dg/test-threads.c due to the
> preprocessor tricks that that test plays in order to make
> dejagnu.h be threadsafe).
>
> This is the first time I've touched the "fixincludes" directory;
> is this the correct way to make a change here?
>
> Successfully bootstrapped & regrtested on x86_64-unknown-linux-gnu
> (Fedora 20).
>
> OK for trunk?
>
> fixincludes/ChangeLog:
> 	PR other/63613
> 	* inclhack.def (dejagnu_h_make_inline_functions_static): New fix.
> 	* fixincl.x: Regenerate.
> 	* tests/base/dejagnu.h: New.
OK.
jeff

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Bruce Korb]

>> This is the first time I've touched the "fixincludes" directory;
>> is this the correct way to make a change here?

Well, I'd like to see it -- especially since it's your first.
Please send to this gmail account or wait until I get my GNU email this weekend.
Thanks!

>> Successfully bootstrapped & regrtested on x86_64-unknown-linux-gnu
>> (Fedora 20).
>>
>> OK for trunk?

More likely than not :)

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Bruce Korb]

On 12/05/14 12:32, Jeff Law wrote:1
>> fixincludes/ChangeLog:
>>     PR other/63613
>>     * inclhack.def (dejagnu_h_make_inline_functions_static): New fix.
>>     * fixincl.x: Regenerate.
>>     * tests/base/dejagnu.h: New.
> OK.

No, actually not.

+fix = {
+    hackname = dejagnu_h_make_inline_functions_static;
+    files = dejagnu.h;
+
+    sed = 's@^inline void$'
+           '@static inline void@';

I guess I should elaborate on the preferred substitution mechanism: the "format" fix.
This is functionally equivalent but does not require loading the "sed" program:

fix = {
   hackname = dejagnu_static_inline;
   files = dejagnu.h;
   select = '^inline void$';
   c-fix = format;
   c-fix-arg = 'static %0';

Thank you.

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Rainer Orth]

Jeff Law <law@redhat.com> writes:

> On 12/04/14 15:42, Rainer Orth wrote:
>> David Malcolm <dmalcolm@redhat.com> writes:
>>
>>> <dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
>>> see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
>>>
>>> Remove the workaround from jit.exp that used -fgnu89-inline
>>> in favor of a fixincludes to dejagnu.h that applies the upstream fix
>>> to a local copy.
>>>
>>> This should make it easier to support C++ testcases from jit.exp.
>>
>> I wonder how this would work if dejagnu.h doesn't live in a system
>> include dir (e.g. a self-compiled version)?  fixincludes won't touch
>> those AFAIU.  The previous version with -fgnu89-inline would still work
>> in that case provided dejagnu.h is found at all.
> Presumably in that case the answer is upgrade dejagnu? :-)

I've two problems with this:

* There's not yet a DejaGnu release available with the fix and I've no
  idea if there are any planned any time soon.  Not everyone is
  comfortable with random git (or whatever) snapshots.

* I don't consider this a critical issue that cannot work without
  current releases.  We're already working around several upstream
  DejaGnu issues in our codebase, and I don't consider this particular
  one important enough to require everyone to upgrade to a not-a-release
  version.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[David Malcolm]

On Mon, 2014-12-08 at 14:13 +0100, Rainer Orth wrote:
> Jeff Law <law@redhat.com> writes:
> 
> > On 12/04/14 15:42, Rainer Orth wrote:
> >> David Malcolm <dmalcolm@redhat.com> writes:
> >>
> >>> <dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
> >>> see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
> >>>
> >>> Remove the workaround from jit.exp that used -fgnu89-inline
> >>> in favor of a fixincludes to dejagnu.h that applies the upstream fix
> >>> to a local copy.
> >>>
> >>> This should make it easier to support C++ testcases from jit.exp.
> >>
> >> I wonder how this would work if dejagnu.h doesn't live in a system
> >> include dir (e.g. a self-compiled version)?  fixincludes won't touch
> >> those AFAIU.  The previous version with -fgnu89-inline would still work
> >> in that case provided dejagnu.h is found at all.
> > Presumably in that case the answer is upgrade dejagnu? :-)
> 
> I've two problems with this:
> 
> * There's not yet a DejaGnu release available with the fix and I've no
>   idea if there are any planned any time soon.  Not everyone is
>   comfortable with random git (or whatever) snapshots.

FWIW I've asked on the DejaGnu mailing list, and Ben Elliston said:
> Yes. I plan on releasing 1.6 over the holidays.
http://lists.gnu.org/archive/html/dejagnu/2014-12/msg00001.html

> * I don't consider this a critical issue that cannot work without
>   current releases.  We're already working around several upstream
>   DejaGnu issues in our codebase, and I don't consider this particular
>   one important enough to require everyone to upgrade to a not-a-release
>   version.
> 
> 	Rainer
> 

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Rainer Orth]

David Malcolm <dmalcolm@redhat.com> writes:

> On Mon, 2014-12-08 at 14:13 +0100, Rainer Orth wrote:
>> Jeff Law <law@redhat.com> writes:
>> 
>> > On 12/04/14 15:42, Rainer Orth wrote:
>> >> David Malcolm <dmalcolm@redhat.com> writes:
>> >>
>> >>> <dejagnu.h> assumed -fgnu89-inline until a recent upstream fix;
>> >>> see http://lists.gnu.org/archive/html/dejagnu/2014-10/msg00011.html
>> >>>
>> >>> Remove the workaround from jit.exp that used -fgnu89-inline
>> >>> in favor of a fixincludes to dejagnu.h that applies the upstream fix
>> >>> to a local copy.
>> >>>
>> >>> This should make it easier to support C++ testcases from jit.exp.
>> >>
>> >> I wonder how this would work if dejagnu.h doesn't live in a system
>> >> include dir (e.g. a self-compiled version)?  fixincludes won't touch
>> >> those AFAIU.  The previous version with -fgnu89-inline would still work
>> >> in that case provided dejagnu.h is found at all.
>> > Presumably in that case the answer is upgrade dejagnu? :-)
>> 
>> I've two problems with this:
>> 
>> * There's not yet a DejaGnu release available with the fix and I've no
>>   idea if there are any planned any time soon.  Not everyone is
>>   comfortable with random git (or whatever) snapshots.
>
> FWIW I've asked on the DejaGnu mailing list, and Ben Elliston said:
>> Yes. I plan on releasing 1.6 over the holidays.
> http://lists.gnu.org/archive/html/dejagnu/2014-12/msg00001.html

Thanks for checking this, but ...

>> * I don't consider this a critical issue that cannot work without
>>   current releases.  We're already working around several upstream
>>   DejaGnu issues in our codebase, and I don't consider this particular
>>   one important enough to require everyone to upgrade to a not-a-release
>>   version.

... a DejaGnu 1.6 release would only address one part of my concern: I
still don't believe this minor issues warrants us demanding all gcc
testers upgrading to a newer DejaGnu release.  I'd like my fellow
testsuite maintainers to weigh in, though.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] PR other/63613: Add fixincludes for dejagnu.h

[Mike Stump]

On Dec 11, 2014, at 5:07 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
> David Malcolm <dmalcolm@redhat.com> writes:
>>> * I don't consider this a critical issue that cannot work without
>>>  current releases.  We're already working around several upstream
>>>  DejaGnu issues in our codebase, and I don't consider this particular
>>>  one important enough to require everyone to upgrade to a not-a-release
>>>  version.
> 
> ... a DejaGnu 1.6 release would only address one part of my concern: I
> still don't believe this minor issues warrants us demanding all gcc
> testers upgrading to a newer DejaGnu release.  I'd like my fellow
> testsuite maintainers to weigh in, though.

I’m fine with how this is being done.  If the jit people want to fixincludes it, fine.  If they submit fixes to dejagnu and want to recommend a newer dejgnu for jit testing, that’s fine.  A tester should be free to use the current dejagnu they use and it should result in no regressions in the test suite for the non-jit parts.  They should also be free to update to the latest dejagnu and use it and see non-regressions across the non-jit suite.  If the jit people want to simplify their lives and do a return if the dejagnu version is too old from their top-level, essentially enforcing a tester to use a newer dejegnu if they want to see jit test suite results, even that is fine with me.  We can even update the recommended version of dejagnu while still keeping the current recommended version as working, I’m not opposed to that.