On Fri, Sep 6, 2024, 9:38 AM Dora, Sunil Kumar < 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 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://gcc.gnu.org/bugzilla/show_bug.cgi?id=111527#c19>. > 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 > > Thanks, > Sunil Dora > ------------------------------ > *From:* Andrew Pinski <pins...@gmail.com> > *Sent:* Friday, August 30, 2024 8:05 PM > *To:* Hemraj, Deepthi <deepthi.hem...@windriver.com> > *Cc:* gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; rguent...@suse.de > <rguent...@suse.de>; jeffreya...@gmail.com <jeffreya...@gmail.com>; > josmy...@redhat.com <josmy...@redhat.com>; MacLeod, Randy < > randy.macl...@windriver.com>; Gowda, Naveen <naveen.go...@windriver.com>; > Dora, Sunil Kumar <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> wrote: > > > > From: Deepthi Hemraj <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 > > > > 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> > > Signed-off-by: Topi Kuutela <topi.kuut...@nokia.com> > > Signed-off-by: Deepthi Hemraj <deepthi.hem...@windriver.com> > > --- > > gcc/collect2.cc | 39 ++++++++++++++++++-- > > gcc/gcc.cc | 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 b/gcc/collect2.cc > > index 902014a9cc1..1f56963b1ce 100644 > > --- a/gcc/collect2.cc > > +++ b/gcc/collect2.cc > > @@ -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 b/gcc/gcc.cc > > index d07a8e172a4..3d7fd8dff5b 100644 > > --- a/gcc/gcc.cc > > +++ b/gcc/gcc.cc > > @@ -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 > > >
