Skip to content

Conversation

@dgg5503
Copy link
Contributor

@dgg5503 dgg5503 commented Apr 23, 2025

Lifetime intrinsics required for detection of use-after-scope are not emitted under kernel-address sanitizer (-fsanitize=kernel-address) when paired with -O0 & -fsanitize-address-use-after-scope.

This is because with -fsanitize=kernel-address -O0 under shouldEmitLifetimeMarkers in clang\lib\CodeGen\CodeGenFunction.cpp, CGOpts.SanitizeAddressUseAfterScope is set to false. Therefore, the following check, CGOpts.OptimizationLevel != 0, is run which evaluates to false thus preventing the emission of lifetime markers:

/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
/// markers.
static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
const LangOptions &LangOpts) {
if (CGOpts.DisableLifetimeMarkers)
return false;
// Sanitizers may use markers.
if (CGOpts.SanitizeAddressUseAfterScope ||
LangOpts.Sanitize.has(SanitizerKind::HWAddress) ||
LangOpts.Sanitize.has(SanitizerKind::Memory))
return true;
// For now, only in optimized builds.
return CGOpts.OptimizationLevel != 0;
}

The reason CGOpts.SanitizeAddressUseAfterScope is false stems from the fact that this variable is normally set via the frontend flag -fsanitize-address-use-after-scope, however, this flag only takes effect under normal address sanitizer due to the gated logic in clang\lib\Driver\SanitizerArgs.cpp, specifically, if (AllAddedKinds & SanitizerKind::Address):

if (AllAddedKinds & SanitizerKind::Address) {

And later on down in this block:
AsanUseAfterScope = Args.hasFlag(
options::OPT_fsanitize_address_use_after_scope,
options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);

This check excludes SanitizerKind::KernelAddress from consideration, so even if -fsanitize-address-use-after-scope is supplied as a front-end argument, it won't be passed to cc1 thus preventing use-after-scope checks from being emitted under -fsanitize-kernel-address -O0. Higher optimization levels will allow emission of lifetime markers regardless thanks to the logic in shouldEmitLifetimeMarkers.

This PR allows -fsanitize-address-use-after-scope to take effect under kernel-address sanitizer.

…ze=kernel-address

Lifetime intrinsics required for detection of use-after-scope are not emitted
under kernel-address sanitizer (`-fsanitize=kernel-address`) when paired with
`-O0` & `-fsanitize-address-use-after-scope`.

This is because with `-fsanitize=kernel-address -O0` under
`shouldEmitLifetimeMarkers` in `clang\lib\CodeGen\CodeGenFunction.cpp`,
`CGOpts.SanitizeAddressUseAfterScope` is set to `false`. Therefore, the
following check, `CGOpts.OptimizationLevel != 0`, is run which evaluates to
`false` thus preventing the emission of lifetime markers.

The reason `CGOpts.SanitizeAddressUseAfterScope` is false stems from the fact
that this variable is normally set via the frontend flag
`-fsanitize-address-use-after-scope`, however, this flag only takes effect
under normal address sanitizer due to the gated logic in
`clang\lib\Driver\SanitizerArgs.cpp`, specifically,
`if (AllAddedKinds & SanitizerKind::Address)`. This check excludes
`SanitizerKind::KernelAddress` from consideration, so even if
`-fsanitize-address-use-after-scope` is supplied as a front-end argument, it
won't be passed to `cc1` thus preventing `use-after-scope` checks from
being emitted under `-fsanitize-kernel-address -O0`. Higher optimization levels
will allow emission of lifetime markers regardless thanks to the logic in
`shouldEmitLifetimeMarkers`.

This PR allows `-fsanitize-address-use-after-scope` to take effect under
kernel-address sanitizer.
@dgg5503 dgg5503 requested a review from nikic April 23, 2025 16:27
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Apr 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2025

@llvm/pr-subscribers-clang

Author: Douglas (dgg5503)

Changes

Lifetime intrinsics required for detection of use-after-scope are not emitted under kernel-address sanitizer (-fsanitize=kernel-address) when paired with -O0 & -fsanitize-address-use-after-scope.

This is because with -fsanitize=kernel-address -O0 under shouldEmitLifetimeMarkers in clang\lib\CodeGen\CodeGenFunction.cpp, CGOpts.SanitizeAddressUseAfterScope is set to false. Therefore, the following check, CGOpts.OptimizationLevel != 0, is run which evaluates to false thus preventing the emission of lifetime markers:

/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
/// markers.
static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
const LangOptions &LangOpts) {
if (CGOpts.DisableLifetimeMarkers)
return false;
// Sanitizers may use markers.
if (CGOpts.SanitizeAddressUseAfterScope ||
LangOpts.Sanitize.has(SanitizerKind::HWAddress) ||
LangOpts.Sanitize.has(SanitizerKind::Memory))
return true;
// For now, only in optimized builds.
return CGOpts.OptimizationLevel != 0;
}

