Skip to content
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

[LLVM] Make sanitizers respect the disable_santizer_instrumentation attribute. #91732

Merged

Conversation

DanielKristofKiss
Copy link
Member

@DanielKristofKiss DanielKristofKiss commented May 10, 2024

disable_sanitizer_instrumetation is attached to functions that shall not be instrumented e.g. ifunc resolver because those run before everything is initialised.
Some sanitizer already handles this attribute, this patch adds it to DataFLow and Coverage too.

@llvmbot
Copy link
Collaborator

llvmbot commented May 10, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Daniel Kiss (DanielKristofKiss)

Changes

disable_sanitizer_instrumetation is attached to functions that shall not be instrumented e.g. ifunc resolver because those run before everything is initialised.
Some sanitizer already handles this attribute, this patch adds it to DataFLow, GCOV, MemProfiler and Coverage too.


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

7 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp (+2-1)
  • (modified) llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp (+2)
  • (modified) llvm/lib/Transforms/Instrumentation/MemProfiler.cpp (+2)
  • (modified) llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp (+2)
  • (added) llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll (+48)
  • (added) llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll (+47)
  • (added) llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll (+47)
diff --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
index 851edb4ce829e..20d11e0ab55f2 100644
--- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -1546,7 +1546,8 @@ bool DataFlowSanitizer::runImpl(
   SmallPtrSet<Constant *, 1> PersonalityFns;
   for (Function &F : M)
     if (!F.isIntrinsic() && !DFSanRuntimeFunctions.contains(&F) &&
-        !LibAtomicFunction(F)) {
+        !LibAtomicFunction(F) &&
+        !F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation)) {
       FnsToInstrument.push_back(&F);
       if (F.hasPersonalityFn())
         PersonalityFns.insert(F.getPersonalityFn()->stripPointerCasts());
diff --git a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
index c7f6f2a43c17f..942d197586cfa 100644
--- a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -803,6 +803,8 @@ bool GCOVProfiler::emitProfileNotes(
         continue;
       if (F.hasFnAttribute(llvm::Attribute::SkipProfile))
         continue;
+      if (F.hasFnAttribute(llvm::Attribute::DisableSanitizerInstrumentation))
+        continue;
 
       // Add the function line number to the lines of the entry block
       // to have a counter for the function definition.
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index c0a3bf8464d2d..c826c8665ccf7 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -560,6 +560,8 @@ bool MemProfiler::instrumentFunction(Function &F) {
     return false;
   if (F.getName().starts_with("__memprof_"))
     return false;
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+    return false;
 
   bool FunctionModified = false;
 
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index 56d4907ae47a9..6a89cee9aaf6c 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -631,6 +631,8 @@ void ModuleSanitizerCoverage::instrumentFunction(Function &F) {
     return;
   if (F.hasFnAttribute(Attribute::NoSanitizeCoverage))
     return;
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+    return;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge) {
     SplitAllCriticalEdges(
         F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
diff --git a/llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll
new file mode 100644
index 0000000000000..c39c3dfd0a99a
--- /dev/null
+++ b/llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,48 @@
+
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='module(msan)' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_memory is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(ptr %a) sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: %0 = load i64, ptr @__msan_param_tls
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(ptr %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_memory.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(ptr %a) disable_sanitizer_instrumentation sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
diff --git a/llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll
new file mode 100644
index 0000000000000..7cb50be7fddd8
--- /dev/null
+++ b/llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='module(msan)' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_memory is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(ptr %a) sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: %0 = load i64, ptr @__msan_param_tls
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(ptr %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_memory.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(ptr %a) disable_sanitizer_instrumentation sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll
new file mode 100644
index 0000000000000..bb70dcc161723
--- /dev/null
+++ b/llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-control-flow -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(ptr %a) sanitize_address {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__sanitizer_cov_trace_pc_guard(
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(ptr %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc_guard(
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(ptr %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc_guard(
+

@llvmbot
Copy link
Collaborator

llvmbot commented May 10, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Daniel Kiss (DanielKristofKiss)

Changes

disable_sanitizer_instrumetation is attached to functions that shall not be instrumented e.g. ifunc resolver because those run before everything is initialised.
Some sanitizer already handles this attribute, this patch adds it to DataFLow, GCOV, MemProfiler and Coverage too.


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

7 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp (+2-1)
  • (modified) llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp (+2)
  • (modified) llvm/lib/Transforms/Instrumentation/MemProfiler.cpp (+2)
  • (modified) llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp (+2)
  • (added) llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll (+48)
  • (added) llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll (+47)
  • (added) llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll (+47)
diff --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
index 851edb4ce829e..20d11e0ab55f2 100644
--- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -1546,7 +1546,8 @@ bool DataFlowSanitizer::runImpl(
   SmallPtrSet<Constant *, 1> PersonalityFns;
   for (Function &F : M)
     if (!F.isIntrinsic() && !DFSanRuntimeFunctions.contains(&F) &&
-        !LibAtomicFunction(F)) {
+        !LibAtomicFunction(F) &&
+        !F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation)) {
       FnsToInstrument.push_back(&F);
       if (F.hasPersonalityFn())
         PersonalityFns.insert(F.getPersonalityFn()->stripPointerCasts());
diff --git a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
index c7f6f2a43c17f..942d197586cfa 100644
--- a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -803,6 +803,8 @@ bool GCOVProfiler::emitProfileNotes(
         continue;
       if (F.hasFnAttribute(llvm::Attribute::SkipProfile))
         continue;
+      if (F.hasFnAttribute(llvm::Attribute::DisableSanitizerInstrumentation))
+        continue;
 
       // Add the function line number to the lines of the entry block
       // to have a counter for the function definition.
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index c0a3bf8464d2d..c826c8665ccf7 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -560,6 +560,8 @@ bool MemProfiler::instrumentFunction(Function &F) {
     return false;
   if (F.getName().starts_with("__memprof_"))
     return false;
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+    return false;
 
   bool FunctionModified = false;
 
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index 56d4907ae47a9..6a89cee9aaf6c 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -631,6 +631,8 @@ void ModuleSanitizerCoverage::instrumentFunction(Function &F) {
     return;
   if (F.hasFnAttribute(Attribute::NoSanitizeCoverage))
     return;
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+    return;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge) {
     SplitAllCriticalEdges(
         F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
diff --git a/llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll
new file mode 100644
index 0000000000000..c39c3dfd0a99a
--- /dev/null
+++ b/llvm/test/Instrumentation/DataFlowSanitizer/dataflow-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,48 @@
+
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='module(msan)' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_memory is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(ptr %a) sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: %0 = load i64, ptr @__msan_param_tls
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(ptr %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_memory.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(ptr %a) disable_sanitizer_instrumentation sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
diff --git a/llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll
new file mode 100644
index 0000000000000..7cb50be7fddd8
--- /dev/null
+++ b/llvm/test/Instrumentation/MemorySanitizer/msan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='module(msan)' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_memory is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(ptr %a) sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: %0 = load i64, ptr @__msan_param_tls
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(ptr %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_memory.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(ptr %a) disable_sanitizer_instrumentation sanitize_memory {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: %0 = load i64, ptr @__msan_param_tls
+
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll
new file mode 100644
index 0000000000000..bb70dcc161723
--- /dev/null
+++ b/llvm/test/Instrumentation/SanitizerCoverage/coverage-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-control-flow -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(ptr %a) sanitize_address {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__sanitizer_cov_trace_pc_guard(
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(ptr %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc_guard(
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(ptr %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, ptr %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, ptr %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc_guard(
+

@DanielKristofKiss
Copy link
Member Author

@@ -803,6 +803,8 @@ bool GCOVProfiler::emitProfileNotes(
continue;
if (F.hasFnAttribute(llvm::Attribute::SkipProfile))
continue;
if (F.hasFnAttribute(llvm::Attribute::DisableSanitizerInstrumentation))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is not about sanitizers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see why we need this, it's just a little bit inconsistent to use attribute with sanitizer in name to suppress profiling.
would be possible to add a test for GCOV and memprof as well?

Copy link
Contributor

@melver melver May 13, 2024

Choose a reason for hiding this comment

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

As Vitaly said this is not a sanitizer, so it'll be unintuitive if the attribute also kills instrumentation here.

For code that needs to disable all kinds of instrumentation, something like Linux's noinstr is needed, and you have to build a list of attributes that also includes __attribute__((no_profile_instrument_function)) (<- disables GCOV).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I move all profiling to a separate PR.
no_profile_instrument_function sound better for profiles as a common attribute.

@@ -560,6 +560,8 @@ bool MemProfiler::instrumentFunction(Function &F) {
return false;
if (F.getName().starts_with("__memprof_"))
return false;
if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
Copy link
Contributor

@melver melver May 13, 2024

Choose a reason for hiding this comment

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

Also not a sanitizer, but it looks like there's no existing attribute to disable it?

Copy link
Member Author

Choose a reason for hiding this comment

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

there isn't as I see, let's make no_profile_instrument_function be the one.

…ttribute.

Disable_sanitizer_instrumetation is attached to functions that shall not
be instrumented, e.g. ifunc resolver because those run before everything
is initialized.
Some sanitizer already handles this attribute, this patch adds it to
DataFLow and Coverage.
@DanielKristofKiss DanielKristofKiss merged commit 45726c1 into llvm:main May 15, 2024
3 of 4 checks passed
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.

None yet

4 participants