Skip to content

Commit

Permalink
[clang][Driver] Fix safestack -u ordering
Browse files Browse the repository at this point in the history
When re-enabling safestack testing on Solaris after the unexplained
b0260c5, all tests `FAIL`ed to link:

```
Undefined			first referenced
 symbol  			    in file
__safestack_unsafe_stack_ptr        buffer-copy-vla.o
__safestack_init                    (command line)
ld: fatal: symbol referencing errors
```

The problem is that `-u __safestack_init` was passed to the linker after
the corresponding version of `libclang_rt.safestack-*.a`.  Since the
Solaris linker (like Unix linkers for decades) respects the command line
argument order (unlike e.g. GNU ld which uses GNU getopt), this cannot
work.  Fixed by moving the `-u` arg further to the front.  Two affected
testcases were fixed accordingly.

Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`,
`x86_64-pc-linux-gnu`, and `sparc64-unknown-linux-gnu`.
  • Loading branch information
rorth committed Jul 11, 2024
1 parent d4e46f0 commit 88dfd3a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 6 deletions.
10 changes: 6 additions & 4 deletions clang/lib/Driver/ToolChains/CommonArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,12 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
RequiredSymbols);
}

// -u options must be added before the runtime libs that resolve them.
for (auto S : RequiredSymbols) {
CmdArgs.push_back("-u");
CmdArgs.push_back(Args.MakeArgString(S));
}

// Inject libfuzzer dependencies.
if (SanArgs.needsFuzzer() && SanArgs.linkRuntimes() &&
!Args.hasArg(options::OPT_shared)) {
Expand Down Expand Up @@ -1566,10 +1572,6 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
addSanitizerRuntime(TC, Args, CmdArgs, RT, false, false);
AddExportDynamic |= !addSanitizerDynamicList(TC, Args, CmdArgs, RT);
}
for (auto S : RequiredSymbols) {
CmdArgs.push_back("-u");
CmdArgs.push_back(Args.MakeArgString(S));
}
// If there is a static runtime with no dynamic list, force all the symbols
// to be dynamic to be sure we export sanitizer interface functions.
if (AddExportDynamic)
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/ohos.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@
// RUN: | FileCheck %s -check-prefix=CHECK-SAFESTACK
// CHECK-SAFESTACK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
// CHECK-SAFESTACK: "-fsanitize=safe-stack"
// CHECK-SAFESTACK: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}arm-liteos-ohos{{/|\\\\}}libclang_rt.safestack.a"
// CHECK-SAFESTACK: "__safestack_init"
// CHECK-SAFESTACK: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}arm-liteos-ohos{{/|\\\\}}libclang_rt.safestack.a"

// RUN: %clang %s -### --target=arm-liteos \
// RUN: -fsanitize=address 2>&1 \
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/sanitizer-ld.c
Original file line number Diff line number Diff line change
Expand Up @@ -753,8 +753,8 @@
// CHECK-SAFESTACK-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
// CHECK-SAFESTACK-LINUX-NOT: "-lc"
// CHECK-SAFESTACK-LINUX-NOT: whole-archive
// CHECK-SAFESTACK-LINUX: libclang_rt.safestack.a"
// CHECK-SAFESTACK-LINUX: "-u" "__safestack_init"
// CHECK-SAFESTACK-LINUX: libclang_rt.safestack.a"
// CHECK-SAFESTACK-LINUX: "-lpthread"
// CHECK-SAFESTACK-LINUX: "-ldl"
// CHECK-SAFESTACK-LINUX: "-lresolv"
Expand Down

0 comments on commit 88dfd3a

Please sign in to comment.