[asan-assembly-instrumentation] Fixed operations with stack in assembly instrumentation.

[ygorshenin]

Summary

Fixed operations with stack in assembly instrumentation.
Implemented test which checks that flags are correctly restored after assembly instrumentation.

[ygorshenin]

Fixes.

Updating D3839: Fixed operations with stack in assembly instrumentation.

Implemented test which checks that flags are correctly restored after assembly instrumentation.

[ygorshenin]

PTAL

[ygorshenin]

Style fix.

[eugenis]

lib/asan/tests/CMakeLists.txt

63

Please add a simple test case that's affected by this flag!
Something like AddressSanitizer.BitFieldPositiveTest but with asm store or load instead of C assingment.

lib/asan/tests/asan_asm_test.cc

232

Could you make it a separate test case?

[ygorshenin]

Fixes

[eugenis]

lib/asan/tests/asan_asm_test.cc

245

remove please

[ygorshenin]

Added simple test for load/store in inline assembly.

[ygorshenin]

Small fix.

[ygorshenin]

PTAL

lib/asan/tests/CMakeLists.txt

63

Done.

lib/asan/tests/asan_asm_test.cc

232

Done.

245

Done.

[eugenis]

LGTM

[eugenis]

r209282.

lib/asan/asan_asm_instrumentation.S

.sanitize_load16_done:
   ret
 #endif // defined(__i386__)
 #if defined(__x86_64__)
 // Sanitize 1-byte store. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_store1
 .type __sanitizer_sanitize_store1, @function
 __sanitizer_sanitize_store1:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je .sanitize_store1_done
   movl %edi, %ecx
   andl $0x7, %ecx
   movsbl %al, %eax
   cmpl %eax, %ecx
   jl .sanitize_store1_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_store1@PLT
 .sanitize_store1_done:
   popfq
   popq %rcx
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 1-byte load. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_load1
 .type __sanitizer_sanitize_load1, @function
 __sanitizer_sanitize_load1:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je .sanitize_load1_done
   movl %edi, %ecx
   andl $0x7, %ecx
   movsbl %al, %eax
   cmpl %eax, %ecx
   jl .sanitize_load1_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_load1@PLT
 .sanitize_load1_done:
   popfq
   popq %rcx
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 2-byte store. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_store2
 .type __sanitizer_sanitize_store2, @function
 __sanitizer_sanitize_store2:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je .sanitize_store2_done
   movl %edi, %ecx
   andl $0x7, %ecx
   incl %ecx
   movsbl %al, %eax
   cmpl %eax, %ecx
   jl .sanitize_store2_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_store2@PLT
 .sanitize_store2_done:
   popfq
   popq %rcx
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 2-byte load. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_load2
 .type __sanitizer_sanitize_load2, @function
 __sanitizer_sanitize_load2:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je .sanitize_load2_done
   movl %edi, %ecx
   andl $0x7, %ecx
   incl %ecx
   movsbl %al, %eax
   cmpl %eax, %ecx
   jl .sanitize_load2_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_load2@PLT
 .sanitize_load2_done:
   popfq
   popq %rcx
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 4-byte store. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_store4
 .type __sanitizer_sanitize_store4, @function
 __sanitizer_sanitize_store4:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je .sanitize_store4_done
   movl %edi, %ecx
   andl $0x7, %ecx
   addl $0x3, %ecx
   movsbl %al, %eax
   cmpl %eax, %ecx
   jl .sanitize_store4_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_store4@PLT
 .sanitize_store4_done:
   popfq
   popq %rcx
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 4-byte load. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_load4
 .type __sanitizer_sanitize_load4, @function
 __sanitizer_sanitize_load4:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je .sanitize_load4_done
   movl %edi, %ecx
   andl $0x7, %ecx
   addl $0x3, %ecx
   movsbl %al, %eax
   cmpl %eax, %ecx
   jl .sanitize_load4_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_load4@PLT
 .sanitize_load4_done:
   popfq
   popq %rcx
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 8-byte store. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_store8
 .type __sanitizer_sanitize_store8, @function
 __sanitizer_sanitize_store8:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   cmpb $0x0, 0x7fff8000(%rax)
   je .sanitize_store8_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_store8@PLT
 .sanitize_store8_done:
   popfq
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 8-byte load. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_load8
 .type __sanitizer_sanitize_load8, @function
 __sanitizer_sanitize_load8:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   cmpb $0x0, 0x7fff8000(%rax)
   je .sanitize_load8_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_load8@PLT
 .sanitize_load8_done:
   popfq
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 16-byte store. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_store16
 .type __sanitizer_sanitize_store16, @function
 __sanitizer_sanitize_store16:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   cmpw $0x0, 0x7fff8000(%rax)
   je .sanitize_store16_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_store16@PLT
 .sanitize_store16_done:
   popfq
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 // Sanitize 16-byte load. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl __sanitizer_sanitize_load16
 .type __sanitizer_sanitize_load16, @function
 __sanitizer_sanitize_load16:
