driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850]

Message ID CAHyHGCnvvm=9dvGFYebmKw_jo+S7NfmWERb0ZWNOsYiCaX+ynA@mail.gmail.com
State New
Headers
Series driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850] |

Commit Message

Costas Argyris March 2, 2023, 7:25 p.m. UTC
  This is a proposal to fix PR71850 by applying the existing logic for
passing include paths to cc1 to as.

Thanks,
Costas
  

Comments

Costas Argyris March 9, 2023, 1:39 p.m. UTC | #1
Pinging list and driver reviewer.

Details here:

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

On Thu, 2 Mar 2023 at 19:25, Costas Argyris <costas.argyris@gmail.com>
wrote:

> This is a proposal to fix PR71850 by applying the existing logic for
> passing include paths to cc1 to as.
>
> Thanks,
> Costas
>
  
Costas Argyris March 20, 2023, 9:47 a.m. UTC | #2
ping

On Thu, 9 Mar 2023 at 13:39, Costas Argyris <costas.argyris@gmail.com>
wrote:

> Pinging list and driver reviewer.
>
> Details here:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71850
>
> On Thu, 2 Mar 2023 at 19:25, Costas Argyris <costas.argyris@gmail.com>
> wrote:
>
>> This is a proposal to fix PR71850 by applying the existing logic for
>> passing include paths to cc1 to as.
>>
>> Thanks,
>> Costas
>>
>
  
Costas Argyris March 27, 2023, 9:36 a.m. UTC | #3
[ping^3]

This looks like it fixes the bug and also unifies the way include paths are
passed from the driver to the compiler and assembler (when a @file has
been passed to the driver in the first place).

That is, when @file has been passed to the driver, put the include paths
in a temp @file and pass them to the assembler.    Note this is already
happening for the compiler, so this patch merely extends this logic to the
assembler.

Is there any reason not to go for it?

On Mon, 20 Mar 2023 at 09:47, Costas Argyris <costas.argyris@gmail.com>
wrote:

> ping
>
> On Thu, 9 Mar 2023 at 13:39, Costas Argyris <costas.argyris@gmail.com>
> wrote:
>
>> Pinging list and driver reviewer.
>>
>> Details here:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71850
>>
>> On Thu, 2 Mar 2023 at 19:25, Costas Argyris <costas.argyris@gmail.com>
>> wrote:
>>
>>> This is a proposal to fix PR71850 by applying the existing logic for
>>> passing include paths to cc1 to as.
>>>
>>> Thanks,
>>> Costas
>>>
>>
  
Xi Ruoyao March 27, 2023, 9:59 a.m. UTC | #4
On Mon, 2023-03-27 at 10:36 +0100, Costas Argyris via Gcc-patches wrote:
> [ping^3]
> 
> This looks like it fixes the bug and also unifies the way include paths are
> passed from the driver to the compiler and assembler (when a @file has
> been passed to the driver in the first place).
> 
> That is, when @file has been passed to the driver, put the include paths
> in a temp @file and pass them to the assembler.    Note this is already
> happening for the compiler, so this patch merely extends this logic to the
> assembler.
> 
> Is there any reason not to go for it?

It's not supported by all GNU assembler releases.  For example, GCC
installation doc says we require Binutils >= 2.13.1 for i?86-*-linux*.
Binutils 2.13.1 was released in 2002, but @FILE support was added into
Binutils in 2005.
> >
  
Costas Argyris March 27, 2023, 10:11 a.m. UTC | #5
Would it be possible to make it version-dependent, then?

As in, if GNU assembler is greater or equal to the version that
supports @FILE, then pass @FILE to it, otherwise fall back to
the current behavior.

I assume most people nowadays would have a version of
Binutils later than 2005, but if we could make it conditional on
the version then even those with earlier version wouldn't break,
they would just get the current behavior.

On Mon, 27 Mar 2023 at 11:00, Xi Ruoyao <xry111@xry111.site> wrote:

