Hi Andrew, "Apologies, I just realized that my previous reply may not have gone through. I'm resending it for your reference."
Thank you for your response. I just wanted to clarify the issue I’m facing. The
concern is not that new files are being generated,
but rather that the LTO debug files are not being generated at all when using
the response file format inside
the set_collect_gcc_option(). (Attached : response_f.patch )
I understand that you’re suggesting adjusting the test case to accommodate the
new files, but in this case,
The files themselves aren’t being created to begin with when following this
approach.
For example : With host gcc if i pass "-flto" flag
$ gcc -O2 -flto -flto-partition=one -fdump-ipa-icf-optimized -fdump-rtl-final
-fstack-usage a.c
a.c a.c.079i.icf a.ltrans0.ltrans.337r.final a.ltrans0.ltrans.su a.out
a.wpa.079i.icf
The above debug files are not generating with Attached : response_f.patch
approach.
Could you please advise on how we might resolve this, or if there’s something
we might be missing in the configuration to
ensure the debug files are generated?
Moving the response file creation to xputenv resolves this. Please see
wrapper_lto.patch.
Would it be appropriate to send this patch? Please advise.
$make check-gcc RUNTESTFLAGS="outputs.exp"
=== gcc Summary ===
# of expected passes 2659
/home/sunild/snap/LTO_WRAPPER_gcc/build/gcc/xgcc version 15.0.0 20241016
(experimental) (GCC)
Thank you for your help!
Best regards,
Sunil Dora
________________________________
From: Andrew Pinski <pins...@gmail.com>
Sent: Monday, November 18, 2024 11:14 AM
To: Dora, Sunil Kumar <sunilkumar.d...@windriver.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>; Richard Guenther
<rguent...@suse.de>; Jeff Law <jeffreya...@gmail.com>; Joseph Myers
<josmy...@redhat.com>; MacLeod, Randy <randy.macl...@windriver.com>; Gowda,
Naveen <naveen.go...@windriver.com>
Subject: Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know
the content is safe.
On Tue, Sep 17, 2024, 3:40 AM Dora, Sunil Kumar
<sunilkumar.d...@windriver.com<mailto:sunilkumar.d...@windriver.com>> wrote:
Hi Andrew,
Initially, I thought to address long command line options (when exceeding
128KB) without disrupting the existing GCC driver behavior.
As you suggested, I implemented changes to use the response file format (@file)
within the set_collect_gcc_options function and ensured that this was passed
through COLLECT_GCC_OPTIONS.
However, these changes have introduced a side effect: they impact the behavior
of the -save-temps switch by generating additional .args.N files. As a result,
some existing test cases, including the one reported by the Linaro team, are
now failing.
(File: Attached)
Could you please advise on how we should proceed? Specifically, should we
adjust the test cases to accommodate the impact on the -save-temps switch, or
is there an alternative approach you would recommend? Your guidance on how to
address these issues while implementing the response file approach would be
greatly appreciated.
Sounds like the test case need to adjusted for the new files that are saved
now. Since we want to have this file around when using -saves-temps to be able
to reproduce what is being invoked.
Thanks,
Andrew
Thank you for your support.
Thanks,
Sunil Dora
________________________________
From: Andrew Pinski <pins...@gmail.com<mailto:pins...@gmail.com>>
Sent: Friday, September 6, 2024 11:33 PM
To: Dora, Sunil Kumar
<sunilkumar.d...@windriver.com<mailto:sunilkumar.d...@windriver.com>>
Cc: Hemraj, Deepthi
<deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com>>; GCC
Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>; Richard
Guenther <rguent...@suse.de<mailto:rguent...@suse.de>>; Jeff Law
<jeffreya...@gmail.com<mailto:jeffreya...@gmail.com>>;
josmy...@redhat.com<mailto:josmy...@redhat.com>
<josmy...@redhat.com<mailto:josmy...@redhat.com>>; MacLeod, Randy
<randy.macl...@windriver.com<mailto:randy.macl...@windriver.com>>; Gowda,
Naveen <naveen.go...@windriver.com<mailto:naveen.go...@windriver.com>>
Subject: Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know
the content is safe.
On Fri, Sep 6, 2024, 9:38 AM Dora, Sunil Kumar
<sunilkumar.d...@windriver.com<mailto:sunilkumar.d...@windriver.com>> wrote:
Hi Andrew,
Thank you for your feedback. Initially, we attempted to address the issue by
utilizing GCC’s response files. However, we discovered that the
COLLECT_GCC_OPTIONS variable already contains the expanded contents of the
response files.
As a result, using response files only mitigates the multiplication factor but
does not bypass the 128KB limit.
I think you missed understood me fully. What I was saying instead of creating a
string inside set_collect_gcc_options, create the response file and pass that
via COLLECT_GCC_OPTIONS with the @file format. And then inside
collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biGX2H76Q$>
when using COLLECT_GCC_OPTIONS/extract_string instead read in the response
file options if there was an @file instead of those 2 loops. This requires more
than what you did. Oh and should be less memory hungry and maybe slightly
faster.
Thanks,
Andrew
I have included the response file usage logs and the complete history in the
Bugzilla report for your reference: Bugzilla
Link<https://urldefense.com/v3/__https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527*c19__;Iw!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biH6f_e3g$>.
Following your suggestion, I have updated the logic to avoid hardcoding /tmp.
Please find the revised version of patch at the following link:
https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662519.html<https://urldefense.com/v3/__https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662519.html__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bgeSIJoIg$>
Thanks,
Sunil Dora
________________________________
From: Andrew Pinski <pins...@gmail.com<mailto:pins...@gmail.com>>
Sent: Friday, August 30, 2024 8:05 PM
To: Hemraj, Deepthi
<deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com>>
Cc: gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>
<gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>;
rguent...@suse.de<mailto:rguent...@suse.de>
<rguent...@suse.de<mailto:rguent...@suse.de>>;
jeffreya...@gmail.com<mailto:jeffreya...@gmail.com>
<jeffreya...@gmail.com<mailto:jeffreya...@gmail.com>>;
josmy...@redhat.com<mailto:josmy...@redhat.com>
<josmy...@redhat.com<mailto:josmy...@redhat.com>>; MacLeod, Randy
<randy.macl...@windriver.com<mailto:randy.macl...@windriver.com>>; Gowda,
Naveen <naveen.go...@windriver.com<mailto:naveen.go...@windriver.com>>; Dora,
Sunil Kumar
<sunilkumar.d...@windriver.com<mailto:sunilkumar.d...@windriver.com>>
Subject: Re: [PATCH v2] GCC Driver : Enable very long gcc command-line option
CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know
the content is safe.
On Fri, Aug 30, 2024 at 12:34 AM
<deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com>> wrote:
>
> From: Deepthi Hemraj
> <deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com>>
>
> For excessively long environment variables i.e >128KB
> Store the arguments in a temporary file and collect them back together in
> collect2.
>
> This commit patches for COLLECT_GCC_OPTIONS issue:
> GCC should not limit the length of command line passed to collect2.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527<https://urldefense.com/v3/__https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bjOT12gOQ$>
>
> The Linux kernel has the following limits on shell commands:
> I. Total number of bytes used to specify arguments must be under 128KB.
> II. Each environment variable passed to an executable must be under 128 KiB
>
> In order to circumvent these limitations, many build tools support
> response-files, i.e. files that contain the arguments for the executed
> command. These are typically passed using @<filepath> syntax.
>
> Gcc uses the COLLECT_GCC_OPTIONS environment variable to transfer the
> expanded command line to collect2. With many options, this exceeds the limit
> II.
>
> GCC : Added Testcase for PR111527
>
> TC1 : If the command line argument less than 128kb, gcc should use
> COLLECT_GCC_OPTION to communicate and compile fine.
> TC2 : If the command line argument in the range of 128kb to 2mb,
> gcc should copy arguments in a file and use FILE_GCC_OPTIONS
> to communicate and compile fine.
> TC3 : If the command line argument greater thean 2mb, gcc shuld
> fail the compile and report error. (Expected FAIL)
>
> Signed-off-by: sunil dora
> <sunilkumar.d...@windriver.com<mailto:sunilkumar.d...@windriver.com>>
> Signed-off-by: Topi Kuutela
> <topi.kuut...@nokia.com<mailto:topi.kuut...@nokia.com>>
> Signed-off-by: Deepthi Hemraj
> <deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com>>
> ---
>
> gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biGX2H76Q$>
> | 39 ++++++++++++++++++--
>
> gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bjFGXxX9w$>
> | 37 +++++++++++++++++--
> gcc/testsuite/gcc.dg/longcmd/longcmd.exp | 16 +++++++++
> gcc/testsuite/gcc.dg/longcmd/pr111527-1.c | 44 +++++++++++++++++++++++
> gcc/testsuite/gcc.dg/longcmd/pr111527-2.c | 9 +++++
> gcc/testsuite/gcc.dg/longcmd/pr111527-3.c | 10 ++++++
> gcc/testsuite/gcc.dg/longcmd/pr111527-4.c | 10 ++++++
> 7 files changed, 159 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> create mode 100644 gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
>
> diff --git
> a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biGX2H76Q$>
>
> b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biGX2H76Q$>
> index 902014a9cc1..1f56963b1ce 100644
> ---
> a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biGX2H76Q$>
> +++
> b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-biGX2H76Q$>
> @@ -376,6 +376,39 @@ typedef int scanfilter;
>
> static void scan_prog_file (const char *, scanpass, scanfilter);
>
> +char* getenv_extended (const char* var_name)
> +{
> + int file_size;
> + char* buf = NULL;
> + const char* prefix = "/tmp";
> +
> + char* string = getenv (var_name);
> + if (strncmp (var_name, prefix, strlen(prefix)) == 0)
This is not what was meant by saying using the same env and supporting
response files.
Instead what Richard meant was use `@file` as the option that gets
passed via COLLECT_GCC_OPTIONS and then if you see `@` expand the
options like what is done for the normal command line.
Hard coding "/tmp" here is wrong because TMPDIR might not be set to
"/tmp" and even more with -save-temps, the response file should stay
around afterwards and be in the working directory rather than TMPDIR.
Thanks,
Andrew Pinski
> + {
> + FILE *fptr;
> + fptr = fopen (string, "r");
> + if (fptr == NULL)
> + return (0);
> + /* Copy contents from temporary file to buffer */
> + if (fseek (fptr, 0, SEEK_END) == -1)
> + return (0);
> + file_size = ftell (fptr);
> + rewind (fptr);
> + buf = (char *) xmalloc (file_size + 1);
> + if (buf == NULL)
> + return (0);
> + if (fread ((void *) buf, file_size, 1, fptr) <= 0)
> + {
> + free (buf);
> + fatal_error (input_location, "fread failed");
> + return (0);
> + }
> + buf[file_size] = '\0';
> + return buf;
> + }
> + return string;
> +}
> +
>
> /* Delete tempfiles and exit function. */
>
> @@ -1004,7 +1037,7 @@ main (int argc, char **argv)
> /* Now pick up any flags we want early from COLLECT_GCC_OPTIONS
> The LTO options are passed here as are other options that might
> be unsuitable for ld (e.g. -save-temps). */
> - p = getenv ("COLLECT_GCC_OPTIONS");
> + p = getenv_extended ("COLLECT_GCC_OPTIONS");
> while (p && *p)
> {
> const char *q = extract_string (&p);
> @@ -1200,7 +1233,7 @@ main (int argc, char **argv)
> AIX support needs to know if -shared has been specified before
> parsing commandline arguments. */
>
> - p = getenv ("COLLECT_GCC_OPTIONS");
> + p = getenv_extended ("COLLECT_GCC_OPTIONS");
> while (p && *p)
> {
> const char *q = extract_string (&p);
> @@ -1594,7 +1627,7 @@ main (int argc, char **argv)
> fprintf (stderr, "o_file = %s\n",
> (o_file ? o_file : "not found"));
>
> - ptr = getenv ("COLLECT_GCC_OPTIONS");
> + ptr = getenv_extended ("COLLECT_GCC_OPTIONS");
> if (ptr)
> fprintf (stderr, "COLLECT_GCC_OPTIONS = %s\n", ptr);
>
> diff --git
> a/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bjFGXxX9w$>
>
> b/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bjFGXxX9w$>
> index d07a8e172a4..3d7fd8dff5b 100644
> ---
> a/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bjFGXxX9w$>
> +++
> b/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!bB1wlMkDkX1X5ykXc7p8DUV8HqhVBevZye8j1QQ39i0QeMJNHHaWWqdg5GbO8D4cL9UiiNe1PmcL-bjFGXxX9w$>
> @@ -2953,12 +2953,43 @@ add_to_obstack (char *path, void *data)
> return NULL;
> }
>
> -/* Add or change the value of an environment variable, outputting the
> - change to standard error if in verbose mode. */
> +/* Add or change the value of an environment variable,
> + * outputting the change to standard error if in verbose mode. */
> static void
> xputenv (const char *string)
> {
> - env.xput (string);
> + static const size_t MAX_ENV_VAR_LEN = 128*1024;
> + const char *prefix = "COLLECT_GCC_OPTIONS=";
> + size_t prefix_len = strlen (prefix);
> + FILE *fptr;
> +
> + const size_t string_length = strlen (string);
> + if ( string_length < MAX_ENV_VAR_LEN )
> + {
> + env.xput (string);
> + return;
> + }
> + /* For excessively long environment variables i.e >128KB
> + Store the arguments in a temporary file and collect
> + them back together in collect2 */
> + if (strncmp (string, prefix, prefix_len) == 0)
> + {
> + const char *data_to_write = string + prefix_len;
> + char *temp_file = make_at_file ();
> + fptr = fopen (temp_file, "w");
> + if (fptr == NULL)
> + {
> + fatal_error (input_location, "Cannot open temporary file");
> + return;
> + }
> + /* Copy contents into temporary file */
> + fwrite (data_to_write, sizeof(char), strlen(data_to_write), fptr);
> + char *env_val = (char *) xmalloc (strlen (temp_file) + 21);
> + sprintf (env_val, "COLLECT_GCC_OPTIONS=%s", temp_file);
> + env.xput (env_val);
> + record_temp_file (temp_file, !save_temps_flag, !save_temps_flag);
> + fclose (fptr);
> + }
> }
>
> /* Build a list of search directories from PATHS.
> diff --git a/gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> b/gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> new file mode 100644
> index 00000000000..bac8b6d6fff
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/longcmd/longcmd.exp
> @@ -0,0 +1,16 @@
> +# GCC testsuite that uses the `dg.exp' driver.
> +# Load support procs.
> +load_lib gcc-dg.exp
> +
> +# If a testcase doesn't have special options, use these.
> +global DEFAULT_CFLAGS
> +if ![info exists DEFAULT_CFLAGS] then {
> + set DEFAULT_CFLAGS ""
> +}
> +
> +dg-init
> +
> +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
> + "" $DEFAULT_CFLAGS
> +
> +dg-finish
> diff --git a/gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> b/gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> new file mode 100644
> index 00000000000..a5373f57790
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/longcmd/pr111527-1.c
> @@ -0,0 +1,44 @@
> +/* { dg-do run } */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +
> +#define ARGV_LIMIT_SIZE 128 * 1024
> +#define SYSTEM_LIMIT_SIZE 2 * 1024 * 1024
> +#define STR_TO_WRITE "-DTEST "
> +void create_large_response_file ()
> +{
> + FILE *fp1, *fp2;
> + char buffer[1024];
> +
> + strcpy (buffer, STR_TO_WRITE);
> +
> + fp1 = fopen ("options_128kb_to_2mb.txt", "wb");
> + if (fp1 == NULL)
> + {
> + abort ();
> + }
> + while (ftell (fp1) < (ARGV_LIMIT_SIZE + 10))
> + {
> + fwrite (buffer, strlen (buffer), 1, fp1);
> + }
> + fclose (fp1);
> +
> + fp2 = fopen ("options_greater_then_2mb.txt", "wb");
> + if (fp2 == NULL)
> + {
> + abort ();
> + }
> + while (ftell (fp2) < (SYSTEM_LIMIT_SIZE +10))
> + {
> + fwrite (buffer, strlen (buffer), 1, fp2);
> + }
> + fclose (fp2);
> +}
> +
> +int main()
> +{
> + create_large_response_file ();
> +}
> diff --git a/gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> b/gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> new file mode 100644
> index 00000000000..397d70f7b03
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/longcmd/pr111527-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do run } */
> +
> +#include <stdio.h>
> +
> +int main()
> +{
> + printf("Hello World\n");
> +}
> +/* { dg-final { output-exists { target *-*-* } } } */
> diff --git a/gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> b/gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> new file mode 100644
> index 00000000000..394e54b1074
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/longcmd/pr111527-3.c
> @@ -0,0 +1,10 @@
> +/* { dg-do run } */
> +/* { dg-options "@options_128kb_to_2mb.txt" } */
> +
> +#include <stdio.h>
> +
> +int main()
> +{
> + printf("Hello world\n");
> +}
> +/* { dg-final { output-exists { target *-*-* } } } */
> diff --git a/gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
> b/gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
> new file mode 100644
> index 00000000000..eaa427f24a4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/longcmd/pr111527-4.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "@options_greater_then_2mb.txt" } */
> +/* { dg-excess-errors "warnings about argument list too long" } */
> +
> +#include <stdio.h>
> +
> +int main()
> +{
> + /* { xfail *-*-* } */
> +}
> --
> 2.43.0
>
response_f.patch
Description: response_f.patch
wrapper_lto.patch
Description: wrapper_lto.patch