-  subq $128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushfq
   movq %rdi, %rax
   shrq $0x3, %rax
   cmpw $0x0, 0x7fff8000(%rax)
   je .sanitize_load16_done
   subq $8, %rsp
   andq $-16, %rsp
   cld
   emms
   call __asan_report_load16@PLT
 .sanitize_load16_done:
   popfq
   popq %rax
-  addq $128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 #endif // defined(__x86_64__)
 /* We do not need executable stack. */
 #if defined(__arm__)
   .section .note.GNU-stack,"",%progbits
 #else
   .section .note.GNU-stack,"",@progbits
 #endif // defined(__arm__)
 #endif // __linux__

lib/asan/scripts/gen_asm_instrumentation.sh

do
   for at in store load
   do
 cat <<EOF
 // Sanitize $as-byte $at. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl $(func_name $at $as)
 .type $(func_name $at $as), @function
 $(func_name $at $as):
-  subq \$128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushq %rcx
   pushfq
   movq %rdi, %rax
   shrq \$0x3, %rax
   movb 0x7fff8000(%rax), %al
   test %al, %al
   je $(func_label $at $as)
cat <<EOF
   cmpl %eax, %ecx
   jl $(func_label $at $as)
 $(emit_stack_align)
 $(emit_call_report $at $as)
 $(func_label $at $as):
   popfq
   popq %rcx
   popq %rax
-  addq \$128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 EOF
   done
 done
 
 # Functions for x86-64 8- and 16-byte accesses.
 for as in 8 16
 do
   for at in store load
   do
 cat <<EOF
 // Sanitize $as-byte $at. Takes one 8-byte address as an argument in %rdi,
 // nothing is returned.
 .globl $(func_name $at $as)
 .type $(func_name $at $as), @function
 $(func_name $at $as):
-  subq \$128, %rsp
+  leaq -128(%rsp), %rsp
   pushq %rax
   pushfq
   movq %rdi, %rax
   shrq \$0x3, %rax
 EOF
 
     case ${as} in
       8) echo '  cmpb $0x0, 0x7fff8000(%rax)' ;;
      16) echo '  cmpw $0x0, 0x7fff8000(%rax)' ;;
       *) echo "Incorrect access size: ${as}" 1>&2; exit 1 ;;
     esac
 
 cat <<EOF
   je $(func_label $at $as)
 $(emit_stack_align)
 $(emit_call_report $at $as)
 $(func_label $at $as):
   popfq
   popq %rax
-  addq \$128, %rsp
+  leaq 128(%rsp), %rsp
   ret
 EOF
   done
 done
 echo "#endif // defined(__x86_64__)"
 
 cat <<EOF
 /* We do not need executable stack. */
 #if defined(__arm__)
   .section .note.GNU-stack,"",%progbits
 #else
   .section .note.GNU-stack,"",@progbits
 #endif // defined(__arm__)
 #endif // __linux__
 EOF

lib/asan/tests/CMakeLists.txt

list(APPEND ASAN_UNITTEST_COMMON_CFLAGS
   -DASAN_HAS_EXCEPTIONS=1
   -DASAN_UAR=0)
 
 set(ASAN_BLACKLIST_FILE "${CMAKE_CURRENT_SOURCE_DIR}/asan_test.ignore")
 set(ASAN_UNITTEST_INSTRUMENTED_CFLAGS
   ${ASAN_UNITTEST_COMMON_CFLAGS}
   -fsanitize=address
   "-fsanitize-blacklist=${ASAN_BLACKLIST_FILE}"
+  -mllvm -asan-instrument-assembly
 )
 
 if(NOT MSVC)
   list(APPEND ASAN_UNITTEST_COMMON_LINKFLAGS --driver-mode=g++)
 endif()
 
 # x86_64 FreeBSD 9.2 additionally requires libc++ to build the tests.
 if(CMAKE_SYSTEM MATCHES "FreeBSD-9.2-RELEASE")

lib/asan/tests/asan_asm_test.cc

if (((uintptr_t) p % 16) != 0)
     p = buf + 8;
   assert(((uintptr_t) p % 16) == 0);
   EXPECT_DEATH(asm_read<__m128i>((__m128i*) p), DeathPattern);
   __m128i val = _mm_set1_epi16(0x1234);
   ASSERT_EQ(0x1234, _mm_extract_epi16(asm_read(&val), 0));
   delete [] buf;
 }
 
+U4 AsmLoad(U4 *a) {
+  U4 r;
+  __asm__("movl (%[a]), %[r]  \n\t" : [r] "=r" (r) : [a] "r" (a) : "memory");
+  return r;
+}
+
+void AsmStore(U4 r, U4 *a) {
+  __asm__("movl %[r], (%[a])  \n\t" : : [a] "r" (a), [r] "r" (r) : "memory");
+}
+
 } // End of anonymous namespace
 
+TEST(AddressSanitizer, asm_load_store) {
+  U4* buf = new U4[2];
+  EXPECT_DEATH(AsmLoad(&buf[3]), "READ of size 4");
+  EXPECT_DEATH(AsmStore(0x1234, &buf[3]), "WRITE of size 4");
+  delete [] buf;
+}
+
 TEST(AddressSanitizer, asm_rw) {
   TestAsmWrite<U1>("WRITE of size 1");
   TestAsmWrite<U2>("WRITE of size 2");
   TestAsmWrite<U4>("WRITE of size 4");
   TestAsmWrite<U8>("WRITE of size 8");
   TestAsmWrite<__m128i>("WRITE of size 16");
 
   TestAsmRead<U1>("READ of size 1");
   TestAsmRead<U2>("READ of size 2");
   TestAsmRead<U4>("READ of size 4");
   TestAsmRead<U8>("READ of size 8");
   TestAsmRead<__m128i>("READ of size 16");
 }
 
+TEST(AddressSanitizer, asm_flags) {
+  long magic = 0x1234;
+  long r = 0x0;
+
+#if defined(__x86_64__)
+  __asm__("xorq %%rax, %%rax  \n\t"
+          "movq (%[p]), %%rax \n\t"
+          "sete %%al          \n\t"
+          "movzbq %%al, %[r]  \n\t"
+          : [r] "=r"(r)
+          : [p] "r"(&magic)
+          : "rax", "memory");
+#else
+  __asm__("xorl %%eax, %%eax  \n\t"
+          "movl (%[p]), %%eax \n\t"
+          "sete %%al          \n\t"
+          "movzbl %%al, %[r]  \n\t"
+          : [r] "=r"(r)
+          : [p] "r"(&magic)
+          : "eax", "memory");
+#endif // defined(__x86_64__)
+
+  ASSERT_EQ(0x1, r);
+}
+
 #endif // defined(__x86_64__) || (defined(__i386__) && defined(__SSE2__))
 
 #endif // defined(__linux__)