On Thu, 14 Jan 2021, Martin Liška wrote:
> PING^3
I see no particular reason to allow arbitrary garbage to be used as
linker. It just asks for users to shoot themselves in the foot and
for strange bugreports to pop up.
Richard.
> On 1/6/21 3:22 PM, Martin Liška wrote:
> > PING^2
> >
> > On 12/4/20 2:45 PM, Martin Liška wrote:
> >> PING
> >>
> >> May I please ping the patch, it's waiting here for a review
> >> for quite some time.
> >>
> >> Thanks,
> >> Martin
> >>
> >> On 7/23/20 12:17 PM, Martin Liška wrote:
> >>> On 7/21/20 6:07 AM, Fangrui Song wrote:
> >>>> If the value does not contain any path component separator (e.g. a
> >>>> slash), the linker will be searched for using COMPILER_PATH followed by
> >>>> PATH. Otherwise, it is either an absolute path or a path relative to the
> >>>> current working directory.
> >>>>
> >>>> --ld-path= complements and overrides -fuse-ld={bfd,gold,lld}. If in the
> >>>> future, we want to make dfferent linker option decisions we can let
> >>>> -fuse-ld= represent the linker flavor and --ld-path= the linker path.
> >>>
> >>> Hello.
> >>>
> >>> I have just few nits:
> >>>
> >>> === ERROR type #3: trailing operator (1 error(s)) ===
> >>> gcc/collect2.c:1155:14: ld_file_name =
> >>>
> >>>>
> >>>> PR driver/93645
> >>>> * common.opt (--ld-path=): Add --ld-path=
> >>>> * opts.c (common_handle_option): Handle OPT__ld_path_.
> >>>> * gcc.c (driver_handle_option): Likewise.
> >>>> * collect2.c (main): Likewise.
> >>>> * doc/invoke.texi: Document --ld-path=.
> >>>>
> >>>> ---
> >>>> Changes in v2:
> >>>> * Renamed -fld-path= to --ld-path= (clang 12.0.0 new option).
> >>>> The option does not affect code generation and is not a language
> >>>> feature,
> >>>> -f* is not suitable. Additionally, clang has other similar --*-path=
> >>>> options, e.g. --cuda-path=.
> >>>> ---
> >>>> gcc/collect2.c | 63 +++++++++++++++++++++++++++++++++++----------
> >>>> gcc/common.opt | 4 +++
> >>>> gcc/doc/invoke.texi | 9 +++++++
> >>>> gcc/gcc.c | 2 +-
> >>>> gcc/opts.c | 1 +
> >>>> 5 files changed, 64 insertions(+), 15 deletions(-)
> >>>>
> >>>> diff --git a/gcc/collect2.c b/gcc/collect2.c
> >>>> index f8a5ce45994..caa1b96ab52 100644
> >>>> --- a/gcc/collect2.c
> >>>> +++ b/gcc/collect2.c
> >>>> @@ -844,6 +844,7 @@ main (int argc, char **argv)
> >>>> const char **ld1;
> >>>> bool use_plugin = false;
> >>>> bool use_collect_ld = false;
> >>>> + const char *ld_path = NULL;
> >>>> /* The kinds of symbols we will have to consider when scanning the
> >>>> outcome of a first pass link. This is ALL to start with, then
> >>>> might
> >>>> @@ -961,12 +962,21 @@ main (int argc, char **argv)
> >>>> if (selected_linker == USE_DEFAULT_LD)
> >>>> selected_linker = USE_PLUGIN_LD;
> >>>> }
> >>>> - else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
> >>>> - selected_linker = USE_BFD_LD;
> >>>> - else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
> >>>> - selected_linker = USE_GOLD_LD;
> >>>> - else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
> >>>> - selected_linker = USE_LLD_LD;
> >>>> + else if (strncmp (argv[i], "-fuse-ld=", 9) == 0
> >>>> + && selected_linker != USE_LD_MAX)
> >>>> + {
> >>>> + if (strcmp (argv[i] + 9, "bfd") == 0)
> >>>> + selected_linker = USE_BFD_LD;
> >>>> + else if (strcmp (argv[i] + 9, "gold") == 0)
> >>>> + selected_linker = USE_GOLD_LD;
> >>>> + else if (strcmp (argv[i] + 9, "lld") == 0)
> >>>> + selected_linker = USE_LLD_LD;
> >>>> + }
> >>>> + else if (strncmp (argv[i], "--ld-path=", 10) == 0)
> >>>> + {
> >>>> + ld_path = argv[i] + 10;
> >>>> + selected_linker = USE_LD_MAX;
> >>>> + }
> >>>> else if (strncmp (argv[i], "-o", 2) == 0)
> >>>> {
> >>>> /* Parse the output filename if it's given so that we can make
> >>>> @@ -1117,14 +1127,34 @@ main (int argc, char **argv)
> >>>> ld_file_name = find_a_file (&cpath, collect_ld_suffix, X_OK);
> >>>> use_collect_ld = ld_file_name != 0;
> >>>> }
> >>>> - /* Search the compiler directories for `ld'. We have protection
> >>>> against
> >>>> - recursive calls in find_a_file. */
> >>>> - if (ld_file_name == 0)
> >>>> - ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker],
> >>>> X_OK);
> >>>> - /* Search the ordinary system bin directories
> >>>> - for `ld' (if native linking) or `TARGET-ld' (if cross). */
> >>>> - if (ld_file_name == 0)
> >>>> - ld_file_name = find_a_file (&path,
> >>>> full_ld_suffixes[selected_linker], X_OK);
> >>>> + if (selected_linker == USE_LD_MAX)
> >>>> + {
> >>>> + /* If --ld-path= does not contain a path component separator,
> >>>> search for
> >>>> + the command using cpath, then using path. Otherwise find the
> >>>> linker
> >>>> + relative to the current working directory. */
> >>>> + if (lbasename (ld_path) == ld_path)
> >>>> + {
> >>>> + ld_file_name = find_a_file (&cpath, ld_path, X_OK);
> >>>> + if (ld_file_name == 0)
> >>>> + ld_file_name = find_a_file (&path, ld_path, X_OK);
> >>>> + }
> >>>> + else if (file_exists (ld_path))
> >>>> + {
> >>>
> >>> ^^^ these braces are not needed.
> >>>
> >>>> + ld_file_name = ld_path;
> >>>> + }
> >>>> + }
> >>>> + else
> >>>> + {
> >>>> + /* Search the compiler directories for `ld'. We have protection
> >>>> against
> >>>> + recursive calls in find_a_file. */
> >>>> + if (ld_file_name == 0)
> >>>
> >>> I would prefer '== NULL'.
> >>>
> >>>> + ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker],
> >>>> X_OK);
> >>>> + /* Search the ordinary system bin directories
> >>>> + for `ld' (if native linking) or `TARGET-ld' (if cross). */
> >>>> + if (ld_file_name == 0)
> >>>> + ld_file_name =
> >>>> + find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
> >>>> + }
> >>>> #ifdef REAL_NM_FILE_NAME
> >>>> nm_file_name = find_a_file (&path, REAL_NM_FILE_NAME, X_OK);
> >>>> @@ -1461,6 +1491,11 @@ main (int argc, char **argv)
> >>>> ld2--;
> >>>> #endif
> >>>> }
> >>>> + else if (strncmp (arg, "--ld-path=", 10) == 0)
> >>>> + {
> >>>> + ld1--;
> >>>> + ld2--;
> >>>> + }
> >>>
> >>> Can you please explain more this change?
> >>>
> >>> Thank you,
> >>> Martin
> >>>
> >>>> else if (strncmp (arg, "--sysroot=", 10) == 0)
> >>>> target_system_root = arg + 10;
> >>>> else if (strcmp (arg, "--version") == 0)
> >>>> diff --git a/gcc/common.opt b/gcc/common.opt
> >>>> index a3893a4725e..5adbd8c18a3 100644
> >>>> --- a/gcc/common.opt
> >>>> +++ b/gcc/common.opt
> >>>> @@ -2940,6 +2940,10 @@ fuse-ld=lld
> >>>> Common Driver Negative(fuse-ld=lld)
> >>>> Use the lld LLVM linker instead of the default linker.
> >>>> +-ld-path=
> >>>> +Common Driver Joined
> >>>> +Use the specified executable instead of the default linker.
> >>>> +
> >>>> fuse-linker-plugin
> >>>> Common Undocumented Var(flag_use_linker_plugin)
> >>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> >>>> index ba18e05fb1a..e185e40ffac 100644
> >>>> --- a/gcc/doc/invoke.texi
> >>>> +++ b/gcc/doc/invoke.texi
> >>>> @@ -14718,6 +14718,15 @@ Use the @command{gold} linker instead of the
> >>>> default linker.
> >>>> @opindex fuse-ld=lld
> >>>> Use the LLVM @command{lld} linker instead of the default linker.
> >>>> +@item --ld-path=@var{linker}
> >>>> +@opindex -ld-path=linker
> >>>> +Use the specified executable named @var{linker} instead of the default
> >>>> linker.
> >>>> +If @var{linker} does not contain any path component separator (e.g. a
> >>>> slash),
> >>>> +the linker will be searched for using @env{COMPILER_PATH} followed by
> >>>> +@env{PATH}. Otherwise, it is either an absolute path or a path relative
> >>>> to the
> >>>> +current working directory. If @option{-fuse-ld=} is also specified, the
> >>>> linker
> >>>> +path is specified by @option{--ld-path=}.
> >>>> +
> >>>> @cindex Libraries
> >>>> @item -l@var{library}
> >>>> @itemx -l @var{library}
> >>>> diff --git a/gcc/gcc.c b/gcc/gcc.c
> >>>> index c0eb3c10cfd..05fa5375f06 100644
> >>>> --- a/gcc/gcc.c
> >>>> +++ b/gcc/gcc.c
> >>>> @@ -1077,7 +1077,7 @@ proper position among the other output files. */
> >>>> LINK_PLUGIN_SPEC \
> >>>> "%{flto|flto=*:%<fcompare-debug*} \
> >>>> %{flto} %{fno-lto} %{flto=*} %l " LINK_PIE_SPEC \
> >>>> - "%{fuse-ld=*:-fuse-ld=%*} " LINK_COMPRESS_DEBUG_SPEC \
> >>>> + "%{fuse-ld=*:-fuse-ld=%*} %{-ld-path=*:--ld-path=%*} "
> >>>> LINK_COMPRESS_DEBUG_SPEC \
> >>>> "%X %{o*} %{e*} %{N} %{n} %{r}\
> >>>> %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!r:%{!nostartfiles:%S}}} \
> >>>> %{static|no-pie|static-pie:} %@{L*} %(mfwrap) %(link_libgcc) " \
> >>>> diff --git a/gcc/opts.c b/gcc/opts.c
> >>>> index 499eb900643..5cbf9b85549 100644
> >>>> --- a/gcc/opts.c
> >>>> +++ b/gcc/opts.c
> >>>> @@ -2755,6 +2755,7 @@ common_handle_option (struct gcc_options *opts,
> >>>> dc->max_errors = value;
> >>>> break;
> >>>> + case OPT__ld_path_:
> >>>> case OPT_fuse_ld_bfd:
> >>>> case OPT_fuse_ld_gold:
> >>>> case OPT_fuse_ld_lld:
> >>>>
> >>>
> >>
> >
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
