Skip to content

Correct sysroot-prepend conditions for script inputs #809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
parth-07 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Correct sysroot-prepend conditions for script inputs #809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/eld/Driver/GnuLinkerOptions.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ defm output_file : smDashTwoWithOpt<"o", "output", "output_file",
MetaVarName<"<outputfile>">,
Group<grp_general>;
defm sysroot : smDash<"sysroot", "sysroot", "Set the system root">,
MetaVarName<"<sysroot-path>">,
MetaVarName<"<sysroot-path>">,
Group<grp_general>;
def version : Flag<["--"], "version">,
HelpText<"Print the Linker version">,
Expand Down
15 changes: 14 additions & 1 deletion include/eld/Input/Input.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ class Input {

void setResolvedPath(std::string Path) { ResolvedPath = Path; }

/// Set the parent script file for inputs from INPUT/GROUP commands.
void setParentScriptFile(InputFile *File) { ParentScriptFile = File; }

/// Get the parent script file.
InputFile *getParentScriptFile() const { return ParentScriptFile; }

uint32_t getInputOrdinal() { return InputOrdinal; }

Attribute &getAttribute() { return Attr; }
Expand Down Expand Up @@ -179,10 +185,16 @@ class Input {
static MemoryArea *createMemoryArea(const std::string &Filepath,
DiagnosticEngine *DiagEngine);

private:
private:
// Check if a path is valid and emit any errors
bool isPathValid(const std::string &Path) const;

// Return true if sysroot should be prepended for a Script-typed input whose
// filename starts with '/'. This is used for INPUT/GROUP entries coming from
// a linker script: sysroot is applied only when the parent script is within
// the sysroot directory.
bool shouldPrependSysrootToScriptInput(const LinkerConfig &Config) const;

protected:
InputFile *IF = nullptr;
MemoryArea *MemArea = nullptr;
Expand All @@ -201,6 +213,7 @@ class Input {
llvm::DenseMap<const WildcardPattern *, bool> MemberPatternMap;
bool PatternMapInitialized = false;
DiagnosticEngine *DiagEngine = nullptr;
InputFile *ParentScriptFile = nullptr; // Parent script for INPUT/GROUP inputs.

/// Keeps track of already created MemoryAreas.
///
Expand Down
27 changes: 25 additions & 2 deletions lib/Input/Input.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ bool Input::resolvePath(const LinkerConfig &PConfig) {
}

if (Type == Input::Script) {
if (PSearchDirs.hasSysRoot() &&
(FileName.size() > 0 && FileName[0] == '/')) {
if (shouldPrependSysrootToScriptInput(PConfig)) {
ResolvedPath = PSearchDirs.sysroot();
ResolvedPath->append(FileName);
}
Expand Down Expand Up @@ -151,6 +150,30 @@ bool Input::resolvePath(const LinkerConfig &PConfig) {
return true;
}

bool Input::shouldPrependSysrootToScriptInput(
const LinkerConfig &Config) const {
auto &searchDirs = Config.directories();
if (!searchDirs.hasSysRoot())
return false;
if (Type != Input::Script)
return false;
if (FileName.empty() || FileName[0] != '/')
Copy link
Contributor

@quic-seaswara quic-seaswara Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how this will be effective on windows. Needs to be tested on windows too.

Copy link
Contributor Author

@parth-07 parth-07 Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay!

return false;

// Only apply sysroot for INPUT/GROUP entries when we know which script they
// came from and that script is inside sysroot.
if (!ParentScriptFile)
return false;

Input *scriptInput = ParentScriptFile->getInput();

std::string scriptPath = scriptInput->getResolvedPath().getFullPath();
std::string sysrootPath = searchDirs.sysroot().getFullPath();
Copy link
Contributor

@quic-seaswara quic-seaswara Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work with reproduce option ?

Copy link
Contributor Author

@parth-07 parth-07 Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. We already had this code. Previously, we used to unconditionally prepend the sysroot for script inputs that began with /. Now, we only prepend sysroot for script inputs that begins with / if the parent script is within the sysroot directory.

Copy link
Contributor

@quic-seaswara quic-seaswara Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check ?

Copy link
Contributor Author

@parth-07 parth-07 Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked. The linker crashes :'). However, this is not a regression with this patch. We crash without this patch as well.

Reproducer (nightly linker crashes with this):

#!/usr/bin/env bash

mkdir -p A/lib64
mkdir -p A/script

cat >1.c <<\EOF
int foo() { return 1; }
EOF

cat >script.t <<\EOF
GROUP(/lib64/lib1.so)
EOF

cat >main.c <<\EOF
int main() { return 0; }
EOF

clang -o lib1.so -target x86_64-unknown-elf 1.c -shared -fPIC
clang -o main.o -target x86_64-unknown-elf main.c -c 
mv lib1.so A/lib64/lib1.so
mv script.t A/script.t

ld.eld --sysroot=A -T A/script.t main.o --reproduce a.tar
rm -rf unpacked && mkdir unpacked
tar -xvf a.tar -C unpacked --strip-components=1
cd unpacked
bash -x response.txt

I believe this should be treated as a separate issue.

Copy link
Contributor Author

@parth-07 parth-07 Feb 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created an issue to track this #818


return scriptPath.size() >= sysrootPath.size() &&
Copy link
Contributor

@quic-areg quic-areg Feb 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about a hypothetical /sdk/sysroot and a sibling path /sdk/sysroot_extra? we should probably use llvm::sys::fs::equivalent or similar instead of a raw prefix check

Copy link
Contributor Author

@parth-07 parth-07 Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we just need to determine whether the script is within or outside the sysroot. If a script is contained within /sdk/sysroot_extra and the sysroot is /sdk/sysroot, then as per the linker the script is correctly outside the sysroot. The behavior seems correct to me. Can you please let me know if I have missed something?

scriptPath.compare(0, sysrootPath.size(), sysrootPath) == 0;
}

void Input::setInputFile(InputFile *Inp) { IF = Inp; }

void Input::overrideInputFile(InputFile *Inp) { IF = Inp; }
Expand Down
9 changes: 7 additions & 2 deletions lib/Script/GroupCmd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ eld::Expected<void> GroupCmd::activate(Module &CurModule) {
// --start-group
ThisBuilder.enterGroup();

// Prefer ScriptCommand context (handles included scripts).
InputFile *parentScriptFile = &ThisScriptFile.getLinkerScriptFile();

for (auto &S : ThisStringList) {
InputToken *Token = llvm::cast<InputToken>(S);
Token = ThisScriptFile.findResolvedFilename(Token);
Expand All @@ -67,9 +70,11 @@ eld::Expected<void> GroupCmd::activate(Module &CurModule) {
else
ThisBuilder.getAttributes().unsetAsNeeded();
switch (Token->type()) {
case InputToken::File:
ThisBuilder.createDeferredInput(Token->name(), Input::Script);
case InputToken::File: {
Input *I = ThisBuilder.createDeferredInput(Token->name(), Input::Script);
I->setParentScriptFile(parentScriptFile);
break;
}
case InputToken::NameSpec: {
ThisBuilder.createDeferredInput(Token->name(), Input::Namespec);
break;
Expand Down
9 changes: 7 additions & 2 deletions lib/Script/InputCmd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ void InputCmd::dump(llvm::raw_ostream &Outs) const {
}

eld::Expected<void> InputCmd::activate(Module &CurModule) {
// Prefer ScriptCommand context (handles included scripts).
InputFile *parentScriptFile = &ThisScriptFile.getLinkerScriptFile();

for (auto &S : ThisStringList) {
InputToken *Token = llvm::cast<InputToken>(S);
Token = ThisScriptFile.findResolvedFilename(Token);
Expand All @@ -64,9 +67,11 @@ eld::Expected<void> InputCmd::activate(Module &CurModule) {
else
ThisBuilder.getAttributes().unsetAsNeeded();
switch (Token->type()) {
case InputToken::File:
ThisBuilder.createDeferredInput(Token->name(), Input::Script);
case InputToken::File: {
Input *I = ThisBuilder.createDeferredInput(Token->name(), Input::Script);
I->setParentScriptFile(parentScriptFile);
break;
}
case InputToken::NameSpec: {
ThisBuilder.createDeferredInput(Token->name(), Input::Namespec);
Copy link
Contributor

@quic-seaswara quic-seaswara Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add the parent script file in all cases, and check if the command is INPUT or GROUP ?

Copy link
Contributor Author

@parth-07 parth-07 Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain this in more detail? Currently, we only add parent script for INPUT and GROUP inputs. If I understand correctly, then for the other cases, we do not have any associated parent script.

Copy link
Contributor

@quic-seaswara quic-seaswara Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Do we need to add this for InputToken::NameSpec too ?

Copy link
Contributor Author

@parth-07 parth-07 Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we do not need this for NameSpec. But we do need to fix our behavior for NameSpec with sysroot. GNU LD finds the NameSpec library in all the standard paths inside sysroot. (Paths such as /usr/lib, /lib, /lib64, and so on).

Copy link
Contributor

@quic-seaswara quic-seaswara Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open a follow up ticket on this, We do have this code in eld::ELDEmulateELF but that code is not getting triggered.

Copy link
Contributor Author

@parth-07 parth-07 Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created an issue to track this: #819

break;
Expand Down
1 change: 1 addition & 0 deletions test/Common/standalone/linkerscript/SysrootInputGroup/Inputs/1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int foo() { return 1; }
1 change: 1 addition & 0 deletions test/Common/standalone/linkerscript/SysrootInputGroup/Inputs/2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int bar() { return 2; }
1 change: 1 addition & 0 deletions test/Common/standalone/linkerscript/SysrootInputGroup/Inputs/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int main() { return 0; }
1 change: 1 addition & 0 deletions test/Common/standalone/linkerscript/SysrootInputGroup/Inputs/script1.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GROUP(/lib64/lib1.so)
1 change: 1 addition & 0 deletions test/Common/standalone/linkerscript/SysrootInputGroup/Inputs/script2.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
INPUT(/lib64/lib2.so)
32 changes: 32 additions & 0 deletions test/Common/standalone/linkerscript/SysrootInputGroup/SysrootInputGroup.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#---SysrootInputGroup.test---------------- Linker Script ----------------#
#BEGIN_COMMENT
# Validate sysroot handling for INPUT/GROUP script commands (issue #808).
# Sysroot is prepended for "/..." only when the linker script itself is within
# the sysroot directory.
#END_COMMENT
#START_TEST

RUN: rm -rf %t.dir && mkdir -p %t.dir/lib64 %t.dir/scripts
RUN: %clang %clangopts -o %t.1.o %p/Inputs/1.c -c -fPIC
RUN: %clang %clangopts -o %t.2.o %p/Inputs/2.c -c -fPIC
RUN: %clang %clangopts -o %t.main.o %p/Inputs/main.c -c
RUN: %link %linkopts -o %t.dir/lib64/lib1.so -shared %t.1.o
RUN: %link %linkopts -o %t.dir/lib64/lib2.so -shared %t.2.o

# Script outside sysroot: do not prepend sysroot for "/...".
RUN: %not %link %linkopts -o %t.out1 --sysroot=%t.dir -T %p/Inputs/script1.t %t.main.o 2>&1 | %filecheck %s --check-prefix=ERR1
RUN: %not %link %linkopts -o %t.out2 --sysroot=%t.dir -T %p/Inputs/script2.t %t.main.o 2>&1 | %filecheck %s --check-prefix=ERR2

# Script in sysroot: prepend sysroot for "/...".
RUN: %cp %p/Inputs/script1.t %t.dir/scripts/script1.t
RUN: %cp %p/Inputs/script2.t %t.dir/scripts/script2.t
RUN: %link %linkopts -o %t.out3 --sysroot=%t.dir -T %t.dir/scripts/script1.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=SYSROOT1
RUN: %link %linkopts -o %t.out4 --sysroot=%t.dir -T %t.dir/scripts/script2.t %t.main.o --verbose 2>&1 | %filecheck %s --check-prefix=SYSROOT2
#END_TEST

ERR1: Fatal: cannot read file /lib64/lib1.so
ERR2: Fatal: cannot read file /lib64/lib2.so

SYSROOT1: Verbose: Mapping input file '{{.*}}dir/lib64/lib1.so' into memory
SYSROOT2: Verbose: Mapping input file '{{.*}}dir/lib64/lib2.so' into memory

4 changes: 4 additions & 0 deletions test/Common/standalone/sysRoot/VersionAndDynamicScripts/Inputs/1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Simple library with exported functions
int exported_func_v1() { return 1; }
int exported_func_v2() { return 2; }
int hidden_func() { return 3; }
4 changes: 4 additions & 0 deletions test/Common/standalone/sysRoot/VersionAndDynamicScripts/Inputs/dynamic.list
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
exported_func_v1;
exported_func_v2;
};
6 changes: 6 additions & 0 deletions test/Common/standalone/sysRoot/VersionAndDynamicScripts/Inputs/version.script
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
global:
exported_func_v1;
local:
*;
};
44 changes: 44 additions & 0 deletions ...mmon/standalone/sysRoot/VersionAndDynamicScripts/VersionAndDynamicScriptAbsolutePath.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#---VersionAndDynamicScripts.test---- sysRoot ----------------#

#BEGIN_COMMENT
# Test that the version scripts and dynamic lists paths are properly handled
# when sysroot is used.
#END_COMMENT
#START_TEST
RUN: %clang %clangopts -o %t1.1.o %p/Inputs/1.c -c
RUN: %rm -rf %t1.sysroot %t1.dir
RUN: %mkdir %t.sysroot
RUN: %link %linkopts -o %t1.lib1.so %t1.1.o -shared --sysroot=%t.sysroot -L=/ \
RUN: --version-script=%p/Inputs/version.script --verbose 2>&1 \
RUN: | %filecheck %s --check-prefix VersionScriptAbs
RUN: %link %linkopts -o %t1.lib2.so %t1.1.o -shared --sysroot=%t.sysroot -L=/ \
RUN: --dynamic-list=%p/Inputs/dynamic.list --verbose 2>&1 \
RUN: | %filecheck %s --check-prefix DynScriptAbs
RUN: %mkdir %t1.dir
RUN: cd %t1.dir
RUN: %link %linkopts -o %t1.lib1.so %t1.1.o -shared --sysroot=%p/Inputs -L=/ \
RUN: --version-script=version.script --verbose 2>&1 \
RUN: | %filecheck %s --check-prefix VersionScriptSysRoot
RUN: %link %linkopts -o %t1.lib2.so %t1.1.o -shared --sysroot=%p/Inputs -L=/ \
RUN: --dynamic-list=dynamic.list --verbose 2>&1 \
RUN: | %filecheck %s --check-prefix DynScriptSysRoot
RUN: %cp %p/Inputs/version.script version.script
RUN: %cp %p/Inputs/dynamic.list dynamic.list
RUN: %link %linkopts -o %t1.lib1.so %t1.1.o -shared --sysroot=%p/Inputs -L=/ \
RUN: --version-script=version.script --verbose 2>&1 \
RUN: | %filecheck %s --check-prefix VersionScriptRel
RUN: %link %linkopts -o %t1.lib2.so %t1.1.o -shared --sysroot=%p/Inputs -L=/ \
RUN: --dynamic-list=dynamic.list --verbose 2>&1 \
RUN: | %filecheck %s --check-prefix DynScriptSysRel
#END_TEST

VersionScriptAbs: Verbose: Parsing version script

DynScriptAbs: Verbose: Dynamic List[{{.*}}] : exported_func_v1
DynScriptAbs: Verbose: Dynamic List[{{.*}}] : exported_func_v2

VersionScriptSysRoot: Verbose: Trying to open input `{{.*}}/Inputs//version.script' of type `linker script' for namespec `version.script': found
DynScriptSysRoot: Verbose: Trying to open input `{{.*}}/Inputs//dynamic.list' of type `linker script' for namespec `dynamic.list': found

VersionScriptRel: Verbose: Mapping input file 'version.script' into memory
DynScriptSysRel: Verbose: Mapping input file 'dynamic.list' into memory
Loading