[Bug modula2/108182] New: gm2 driver mishandles target and multilib options

[Bug modula2/108182] New: gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

            Bug ID: 108182
           Summary: gm2 driver mishandles target and multilib options
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: modula2
          Assignee: gaius at gcc dot gnu.org
          Reporter: iains at gcc dot gnu.org
  Target Milestone: ---

The language_specific_driver for modula 2 is treating every command-line option
beginning with '-m' as a multilib specifier, also expecting that the following
characters are the name of the multilib_os_dir.

This is not correct:
 1. there are many other target options beginning with '-m' (e.g. -march=)

 2. the characters following -m are not usually the multilib_os_dir.

The language_specific_driver code needs to translate the command line options
appropriately to determine the multilib_os_dir (there is code that does this in
gcc/gcc.cc)

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gaius at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-12-19

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Even some -f options are mutlilib'ed.
Eg.:
cr16/t-cr16:MULTILIB_OPTIONS     = fPIC mint32

And MIPS does EL/EB:
mips/t-img-linux:MULTILIB_OPTIONS = mips64r6 mabi=64 EL

So this is definitely a bad assumption.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

--- Comment #2 from Iain Sandoe <iains at gcc dot gnu.org> ---
computing the multilib_os_dir in the language driver is not going to be
easy/reliably correct, since that code is called very early and the specs
applied later could well modify the command line options.

On IRC I had suggested that to mitigate thus it might be possible to defer the
computation by using the "%M" spec to insert the relevant multilib_os_dir
entries.  However, that would mean moving the production of the -I command line
entries to a language spec.   At the moment I cannot see how to implement that
- since there are no specs to substitute for $libdir etc.

So, at present, ISTM that the most reliable approach would be to follow what
the c-fmaily does and compute the language-specific include paths early in the
front end.

----

As noted in PR108259, the '-L' entries are not, in fact, needed (actually they
break the discovery of the shared libraries) so that this part of the process
can be simplified (and there is no need to consider the pre-link callback we
were discussing).

It seems increasingly likely that simplifying the library to a single runtime
with the process of deciding which APIs are available decided by the FE is
probably going to resolve a bunch of difficulties and simplify the driver at
the same time (0.02GBP, only, no patches at this time).

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[gaius at gcc dot gnu.org]

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

--- Comment #3 from Gaius Mulley <gaius at gcc dot gnu.org> ---
I believe I have a fix for the target options.  I've built gm2 as a cross
compiler for the avr series of microprocessors with the fix and now get a
cc1gm2 ICE (after the correct arguments are presented to cc1gm2 :-).  Possibly
due the separate ID space and scaffold issue?   Anyway I'll attach the proposed
patches to this PR shortly.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[gaius at gcc dot gnu.org]

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

--- Comment #4 from Gaius Mulley <gaius at gcc dot gnu.org> ---
Created attachment 54184
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54184&action=edit
Potential fix for target multilib_dir handling -m and -f.

Work in progress.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Gaius Mulley from comment #4)
> Created attachment 54184 [details]
> Potential fix for target multilib_dir handling -m and -f.
> 
> Work in progress.

1. (I think) the string you need is "multilib_os_dir" not "multilib_dir" (but
ICBW about that .. there are many twisty passages in the determination of these
things).

2. The concern remains that the language spec handler is run pretty much first
in the stack and there are several other claims on the command line that could
have specs that alter (or add) flags to the command line, giving a different
outcome to the computation of mulilib_{,os_}dir.