The reason CGOpts.SanitizeAddressUseAfterScope is false stems from the fact that this variable is normally set via the frontend flag -fsanitize-address-use-after-scope, however, this flag only takes effect under normal address sanitizer due to the gated logic in clang\lib\Driver\SanitizerArgs.cpp, specifically, if (AllAddedKinds & SanitizerKind::Address):

if (AllAddedKinds & SanitizerKind::Address) {

This check excludes SanitizerKind::KernelAddress from consideration, so even if -fsanitize-address-use-after-scope is supplied as a front-end argument, it won't be passed to cc1 thus preventing use-after-scope checks from being emitted under -fsanitize-kernel-address -O0. Higher optimization levels will allow emission of lifetime markers regardless thanks to the logic in shouldEmitLifetimeMarkers.

This PR allows -fsanitize-address-use-after-scope to take effect under kernel-address sanitizer.


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

3 Files Affected:

  • (modified) clang/lib/Driver/SanitizerArgs.cpp (+7-1)
  • (modified) clang/test/CodeGen/lifetime-sanitizer.c (+3)
  • (modified) clang/test/CodeGenCXX/lifetime-sanitizer.cpp (+3)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index f27cb813012f2..b428ded90a72e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1099,7 +1099,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
     }
 
   } else {
-    AsanUseAfterScope = false;
+    if (AllAddedKinds & SanitizerKind::KernelAddress) {
+      AsanUseAfterScope = Args.hasFlag(
+          options::OPT_fsanitize_address_use_after_scope,
+          options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);
+    } else {
+      AsanUseAfterScope = false;
+    }
     // -fsanitize=pointer-compare/pointer-subtract requires -fsanitize=address.
     SanitizerMask DetectInvalidPointerPairs =
         SanitizerKind::PointerCompare | SanitizerKind::PointerSubtract;
diff --git a/clang/test/CodeGen/lifetime-sanitizer.c b/clang/test/CodeGen/lifetime-sanitizer.c
index b15d692b79e36..68879fda1e1a5 100644
--- a/clang/test/CodeGen/lifetime-sanitizer.c
+++ b/clang/test/CodeGen/lifetime-sanitizer.c
@@ -4,6 +4,9 @@
 // RUN:     -fsanitize=address -fsanitize-address-use-after-scope \
 // RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefix=LIFETIME
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN:     -fsanitize=kernel-address -fsanitize-address-use-after-scope \
+// RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN:     -fsanitize=memory -Xclang -disable-llvm-passes %s | \
 // RUN:     FileCheck %s -check-prefix=LIFETIME
 // RUN: %clang -target aarch64-linux-gnu -S -emit-llvm -o - -O0 \
diff --git a/clang/test/CodeGenCXX/lifetime-sanitizer.cpp b/clang/test/CodeGenCXX/lifetime-sanitizer.cpp
index 33a8566092519..225d5e28921b8 100644
--- a/clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ b/clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -5,6 +5,9 @@
 // RUN:     -fsanitize=address -fsanitize-address-use-after-scope \
 // RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefixes=CHECK,LIFETIME
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN:     -fsanitize=kernel-address -fsanitize-address-use-after-scope \
+// RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN:     -fsanitize=memory -Xclang -disable-llvm-passes %s | \
 // RUN:     FileCheck %s -check-prefixes=CHECK,LIFETIME
 // RUN: %clang -w -target aarch64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \

@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2025

@llvm/pr-subscribers-clang-driver

Author: Douglas (dgg5503)

Changes

Lifetime intrinsics required for detection of use-after-scope are not emitted under kernel-address sanitizer (-fsanitize=kernel-address) when paired with -O0 & -fsanitize-address-use-after-scope.

This is because with -fsanitize=kernel-address -O0 under shouldEmitLifetimeMarkers in clang\lib\CodeGen\CodeGenFunction.cpp, CGOpts.SanitizeAddressUseAfterScope is set to false. Therefore, the following check, CGOpts.OptimizationLevel != 0, is run which evaluates to false thus preventing the emission of lifetime markers:

/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
/// markers.
static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
const LangOptions &LangOpts) {
if (CGOpts.DisableLifetimeMarkers)
return false;
// Sanitizers may use markers.
if (CGOpts.SanitizeAddressUseAfterScope ||
LangOpts.Sanitize.has(SanitizerKind::HWAddress) ||
LangOpts.Sanitize.has(SanitizerKind::Memory))
return true;
// For now, only in optimized builds.
return CGOpts.OptimizationLevel != 0;
}

