[Bug testsuite/108675] New: FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[Bug testsuite/108675] New: FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[nightstrike at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

            Bug ID: 108675
           Summary: FAIL: gcc.c-torture/execute/builtins/*printf.c when
                    stdio.h includes definitions
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: testsuite
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nightstrike at gmail dot com
  Target Milestone: ---

Failing tests:
gcc.c-torture/execute/builtins/printf.c
gcc.c-torture/execute/builtins/fprintf.c
gcc.c-torture/execute/builtins/sprintf.c

On mingw-w64, the gcc.c-torture/execute/builtins/*printf.c tests fail.  We
include definitions of various functions in stdio.h instead of just
declarations, leading to redefinition errors.  For example,

https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-headers/crt/stdio.h:
__mingw_ovr
__attribute__((__format__ (gnu_printf, 2, 3))) __MINGW_ATTRIB_NONNULL(2)
int fprintf (FILE *__stream, const char *__format, ...)
{
  int __retval;
  __builtin_va_list __local_argv; __builtin_va_start( __local_argv, __format );
  __retval = __mingw_vfprintf( __stream, __format, __local_argv );
  __builtin_va_end( __local_argv );
  return __retval;
}

This causes the test to fail to compile at all optimization levels:
gcc.c-torture/execute/builtins/lib/fprintf.c:8:1: error: redefinition of
'fprintf'
mingw13/x86_64-w64-mingw32/include/stdio.h:357:5: note: previous definition of
'fprintf' with type 'int(FILE *, const char *, ...)' {aka 'int(struct _iobuf *,
const char *, ...)'}

The tests were originally added in r38065, r38788, and r48335.  They were later
moved in r84044 and tweaked a bit.

Is there a better way to verify that the builtin was used instead of mingw
version?

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
These tests are known to be a bit awkwardly implemented to check for
optimizations done ...

There might be a better way to provide definitions of fprintf, but is it
possible to short-circuit those definitions in the stdio.h header with
some macro definition?  And would that make the execution succeed?

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[nightstrike at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #2 from nightstrike <nightstrike at gmail dot com> ---
(In reply to Richard Biener from comment #1)
> These tests are known to be a bit awkwardly implemented to check for
> optimizations done ...

How would you do it if you were writing the test today?

> There might be a better way to provide definitions of fprintf, but is it
> possible to short-circuit those definitions in the stdio.h header with
> some macro definition?  And would that make the execution succeed?

If I modify stdio.h to #if 0 the definitions, then at least 16 fprintf tests
pass (that's the only one I modified).  We don't currently provide a macro
mechanism to disable these, though.  @jon_y, is this a reasonable workaround? 
(Ideally, fixing the test would happen instead, but if it's desirable that the
headers can optionally define *printf outside of the testsuite, then we should
entertain that as well).

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[10walls at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #3 from jon_y <10walls at gmail dot com> ---
I'm fine with something like __HIDE_PRINTF_PROTOTYPES to prevent them from
being exposed, though there may be places that needs to be fixed up from
assuming the prototypes exist.

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #4 from LIU Hao <lh_mouse at 126 dot com> ---
Does it make any sense to remove `#include <stdio.h>` from
'gcc.c-torture/execute/builtins/lib/fprintf.c' ?

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[nightstrike at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #5 from nightstrike <nightstrike at gmail dot com> ---
(In reply to LIU Hao from comment #4)
> Does it make any sense to remove `#include <stdio.h>` from
> 'gcc.c-torture/execute/builtins/lib/fprintf.c' ?

That will prevent the FILE type from existing, so the replacement functions
won't compile.

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[zack+srcbugz at owlfolio dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #6 from Zack Weinberg <zack+srcbugz at owlfolio dot org> ---
Note that what you (mingw) have in this header is a pretty serious
anti-optimization.  Functions that call __builtin_va_start cannot be inlined
under any circumstances whatsoever (try tagging it
__attribute__((always_inline)) and see what happens) so you're emitting a
redundant copy of this function in every translation unit that calls fprintf,
and you're not getting any actual codegen improvement in exchange.

You _could_ do like glibc bits/stdio2.h

__mingw_ovr
__attribute__((__format__ (gnu_printf, 1, 2))) __MINGW_ATTRIB_NONNULL(1)
int printf (const char *__format, ...)
{
  return __mingw_fprintf(stdout, __format, __builtin_va_arg_pack());
}

but this doesn't let you synthesize a direct call to __mingw_vfprintf.

I'd recommend scrapping the entire mess, abandoning the idea of getting direct
calls to v*printf/v*scanf, and using asm() symbol renaming to add the __mingw_
prefix.  That should also avoid stepping on the GCC testcases' toes.

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[lh_mouse at 126 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #7 from LIU Hao <lh_mouse at 126 dot com> ---
(In reply to nightstrike from comment #5)
> (In reply to LIU Hao from comment #4)
> > Does it make any sense to remove `#include <stdio.h>` from
> > 'gcc.c-torture/execute/builtins/lib/fprintf.c' ?
> 
> That will prevent the FILE type from existing, so the replacement functions
> won't compile.

That file never uses any fields of `FILE` directly. `FILE*` is always passed as
a pointer to `vfprintf()`, so it is perfectly valid to declare

   typedef struct _iobuf FILE;

or even 

   int fprintf (void* fp, const char* fmt, ...);

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[nightstrike at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #8 from nightstrike <nightstrike at gmail dot com> ---
(In reply to LIU Hao from comment #7)
> (In reply to nightstrike from comment #5)
> > (In reply to LIU Hao from comment #4)
> > > Does it make any sense to remove `#include <stdio.h>` from
> > > 'gcc.c-torture/execute/builtins/lib/fprintf.c' ?
> > 
> > That will prevent the FILE type from existing, so the replacement functions
> > won't compile.
> 
> That file never uses any fields of `FILE` directly. `FILE*` is always passed
> as a pointer to `vfprintf()`, so it is perfectly valid to declare
> 
>    typedef struct _iobuf FILE;
> 
> or even 
> 
>    int fprintf (void* fp, const char* fmt, ...);

Removing "include stdio" and changing both FILE*'s to void*'s does in fact make
the test pass.  Is this an acceptable way to pass the test?

diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
index 853a705e86d..75406298856 100644
--- a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
+++ b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
@@ -1,11 +1,11 @@
-#include <stdio.h>
+//#include <stdio.h>
 #include <stdarg.h>
 extern void abort (void);
 extern int inside_main;

 __attribute__ ((__noinline__))
 int
-fprintf (FILE *fp, const char *string, ...)
+fprintf (void *fp, const char *string, ...)
 {
   va_list ap;
   int r;
@@ -22,7 +22,7 @@ fprintf (FILE *fp, const char *string, ...)
 /* Locking stdio doesn't matter for the purposes of this test.  */
 __attribute__ ((__noinline__))
 int
-fprintf_unlocked (FILE *fp, const char *string, ...)
+fprintf_unlocked (void *fp, const char *string, ...)
 {
   va_list ap;
   int r;

[Bug testsuite/108675] FAIL: gcc.c-torture/execute/builtins/*printf.c when stdio.h includes definitions

[nightstrike at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108675

--- Comment #9 from nightstrike <nightstrike at gmail dot com> ---
I understand it's not ideal based on comment #6, but this fixes all the tests:

diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
index 853a705e86d..499c81b0fa7 100644
--- a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
+++ b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/fprintf.c
@@ -1,11 +1,10 @@
-#include <stdio.h>
 #include <stdarg.h>
 extern void abort (void);
 extern int inside_main;

 __attribute__ ((__noinline__))
 int
-fprintf (FILE *fp, const char *string, ...)
+fprintf (void *fp, const char *string, ...)
 {
   va_list ap;
   int r;
@@ -22,7 +21,7 @@ fprintf (FILE *fp, const char *string, ...)
 /* Locking stdio doesn't matter for the purposes of this test.  */
 __attribute__ ((__noinline__))
 int
-fprintf_unlocked (FILE *fp, const char *string, ...)
+fprintf_unlocked (void *fp, const char *string, ...)
 {
   va_list ap;
   int r;
diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/printf.c
b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/printf.c
index 4be7578d124..0d405241cfe 100644
--- a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/printf.c
+++ b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/printf.c
@@ -1,4 +1,3 @@
-#include <stdio.h>
 #include <stdarg.h>
 extern void abort (void);
 extern int inside_main;
diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/sprintf.c
b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/sprintf.c
index 3ac447b117f..6de24cd7df4 100644
--- a/gcc/testsuite/gcc.c-torture/execute/builtins/lib/sprintf.c
+++ b/gcc/testsuite/gcc.c-torture/execute/builtins/lib/sprintf.c
@@ -1,4 +1,3 @@
-#include <stdio.h>
 #include <stdarg.h>
 extern void abort (void);
 extern int inside_main;