perhaps it would be a good idea to post this and ask the opinion of someone
like Joseph on the direction.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #5)
> (In reply to Gaius Mulley from comment #4)
> > Created attachment 54184 [details]
> > Potential fix for target multilib_dir handling -m and -f.
> > 
> > Work in progress.
> 
> 1. (I think) the string you need is "multilib_os_dir" not "multilib_dir"
> (but ICBW about that .. there are many twisty passages in the determination
> of these things).

It seems that I was wrong about this ... at least on
 x86_65-linux-gnu (which does use MULTILIB_OSDIRNAMES) and
 *-darwin* (which does not).

It seems to DTRT.
(tested on linux with a 32b multilib)

> 2. The concern remains that the language spec handler is run pretty much
> first in the stack and there are several other claims on the command line
> that could have specs that alter (or add) flags to the command line, giving
> a different outcome to the computation of mulilib_{,os_}dir.
> 
> perhaps it would be a good idea to post this and ask the opinion of someone
> like Joseph on the direction.

So ... the proposed patch seems to fix the problem for Darwin and X86_64 Linux.
definitely post it!

(we still need to fix the issue with the shared libraries hidden by the
additional -Ls but that's a one-line patch)

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[gaius at gcc dot gnu.org]

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

--- Comment #7 from Gaius Mulley <gaius at gcc dot gnu.org> ---
About to post - I've cleaned up the patch and modified it not to skip the -m
option.  It is fine on x86_64 gnu/linux - it would be great to know I've not
broken anything on Darwin :-)

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[gaius at gcc dot gnu.org]

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

--- Comment #8 from Gaius Mulley <gaius at gcc dot gnu.org> ---
Created attachment 54208
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54208&action=edit
Potential fix for target multilib_dir handling -m and -f (version 2)

Cleaned up and revised patch (no skip on -m).

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[gaius at gcc dot gnu.org]

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

--- Comment #9 from Gaius Mulley <gaius at gcc dot gnu.org> ---
Created attachment 54214
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54214&action=edit
Potential fix for target multilib_dir handling -m and -f (version 3)

version3 work in progress, implemented using lang-specs.h and gm2spec.cc.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[gaius at gcc dot gnu.org]

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

--- Comment #10 from Gaius Mulley <gaius at gcc dot gnu.org> ---
here is version 4 of the bugfix which enables the driver to link against shared
libraries.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54184|0                           |1
        is obsolete|                            |
  Attachment #54208|0                           |1
        is obsolete|                            |
  Attachment #54214|0                           |1
        is obsolete|                            |

--- Comment #11 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 54220
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54220&action=edit
patch version 3.1


This is patch v3 + some specific changes. [hence it is 3.1 :) ]

