diff mbox series

fixincludes breaks mingw64 build

Message ID 3eb89802-0cfb-1888-00b6-357d76370894@sft.mx
State New
Headers show
Series fixincludes breaks mingw64 build | expand

Commit Message

Jonas Jelten May 31, 2019, 1:35 p.m. UTC
Hi!

I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
 It breaks because a fixincludes-fix is applied at a place where it should not be applied.

This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that made it tricky to figure out what was
really going on.


This code snippet of mingw's stdio.h:


#ifdef _WIN64
  _CRTIMP FILE *__cdecl __iob_func(void);
#define _iob  __iob_func()
#else
[.....]
#if (!defined(NO_OLDNAMES) || defined(__GNUC__))
  __MINGW_EXTENSION typedef __int64 fpos_t;
#define _FPOSOFF(fp) ((long)(fp))


============= is modified by fixincludes to be:


#ifdef _WIN64
  _CRTIMP FILE *__cdecl __iob_func(void);
#define _iob  __iob

#  if defined(__STDC__) || defined(__cplusplus)
     extern int snprintf(char *, size_t, const char *, ...);
     extern int vsnprintf(char *, size_t, const char *, __gnuc_va_list);
#  else /* not __STDC__) || __cplusplus */
     extern int snprintf();
     extern int vsnprintf();
#  endif /* __STDC__) || __cplusplus */
_func()
#else
[....]
#if (!defined(NO_OLDNAMES) || defined(__GNUC__))
  __MINGW_EXTENSION typedef __int64 fpos_t;
#define _FPOSOFF(fp) ((long)(fp))


which leads to this compiler error:

In file included from /usr/x86_64-w64-mingw64/sys-include/_mingw.h:12,
                 from /usr/x86_64-w64-mingw64/sys-include/crtdefs.h:10,
                 from /usr/x86_64-w64-mingw64/sys-include/stddef.h:7,
                 from /tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/build/gcc/include/stddef.h:1,
                 from /tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/gcc-8.3.0/libgcc/../gcc/tsystem.h:44,
                 from /tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/gcc-8.3.0/libgcc/libgcc2.c:27:
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/build/gcc/include-fixed/stdio.h: In function ‘_func’:
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/build/gcc/include-fixed/stdio.h:136:3: error: expected
declaration specifiers before ‘__extension__’
   __MINGW_EXTENSION typedef __int64 fpos_t;
   ^~~~~~~~~~~~~~~~~

and many many more follow-up errors.



A simple fix seems to be to check for a line ending in the select-regex:

Author: Jonas Jelten <jj@sft.mx>
Date:   Wed May 31 15:10:52 2019 +0200

    fixincludes: check for lineending to not break mingw64-build




Cheers

-- Jonas

Comments

Jonathan Wakely May 31, 2019, 2:38 p.m. UTC | #1
On 31/05/19 15:35 +0200, Jonas Jelten wrote:
>Hi!
>
>I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
> It breaks because a fixincludes-fix is applied at a place where it should not be applied.
>
>This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that made it tricky to figure out what was
>really going on.

[snip]

N.B. The gcc-bugs mailing list is for automated email from Bugzilla,
not for reporting bugs. Sending the patch to gcc-patches (and CCing a
relevant maintainer) is correct, but there's no need (and no point) to
also send it to gcc-bugs.
Jonas Jelten May 31, 2019, 3:18 p.m. UTC | #2
On 31/05/2019 16.38, Jonathan Wakely wrote:
> On 31/05/19 15:35 +0200, Jonas Jelten wrote:
>> Hi!
>>
>> I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
>> It breaks because a fixincludes-fix is applied at a place where it should not be applied.
>>
>> This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that made it tricky to figure out what was
>> really going on.
> 
> [snip]
> 
> N.B. The gcc-bugs mailing list is for automated email from Bugzilla,
> not for reporting bugs. Sending the patch to gcc-patches (and CCing a
> relevant maintainer) is correct, but there's no need (and no point) to
> also send it to gcc-bugs.
> 
> 


Alright, thanks for the hint. I was just following the instructions in gcc-9.1.0/fixincludes/README:

"""
Please also send relevant information to gcc-bugs@gcc.gnu.org, gcc-patches@gcc.gnu.org and,
please, to me: bkorb@gnu.org.
"""

