[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 ↗

(On Diff #9619)

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

245

Could you make it a separate test case?

[ygorshenin]

Fixes

[eugenis]

lib/asan/tests/asan_asm_test.cc

260

remove please

[ygorshenin]

Added simple test for load/store in inline assembly.

[ygorshenin]

Small fix.

[ygorshenin]

PTAL

lib/asan/tests/CMakeLists.txt

63 ↗

(On Diff #9619)

Done.

lib/asan/tests/asan_asm_test.cc

245

Done.

260

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/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;
 }
 
+#if defined(__x86_64__)
+
+void TestFlags() {
+  long magic = 0x1234;
+  long r = 0x0;
+  __asm__("xorq %%rax, %%rax  \n\t"
+          "cmove (%[p]), %[r] \n\t"
+          : [r] "=r"(r)
+          : [p] "r"(&magic)
+          : "rax", "memory");
+  ASSERT_EQ(0x1234, r);
+}
+
+#endif // defined(__x86_64__)
+
+#if defined(__i386__)
+
+void TestFlags() {
+  long magic = 0x1234;
+  long r = 0x0;
+  __asm__("xorl %%eax, %%eax  \n\t"
+          "cmove (%[p]), %[r] \n\t"
+          : [r] "=r"(r)
+          : [p] "r"(&magic)
+          : "eax", "memory");
+  ASSERT_EQ(0x1234, r);
+}
+
+#endif // defined(__i386__)
+
 } // End of anonymous namespace
 
 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");
+
+  TestFlags();
 }
 
 #endif // defined(__x86_64__) || (defined(__i386__) && defined(__SSE2__))
 
 #endif // defined(__linux__)