Skip to content

[hwasan] Consider order of mapping copts #109621

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

Merged

Conversation

vitalybuka
Copy link
Collaborator

Flags "-hwasan-mapping-offset" and
"-hwasan-mapping-offset-dynamic" are mutually
exclusive, use the last one.

@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

Changes

Flags "-hwasan-mapping-offset" and
"-hwasan-mapping-offset-dynamic" are mutually
exclusive, use the last one.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+8-4)
  • (added) llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll (+50)
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index e386fa5d50b4d6..a70bfd121a2647 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1938,14 +1938,18 @@ void HWAddressSanitizer::ShadowMapping::init(Triple &TargetTriple,
     // Fuchsia is always PIE, which means that the beginning of the address
     // space is always available.
     SetFixed(0);
-  } else if (ClMappingOffset.getNumOccurrences() > 0) {
-    SetFixed(ClMappingOffset);
   } else if (ClEnableKhwasan || InstrumentWithCalls) {
     SetFixed(0);
     WithFrameRecord = false;
-  } else if (ClMappingOffsetDynamic.getNumOccurrences() > 0) {
-    Kind = ClMappingOffsetDynamic;
   }
 
   WithFrameRecord = optOr(ClFrameRecords, WithFrameRecord);
+
+  // Apply the last of ClMappingOffset and ClMappingOffsetDynamic.
+  Kind = optOr(ClMappingOffsetDynamic, Kind);
+  if (ClMappingOffset.getNumOccurrences() > 0 &&
+      !(ClMappingOffsetDynamic.getNumOccurrences() > 0 &&
+        ClMappingOffsetDynamic.getPosition() > ClMappingOffset.getPosition())) {
+    SetFixed(ClMappingOffset);
+  }
 }
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll b/llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll
new file mode 100644
index 00000000000000..5cd23f3ebe2b07
--- /dev/null
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll
@@ -0,0 +1,50 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+
+; RUN: opt < %s -passes=hwasan -S | FileCheck %s
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset-dynamic=global -S | FileCheck %s --check-prefixes=GLOBAL
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset=567 -S | FileCheck %s --check-prefixes=FIXED
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset=567 -hwasan-mapping-offset-dynamic=global -S | FileCheck %s --check-prefixes=FIXED-GLOBAL
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset-dynamic=global -hwasan-mapping-offset=567 -S | FileCheck %s --check-prefixes=GLOBAL-FIXED
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64--linux-android"
+
+define i8 @test_load8(ptr %a) sanitize_hwaddress {
+; CHECK-LABEL: define i8 @test_load8
+; CHECK-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr @__hwasan_shadow)
+; CHECK-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[DOTHWASAN_SHADOW]], ptr [[A]], i32 0)
+; CHECK-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; CHECK-NEXT:    ret i8 [[B]]
+;
+; GLOBAL-LABEL: define i8 @test_load8
+; GLOBAL-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; GLOBAL-NEXT:    [[TMP1:%.*]] = load ptr, ptr @__hwasan_shadow_memory_dynamic_address, align 8
+; GLOBAL-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[TMP1]], ptr [[A]], i32 0)
+; GLOBAL-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; GLOBAL-NEXT:    ret i8 [[B]]
+;
+; FIXED-LABEL: define i8 @test_load8
+; FIXED-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; FIXED-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr inttoptr (i64 567 to ptr))
+; FIXED-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[DOTHWASAN_SHADOW]], ptr [[A]], i32 0)
+; FIXED-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; FIXED-NEXT:    ret i8 [[B]]
+;
+; FIXED-GLOBAL-LABEL: define i8 @test_load8
+; FIXED-GLOBAL-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; FIXED-GLOBAL-NEXT:    [[TMP1:%.*]] = load ptr, ptr @__hwasan_shadow_memory_dynamic_address, align 8
+; FIXED-GLOBAL-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[TMP1]], ptr [[A]], i32 0)
+; FIXED-GLOBAL-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; FIXED-GLOBAL-NEXT:    ret i8 [[B]]
+;
+; GLOBAL-FIXED-LABEL: define i8 @test_load8
+; GLOBAL-FIXED-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; GLOBAL-FIXED-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr inttoptr (i64 567 to ptr))
+; GLOBAL-FIXED-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[DOTHWASAN_SHADOW]], ptr [[A]], i32 0)
+; GLOBAL-FIXED-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; GLOBAL-FIXED-NEXT:    ret i8 [[B]]
+;
+  %b = load i8, ptr %a, align 4
+  ret i8 %b
+}

@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