> On Mon, 2023-03-27 at 10:36 +0100, Costas Argyris via Gcc-patches wrote:
> > [ping^3]
> >
> > This looks like it fixes the bug and also unifies the way include paths
> are
> > passed from the driver to the compiler and assembler (when a @file has
> > been passed to the driver in the first place).
> >
> > That is, when @file has been passed to the driver, put the include paths
> > in a temp @file and pass them to the assembler.    Note this is already
> > happening for the compiler, so this patch merely extends this logic to
> the
> > assembler.
> >
> > Is there any reason not to go for it?
>
> It's not supported by all GNU assembler releases.  For example, GCC
> installation doc says we require Binutils >= 2.13.1 for i?86-*-linux*.
> Binutils 2.13.1 was released in 2002, but @FILE support was added into
> Binutils in 2005.
> > >
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
>
  
Costas Argyris April 17, 2023, 8:29 a.m. UTC | #6
Pinging list one last time about this.

Proposed fix is to simply pass include paths to GNU AS through a @file, if
a @file has been provided to gcc in the first place:

https://gcc.gnu.org/bugzilla/attachment.cgi?id=54575&action=diff

That is, simply change %{I*} to %@{I*} in asm_options.

This mimics what is already happening for cpp_unique_options, and avoids
overflowing the windows command line
when many include paths are passed to gcc through @file.

On Mon, 27 Mar 2023 at 11:11, Costas Argyris <costas.argyris@gmail.com>
wrote:

> Would it be possible to make it version-dependent, then?
>
> As in, if GNU assembler is greater or equal to the version that
> supports @FILE, then pass @FILE to it, otherwise fall back to
> the current behavior.
>
> I assume most people nowadays would have a version of
> Binutils later than 2005, but if we could make it conditional on
> the version then even those with earlier version wouldn't break,
> they would just get the current behavior.
>
> On Mon, 27 Mar 2023 at 11:00, Xi Ruoyao <xry111@xry111.site> wrote:
>
>> On Mon, 2023-03-27 at 10:36 +0100, Costas Argyris via Gcc-patches wrote:
>> > [ping^3]
>> >
>> > This looks like it fixes the bug and also unifies the way include paths
>> are
>> > passed from the driver to the compiler and assembler (when a @file has
>> > been passed to the driver in the first place).
>> >
>> > That is, when @file has been passed to the driver, put the include paths
>> > in a temp @file and pass them to the assembler.    Note this is already
>> > happening for the compiler, so this patch merely extends this logic to
>> the
>> > assembler.
>> >
>> > Is there any reason not to go for it?
>>
>> It's not supported by all GNU assembler releases.  For example, GCC
>> installation doc says we require Binutils >= 2.13.1 for i?86-*-linux*.
>> Binutils 2.13.1 was released in 2002, but @FILE support was added into
>> Binutils in 2005.
>> > >
>>
>> --
>> Xi Ruoyao <xry111@xry111.site>
>> School of Aerospace Science and Technology, Xidian University
>>
>
  

Patch

From 393aff0d006ee9372cc8b9321c612c2dfb4b0a31 Mon Sep 17 00:00:00 2001
From: Costas Argyris <costas.argyris@gmail.com>
Date: Thu, 2 Mar 2023 18:27:22 +0000
Subject: [PATCH] driver: Treat include path args the same way between
 cpp_unique_options and asm_options. [PR71850]

On Windows, when a @file with many include paths is passed to gcc, it forwards those include paths to cc1 through a temporary @file as well, so they don't end up in the command line.    This is because cpp_unique_options has %@{I* which passes -I args in a temporary file, if a temporary file was passed to the driver in the first place.

The same logic is not applied in asm_options, and this leads to the include paths being passed as command line arguments to the assembler, which causes the failure on Windows seen in PR71850.

Treating the -I args to the assembler the same way as to the compiler (that is, through a @tempfile if @file was passed to gcc) solves the issue, allowing a large number of include paths to be passed to gcc on Windows through a @file.
---
 gcc/gcc.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index becc56051a8..b1fa80cde4f 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -1278,7 +1278,7 @@  static const char *asm_options =
 #if HAVE_GNU_AS
 /* If GNU AS is used, then convert -w (no warnings), -I, and -v
    to the assembler equivalents.  */
-"%{v} %{w:-W} %{I*} "
+"%{v} %{w:-W} %@{I*} "
 #endif
 "%(asm_debug_option)"
 ASM_COMPRESS_DEBUG_SPEC
-- 
2.30.2