-- JJ
Jonathan Wakely May 31, 2019, 7:15 p.m. UTC | #3
On 31/05/19 17:18 +0200, Jonas Jelten wrote:
>On 31/05/2019 16.38, Jonathan Wakely wrote:
>> On 31/05/19 15:35 +0200, Jonas Jelten wrote:
>>> Hi!
>>>
>>> I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
>>> It breaks because a fixincludes-fix is applied at a place where it should not be applied.
>>>
>>> This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that made it tricky to figure out what was
>>> really going on.
>>
>> [snip]
>>
>> N.B. The gcc-bugs mailing list is for automated email from Bugzilla,
>> not for reporting bugs. Sending the patch to gcc-patches (and CCing a
>> relevant maintainer) is correct, but there's no need (and no point) to
>> also send it to gcc-bugs.
>>
>>
>
>
>Alright, thanks for the hint. I was just following the instructions in gcc-9.1.0/fixincludes/README:
>
>"""
>Please also send relevant information to gcc-bugs@gcc.gnu.org, gcc-patches@gcc.gnu.org and,
>please, to me: bkorb@gnu.org.
>"""

Oh, indeed! 

Bruce, should that suggest submitting something to Bugzilla instead?
Mails sent directly to gcc-bugs tend to get lost among all the
automated email from Bugzilla.
Bruce Korb June 11, 2019, 8:32 p.m. UTC | #4
On 5/31/19 6:35 AM, Jonas Jelten wrote:
> This code snippet of mingw's stdio.h:
> 
> 
> #ifdef _WIN64
>   _CRTIMP FILE *__cdecl __iob_func(void);
> #define _iob  __iob_func()
> #else
> [.....]
> #if (!defined(NO_OLDNAMES) || defined(__GNUC__))
>   __MINGW_EXTENSION typedef __int64 fpos_t;
> #define _FPOSOFF(fp) ((long)(fp))
> 
> 
> ============= is modified by fixincludes to be:

<<mutilated>>

