Attached v3 to address nits. On 2020-07-23, 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. Usually, if the 'if' branch has braces, I'd add braces to 'else' as well. Updated to follow your advice anyway. >>+ 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'. Done. >>+ 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? This is to mimick nearly lines. collect2 should filter out options like -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. --ld-path needs its own `else if`. Honestly I don't know why both both ld1 and ld2 exist and I suspect this can be cleaned up/at least documented, but it is not the scope of this patch. >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=*:%> %{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: >> >