The main issues with v3 (and v4 on PR108261) are:

 - link items are positional (you need to ensure that the runtime libraries
appear after the user's objects).
 - adding {f*} to the cc1gm2 line causes f options to be duplicated, this could
(potentially alter the behaviour of the command line when final values of
opposite switches are used - which is the 'usual' mechanism).
 - V3 was still adding the '-L' options for the various libraries which are not
needed (v4 fixes this, but not the other issues)

 - Supporting the target's ability to handle -Bstatic/dynamic in specs is going
to be hard.

---- so ... 


1. we use the specs now to insert the include paths; this works very nicely.

2. we use the existing sequencing the language-driver to ensure that the link
positional arguments are in the right places (and to handle the Bstatic/dynamic
stuff)

3. We remove the {f*} from the cc1gm2 spec [note it is possible that other
similar  entries will cause duplication of their content .. I did not check
this yet]

This means that we can drop the linker-related extra specs and code (and
actually simplify things a bit in the lang-specific driver).

4. We skip options that we will re-insert to avoid duplication there too.

-----

NOTE: with specs, it is usually necessary to ensure that they being and/or end
with whitespace because they can be arbitrarily concatenated.

-----

This does not fix PR108261 (neither does v4, FWIW) on Darwin.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

--- Comment #12 from Iain Sandoe <iains at gcc dot gnu.org> ---

unfortunately, neither this nor the v4.1 (WIP) is still quite right.

Using LIBDIR in the computation of the include paths means that the compiler
does not work when it is relocated .. the directory prefix needs to be computed
at runtime.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

--- Comment #13 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 54248
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54248&action=edit
Revised fix

This essentially makes Modula-2 build its include paths in the Front End (which
is how all the other compilers in GCC work too).

the huge advantages there are that the prefix and multilib info are all
available (as is the sysroot) with no changes needed to gcc/gcc.cc
.. + the prefix correctly follows relocation of the compiler.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54220|0                           |1
        is obsolete|                            |
  Attachment #54248|0                           |1
        is obsolete|                            |

--- Comment #14 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 54261
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54261&action=edit
Updated patch that honours the order of include use.

This is V5 ...

... so here we collect the incoming search paths and then apply (most of) the
rules in 
https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html#Directory-Options

... TODO:
 - eliminate duplicate -I entries that have a system one
 - figure out the right way to implement -nostdinc 
 - handle -idirafter

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

--- Comment #15 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #14)
> Created attachment 54261 [details]
> Updated patch that honours the order of include use.
> 
> This is V5 ...
> 
> ... so here we collect the incoming search paths and then apply (most of)
> the rules in 
> https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html#Directory-Options
> 
> ... TODO:
>  - eliminate duplicate -I entries that have a system one
>  - figure out the right way to implement -nostdinc 
>  - handle -idirafter

add this missed hunk.

diff --git a/gcc/m2/gm2-lang.cc b/gcc/m2/gm2-lang.cc
index 073d1dd55db..cd59784d7b2 100644
--- a/gcc/m2/gm2-lang.cc
+++ b/gcc/m2/gm2-lang.cc
@@ -513,6 +513,7 @@ add_m2_import_paths (const char *liblist)
     {
       char *libname = (char *) alloca (6);
       strncpy (libname, liblist, 5);
+      libname[5] = 0;
       add_one_import_path (libname);
       liblist += 5;
       if (*liblist == ',')

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54261|0                           |1
        is obsolete|                            |
           Assignee|gaius at gcc dot gnu.org           |iains at gcc dot gnu.org

--- Comment #16 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 54290
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54290&action=edit
Patch to be posted,

This is the patch I intend to post (here in case anyone is testing)

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[cvs-commit at gcc dot gnu.org]

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

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:47b269caf87904fd0112e8c9e96884dd0313ed15

commit r13-5308-g47b269caf87904fd0112e8c9e96884dd0313ed15
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Wed Jan 11 10:22:34 2023 +0000

    modula-2, driver, Front end: Revise handling of I and L paths [PR108182].

    The adds the includes in the FE as done in other GCC languages.
    It also revises the library handling to avoid additional -L options
    from hiding LIBDIR.

    For the include/import paths as presented to the front end initialisation,
    we capture them and then arrange to emit the 'standard library' paths in
    the same order as specified for C.

    The specs are tidied up.

    The use of the internal prefix also fixes searching in a relocated
compiler.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR modula2/108182
            PR modula2/108480

    gcc/m2/ChangeLog:

            * Make-lang.in: Pass libsubdir to the language init
            build.
            * gm2-lang.cc (INCLUDE_VECTOR): Define.
            (add_one_import_path): New.
            (add_m2_import_paths): New.
            (gm2_langhook_post_options): Arrange to add the include
            paths (and add the system ones) in the same order as C
            uses.
            * gm2spec.cc (build_archive_path): Remove.
            (add_default_combination): Remove.
            (add_default_archives): Remove.
            (add_default_libs): We no longer need a '-L' option, just
            emit the -l and each library in use.
            (build_include_path): Remove.
            (add_include): Remove.
            (add_default_includes): Remove.
            (library_installed): Remove.
            (check_valid_library): Remove.
            (check_valid_list): Remove.
            (convert_abbreviation): Diagnose unhandled cases.
            (lang_specific_driver): Skip options where we will add back
            a validated version.
            * lang-specs.h (M2CPP): Reformat, append %I when -fcpp is not
            in use.  Revise the cc1gm2 spec to omit mentioning options that
            are handled in the c pre-processor line.
            * lang.opt: Allow preprocessing and path options as input to the
            cc1gm2 invocation, so that they can be passed to the preprocessor
            invocation.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=102343

--- Comment #18 from Iain Sandoe <iains at gcc dot gnu.org> ---
there are still fixes needed - to the passing of parameters to C preprocessor
jobs that ensures the multilib settings are correct there .. see PR102343 for
the candidate patch.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[cvs-commit at gcc dot gnu.org]

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

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:80cf2c5e8f496bed9c6facf55f9ae31d0d90fd28

commit r13-5373-g80cf2c5e8f496bed9c6facf55f9ae31d0d90fd28
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Mon Jan 16 14:07:20 2023 +0000

    modula-2: Fixes for preprocessing [PR102343, PR108182].

    Modula-2 uses the C preprocessor to implement handling for conditional
    code and macros.  However, this is not done directly, because the process
    is applied recursively to imported definitions and modules.

    The cc1gm2 executable records the parameters as a template command line
    needed to create a composite 'cc1 -E' for each file to be preprocessed
    starting with the main file from the original command line.

    This patch fixes the capture of the C preprocessor template to include
    the target information needed for correct multilib operation.

    In order to match the existing semantics of '-E, -M and -MM' these have
    to be handled as a 'pre-processor only' job (i.e. the recursion is omitted
    and only the main file is processed).

    Whereas C-family front ends always pre-process, Modula-2 only does so
    when specifically requested (via the -fcpp option).

    '-MD, -MMD and -MQ' also require special handling, since (in principle)
    these options can be applied to any command line (with -fcpp) providing
    dependency information as a by-product.

    TODO: the preprocessor is not able to determine def and mod dependencies
    for Modula-2 and so the output of this only shows the object to module
    dep.  We should be able to append the .def and .mod dependencies.

    The patch amends save-temps handling to cater for the preprocessor
    recursion and to avoid writing saved files into the source directories.

    The patch changes the extension for Modula-2 preprocessed source to .m2i
    to avoid clashes with .i.

    The main driver code is amended to add default handlers for .mod and .m2i
    so that a useful error message will be emitted if the Modula-2 compiler
    is not built-in.

    The compiler will now also handle code generation from a .m2i preprocessed
    source.

    TODO: We should not need to pass the '-c' option to the compiler to alter
    the processing of init code.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR modula2/102343
            PR modula2/108182

    gcc/ChangeLog:

            * gcc.cc: Provide default specs for Modula-2 so that when the
            language is not built-in better diagnostics are emitted for
            attempts to use .mod or .m2i file extensions.

    gcc/m2/ChangeLog:

            * gm2-compiler/M2Comp.mod: Early exit for pre-processor-only jobs.
            * gm2-compiler/M2Options.def (SetPPOnly, GetPPOnly, SetMD, GetMD,
            SetMMD, GetMMD, SetMQ, GetMQ, SetObj, GetObj, SetDumpDir,
            GetDumpDir):New.
            * gm2-compiler/M2Options.mod:(SetPPOnly, GetPPOnly, SetMD, GetMD,
            SetMMD, GetMMD, SetMQ, GetMQ, SetObj, GetObj, SetDumpDir,
            GetDumpDir):New.
            * gm2-compiler/M2Preprocess.def (PreprocessModule): Add flag to
            indicate the main file.
            * gm2-compiler/M2Preprocess.mod: Handle Preprocess-only jobs,
            handle MD, MMD and MQ options.
            * gm2-gcc/m2options.h (M2Options_SetPPOnly, M2Options_GetPPOnly,
            M2Options_SetDumpDir, M2Options_SetMD, M2Options_GetMD,
            M2Options_SetMMD, M2Options_GetMMD, M2Options_SetMQ,
M2Options_GetMQ,
            M2Options_SetObj, M2Options_GetObj): New.
            * gm2-gcc/m2type.cc (m2type_InitBaseTypes): Early exit for pre-
            processor-only jobs.
            * gm2-lang.cc (gm2_langhook_init): Handle preprocess-only commands.
            (gm2_langhook_option_lang_mask): Claim C and Driver options so that
            we can intercept them for building pre-processor commands.
            (gm2_langhook_init_options): Collect the preprocessor line here.
            Save options that have different actions for preprocessor and
compile
            commands.
            (gm2_langhook_handle_option): Only handle the modula-2 options
here.
            (gm2_langhook_post_options): Do not create a back-end for pre-
            processor-only jobs.
            * gm2spec.cc (lang_specific_driver): Ignore PCH options, append a
            scaffold-main for cases where we are building a main module with
            -c.
            * lang-specs.h: Revise to handle preprocessor-only jobs and to
            consume pre-processed files.
            * lang.opt: Remove Driver and C options copies (we claim these
            separately).

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #20 from Iain Sandoe <iains at gcc dot gnu.org> ---
there are undoubtedly improvements we can make to the driver - but in terms of
basic correctness, this can be considered fixed on trunk.

[Bug modula2/108182] gm2 driver mishandles target and multilib options

[cvs-commit at gcc dot gnu.org]

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

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:83979240c072599bc8595a9324c3f4371eedbc7c

commit r13-5426-g83979240c072599bc8595a9324c3f4371eedbc7c
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Jan 26 13:49:11 2023 +0000

    Modula-2: Add claimed command line options to lang.opt [PR108555].

    This is a partial reversion of the changes in r13-5373-g80cf2c5e8f496b.

    As observed in the PR, handling the C and Driver options in the Modula-2
    lang-specific code could be difficult to emulate; This reverts to adding
    the required options to the language-specific .opt file.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR modula2/108555
            PR modula2/108182
            PR modula2/102343

    gcc/m2/ChangeLog:

            * gm2-lang.cc (gm2_langhook_option_lang_mask): Do not claim CL_C
            or CL_DRIVER.
            (gm2_langhook_init_options): Handle options that we want to pass
            to the preprocessor.
            * lang-specs.h: Pass -B and -save-temps to regular compile lines.
            * lang.opt: Add C and Driver options that Modula-2 intercepts for
            internal use. Reorder options into two sections and to collate.