> Author: Jonas Jelten <jj@sft.mx>
> Date:   Wed May 31 15:10:52 2019 +0200
> 
>     fixincludes: check for lineending to not break mingw64-build
> 
> --- a/fixincludes/inclhack.def  2019-05-31 15:01:31.841235934 +0200
> +++ b/fixincludes/inclhack.def  2019-05-31 15:09:31.068347492 +0200
> @@ -2163,7 +2163,7 @@
>  fix = {
>      hackname = hpux10_stdio_declarations;
>      files    = stdio.h;
> -    select   = "^#[ \t]*define _iob[ \t]*__iob";
> +    select   = "^#[ \t]*define _iob[ \t]*__iob[ \t]*\n";
>      bypass   = "^[ \t]*extern[ \t]*int[ \t]*vsnprintf[ \t]*\\(";
>      c_fix     = format;
>      c_fix_arg = "%0\n\n"
> --- a/fixincludes/fixincl.x     2019-05-31 15:13:20.430156243 +0200
> +++ b/fixincludes/fixincl.x     2019-05-31 15:13:24.953156662 +0200
> @@ -4272,7 +4272,7 @@
>   *  content selection pattern - do fix if pattern found
>   */
>  tSCC zHpux10_Stdio_DeclarationsSelect0[] =
> -       "^#[ \t]*define _iob[ \t]*__iob";
> +       "^#[ \t]*define _iob[ \t]*__iob[ \t]*\n";
> 
>  /*
>   *  content bypass pattern - skip fix if pattern found

Looks reasonable to me, but I don't see a test case.

> #define _iob  __iob_func()

should be added to the test code and validated that it remains
unmodified in the output.
Bruce Korb June 11, 2019, 8:38 p.m. UTC | #5
On 5/31/19 12:15 PM, Jonathan Wakely wrote:
>> Alright, thanks for the hint. I was just following the instructions in
>> gcc-9.1.0/fixincludes/README:
>>
>> """
>> Please also send relevant information to gcc-bugs@gcc.gnu.org,
>> gcc-patches@gcc.gnu.org and,
>> please, to me: bkorb@gnu.org.
>> """
> 
> Oh, indeed!
> Bruce, should that suggest submitting something to Bugzilla instead?
> Mails sent directly to gcc-bugs tend to get lost among all the
> automated email from Bugzilla.

Looks like old stuff that wasn't updated. I'd copied that text from
other sources and seems out of date.

I should probably update the e-address as well. Yes, I still receive
gnu.org email, but not daily. As infrequent as monthly. Can I do
something to auto-forward it to an e-address I process daily?

Finally, my gcc sources are also years out of date, so if someone is
willing to tweak the docs for me, it'd save some bother. :) I'm actually
retired now and spend more time with a camera than with code.

Cheers - Bruce
Gerald Pfeifer Aug. 12, 2019, 9:57 p.m. UTC | #6
On Tue, 11 Jun 2019, Bruce Korb wrote:
> I should probably update the e-address as well. Yes, I still receive
> gnu.org email, but not daily. As infrequent as monthly. Can I do
> something to auto-forward it to an e-address I process daily?

You can log in to fencepost.gnu.org and edit /com/mailer/aliases 
there; that is pretty straightforward once you're logged in.

> Finally, my gcc sources are also years out of date, so if someone is
> willing to tweak the docs for me, it'd save some bother. :) 

Let me know how you'd like to see fixincludes/README updated
(current version attached) and I'll take care.

> I'm actually retired now and spend more time with a camera than 
> with code.

Enjoy!

Gerald
GCC MAINTAINER INFORMATION
==========================

If you are having some problem with a system header that is either
broken by the manufacturer, or is broken by the fixinclude process,
then you will need to alter or add information to the include fix
definitions file, ``inclhack.def''.  Please also send relevant
information to gcc-bugs@gcc.gnu.org, gcc-patches@gcc.gnu.org and,
please, to me:  bkorb@gnu.org.

To make your fix, you will need to do several things:

1.  Obtain access to the AutoGen program on some platform.  It does
    not have to be your build platform, but it is more convenient.

2.  Edit "inclhack.def" to reflect the changes you need to make.
    See below for information on how to make those changes.

3.  Run the "genfixes" shell script to produce a new copy of
    the "fixincl.x" file.

4.  Rebuild the compiler and check the header causing the issue.
    Make sure it is now properly handled.  Add tests to the
    "test_text" entry(ies) that validate your fix.  This will
    help ensure that future fixes won't negate your work.
    Do *NOT* specify test text for "wrap" or "replacement" fixes.
    There is no real possibility that these fixes will fail.
    If they do, you will surely know straight away.

    NOTE:  "test_text" is interpreted by the shell as it gets
    copied into the test header.  THEREFORE you must quote
    dollar sign characters and back quotes -- unless you mean
    for them to be interpreted by the shell.

    e.g. the math_huge_val_from_dbl_max test_text needs to
    put text into both float.h and math.h, so it includes a
    back-quoted script to add text to float.h.

5.  Go into the fixincludes build directory and type, "make check".
    You are guaranteed to have issues printed out as a result.
    Look at the diffs produced.  Make sure you have not clobbered
    the proper functioning of a different fix.  Make sure your
    fix is properly tested and it does what it is supposed to do.

6.  Now that you have the right things happening, synchronize the
    $(srcdir)/tests/base directory with the $(builddir)/tests/res
    directory.  The output of "make check" will be some diffs that
    should give you some hints about what to do.

7.  Rerun "make check" and verify that there are no issues left.


MAKING CHANGES TO INCLHACK.DEF
==============================

0.  If you are not the fixincludes maintainer, please send that
    person email about any changes you may want to make.  Thanks!

1.  Every fix must have a "hackname" that is compatible with C syntax
    for variable names and is unique without regard to alphabetic case.
    Please keep them alphabetical by this name.  :-)

2.  If the problem is known to exist only in certain files, then
    identify the files with "files = " entries.  If you use fnmatch(3C)
    wild card characters in a "files" entry, be certain that the first
    "files" entry has no such character.  Otherwise, the "make check"
    machinery will attempt to create files with those characters in the
    name.  That is inconvenient.

3.  It is relatively expensive to fire off a process to fix a source
    file, therefore write apply tests to avoid unnecessary fix
    processes.  The preferred apply tests are "select", "bypass", "mach"
    "sum", and "c-test" because they are performed internally:

    * select - Run a regex on the contents of the file being considered.
               All such regex-es must match.  Matching is done with
               extended regular expressions.

    * bypass - Run a regex on the contents of the file being considered.
               No such regex may match.

    * sum    - Select a specific version of a file that has a matching
               check sum.  The BSD version of checksum ["sum(1BSD)"]
               is used.  Each "sum" entry should contain exactly three
               space separated tokens:  the sum, some number and the
               basename of the file.  The "some number" is ignored.
               If there are multiple "sum" entries, only one needs to
               match in order to pass.  For example:

                   sum = '1234 3 foobar.h';

               specifies that the "foobar.h" header in any directory
               will match if it has the checksum 1234.

    * c_test - call a function in fixtests.c.  See that file.

    * files  - the "fnmatch" pattern of the file(s) to examine for
               the issue.  There may be several copies of this attribute.
               If the header lives in a /usr/include subdirectory, be
               sure to include that subdirectory in the name. e.g. net/if.h

    * mach   - Match the output of config.guess against a series of fnmatch
               patterns.  It must match at least one of the patterns, unless
               "not-machine" has also been specified.  In that case, the
               config.guess output must not match any of the patterns.

    The next test is relatively slow because it must be handled in a
    separate shell process.  Some platforms do not support server shells,
    so the whole process is even slower and more cumbersome there.

    * test   - These should be arguments to the program, "/bin/test".
               You may perform multiple commands, if you enclose them
               in backquotes and echo out valid test arguments.  For
               example, you might echo out '0 -eq 1' if you want a false
               result, or '0 -eq 0' for a true result.

    These tests are required to:

    1.  Be positive for all header files that require the fix.

    It is desirable to:

    2.  Be negative as often as possible whenever the fix is not
        required, avoiding the process overhead.

    It is nice if:

    3.  The expression is as simple as possible to both
        process and understand by people.  :-)

        Please take advantage of the fact AutoGen will glue
        together string fragments.  It helps.  Also take note
        that double quote strings and single quote strings have
        different formation rules.  Double quote strings are a
        tiny superset of ANSI-C string syntax.  Single quote
        strings follow shell single quote string formation
        rules, except that the backslash is processed before
        '\\', '\'' and '#' characters (using C character syntax).

    Each test must pass or the fix is not applied.  For example,
    all "select" expressions must be found and not one "bypass"
    selection may be found.

    Examples of test specifications:

      hackname = broken_assert_stdio;
      files    = assert.h;
      select   = stderr;
      bypass   = "include.*stdio.h";

    The ``broken_assert_stdio'' fix will be applied only to a file
    named "assert.h" if it contains the string "stderr" _and_ it
    does _not_ contain the expression "include.*stdio.h".

      hackname = no_double_slash;
      c_test   = "double_slash";

    The ``no_double_slash'' fix will be applied if the
    ``double_slash_test()'' function says to.  See ``fixtests.c''
    for documentation on how to include new functions into that
    module.

4.  There are currently four methods of fixing a file:

    1.  a series of sed expressions.  Each will be an individual
        "-e" argument to a single invocation of sed.  Unless you
        need to use multiple or complex sed expressions, please
        use the "replacement text" method instead.

    2.  a shell script.  These scripts are _required_ to read all
        of stdin in order to avoid pipe stalls.  They may choose to
        discard the input.

    3.  Replacement text.  If the replacement is empty, then no
        fix is applied.  Otherwise, the replacement text is
        written to the output file and no further fixes are
        applied.  If you really want a no-op file, replace the
        file with a comment.

        Replacement text "fixes" must be first in this file!!

    4.  A C language subroutine method for both tests and fixes.
        See ``fixtests.c'' for instructions on writing C-language
        applicability tests and ``fixfixes.c'' for C-language fixing.
        These files also contain tables that describe the currently
        implemented fixes and tests.

    If at all possible, you should try to use one of the C language
    fixes as it is far more efficient.  There are currently five
    such fixes, three of which are very special purpose:

    i) char_macro_def - This function repairs the definition of an
        ioctl macro that presumes CPP macro substitution within
        pairs of single quote characters.

    ii) char_macro_use - This function repairs the usage of ioctl
        macros that no longer can wrap an argument with single quotes.

    iii) machine_name - This function will look at "#if", "#ifdef",
        "#ifndef" and "#elif" directive lines and replace the first
        occurrence of a non-reserved name that is traditionally
        pre-defined by the native compiler.

    The next two are for general use:

    iv) wrap - wraps the entire file with "#ifndef", "#define" and
        "#endif" self-exclusionary text.  It also, optionally, inserts
        a prolog after the "#define" and an epilog just before the
        "#endif".  You can use this for a fix as follows:

            c_fix     = wrap;
            c_fix_arg = "/* prolog text */";
            c_fix_arg = "/* epilog text */";

        If you want an epilog without a prolog, set the first "c_fix_arg"
        to the empty string.  Both or the second "c_fix_arg"s may be
        omitted and the file will still be wrapped.

	THERE IS A SPECIAL EXCEPTION TO THIS, HOWEVER:

	If the regular expression '#if.*__need' is found, then it is
	assumed that the file needs to be read and interpreted more
	than once.  However, the prolog and epilog text (if any) will
	be inserted.

    v) format - Replaces text selected with a regular expression with
        a specialized formating string.  The formatting works as follows:
        The format text is copied to the output until a '%' character
        is found.  If the character after the '%' is another '%', then
        one '%' is output and processing continues.  If the following
        character is not a digit, then the '%' and that character are
        copied and processing continues.  Finally, if the '%' *is*
        followed by a digit, that digit is used as an index into the
        regmatch_t array to replace the two characters with the matched
        text.  i.e.: "%0" is replaced by the full matching text, "%1"
        is the first matching sub-expression, etc.

        This is used as follows:

            c_fix     = format;
            c_fix_arg = "#ifndef %1\n%0\n#endif";
            c_fix_arg = "#define[ \t]+([A-Z][A-Z0-9a-z_]*).*";

        This would wrap a one line #define inside of a "#ifndef"/"#endif"
        pair.  The second "c_fix_arg" may be omitted *IF* there is at least
        one select clause and the first one identifies the text you wish to
        reformat.  It will then be used as the second "c_fix_arg".  You may
        delete the selected text by supplying an empty string for the
        replacement format (the first "c_fix_arg").

	Note: In general, a format c_fix may be used in place of one
	sed expression.  However, it will need to be rewritten by
	hand.  For example:

	sed = 's@^#if __GNUC__ == 2 && __GNUC_MINOR__ >= 7$'
	       '@& || __GNUC__ >= 3@';

	may be rewritten using a format c_fix as:

	c_fix     = format;
	c_fix_arg = '%0 || __GNUC__ >= 3';
	c_fix_arg = '^#if __GNUC__ == 2 && __GNUC_MINOR__ >= 7$';

	Multiple sed substitution expressions probably ought to remain sed
	expressions in order to maintain clarity.  Also note that if the
	second sed expression is the same as the first select expression,
	then you may omit the second c_fix_arg.  The select expression will
	be picked up and used in its absence.

