Thank you, Richard, for your feedback. I have removed the FILE_GCC_OPTION and used COLLECT_GCC_OPTION exclusively to manage the long command line option. The revised V2 patch has been posted and can be found at the following link:
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/661842.html I would appreciate it if you could review the updated patch. Best Regards, Sunil Dora ________________________________ From: Richard Biener <rguent...@suse.de> Sent: Tuesday, August 27, 2024 6:09 PM To: Dora, Sunil Kumar <sunilkumar.d...@windriver.com> Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; pins...@gmail.com <pins...@gmail.com>; jeffreya...@gmail.com <jeffreya...@gmail.com>; josmy...@redhat.com <josmy...@redhat.com>; Hemraj, Deepthi <deepthi.hem...@windriver.com>; MacLeod, Randy <randy.macl...@windriver.com>; Gowda, Naveen <naveen.go...@windriver.com>; Kokkonda, Sundeep <sundeep.kokko...@windriver.com> Subject: Re: PING ^1 [PATCH] 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, 27 Aug 2024, Dora, Sunil Kumar wrote: > Dear GCC Team, > > Please consider this as a gentle reminder to review the patch I posted at the > following link: [ > https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660223.html ]. > > BUG Link : [ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527 ] > > Your feedback or approval would be greatly appreciated. > > Best Regards, > Sunil Dora > > On 2024-08-13 2:30 a.m., > deepthi.hem...@windriver.com<mailto:deepthi.hem...@windriver.com> wrote: > > From: sunil dora <sunil.dora1...@gmail.com><mailto:sunil.dora1...@gmail.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!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8ecOLOVBw$> > > 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) Just as a random comment - I'd prefer if COLLECT_GCC_OPTIONS would allow response files instead of indirecting via a new fixed other environment like the proposed FILE_GCC_OPTIONS. That looks like a cleaner interface to me. Richard. > Signed-off-by: Topi Kuutela > <topi.kuut...@nokia.com><mailto:topi.kuut...@nokia.com> > Signed-off-by: sunil dora > <sunil.dora1...@gmail.com><mailto:sunil.dora1...@gmail.com> > > --- > > gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$> > | 40 +++++++++++++++++++-- > > gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$> > | 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, 160 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!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$> > > b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$> > index 902014a9cc1..564d8968648 100644 > --- > a/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$> > +++ > b/gcc/collect2.cc<https://urldefense.com/v3/__http://collect2.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8f5x6pChg$> > @@ -376,6 +376,40 @@ 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; > + > + char* string = getenv (var_name); > + if (!string) > + { > + char* string = getenv ("FILE_GCC_OPTIONS"); > + 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 +1038,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 +1234,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 +1628,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!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$> > > b/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$> > index abdb40bfe6e..fb36d4a979c 100644 > --- > a/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$> > +++ > b/gcc/gcc.cc<https://urldefense.com/v3/__http://gcc.cc__;!!AjveYdw8EvQ!ZKeP8wCVwVcrJUs6gQlE4qeAjJ6qNDbZLBt8Smt01dIdiPvTI9X0LL6LPKtXkQtKzKRsfIVRqXGkPbj4CvwvC8fDr-4p_g$> > @@ -2952,12 +2952,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; > + size_t count; > + 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 i > + them back together in collect2 */ > + 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 */ > + count = fwrite (string, sizeof(char), strlen(string), fptr); > + if ( count != string_length ) > + { > + fatal_error (input_location, "Cannot write into temporary file"); > + return; > + } > + char *env_val = (char *) xmalloc (strlen (temp_file) + 18); > + sprintf (env_val, "FILE_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 *-*-* } */ > +} > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
