Re: [PATCH v7 1/4] driver: add a spec function to join arguments

[Jason Merrill <jason@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

On 7/2/23 12:32, Ben Boeckel wrote:
> When passing `-o` flags to other options, the typical `-o foo` spelling
> leaves a leading whitespace when replacing elsewhere. This ends up
> creating flags spelled as `-some-option-with-arg= foo.ext` which doesn't
> parse properly. When attempting to make a spec function to just remove
> the leading whitespace, the argument splitting ends up masking the
> whitespace. However, the intended extension *also* ends up being its own
> argument.

Odd.  I looked into this to figure out what was going on, and now I 
understand: when process_brace_body handles e.g. %{o*:-fjoined=%.x%*}, 
first it replaces $* with the rest of the flag, i.e. "", resulting in 
-fjoined=, and then adds the argument as a separate argument to the 
result of substitution.  This seems strange, but works fine for the 
existing uses that build one Separate switch from another.

The other oddity you mention comes from

>   /* End of string.  If we are processing a spec function, we need to                                                               
>      end any pending argument.  */
>   if (processing_spec_function)
>     end_going_arg ();

so that when give_switch calls do_spec_1 twice for the basename and 
suffix, they end up as separate arguments to the spec function.  I don't 
know the purpose of this code; it doesn't seem to have been necessary 
for the if-exists spec function that was added in the same patch 
(r59241).  Removing this doesn't seem to break anything for me.

The join function works around both of these issues.  But I notice that 
the use of reconcat is a memory leak, and since we have the obstack 
available I've tweaked the function to use it.  I also added some 
documentation.

Joseph, any thoughts on these issues or the workaround?

Jason

[-- Attachment #2: 0001-driver-add-a-spec-function-to-join-arguments.patch --]
[-- Type: text/x-patch, Size: 4069 bytes --]

From ca7c76c5b44e76e8596bf8db68d6210fd9ddf113 Mon Sep 17 00:00:00 2001
From: Ben Boeckel <ben.boeckel@kitware.com>
Date: Sun, 2 Jul 2023 12:32:08 -0400
Subject: [PATCH] driver: add a spec function to join arguments
To: gcc-patches@gcc.gnu.org

When passing `-o` flags to other options, the typical `-o foo` spelling
leaves a leading whitespace when replacing elsewhere. This ends up
creating flags spelled as `-some-option-with-arg= foo.ext` which doesn't
parse properly. When attempting to make a spec function to just remove
the leading whitespace, the argument splitting ends up masking the
whitespace. However, the intended extension *also* ends up being its own
argument. To perform the desired behavior, the arguments need to be
concatenated together.

gcc/:

	* gcc.cc (join_spec_func): Add a spec function to join all
	arguments.
	* doc/invoke.texi: Document it.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
Co-authored-by: Jason Merrill <jason@redhat.com>
---
 gcc/doc/invoke.texi | 10 ++++++++++
 gcc/gcc.cc          | 23 +++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ef3f4098986..7c475ee5c82 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -35236,6 +35236,16 @@ usage:
 -l%:if-exists-then-else(%:getenv(VSB_DIR rtnet.h) rtnet net)
 @end smallexample
 
+@item @code{join}
+The @code{join} spec function returns the concatenation of its
+arguments.  This is currently used to build e.g. @samp{-fjoined=foo.b}
+from @samp{-fseparate foo.a}, as the behavior of @samp{%*} without this
+function produces the broken @samp{-fjoined= foo.b} instead.
+
+@smallexample
+%@{-fseparate*:-fjoined=%:join(%.b%*)@}
+@end smallexample
+
 @item @code{sanitize}
 The @code{sanitize} spec function takes no arguments.  It returns non-NULL if
 any address, thread or undefined behavior sanitizers are active.
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index fdfac0b4fe4..4c4e81dee50 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -447,6 +447,7 @@ static const char *greater_than_spec_func (int, const char **);
 static const char *debug_level_greater_than_spec_func (int, const char **);
 static const char *dwarf_version_greater_than_spec_func (int, const char **);
 static const char *find_fortran_preinclude_file (int, const char **);
+static const char *join_spec_func (int, const char **);
 static char *convert_white_space (char *);
 static char *quote_spec (char *);
 static char *quote_spec_arg (char *);
@@ -1772,6 +1773,7 @@ static const struct spec_function static_spec_functions[] =
   { "debug-level-gt",		debug_level_greater_than_spec_func },
   { "dwarf-version-gt",		dwarf_version_greater_than_spec_func },
   { "fortran-preinclude-file",	find_fortran_preinclude_file},
+  { "join",			join_spec_func},
 #ifdef EXTRA_SPEC_FUNCTIONS
   EXTRA_SPEC_FUNCTIONS
 #endif
@@ -10975,6 +10977,27 @@ find_fortran_preinclude_file (int argc, const char **argv)
   return result;
 }
 
+/* The function takes any number of arguments and joins them together.
+
+   This seems to be necessary to build "-fjoined=foo.b" from "-fseparate foo.a"
+   with a %{fseparate*:-fjoined=%.b$*} rule without adding undesired spaces:
+   when doing $* replacement we first replace $* with the rest of the switch
+   (in this case ""), and then add any arguments as arguments after the result,
+   resulting in "-fjoined= foo.b".  Using this function with e.g.
+   %{fseparate*:-fjoined=%:join(%.b$*)} gets multiple words as separate argv
+   elements instead of separated by spaces, and we paste them together.  */
+
+static const char *
+join_spec_func (int argc, const char **argv)
+{
+  if (argc == 1)
+    return argv[0];
+  for (int i = 0; i < argc; ++i)
+    obstack_grow (&obstack, argv[i], strlen (argv[i]));
+  obstack_1grow (&obstack, '\0');
+  return XOBFINISH (&obstack, const char *);
+}
+
 /* If any character in ORIG fits QUOTE_P (_, P), reallocate the string
    so as to precede every one of them with a backslash.  Return the
    original string or the reallocated one.  */
-- 
2.39.3