EXAMPLES OF FIXES:
==================

      hackname = AAA_ki_iface;
      replace; /* empty replacement -> no fixing the file */

    When this ``fix'' is invoked, it will prevent any fixes
    from being applied.

    ------------------

      hackname = AAB_svr4_no_varargs;
      replace  = "/* This file was generated by fixincludes.  */\n"
                 "#ifndef _SYS_VARARGS_H\n"
                 "#define _SYS_VARARGS_H\n\n"

                 "#ifdef __STDC__\n"
                 "#include <stdarg.h>\n"
                 "#else\n"
                 "#include <varargs.h>\n"
                 "#endif\n\n"

                 "#endif  /* _SYS_VARARGS_H */\n";

    When this ``fix'' is invoked, the replacement text will be
    emitted into the replacement include file.  No further fixes
    will be applied.

    ------------------

        hackname  = hpux11_fabsf;
        files     = math.h;
        select    = "^[ \t]*#[ \t]*define[ \t]+fabsf\\(.*";
        bypass    = "__cplusplus";

        c_fix     = format;
        c_fix_arg = "#ifndef __cplusplus\n%0\n#endif";

        test_text =
        "#  define fabsf(x) ((float)fabs((double)(float)(x)))\n";

    This fix will ensure that the #define for fabs is wrapped
    with C++ protection, providing the header is not already
    C++ aware.

    ------------------