Flags "-hwasan-mapping-offset" and
"-hwasan-mapping-offset-dynamic" are mutually
exclusive, use the last one.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+8-4)
  • (added) llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll (+50)
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index e386fa5d50b4d6..a70bfd121a2647 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1938,14 +1938,18 @@ void HWAddressSanitizer::ShadowMapping::init(Triple &TargetTriple,
     // Fuchsia is always PIE, which means that the beginning of the address
     // space is always available.
     SetFixed(0);
-  } else if (ClMappingOffset.getNumOccurrences() > 0) {
-    SetFixed(ClMappingOffset);
   } else if (ClEnableKhwasan || InstrumentWithCalls) {
     SetFixed(0);
     WithFrameRecord = false;
-  } else if (ClMappingOffsetDynamic.getNumOccurrences() > 0) {
-    Kind = ClMappingOffsetDynamic;
   }
 
   WithFrameRecord = optOr(ClFrameRecords, WithFrameRecord);
+
+  // Apply the last of ClMappingOffset and ClMappingOffsetDynamic.
+  Kind = optOr(ClMappingOffsetDynamic, Kind);
+  if (ClMappingOffset.getNumOccurrences() > 0 &&
+      !(ClMappingOffsetDynamic.getNumOccurrences() > 0 &&
+        ClMappingOffsetDynamic.getPosition() > ClMappingOffset.getPosition())) {
+    SetFixed(ClMappingOffset);
+  }
 }
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll b/llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll
new file mode 100644
index 00000000000000..5cd23f3ebe2b07
--- /dev/null
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/mapping-override.ll
@@ -0,0 +1,50 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+
+; RUN: opt < %s -passes=hwasan -S | FileCheck %s
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset-dynamic=global -S | FileCheck %s --check-prefixes=GLOBAL
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset=567 -S | FileCheck %s --check-prefixes=FIXED
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset=567 -hwasan-mapping-offset-dynamic=global -S | FileCheck %s --check-prefixes=FIXED-GLOBAL
+; RUN: opt < %s -passes=hwasan -hwasan-mapping-offset-dynamic=global -hwasan-mapping-offset=567 -S | FileCheck %s --check-prefixes=GLOBAL-FIXED
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64--linux-android"
+
+define i8 @test_load8(ptr %a) sanitize_hwaddress {
+; CHECK-LABEL: define i8 @test_load8
+; CHECK-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr @__hwasan_shadow)
+; CHECK-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[DOTHWASAN_SHADOW]], ptr [[A]], i32 0)
+; CHECK-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; CHECK-NEXT:    ret i8 [[B]]
+;
+; GLOBAL-LABEL: define i8 @test_load8
+; GLOBAL-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; GLOBAL-NEXT:    [[TMP1:%.*]] = load ptr, ptr @__hwasan_shadow_memory_dynamic_address, align 8
+; GLOBAL-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[TMP1]], ptr [[A]], i32 0)
+; GLOBAL-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; GLOBAL-NEXT:    ret i8 [[B]]
+;
+; FIXED-LABEL: define i8 @test_load8
+; FIXED-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; FIXED-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr inttoptr (i64 567 to ptr))
+; FIXED-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[DOTHWASAN_SHADOW]], ptr [[A]], i32 0)
+; FIXED-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; FIXED-NEXT:    ret i8 [[B]]
+;
+; FIXED-GLOBAL-LABEL: define i8 @test_load8
+; FIXED-GLOBAL-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; FIXED-GLOBAL-NEXT:    [[TMP1:%.*]] = load ptr, ptr @__hwasan_shadow_memory_dynamic_address, align 8
+; FIXED-GLOBAL-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[TMP1]], ptr [[A]], i32 0)
+; FIXED-GLOBAL-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; FIXED-GLOBAL-NEXT:    ret i8 [[B]]
+;
+; GLOBAL-FIXED-LABEL: define i8 @test_load8
+; GLOBAL-FIXED-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; GLOBAL-FIXED-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr inttoptr (i64 567 to ptr))
+; GLOBAL-FIXED-NEXT:    call void @llvm.hwasan.check.memaccess(ptr [[DOTHWASAN_SHADOW]], ptr [[A]], i32 0)
+; GLOBAL-FIXED-NEXT:    [[B:%.*]] = load i8, ptr [[A]], align 4
+; GLOBAL-FIXED-NEXT:    ret i8 [[B]]
+;
+  %b = load i8, ptr %a, align 4
+  ret i8 %b
+}

@fmayer
Copy link
Contributor

fmayer commented Sep 24, 2024

[hwasan] Check order of mapping flags

I don't understand what "checking" we do here. "Consider"?

@vitalybuka vitalybuka changed the title [hwasan] Check order of mapping flags [hwasan] Consider order of mapping copts Sep 24, 2024
@vitalybuka
Copy link
Collaborator Author

"Consider"?

Done

Copy link
Contributor

@fmayer fmayer left a comment

Choose a reason for hiding this comment

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

LGTM.

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.hwasan-check-order-of-mapping-flags to main September 25, 2024 02:48
@vitalybuka vitalybuka merged commit b218048 into main Sep 25, 2024
8 of 11 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/hwasan-check-order-of-mapping-flags branch September 25, 2024 04:11
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.

3 participants