[compiler-rt] d81d317 - [ASan] Fixed a reporting bug in (load|store)N functions which would print unknown-crash instead of the proper error message when a the data access is unaligned.

Kirill Stoimenov via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 18 15:47:18 PDT 2022


Author: Kirill Stoimenov
Date: 2022-04-18T22:46:37Z
New Revision: d81d317999b350f9ba41e214149899ccd62eb998

URL: https://github.com/llvm/llvm-project/commit/d81d317999b350f9ba41e214149899ccd62eb998
DIFF: https://github.com/llvm/llvm-project/commit/d81d317999b350f9ba41e214149899ccd62eb998.diff

LOG: [ASan] Fixed a reporting bug in (load|store)N functions which would print unknown-crash instead of the proper error message when a the data access is unaligned.

Reviewed By: kda, eugenis

Differential Revision: https://reviews.llvm.org/D123643

Added: 
    compiler-rt/test/asan/TestCases/load_and_store_n.cpp

Modified: 
    compiler-rt/lib/asan/asan_rtl.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp
index bacb452f60c2e..3a5261474b298 100644
--- a/compiler-rt/lib/asan/asan_rtl.cpp
+++ b/compiler-rt/lib/asan/asan_rtl.cpp
@@ -187,7 +187,7 @@ ASAN_MEMORY_ACCESS_CALLBACK(store, true, 16)
 extern "C"
 NOINLINE INTERFACE_ATTRIBUTE
 void __asan_loadN(uptr addr, uptr size) {
-  if (__asan_region_is_poisoned(addr, size)) {
+  if ((addr = __asan_region_is_poisoned(addr, size))) {
     GET_CALLER_PC_BP_SP;
     ReportGenericError(pc, bp, sp, addr, false, size, 0, true);
   }
@@ -196,7 +196,7 @@ void __asan_loadN(uptr addr, uptr size) {
 extern "C"
 NOINLINE INTERFACE_ATTRIBUTE
 void __asan_exp_loadN(uptr addr, uptr size, u32 exp) {
-  if (__asan_region_is_poisoned(addr, size)) {
+  if ((addr = __asan_region_is_poisoned(addr, size))) {
     GET_CALLER_PC_BP_SP;
     ReportGenericError(pc, bp, sp, addr, false, size, exp, true);
   }
@@ -205,7 +205,7 @@ void __asan_exp_loadN(uptr addr, uptr size, u32 exp) {
 extern "C"
 NOINLINE INTERFACE_ATTRIBUTE
 void __asan_loadN_noabort(uptr addr, uptr size) {
-  if (__asan_region_is_poisoned(addr, size)) {
+  if ((addr = __asan_region_is_poisoned(addr, size))) {
     GET_CALLER_PC_BP_SP;
     ReportGenericError(pc, bp, sp, addr, false, size, 0, false);
   }
@@ -214,7 +214,7 @@ void __asan_loadN_noabort(uptr addr, uptr size) {
 extern "C"
 NOINLINE INTERFACE_ATTRIBUTE
 void __asan_storeN(uptr addr, uptr size) {
-  if (__asan_region_is_poisoned(addr, size)) {
+  if ((addr = __asan_region_is_poisoned(addr, size))) {
     GET_CALLER_PC_BP_SP;
     ReportGenericError(pc, bp, sp, addr, true, size, 0, true);
   }
@@ -223,7 +223,7 @@ void __asan_storeN(uptr addr, uptr size) {
 extern "C"
 NOINLINE INTERFACE_ATTRIBUTE
 void __asan_exp_storeN(uptr addr, uptr size, u32 exp) {
-  if (__asan_region_is_poisoned(addr, size)) {
+  if ((addr = __asan_region_is_poisoned(addr, size))) {
     GET_CALLER_PC_BP_SP;
     ReportGenericError(pc, bp, sp, addr, true, size, exp, true);
   }
@@ -232,7 +232,7 @@ void __asan_exp_storeN(uptr addr, uptr size, u32 exp) {
 extern "C"
 NOINLINE INTERFACE_ATTRIBUTE
 void __asan_storeN_noabort(uptr addr, uptr size) {
-  if (__asan_region_is_poisoned(addr, size)) {
+  if ((addr = __asan_region_is_poisoned(addr, size))) {
     GET_CALLER_PC_BP_SP;
     ReportGenericError(pc, bp, sp, addr, true, size, 0, false);
   }

diff  --git a/compiler-rt/test/asan/TestCases/load_and_store_n.cpp b/compiler-rt/test/asan/TestCases/load_and_store_n.cpp
new file mode 100644
index 0000000000000..ce5ddfe14f18a
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/load_and_store_n.cpp
@@ -0,0 +1,78 @@
+// CHECK_REGULAR_LOAD_STORE: call{{[ql]}} __asan_loadN at PLT
+// CHECK_REGULAR_LOAD_STORE: call{{[ql]}} __asan_storeN at PLT
+// RUN: %clangxx_asan -O2 -fsanitize-address-outline-instrumentation %s -o %t
+// RUN: %clangxx_asan -O2 -fsanitize-address-outline-instrumentation %s -o - -S \
+// RUN:   | FileCheck %s --check-prefix=CHECK_REGULAR_LOAD_STORE
+// RUN: not %run %t A 2>&1 | FileCheck %s --check-prefix=CHECK_0_BYTES
+// RUN: not %run %t B 2>&1 | FileCheck %s --check-prefix=CHECK_0_BYTES
+// RUN: not %run %t C 2>&1 | FileCheck %s --check-prefix=CHECK_1_BYTES
+// RUN: not %run %t D 2>&1 | FileCheck %s --check-prefix=CHECK_1_BYTES
+
+// CHECK_NOABORT_LOAD_STORE: call{{[ql]}} __asan_loadN_noabort at PLT
+// CHECK_NOABORT_LOAD_STORE: call{{[ql]}} __asan_storeN_noabort at PLT
+// RUN: %clangxx_asan -O2 -fsanitize-address-outline-instrumentation %s -o %t \
+// RUN:   -mllvm -asan-recover=1
+// RUN: %clangxx_asan -O2 -fsanitize-address-outline-instrumentation %s -o - -S \
+// RUN:   -mllvm -asan-recover=1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK_NOABORT_LOAD_STORE
+// RUN: not %run %t A 2>&1 | FileCheck %s --check-prefix=CHECK_0_BYTES
+// RUN: not %run %t B 2>&1 | FileCheck %s --check-prefix=CHECK_0_BYTES
+// RUN: not %run %t C 2>&1 | FileCheck %s --check-prefix=CHECK_1_BYTES
+// RUN: not %run %t D 2>&1 | FileCheck %s --check-prefix=CHECK_1_BYTES
+
+// CHECK_EXP_LOAD_STORE: call{{[ql]}} __asan_exp_loadN at PLT
+// CHECK_EXP_LOAD_STORE: call{{[ql]}} __asan_exp_storeN at PLT
+// RUN: %clangxx_asan -O2 -fsanitize-address-outline-instrumentation %s -o %t \
+// RUN:   -mllvm -asan-force-experiment=42
+// RUN: %clangxx_asan -O2 -fsanitize-address-outline-instrumentation %s -o - -S \
+// RUN:   -mllvm -asan-force-experiment=42 \
+// RUN:   | FileCheck %s --check-prefix=CHECK_EXP_LOAD_STORE
+// RUN: not %run %t A 2>&1 | FileCheck %s --check-prefix=CHECK_0_BYTES
+// RUN: not %run %t B 2>&1 | FileCheck %s --check-prefix=CHECK_0_BYTES
+// RUN: not %run %t C 2>&1 | FileCheck %s --check-prefix=CHECK_1_BYTES
+// RUN: not %run %t D 2>&1 | FileCheck %s --check-prefix=CHECK_1_BYTES
+
+// CHECK_0_BYTES: ERROR: AddressSanitizer: global-buffer-overflow on address [[ADDR:.*]] at
+// CHECK_0_BYTES: [[ADDR]] is located 0 bytes to the right
+
+// CHECK_1_BYTES: ERROR: AddressSanitizer: global-buffer-overflow on address [[ADDR:.*]] at
+// CHECK_1_BYTES: [[ADDR]] is located 1 bytes to the right
+
+#include <sanitizer/asan_interface.h>
+
+#include <stdlib.h>
+#include <string.h>
+
+static int64_t mem = -1;
+static int64_t *volatile G = &mem;
+
+inline uint16_t UNALIGNED_LOAD(const void *p) {
+  uint16_t data;
+  memcpy(&data, p, sizeof data);
+  return data;
+}
+
+inline void UNALIGNED_STORE(uint16_t data, void *p) {
+  memcpy(p, &data, sizeof data);
+}
+
+int main(int argc, char **argv) {
+  if (argc != 2)
+    return 1;
+  int res = 1;
+  switch (argv[1][0]) {
+  case 'A':
+    res = UNALIGNED_LOAD(reinterpret_cast<char *>(G) + 7);
+    break;
+  case 'B':
+    UNALIGNED_STORE(0, reinterpret_cast<char *>(G) + 7);
+    break;
+  case 'C':
+    res = UNALIGNED_LOAD(reinterpret_cast<char *>(G) + 9);
+    break;
+  case 'D':
+    UNALIGNED_STORE(0, reinterpret_cast<char *>(G) + 9);
+    break;
+  }
+  return res;
+}


        


More information about the llvm-commits mailing list