5.  Testing fixes.

    The brute force method is, of course, to configure and build
    GCC.  But you can also:

        cd ${top_builddir}/gcc
        rm -rf include-fixed/ stmp-fixinc
        make stmp-fixinc

    I would really recommend, however:

        cd ${top_builddir}/fixincludes
        make check

    To do this, you *must* have autogen installed on your system.
    The "check" step will proceed to construct a shell script that
    will exercise all the fixes, using the sample test_text
    provided with each fix.  Once done, the changes made will
    be compared against the changes saved in the source directory.
    If you are changing the tests or fixes, the change will likely
    be highlighted.
diff mbox series

Patch

--- a/fixincludes/inclhack.def  2019-05-31 15:01:31.841235934 +0200
+++ b/fixincludes/inclhack.def  2019-05-31 15:09:31.068347492 +0200
@@ -2163,7 +2163,7 @@ 
 fix = {
     hackname = hpux10_stdio_declarations;
     files    = stdio.h;
-    select   = "^#[ \t]*define _iob[ \t]*__iob";
+    select   = "^#[ \t]*define _iob[ \t]*__iob[ \t]*\n";
     bypass   = "^[ \t]*extern[ \t]*int[ \t]*vsnprintf[ \t]*\\(";
     c_fix     = format;
     c_fix_arg = "%0\n\n"
--- a/fixincludes/fixincl.x     2019-05-31 15:13:20.430156243 +0200
+++ b/fixincludes/fixincl.x     2019-05-31 15:13:24.953156662 +0200
@@ -4272,7 +4272,7 @@ 
  *  content selection pattern - do fix if pattern found
  */
 tSCC zHpux10_Stdio_DeclarationsSelect0[] =
-       "^#[ \t]*define _iob[ \t]*__iob";
+       "^#[ \t]*define _iob[ \t]*__iob[ \t]*\n";

 /*
  *  content bypass pattern - skip fix if pattern found