The reason CGOpts.SanitizeAddressUseAfterScope is false stems from the fact that this variable is normally set via the frontend flag -fsanitize-address-use-after-scope, however, this flag only takes effect under normal address sanitizer due to the gated logic in clang\lib\Driver\SanitizerArgs.cpp, specifically, if (AllAddedKinds & SanitizerKind::Address):

if (AllAddedKinds & SanitizerKind::Address) {

This check excludes SanitizerKind::KernelAddress from consideration, so even if -fsanitize-address-use-after-scope is supplied as a front-end argument, it won't be passed to cc1 thus preventing use-after-scope checks from being emitted under -fsanitize-kernel-address -O0. Higher optimization levels will allow emission of lifetime markers regardless thanks to the logic in shouldEmitLifetimeMarkers.

This PR allows -fsanitize-address-use-after-scope to take effect under kernel-address sanitizer.


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

3 Files Affected:

  • (modified) clang/lib/Driver/SanitizerArgs.cpp (+7-1)
  • (modified) clang/test/CodeGen/lifetime-sanitizer.c (+3)
  • (modified) clang/test/CodeGenCXX/lifetime-sanitizer.cpp (+3)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index f27cb813012f2..b428ded90a72e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1099,7 +1099,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
     }
 
   } else {
-    AsanUseAfterScope = false;
+    if (AllAddedKinds & SanitizerKind::KernelAddress) {
+      AsanUseAfterScope = Args.hasFlag(
+          options::OPT_fsanitize_address_use_after_scope,
+          options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);
+    } else {
+      AsanUseAfterScope = false;
+    }
     // -fsanitize=pointer-compare/pointer-subtract requires -fsanitize=address.
     SanitizerMask DetectInvalidPointerPairs =
         SanitizerKind::PointerCompare | SanitizerKind::PointerSubtract;
diff --git a/clang/test/CodeGen/lifetime-sanitizer.c b/clang/test/CodeGen/lifetime-sanitizer.c
index b15d692b79e36..68879fda1e1a5 100644
--- a/clang/test/CodeGen/lifetime-sanitizer.c
+++ b/clang/test/CodeGen/lifetime-sanitizer.c
@@ -4,6 +4,9 @@
 // RUN:     -fsanitize=address -fsanitize-address-use-after-scope \
 // RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefix=LIFETIME
 // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
+// RUN:     -fsanitize=kernel-address -fsanitize-address-use-after-scope \
+// RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefix=LIFETIME
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
 // RUN:     -fsanitize=memory -Xclang -disable-llvm-passes %s | \
 // RUN:     FileCheck %s -check-prefix=LIFETIME
 // RUN: %clang -target aarch64-linux-gnu -S -emit-llvm -o - -O0 \
diff --git a/clang/test/CodeGenCXX/lifetime-sanitizer.cpp b/clang/test/CodeGenCXX/lifetime-sanitizer.cpp
index 33a8566092519..225d5e28921b8 100644
--- a/clang/test/CodeGenCXX/lifetime-sanitizer.cpp
+++ b/clang/test/CodeGenCXX/lifetime-sanitizer.cpp
@@ -5,6 +5,9 @@
 // RUN:     -fsanitize=address -fsanitize-address-use-after-scope \
 // RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefixes=CHECK,LIFETIME
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
+// RUN:     -fsanitize=kernel-address -fsanitize-address-use-after-scope \
+// RUN:     -Xclang -disable-llvm-passes %s | FileCheck %s -check-prefixes=CHECK,LIFETIME
+// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \
 // RUN:     -fsanitize=memory -Xclang -disable-llvm-passes %s | \
 // RUN:     FileCheck %s -check-prefixes=CHECK,LIFETIME
 // RUN: %clang -w -target aarch64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \

@dgg5503
Copy link
Contributor Author

dgg5503 commented Apr 23, 2025

Hi @nikic,

I added you as a reviewer since your name popped up as a suggestion. Please feel free to remove yourself from the reviewers list if you prefer.

@nikic nikic requested review from MaskRay and vitalybuka April 23, 2025 18:42
Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

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

Thanks!

@vitalybuka
Copy link
Collaborator

vitalybuka commented Apr 24, 2025

However, please be aware that default for Asan is ON. Should Kasan default be OFF in the beginning?

@dgg5503
Copy link
Contributor Author

dgg5503 commented Apr 24, 2025

However, please be aware that default for Asan is ON. Should Kasan default be OFF in the beginning?

I'd be happy to flip the default to OFF for KASAN to preserve existing expectations if there are no objections. I'll wait a few hours for others to respond.

@dgg5503
Copy link
Contributor Author

dgg5503 commented Apr 25, 2025

However, please be aware that default for Asan is ON. Should Kasan default be OFF in the beginning?

@vitalybuka I'm not sure how many users test a kernel exclusively in -O0, but to those who do, I suppose defaulting this to ON could allow the sanitizer to capture never-before-seen use-after-scope issues.

With that said, I'm inclined to keep this defaulted to ON like normal ASAN. Does this sound reasonable, or am I missing some negative side-effects that you had in mind when posting this comment? Would it be worth mentioning this change as a release note regardless?

EDIT: I realize one detail I missed when I wrote the description of the PR. The 'workaround' to get use-after-scope functioning with -fsanitize=kernel-address for -O1 and up before this PR involves adding -mllvm -asan-use-after-scope=1. So by defaulting -fsanitize-address-use-after-scope to ON for -fsanitize=kernel-address in this PR, we'd actually be enabling UAS detection for all optimization levels which may be new to those who haven't employed the workaround. I suppose from that PoV, effects are a bit more far reaching than I initially though. I'm still inclined to keep it ON by default if not to increase coverage, but maybe I'm being optimistic that it wouldn't have negative effects elsewhere...

@dgg5503
Copy link
Contributor Author

dgg5503 commented Apr 28, 2025

Ping for @vitalybuka , I'll merge this in by EOD if there are no objections based on my latest comment (for real this time 😀).

@vitalybuka
Copy link
Collaborator

However, please be aware that default for Asan is ON. Should Kasan default be OFF in the beginning?

@vitalybuka I'm not sure how many users test a kernel exclusively in -O0, but to those who do, I suppose defaulting this to ON could allow the sanitizer to capture never-before-seen use-after-scope issues.

With that said, I'm inclined to keep this defaulted to ON like normal ASAN. Does this sound reasonable, or am I missing some negative side-effects that you had in mind when posting this comment? Would it be worth mentioning this change as a release note regardless?

EDIT: I realize one detail I missed when I wrote the description of the PR. The 'workaround' to get use-after-scope functioning with -fsanitize=kernel-address for -O1 and up before this PR involves adding -mllvm -asan-use-after-scope=1. So by defaulting -fsanitize-address-use-after-scope to ON for -fsanitize=kernel-address in this PR, we'd actually be enabling UAS detection for all optimization levels which may be new to those who haven't employed the workaround. I suppose from that PoV, effects are a bit more far reaching than I initially though. I'm still inclined to keep it ON by default if not to increase coverage, but maybe I'm being optimistic that it wouldn't have negative effects elsewhere...

LGTM, I guess it's easy to add -fno-sanitize-address-use-after-scope into kernel build files if needed.

@dgg5503 dgg5503 merged commit efd46bc into llvm:main Apr 28, 2025
11 checks passed
@dgg5503 dgg5503 deleted the dgg5503/main/kasan-use-after-scope branch April 28, 2025 18:54
@dgg5503
Copy link
Contributor Author

dgg5503 commented Apr 28, 2025

@vitalybuka , great, thanks so much for the review.

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ith -fsanitize=kernel-address (llvm#137015)

Allow `-f[no]-sanitize-address-use-after-scope` to take effect under
kernel-address sanitizer (`-fsanitize=kernel-address`). `use-after-scope` is
now enabled by default under kernel-address sanitizer.

Previously, users may have enabled `use-after-scope` checks for kernel-address
sanitizer via `-mllvm -asan-use-after-scope=true`. While this may have worked
for optimization levels > O0, the required lifetime intrinsics to allow for
`use-after-scope` detection were not emitted under O0. This commit ensures
the required lifetime intrinsics are emitted under O0 with kernel-address
sanitizer.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…ith -fsanitize=kernel-address (llvm#137015)

Allow `-f[no]-sanitize-address-use-after-scope` to take effect under
kernel-address sanitizer (`-fsanitize=kernel-address`). `use-after-scope` is
now enabled by default under kernel-address sanitizer.

Previously, users may have enabled `use-after-scope` checks for kernel-address
sanitizer via `-mllvm -asan-use-after-scope=true`. While this may have worked
for optimization levels > O0, the required lifetime intrinsics to allow for
`use-after-scope` detection were not emitted under O0. This commit ensures
the required lifetime intrinsics are emitted under O0 with kernel-address
sanitizer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants