Skip to content

EntryExitInstrumenter: skip available_externally linkage #121452

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.

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

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jan 2, 2025

gnu::always_inline functions, which lower to available_externally, may
not have definitions external to the module. -finstrument-function
family options instrumentating the function (which takes the function
address) may lead to a linker error if the function is not optimized
out, e.g.

// -std=c++17 or above with libstdc++
 #include <string>
std::string str;
int main() {}

Simplified reproduce:

template <typename T>
struct A {
  [[gnu::always_inline]] T bar(T a) { return a * 2; }
};
extern template class A<int>;
int main(int argc, char **argv) {
  return A<int>().bar(argc);
}

GCC's -finstrument-function instrumentation skips such functions
(https://gcc.gnu.org/PR78333). Let's skip such functions
(available_externally) as well.

Fix #50742

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Fangrui Song (MaskRay)

Changes

gnu::always_inline functions, which lower to available_externally, may
not have definitions external to the module. -finstrument-function
family options instrumentating the function (which takes the function
address) will lead to a linker error, e.g.

// -std=c++17 or above with libstdc++
 #include &lt;string&gt;
std::string str;
int main() {}

Simplified reproduce:

template &lt;typename T&gt;
struct A {
  [[gnu::always_inline]] T bar(T a) { return a * 2; }
};
extern template class A&lt;int&gt;;
int main(int argc, char **argv) {
  return A&lt;int&gt;().bar(argc);
}

GCC's -finstrument-function instrumentation skips such functions
(https://gcc.gnu.org/PR78333). Let's skip such functions
(available_externally) as well.

Fix #50742


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp (+6)
  • (modified) llvm/test/Transforms/EntryExitInstrumenter/mcount.ll (+7)
diff --git a/llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp b/llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
index 5b33edd51cffa6..8dd54a0bc25ebd 100644
--- a/llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ b/llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -103,6 +103,12 @@ static bool runOnFunction(Function &F, bool PostInlining) {
   if (F.hasFnAttribute(Attribute::Naked))
     return false;
 
+  // available_externally functions may not have definitions external to the
+  // module (e.g. gnu::always_inline). Instrumenting them would lead to linker
+  // errors. Skip them like GCC.
+  if (F.hasAvailableExternallyLinkage())
+    return false;
+
   StringRef EntryAttr = PostInlining ? "instrument-function-entry-inlined"
                                      : "instrument-function-entry";
 
diff --git a/llvm/test/Transforms/EntryExitInstrumenter/mcount.ll b/llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
index bd5f4c2b51a896..56ccfb9ed2e7e0 100644
--- a/llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
+++ b/llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
@@ -129,6 +129,13 @@ define void @naked() naked {
   ret void
 }
 
+define available_externally void @always_inline() {
+; CHECK-LABEL: define available_externally void @always_inline() {
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+
 ; The attributes are "consumed" when the instrumentation is inserted.
 ; CHECK: attributes
 ; CHECK-NOT: instrument-function

.
Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit e6f7637 into main Jan 3, 2025
8 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/entryexitinstrumenter-skip-available_externally-linkage branch January 3, 2025 17:25
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 10, 2025
gnu::always_inline functions, which lower to available_externally, may
not have definitions external to the module. -finstrument-function
family options instrumentating the function (which takes the function
address) may lead to a linker error if the function is not optimized
out, e.g.

```
// -std=c++17 or above with libstdc++
 #include <string>
std::string str;
int main() {}
```

Simplified reproduce:
```
template <typename T>
struct A {
  [[gnu::always_inline]] T bar(T a) { return a * 2; }
};
extern template class A<int>;
int main(int argc, char **argv) {
  return A<int>().bar(argc);
}
```

GCC's -finstrument-function instrumentation skips such functions
(https://gcc.gnu.org/PR78333). Let's skip such functions
(available_externally) as well.

Fix #50742

Pull Request: llvm/llvm-project#121452
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-finstrument-functions + libstdc++ + C++17/20 = undefined symbol linker error
3 participants