[asan][x86] Skip memintrinsics that write to special address spaces on x86-64 #129291

cheezeburglar · 2025-02-28T19:37:11Z

Currently code such as

#include "stdint.h"
void test(uintptr_t addr) {
	__builtin_memcpy(( void __seg_fs*)addr, "x", 1);
}

and

...
call void @llvm.memcpy.p257.p0.i64(ptr addrspace(257) align 1 %4, ptr align 1 @.str, i64 1, i1 false)
...

are miscompiled by ASan. When any of mem{set, cpy, move} are intercepted, by ASan, the writes to fs and gs are effectively scrubbed from the calls to ASan's runtime. This commit just causes AddressSanitizer to bail when on instrumenting these sorts of memory intrinsics.

(Really sorry if you got pinged accidentally when I was editing the commit history before - my bad)

llvmbot · 2025-02-28T22:16:02Z

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

Author: Thor Preimesberger (cheezeburglar)

Changes

For #124238

Currently code such as

#include "stdint.h"
void test(uintptr_t addr) {
	__builtin_memcpy(( void __seg_fs*)addr, "x", 1);
}

and

...
call void @<!-- -->llvm.memcpy.p257.p0.i64(ptr addrspace(257) align 1 %4, ptr align 1 @.str, i64 1, i1 false)
...

are miscompiled by ASan. When any of mem{set, cpy, move} are intercepted, by ASan, the writes to fs and gs are effectively scrubbed from the calls to ASan's runtime. This commit just causes AddressSanitizer to bail when on instrumenting these sorts of memory intrinsics.

(Really sorry if you got pinged accidentally when I was editing the commit history before - my bad)

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

2 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+12)
(added) llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll (+60)

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 8d8d56035a48f..85edcb1276efe 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -797,6 +797,7 @@ struct AddressSanitizer {
                                  bool IsWrite, size_t AccessSizeIndex,
                                  Value *SizeArgument, uint32_t Exp,
                                  RuntimeCallInserter &RTCI);
+  bool maybeIgnoreMemIntrinsic(MemIntrinsic *MI, const Triple &TargetTriple);
   void instrumentMemIntrinsic(MemIntrinsic *MI, RuntimeCallInserter &RTCI);
   Value *memToShadow(Value *Shadow, IRBuilder<> &IRB);
   bool suppressInstrumentationSiteForDebug(int &Instrumented);
@@ -1340,10 +1341,21 @@ Value *AddressSanitizer::memToShadow(Value *Shadow, IRBuilder<> &IRB) {
     return IRB.CreateAdd(Shadow, ShadowBase);
 }
 
+bool AddressSanitizer::maybeIgnoreMemIntrinsic(MemIntrinsic *MI,
+                                               const Triple &TargetTriple) {
+  // Ignore FS and GS registers to prevent miscompilation
+  if (MI->getDestAddressSpace() >= 256 &&
+      TargetTriple.getArch() == Triple::x86_64)
+    return true;
+  return false;
+}
+
 // Instrument memset/memmove/memcpy
 void AddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI,
                                               RuntimeCallInserter &RTCI) {
   InstrumentationIRBuilder IRB(MI);
+  if (maybeIgnoreMemIntrinsic(MI, TargetTriple))
+    return;
   if (isa<MemTransferInst>(MI)) {
     RTCI.createRuntimeCall(
         IRB, isa<MemMoveInst>(MI) ? AsanMemmove : AsanMemcpy,
diff --git a/llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll b/llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll
new file mode 100644
index 0000000000000..ce82bc48563a0
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll
@@ -0,0 +1,60 @@
+; RUN: opt -passes=asan %s -S | FileCheck %s
+
+;; Punt AddressSanitizer::instrumentMemIntrinsics out for MemIntrinsics
+;; that need write to unsupported registers on X86
+;; PR124238: https://www.github.com/llvm/llvm-project/issues/124238
+
+target triple = "x86_64-unknown-linux-gnu"
+
+$.str.658906a285b7a0f82dabd9915e07848c = comdat any
+@.str = internal constant { [2 x i8], [30 x i8] } { [2 x i8] c"x\00", [30 x i8] zeroinitializer }, comdat($.str.658906a285b7a0f82dabd9915e07848c), align 32
+@0 = private alias { [2 x i8], [30 x i8] }, ptr @.str
+
+define void @test_memcpy(i64 noundef %addr) sanitize_address #0 {
+entry:
+  %addr.addr = alloca i64, align 8
+  store i64 %addr, ptr %addr.addr, align 8
+  %0 = load i64, ptr %addr.addr, align 8
+  %1 = inttoptr i64 %0 to ptr addrspace(257)
+  call void @llvm.memcpy.p257.p0.i64(ptr addrspace(257) align 1 %1, ptr align 1 @.str, i64 1, i1 false)
+; CHECK: llvm.memcpy
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr addrspace(256)
+  call void @llvm.memcpy.p256.p0.i64(ptr addrspace(256) align 1 %3, ptr align 1 @.str, i64 1, i1 false)
+; CHECK: llvm.memcpy
+  ret void
+}
+
+define void @test_memset(i64 noundef %addr) sanitize_address #0 {
+entry:
+  %addr.addr = alloca i64, align 8
+  store i64 %addr, ptr %addr.addr, align 8
+  %0 = load i64, ptr %addr.addr, align 8
+  %1 = inttoptr i64 %0 to ptr addrspace(257)
+  call void @llvm.memset.p257.i64(ptr addrspace(257) align 1 %1, i8 0, i64 1, i1 false)
+; CHECK: llvm.memset
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr addrspace(256)
+  call void @llvm.memset.p256.i64(ptr addrspace(256) align 1 %3, i8 0, i64 1, i1 false)
+; CHECK: llvm.memset
+  ret void
+}
+
+define void @test_memmove(i64 noundef %addr) sanitize_address #0 {
+entry:
+  %addr.addr = alloca i64, align 8
+  store i64 %addr, ptr %addr.addr, align 8
+  %0 = load i64, ptr %addr.addr, align 8
+  %1 = inttoptr i64 %0 to ptr addrspace(257)
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr
+  call void @llvm.memmove.p257.p0.i64(ptr addrspace(257) align 1 %1, ptr align 1 %3, i64 1, i1 false)
+; CHECK: llvm.memmove
+  %4 = load i64, ptr %addr.addr, align 8
+  %5 = inttoptr i64 %4 to ptr addrspace(256)
+  %6 = load i64, ptr %addr.addr, align 8
+  %7 = inttoptr i64 %6 to ptr
+  call void @llvm.memmove.p256.p0.i64(ptr addrspace(256) align 1 %5, ptr align 1 %7, i64 1, i1 false)
+; CHECK: llvm.memmove
+  ret void
+}

llvmbot · 2025-02-28T22:16:03Z

@llvm/pr-subscribers-llvm-transforms

Author: Thor Preimesberger (cheezeburglar)

Changes

For #124238

Currently code such as

#include "stdint.h"
void test(uintptr_t addr) {
	__builtin_memcpy(( void __seg_fs*)addr, "x", 1);
}

and

...
call void @<!-- -->llvm.memcpy.p257.p0.i64(ptr addrspace(257) align 1 %4, ptr align 1 @.str, i64 1, i1 false)
...

are miscompiled by ASan. When any of mem{set, cpy, move} are intercepted, by ASan, the writes to fs and gs are effectively scrubbed from the calls to ASan's runtime. This commit just causes AddressSanitizer to bail when on instrumenting these sorts of memory intrinsics.

(Really sorry if you got pinged accidentally when I was editing the commit history before - my bad)

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

2 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+12)
(added) llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll (+60)

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 8d8d56035a48f..85edcb1276efe 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -797,6 +797,7 @@ struct AddressSanitizer {
                                  bool IsWrite, size_t AccessSizeIndex,
                                  Value *SizeArgument, uint32_t Exp,
                                  RuntimeCallInserter &RTCI);
+  bool maybeIgnoreMemIntrinsic(MemIntrinsic *MI, const Triple &TargetTriple);
   void instrumentMemIntrinsic(MemIntrinsic *MI, RuntimeCallInserter &RTCI);
   Value *memToShadow(Value *Shadow, IRBuilder<> &IRB);
   bool suppressInstrumentationSiteForDebug(int &Instrumented);
@@ -1340,10 +1341,21 @@ Value *AddressSanitizer::memToShadow(Value *Shadow, IRBuilder<> &IRB) {
     return IRB.CreateAdd(Shadow, ShadowBase);
 }
 
+bool AddressSanitizer::maybeIgnoreMemIntrinsic(MemIntrinsic *MI,
+                                               const Triple &TargetTriple) {
+  // Ignore FS and GS registers to prevent miscompilation
+  if (MI->getDestAddressSpace() >= 256 &&
+      TargetTriple.getArch() == Triple::x86_64)
+    return true;
+  return false;
+}
+
 // Instrument memset/memmove/memcpy
 void AddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI,
                                               RuntimeCallInserter &RTCI) {
   InstrumentationIRBuilder IRB(MI);
+  if (maybeIgnoreMemIntrinsic(MI, TargetTriple))
+    return;
   if (isa<MemTransferInst>(MI)) {
     RTCI.createRuntimeCall(
         IRB, isa<MemMoveInst>(MI) ? AsanMemmove : AsanMemcpy,
diff --git a/llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll b/llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll
new file mode 100644
index 0000000000000..ce82bc48563a0
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll
@@ -0,0 +1,60 @@
+; RUN: opt -passes=asan %s -S | FileCheck %s
+
+;; Punt AddressSanitizer::instrumentMemIntrinsics out for MemIntrinsics
+;; that need write to unsupported registers on X86
+;; PR124238: https://www.github.com/llvm/llvm-project/issues/124238
+
+target triple = "x86_64-unknown-linux-gnu"
+
+$.str.658906a285b7a0f82dabd9915e07848c = comdat any
+@.str = internal constant { [2 x i8], [30 x i8] } { [2 x i8] c"x\00", [30 x i8] zeroinitializer }, comdat($.str.658906a285b7a0f82dabd9915e07848c), align 32
+@0 = private alias { [2 x i8], [30 x i8] }, ptr @.str
+
+define void @test_memcpy(i64 noundef %addr) sanitize_address #0 {
+entry:
+  %addr.addr = alloca i64, align 8
+  store i64 %addr, ptr %addr.addr, align 8
+  %0 = load i64, ptr %addr.addr, align 8
+  %1 = inttoptr i64 %0 to ptr addrspace(257)
+  call void @llvm.memcpy.p257.p0.i64(ptr addrspace(257) align 1 %1, ptr align 1 @.str, i64 1, i1 false)
+; CHECK: llvm.memcpy
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr addrspace(256)
+  call void @llvm.memcpy.p256.p0.i64(ptr addrspace(256) align 1 %3, ptr align 1 @.str, i64 1, i1 false)
+; CHECK: llvm.memcpy
+  ret void
+}
+
+define void @test_memset(i64 noundef %addr) sanitize_address #0 {
+entry:
+  %addr.addr = alloca i64, align 8
+  store i64 %addr, ptr %addr.addr, align 8
+  %0 = load i64, ptr %addr.addr, align 8
+  %1 = inttoptr i64 %0 to ptr addrspace(257)
+  call void @llvm.memset.p257.i64(ptr addrspace(257) align 1 %1, i8 0, i64 1, i1 false)
+; CHECK: llvm.memset
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr addrspace(256)
+  call void @llvm.memset.p256.i64(ptr addrspace(256) align 1 %3, i8 0, i64 1, i1 false)
+; CHECK: llvm.memset
+  ret void
+}
+
+define void @test_memmove(i64 noundef %addr) sanitize_address #0 {
+entry:
+  %addr.addr = alloca i64, align 8
+  store i64 %addr, ptr %addr.addr, align 8
+  %0 = load i64, ptr %addr.addr, align 8
+  %1 = inttoptr i64 %0 to ptr addrspace(257)
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr
+  call void @llvm.memmove.p257.p0.i64(ptr addrspace(257) align 1 %1, ptr align 1 %3, i64 1, i1 false)
+; CHECK: llvm.memmove
+  %4 = load i64, ptr %addr.addr, align 8
+  %5 = inttoptr i64 %4 to ptr addrspace(256)
+  %6 = load i64, ptr %addr.addr, align 8
+  %7 = inttoptr i64 %6 to ptr
+  call void @llvm.memmove.p256.p0.i64(ptr addrspace(256) align 1 %5, ptr align 1 %7, i64 1, i1 false)
+; CHECK: llvm.memmove
+  ret void
+}

ramosian-glider

I think "abort" is a bit too strong word here, how about "skip"?

Also, could you please elaborate on the problem in the patch description?

ramosian-glider · 2025-03-13T15:47:27Z

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

