Skip to content

Commit

Permalink
Fix aggressive params file assumption (bazelbuild#14930)
Browse files Browse the repository at this point in the history
Most programs that accept params files use the `@file` syntax. For Apple
platform builds `@` can be the start of non-params file arguments as
well, such as `-rpath @executable_path/Frameworks`. There is a small
list of options where this is the case, so this new behavior no longer
assumes params files if args start with `@`, they also have to not start
with one of the 3 keywords used with this (from `man dyld` on macOS).
This should always hold since params files generated by bazel should
always start with `bazel-out`, if someone renames the symlinks to one of
the keywords, they're on their own.

Previously the workaround was to always make sure to pass the
`-Wl,-rpath,@executable_path` form of these arguments, but this makes
users not have to worry about this.

In a few other places we check this by checking if the file exists,
which is likely more accurate, but feels excessive and potentially
dangerous in this context.

Related: bazelbuild#13148
Fixes: bazelbuild#14316

Closes bazelbuild#14650.

PiperOrigin-RevId: 430195929
(cherry picked from commit 24e8242)

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
  • Loading branch information
brentleyjones and keith authored Mar 2, 2022
1 parent e624aff commit c1ecca2
Showing 1 changed file with 11 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -276,36 +276,43 @@ CommandLine paramCmdLine() {
paramFile, linkTargetType, forcedToolPath, featureConfiguration, actionName, variables);
}

public static void extractArgumentsForStaticLinkParamFile(
private static void extractArgumentsForStaticLinkParamFile(
List<String> args, List<String> commandlineArgs, List<String> paramFileArgs) {
commandlineArgs.add(args.get(0)); // ar command, must not be moved!
int argsSize = args.size();
for (int i = 1; i < argsSize; i++) {
String arg = args.get(i);
if (arg.startsWith("@")) {
if (isLikelyParamFile(arg)) {
commandlineArgs.add(arg); // params file, keep it in the command line
} else {
paramFileArgs.add(arg); // the rest goes to the params file
}
}
}

public static void extractArgumentsForDynamicLinkParamFile(
private static void extractArgumentsForDynamicLinkParamFile(
List<String> args, List<String> commandlineArgs, List<String> paramFileArgs) {
// Note, that it is not important that all linker arguments are extracted so that
// they can be moved into a parameter file, but the vast majority should.
commandlineArgs.add(args.get(0)); // gcc command, must not be moved!
int argsSize = args.size();
for (int i = 1; i < argsSize; i++) {
String arg = args.get(i);
if (arg.startsWith("@")) {
if (isLikelyParamFile(arg)) {
commandlineArgs.add(arg); // params file, keep it in the command line
} else {
paramFileArgs.add(arg); // the rest goes to the params file
}
}
}

private static boolean isLikelyParamFile(String arg) {
return arg.startsWith("@")
&& !arg.startsWith("@rpath")
&& !arg.startsWith("@loader_path")
&& !arg.startsWith("@executable_path");
}

/**
* Returns a raw link command for the given link invocation, including both command and arguments
* (argv). The version that uses the expander is preferred, but that one can't be used during
Expand Down

0 comments on commit c1ecca2

Please sign in to comment.