[PS4,PS5][Driver] Pass `-L<...>/target/lib -L.` to linker #109796

playstation-edd · 2024-09-24T13:06:47Z

The proprietary PS4 linker implicitly adds =/target/lib and . as library search paths. This behaviour was added to the PS5 linker via a downstream patch in LLD. This really belongs in the driver, instead. This change adds the driver behaviour to allow removal of the downstream patch in LLD.

There are no plans to update the PS4 linker behaviour in the analogous way, so do not pass the same search paths to the PS4 linker.

SIE tracker: TOOLCHAIN-16704

The proprietary PS4 linker implicitly adds `=/target/lib` and `.` as library search paths. This behaviour was added to the PS5 linker via a downstream patch in LLD. This really belongs in the driver, instead. This change adds the driver behaviour to allow removal of the downstream patch in LLD. There are no plans to update the PS4 linker behaviour in the analogous way, so do not pass the same search paths to the PS4 linker. SIE tracker: TOOLCHAIN-16704

llvmbot · 2024-09-24T13:07:24Z

@llvm/pr-subscribers-clang

Author: Edd Dawson (playstation-edd)

Changes

The proprietary PS4 linker implicitly adds =/target/lib and . as library search paths. This behaviour was added to the PS5 linker via a downstream patch in LLD. This really belongs in the driver, instead. This change adds the driver behaviour to allow removal of the downstream patch in LLD.

There are no plans to update the PS4 linker behaviour in the analogous way, so do not pass the same search paths to the PS4 linker.

SIE tracker: TOOLCHAIN-16704

Full diff: https://github.com/llvm/llvm-project/pull/109796.diff

2 Files Affected:

(modified) clang/lib/Driver/ToolChains/PS4CPU.cpp (+6)
(modified) clang/test/Driver/ps5-linker.c (+23)

diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 647580e4e235dc..0a43ab19b434c5 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -186,6 +186,9 @@ void tools::PS4cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
     TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 
+  // Other drivers typically add library search paths (`-L`) here via
+  // TC.AddFilePathLibArgs(). We don't do that on PS4 as the PS4 linker
+  // searches those locations by default.
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
                             options::OPT_s, options::OPT_t});
 
@@ -290,6 +293,7 @@ void tools::PS5cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
     TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 
+  TC.AddFilePathLibArgs(Args, CmdArgs);
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
                             options::OPT_s, options::OPT_t});
 
@@ -382,6 +386,8 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const llvm::Triple &Triple,
     llvm::sys::path::append(Dir, "target/include");
     CheckSDKPartExists(Dir, "system headers");
   }
+
+  getFilePaths().push_back(".");
 }
 
 void toolchains::PS4PS5Base::AddClangSystemIncludeArgs(
diff --git a/clang/test/Driver/ps5-linker.c b/clang/test/Driver/ps5-linker.c
index c0cf0b864028c8..852659ae86d5cf 100644
--- a/clang/test/Driver/ps5-linker.c
+++ b/clang/test/Driver/ps5-linker.c
@@ -46,3 +46,26 @@
 
 // CHECK-SYSROOT: {{ld(\.exe)?}}"
 // CHECK-SYSROOT-SAME: "--sysroot=mysdk"
+
+// Test that "." is always added to library search paths. This is long-standing
+// behavior, unique to PlayStation toolchains.
+
+// RUN: %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LDOT %s
+
+// CHECK-LDOT: {{ld(\.exe)?}}"
+// CHECK-LDOT-SAME: "-L."
+
+// Test that <sdk-root>/target/lib is added to library search paths, if it
+// exists and no --sysroot is specified.
+
+// RUN: rm -rf %t.dir && mkdir -p %t.dir/target/lib
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-TARGETLIB %s
+
+// CHECK-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-TARGETLIB-SAME: "-L{{.*[/\\]}}target/lib"
+
+// RUN: env SCE_PROSPERO_SDK_DIR=missing %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### --sysroot=missing 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+
+// CHECK-NO-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-NO-TARGETLIB-NOT: "-L{{.*[/\\]}}target/lib"

llvmbot · 2024-09-24T13:07:25Z

@llvm/pr-subscribers-clang-driver

Author: Edd Dawson (playstation-edd)

Changes

The proprietary PS4 linker implicitly adds =/target/lib and . as library search paths. This behaviour was added to the PS5 linker via a downstream patch in LLD. This really belongs in the driver, instead. This change adds the driver behaviour to allow removal of the downstream patch in LLD.

There are no plans to update the PS4 linker behaviour in the analogous way, so do not pass the same search paths to the PS4 linker.

SIE tracker: TOOLCHAIN-16704

Full diff: https://github.com/llvm/llvm-project/pull/109796.diff

2 Files Affected:

(modified) clang/lib/Driver/ToolChains/PS4CPU.cpp (+6)
(modified) clang/test/Driver/ps5-linker.c (+23)

diff --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 647580e4e235dc..0a43ab19b434c5 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -186,6 +186,9 @@ void tools::PS4cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
     TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 
+  // Other drivers typically add library search paths (`-L`) here via
+  // TC.AddFilePathLibArgs(). We don't do that on PS4 as the PS4 linker
+  // searches those locations by default.
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
                             options::OPT_s, options::OPT_t});
 
@@ -290,6 +293,7 @@ void tools::PS5cpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
     TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 
+  TC.AddFilePathLibArgs(Args, CmdArgs);
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
                             options::OPT_s, options::OPT_t});
 
@@ -382,6 +386,8 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const llvm::Triple &Triple,
     llvm::sys::path::append(Dir, "target/include");
     CheckSDKPartExists(Dir, "system headers");
   }
+
+  getFilePaths().push_back(".");
 }
 
 void toolchains::PS4PS5Base::AddClangSystemIncludeArgs(
diff --git a/clang/test/Driver/ps5-linker.c b/clang/test/Driver/ps5-linker.c
index c0cf0b864028c8..852659ae86d5cf 100644
--- a/clang/test/Driver/ps5-linker.c
+++ b/clang/test/Driver/ps5-linker.c
@@ -46,3 +46,26 @@
 
 // CHECK-SYSROOT: {{ld(\.exe)?}}"
 // CHECK-SYSROOT-SAME: "--sysroot=mysdk"
+
+// Test that "." is always added to library search paths. This is long-standing
+// behavior, unique to PlayStation toolchains.
+
+// RUN: %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LDOT %s
+
+// CHECK-LDOT: {{ld(\.exe)?}}"
+// CHECK-LDOT-SAME: "-L."
+
+// Test that <sdk-root>/target/lib is added to library search paths, if it
+// exists and no --sysroot is specified.
+
+// RUN: rm -rf %t.dir && mkdir -p %t.dir/target/lib
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-TARGETLIB %s
+
+// CHECK-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-TARGETLIB-SAME: "-L{{.*[/\\]}}target/lib"
+
+// RUN: env SCE_PROSPERO_SDK_DIR=missing %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+// RUN: env SCE_PROSPERO_SDK_DIR=%t.dir %clang --target=x64_64-sie-ps5 %s -### --sysroot=missing 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s
+
+// CHECK-NO-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-NO-TARGETLIB-NOT: "-L{{.*[/\\]}}target/lib"

pogo59 · 2024-09-24T14:17:27Z

clang/test/Driver/ps5-linker.c

+// CHECK-TARGETLIB: {{ld(\.exe)?}}"
+// CHECK-TARGETLIB-SAME: "-L{{.*[/\\]}}target/lib"
+
+// RUN: env SCE_PROSPERO_SDK_DIR=missing %clang --target=x64_64-sie-ps5 %s -### 2>&1 | FileCheck --check-prefixes=CHECK-NO-TARGETLIB %s


This assumes missing is not present. While that's likely to be true, I might prefer to sequence the tests this way:

rm -rf %t.dir && mkdir -p %t.dir env SCE_PROSPERO_SDK_DIR=%t.dir .... CHECK-NO-TARGETLIB mkdir -p %t.dir/target/lib env SCE_PROSPERO_SDK_DIR=%t.dir .... CHECK-TARGETLIB

It's a smidge more robust and self-explanatory IMO.

Yes, much better. Done. Thanks.

pogo59 · 2024-09-24T14:18:03Z

clang/test/Driver/ps5-linker.c

+// CHECK-LDOT-SAME: "-L."
+
+// Test that <sdk-root>/target/lib is added to library search paths, if it
+// exists and no --sysroot is specified.


Demonstrate that --sysroot causes no target/lib to be appended? Or is that tested elsewhere?

This is what the RUN on line 63 does ... I hope! (Prior to 1a6dc7d, this was on line 68).

Ah missed that, sorry!

…tory

pogo59

LGTM

…119875) Responsibility for setting up implicit library search paths was recently transferred to the PS5 driver (#109796). Prior to this, SIE private patches in lld performed this function. During the transition, I failed to maintain the order in which implicit and user-supplied search paths were supplied/considered. This change ensures user-supplied search paths appear before any implicit ones on the link line. SIE tracker: TOOLCHAIN-17490

playstation-edd requested review from jmorse and pogo59 September 24, 2024 13:06

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Sep 24, 2024

pogo59 reviewed Sep 24, 2024

View reviewed changes

Rewrite test cases to remove assumed non-existence of 'missing' direc…

1a6dc7d

…tory

pogo59 approved these changes Sep 24, 2024

View reviewed changes

playstation-edd merged commit 660ddb3 into llvm:main Sep 25, 2024
8 checks passed

playstation-edd deleted the ps5-driver-lib-search-paths branch September 25, 2024 17:08

playstation-edd mentioned this pull request Dec 13, 2024

[PS5][Driver] Pass user search paths to linker before implict ones #119875

Merged

This was referenced Jun 2, 2025

[MTE] [NFC] use vector to collect globals to tag (#120283) #142329

Closed

[MTE] [NFC] use vector to collect globals to tag (#120283) #142330

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PS4,PS5][Driver] Pass `-L<...>/target/lib -L.` to linker #109796

[PS4,PS5][Driver] Pass `-L<...>/target/lib -L.` to linker #109796

Uh oh!

playstation-edd commented Sep 24, 2024

Uh oh!

llvmbot commented Sep 24, 2024

Uh oh!

llvmbot commented Sep 24, 2024

Uh oh!

pogo59 Sep 24, 2024

Uh oh!

playstation-edd Sep 24, 2024

Uh oh!

pogo59 Sep 24, 2024

Uh oh!

playstation-edd Sep 24, 2024

Uh oh!

pogo59 Sep 24, 2024

Uh oh!

pogo59 left a comment

Uh oh!

Uh oh!

Uh oh!

[PS4,PS5][Driver] Pass -L<...>/target/lib -L. to linker #109796

[PS4,PS5][Driver] Pass -L<...>/target/lib -L. to linker #109796

Uh oh!

Conversation

playstation-edd commented Sep 24, 2024

Uh oh!

llvmbot commented Sep 24, 2024

Uh oh!

llvmbot commented Sep 24, 2024

Uh oh!

pogo59 Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

playstation-edd Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

pogo59 Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

playstation-edd Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

pogo59 Sep 24, 2024

Choose a reason for hiding this comment

Uh oh!

pogo59 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[PS4,PS5][Driver] Pass `-L<...>/target/lib -L.` to linker #109796

[PS4,PS5][Driver] Pass `-L<...>/target/lib -L.` to linker #109796