+bool AddressSanitizer::maybeIgnoreMemIntrinsic(MemIntrinsic *MI,
+                                               const Triple &TargetTriple) {
+  // Ignore FS and GS registers to prevent miscompilation
+  if (MI->getDestAddressSpace() >= 256 &&


Do we also want to bail out if the address space is 258 (SS), 259 etc.?

For SS, probably not - in 64 bit mode, all segment registers are disabled except for FS and GS.

The higher (259+) address spaces look like they correspond to compiler directives that originate from Microsoft - I think they get casted the the usual default address space in llvm, so I'd guess not. I'm not entirely sure however

Edit: Latest commit skips if and only if X86AS::FS or X86AS::GS are being targeted

ramosian-glider · 2025-03-13T15:54:41Z

llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll

+; RUN: opt -passes=asan %s -S | FileCheck %s
+
+;; Punt AddressSanitizer::instrumentMemIntrinsics out for MemIntrinsics
+;; that need write to unsupported registers on X86


nit: "need to write"
Also, as far as I understand, these are unsupported address spaces, not unsupported registers?

Yep - corrected in latest commit

ramosian-glider · 2025-03-13T15:55:47Z

llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll

+; CHECK: llvm.memcpy
+  %2 = load i64, ptr %addr.addr, align 8
+  %3 = inttoptr i64 %2 to ptr addrspace(256)
+  call void @llvm.memcpy.p256.p0.i64(ptr addrspace(256) align 1 %3, ptr align 1 @.str, i64 1, i1 false)


Can you add tests for addrspace(0) and addrspace(257) (which should probably be kept instrumented)?

My bad, should be addrspace(258)

Done, in latest commit

…n x86-64 (llvm#129291) Currently, AddressSanitizer's instrumented memintrinsics are unable to write to the address spaces indicated by the segment register fs and gs on x86-64. This patch just skips the instrumentation of such memintrinsics. Fixes llvm#124238 for asan.

ramosian-glider · 2025-03-18T12:23:53Z

llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll

+; CHECK: __asan_memcpy
+  %6 = load i64, ptr %addr.addr, align 8
+  %7 = inttoptr i64 %2 to ptr addrspace(0)
+  call void @llvm.memcpy.p258.p0.i64(ptr addrspace(0) align 1 %7, ptr align 1 @.str, i64 1, i1 false)


This should probably be llvm.memcpy.p0.p0.i64?

Yep - fixed in latest. I also took a more careful look at the test and edited it so that it generates the intended assembly code once compiled all the way down. Original test case in #124238 for asan also works as intended.

ramosian-glider · 2025-03-18T12:24:07Z

llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll

+; CHECK: __asan_memset
+  %6 = load i64, ptr %addr.addr, align 8
+  %7 = inttoptr i64 %2 to ptr addrspace(0)
+  call void @llvm.memset.p258.i64(ptr addrspace(0) align 1 %7, i8 0, i64 1, i1 false)


Done in latest

ramosian-glider · 2025-03-18T12:24:38Z

llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll

+  %9 = inttoptr i64 %4 to ptr addrspace(258)
+  %10 = load i64, ptr %addr.addr, align 8
+  %11 = inttoptr i64 %6 to ptr
+  call void @llvm.memmove.p256.p0.i64(ptr addrspace(258) align 1 %9, ptr align 1 %11, i64 1, i1 false)


llvm.memmove.p258.p0.i64

Done in latest

ramosian-glider · 2025-03-18T12:25:01Z

llvm/test/Instrumentation/AddressSanitizer/X86/bug_124238.ll

+  %13 = inttoptr i64 %4 to ptr addrspace(0)
+  %14 = load i64, ptr %addr.addr, align 8
+  %15 = inttoptr i64 %6 to ptr
+  call void @llvm.memmove.p256.p0.i64(ptr addrspace(0) align 1 %13, ptr align 1 %15, i64 1, i1 false)


llvm.memmove.p0.p0.i64

Done in latest

…n x86-64 (llvm#129291) Currently, AddressSanitizer's instrumented memintrinsics are unable to write to the address spaces indicated by the segment register fs and gs on x86-64. This patch just skips the instrumentation of such memintrinsics. Fixes llvm#124238 for asan.

cheezeburglar · 2025-04-28T20:29:45Z

Bump @ramosian-glider

cheezeburglar marked this pull request as ready for review February 28, 2025 22:15

llvmbot added compiler-rt:sanitizer llvm:transforms labels Feb 28, 2025

ramosian-glider reviewed Mar 13, 2025

View reviewed changes

cheezeburglar force-pushed the asan_bug branch from 5b3295b to 76146ce Compare March 13, 2025 19:20

cheezeburglar changed the title ~~[asan][x86] Abort instrumenting memintrinsics that target fs, gs~~ [asan][x86] Skip memintrinsics that write to special address spaces on x86-64 Mar 13, 2025

cheezeburglar force-pushed the asan_bug branch from 76146ce to 650c721 Compare March 13, 2025 22:46

ramosian-glider reviewed Mar 18, 2025

View reviewed changes

cheezeburglar force-pushed the asan_bug branch from 650c721 to 3f588b9 Compare March 23, 2025 20:36

[asan][x86] Skip memintrinsics that write to special address spaces on x86-64 #129291

Are you sure you want to change the base?

[asan][x86] Skip memintrinsics that write to special address spaces on x86-64 #129291

Uh oh!

Conversation

cheezeburglar commented Feb 28, 2025

Uh oh!

llvmbot commented Feb 28, 2025

Uh oh!

llvmbot commented Feb 28, 2025

Uh oh!

ramosian-glider left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cheezeburglar Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cheezeburglar commented Apr 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cheezeburglar Mar 13, 2025 •

edited

Loading