Bug 109293 - [12 Regression] Missing memmem() prototype in fixincludes/fixfixes.c on Windows/MinGW-W64
Summary: [12 Regression] Missing memmem() prototype in fixincludes/fixfixes.c on Windo...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: 12.3
Assignee: Xi Ruoyao
URL: https://gcc.gnu.org/pipermail/gcc-pat...
Keywords: build, patch
Depends on:
Blocks:
 
Reported: 2023-03-27 09:19 UTC by Jan Dubiec
Modified: 2023-03-28 10:02 UTC (History)
2 users (show)

See Also:
Host: x86_64-w64-mingw32
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-03-27 00:00:00


Attachments
Proposed patch (238 bytes, patch)
2023-03-27 09:19 UTC, Jan Dubiec
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Dubiec 2023-03-27 09:19:56 UTC
Created attachment 54763 [details]
Proposed patch

gcc -c -g -O2     -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wno-overlength-strings -pedantic -Wno-long-long   -DHAVE_CONFIG_H -I. -I../../../gcc/fixincludes -I../include -I../../../gcc/fixincludes/../include ../../../gcc/fixincludes/fixfixes.c
../../../gcc/fixincludes/fixfixes.c: In function 'check_has_inc':
../../../gcc/fixincludes/fixfixes.c:490:12: warning: implicit declaration of function 'memmem'; did you mean 'memset'? [-Wimplicit-function-declaration]
  490 |   for (p = memmem (begin, pos - begin, has_inc, has_inc_len);
      |            ^~~~~~
      |            memset
../../../gcc/fixincludes/fixfixes.c:490:10: warning: assignment to 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  490 |   for (p = memmem (begin, pos - begin, has_inc, has_inc_len);
      |          ^
../../../gcc/fixincludes/fixfixes.c:492:10: warning: assignment to 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  492 |        p = memmem (p, pos - p, has_inc, has_inc_len))
      |          ^
Comment 1 Andrew Pinski 2023-03-27 15:42:39 UTC
Started with r12-1924-g6bf383c37e6131 .
Comment 2 Xi Ruoyao 2023-03-27 16:09:10 UTC
I'll make a patch to check if memmem is declared in configure.ac.  memmem is not a POSIX function, so it may be undeclared on systems other than MinGW as well.
Comment 3 Andrew Pinski 2023-03-27 16:13:21 UTC
(In reply to Xi Ruoyao from comment #2)
> I'll make a patch to check if memmem is declared in configure.ac.  memmem is
> not a POSIX function, so it may be undeclared on systems other than MinGW as
> well.

memmem is included in libiberty so it is more just about the prototype not being declared.
Comment 4 Xi Ruoyao 2023-03-27 16:15:37 UTC
(In reply to Andrew Pinski from comment #3)
> (In reply to Xi Ruoyao from comment #2)
> > I'll make a patch to check if memmem is declared in configure.ac.  memmem is
> > not a POSIX function, so it may be undeclared on systems other than MinGW as
> > well.
> 
> memmem is included in libiberty so it is more just about the prototype not
> being declared.

libiberty configure.ac checked it with AC_CHECK_FUNCS, so if the system libc does not provide memmem, libiberty will build its copy.  And we just need to declare the prototype to use it, as libiberty doc says: https://gcc.gnu.org/onlinedocs/libiberty/Supplemental-Functions.html
Comment 5 Andrew Pinski 2023-03-27 16:19:44 UTC
Something like this:
diff --git a/fixincludes/configure.ac b/fixincludes/configure.ac
index 14813b910f1..00aeb1ce1d9 100644
--- a/fixincludes/configure.ac
+++ b/fixincludes/configure.ac
@@ -89,6 +89,7 @@ define(fixincludes_UNLOCKED_FUNCS, clearerr_unlocked feof_unlocked dnl
   putchar_unlocked putc_unlocked)
 AC_CHECK_FUNCS(fixincludes_UNLOCKED_FUNCS)
 AC_CHECK_DECLS([abort, asprintf, basename(char *), errno, vasprintf])
+AC_CHECK_DECLS([memmem])
 AC_CHECK_DECLS(m4_split(m4_normalize(fixincludes_UNLOCKED_FUNCS)))

 # Checks for typedefs, structures, and compiler characteristics.
diff --git a/fixincludes/system.h b/fixincludes/system.h
index dca5d57b2e3..f7bbd0e952c 100644
--- a/fixincludes/system.h
+++ b/fixincludes/system.h
@@ -209,6 +209,11 @@ extern int errno;
 extern void abort (void);
 #endif

+#if defined (HAVE_DECL_MEMMEM) && !HAVE_DECL_MEMMEM
+extern void *memmem (const void *, size_t, const void *, size_t);
+#endif
+
+
 #if HAVE_SYS_STAT_H
 # include <sys/stat.h>
 #endif
Comment 6 Xi Ruoyao 2023-03-27 16:20:43 UTC
(In reply to Andrew Pinski from comment #5)
> Something like this:
> diff --git a/fixincludes/configure.ac b/fixincludes/configure.ac
> index 14813b910f1..00aeb1ce1d9 100644
> --- a/fixincludes/configure.ac
> +++ b/fixincludes/configure.ac
> @@ -89,6 +89,7 @@ define(fixincludes_UNLOCKED_FUNCS, clearerr_unlocked
> feof_unlocked dnl
>    putchar_unlocked putc_unlocked)
>  AC_CHECK_FUNCS(fixincludes_UNLOCKED_FUNCS)
>  AC_CHECK_DECLS([abort, asprintf, basename(char *), errno, vasprintf])
> +AC_CHECK_DECLS([memmem])
>  AC_CHECK_DECLS(m4_split(m4_normalize(fixincludes_UNLOCKED_FUNCS)))
> 
>  # Checks for typedefs, structures, and compiler characteristics.
> diff --git a/fixincludes/system.h b/fixincludes/system.h
> index dca5d57b2e3..f7bbd0e952c 100644
> --- a/fixincludes/system.h
> +++ b/fixincludes/system.h
> @@ -209,6 +209,11 @@ extern int errno;
>  extern void abort (void);
>  #endif
> 
> +#if defined (HAVE_DECL_MEMMEM) && !HAVE_DECL_MEMMEM
> +extern void *memmem (const void *, size_t, const void *, size_t);
> +#endif
> +
> +
>  #if HAVE_SYS_STAT_H
>  # include <sys/stat.h>
>  #endif

Yes, I'm testing it in a container environment where I've removed memmem from system string.h.
Comment 7 GCC Commits 2023-03-28 07:26:39 UTC
The master branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>:

https://gcc.gnu.org/g:21c74b6ea41d21ef96813b34bfa55c51a82d6c99

commit r13-6888-g21c74b6ea41d21ef96813b34bfa55c51a82d6c99
Author: Xi Ruoyao <xry111@xry111.site>
Date:   Tue Mar 28 01:48:02 2023 +0800

    fixincludes: Declare memmem if it's not declared in system headers [PR109293]
    
    memmem is not POSIX so the system may lack it.  Then libiberty will
    provide an implementation, but it's a "supplemental function" and not
    declared in libiberty.h.  We need to declare the prototype to use it
    then.
    
    See libiberty doc at
    https://gcc.gnu.org/onlinedocs/libiberty/Supplemental-Functions.html.
    
    Tested by bootstrapping GCC in the following container environments on
    x86_64-linux-gnu:
    
    1. "Vanilla" system with memmem in Glibc.
    2. memmem removed from string.h.
    3. memmem removed from both string.h and libc.so.
    
    For 3, also verified that memmem from libiberty is linked into fixincl
    executable.
    
    Ok for trunk?
    
    fixincludes/ChangeLog:
    
            PR other/109293
            * configure.ac (AC_CHECK_DECLS): Add memmem.
            * configure: Regenerate.
            * config.h.in: Regenerate.
            * system.h (memmem): Declare if HAVE_DECL_MEMMEM is zero.
Comment 8 Xi Ruoyao 2023-03-28 07:28:34 UTC
Will backport the fix into gcc-12 after a bootstrap as verification.
Comment 9 Jan Dubiec 2023-03-28 09:30:50 UTC
I can confirm that now everything is fine on this issue on Windows/MingW.
Comment 10 GCC Commits 2023-03-28 10:01:20 UTC
The releases/gcc-12 branch has been updated by Xi Ruoyao <xry111@gcc.gnu.org>:

https://gcc.gnu.org/g:743088c3c7c670f180978dd7b59a57fd9626c5a2

commit r12-9326-g743088c3c7c670f180978dd7b59a57fd9626c5a2
Author: Xi Ruoyao <xry111@xry111.site>
Date:   Tue Mar 28 01:48:02 2023 +0800

    fixincludes: Declare memmem if it's not declared in system headers [PR109293]
    
    memmem is not POSIX so the system may lack it.  Then libiberty will
    provide an implementation, but it's a "supplemental function" and not
    declared in libiberty.h.  We need to declare the prototype to use it
    then.
    
    See libiberty doc at
    https://gcc.gnu.org/onlinedocs/libiberty/Supplemental-Functions.html.
    
    Tested by bootstrapping GCC in the following container environments on
    x86_64-linux-gnu:
    
    1. "Vanilla" system with memmem in Glibc.
    2. memmem removed from string.h.
    3. memmem removed from both string.h and libc.so.
    
    For 3, also verified that memmem from libiberty is linked into fixincl
    executable.
    
    Note that the backport does not contain a complete regeneration of
    configure and config.h.in (attempting such regeneration resulted in all
    the USED_FOR_TARGET conditional disappearing; this already happened in
    trunk at r13-2200).
    
    fixincludes/ChangeLog:
    
            PR other/109293
            * configure.ac (AC_CHECK_DECLS): Add memmem.
            * configure: Regenerate.
            * config.h.in: Regenerate.
            * system.h (memmem): Declare if HAVE_DECL_MEMMEM is zero.
    
    (cherry picked from commit 21c74b6ea41d21ef96813b34bfa55c51a82d6c99)
Comment 11 Xi Ruoyao 2023-03-28 10:02:32 UTC
Fixed for trunk and gcc-12.