Bug 86560 - FAIL: c-c++-common/asan/swapcontext-test-1.c
Summary: FAIL: c-c++-common/asan/swapcontext-test-1.c
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: 9.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: 81652
  Show dependency treegraph
 
Reported: 2018-07-18 04:25 UTC by H.J. Lu
Modified: 2018-07-26 14:50 UTC (History)
0 users

See Also:
Host:
Target: i386,x86-64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-07-18 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2018-07-18 04:25:38 UTC
When CET is enabled, I got

FAIL: c-c++-common/asan/swapcontext-test-1.c   -O0  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O1  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O2  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O3 -g  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -Os  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O0  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O1  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O2  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -O3 -g  execution test
FAIL: c-c++-common/asan/swapcontext-test-1.c   -Os  execution test

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff71fbef3 in __interceptor_swapcontext (oucp=0x404300 <orig_context>, 
    ucp=0x4046e0 <child_context>)
    at /export/gnu/import/git/sources/gcc/libsanitizer/asan/asan_interceptors.cc:270
270	  int res = REAL(swapcontext)(oucp, ucp);
(gdb) disass
Dump of assembler code for function __interceptor_swapcontext(ucontext_t*, ucontext_t*):
   0x00007ffff71fbeb0 <+0>:	endbr64 
   0x00007ffff71fbeb4 <+4>:	push   %rbp
   0x00007ffff71fbeb5 <+5>:	mov    %rdi,%rbp
   0x00007ffff71fbeb8 <+8>:	push   %rbx
   0x00007ffff71fbeb9 <+9>:	mov    %rsi,%rbx
   0x00007ffff71fbebc <+12>:	sub    $0x18,%rsp
   0x00007ffff71fbec0 <+16>:	cmpb   $0x0,0x337bfa(%rip)        # 0x7ffff7533ac1 <_ZZ25__interceptor_swapcontextE16reported_warning>
   0x00007ffff71fbec7 <+23>:	je     0x7ffff71fbf10 <__interceptor_swapcontext(ucontext_t*, ucontext_t*)+96>
   0x00007ffff71fbec9 <+25>:	lea    0x8(%rsp),%rdx
   0x00007ffff71fbece <+30>:	mov    %rsp,%rsi
   0x00007ffff71fbed1 <+33>:	mov    %rbx,%rdi
   0x00007ffff71fbed4 <+36>:	callq  0x7ffff72ad380 <__asan::ReadContextStack(void*, unsigned long*, unsigned long*)>
   0x00007ffff71fbed9 <+41>:	mov    0x8(%rsp),%rsi
   0x00007ffff71fbede <+46>:	mov    (%rsp),%rdi
   0x00007ffff71fbee2 <+50>:	callq  0x7ffff71fbd10 <ClearShadowMemoryForContextStack(__sanitizer::uptr, __sanitizer::uptr)>
   0x00007ffff71fbee7 <+55>:	mov    %rbx,%rsi
   0x00007ffff71fbeea <+58>:	mov    %rbp,%rdi
   0x00007ffff71fbeed <+61>:	callq  *0x337045(%rip)        # 0x7ffff7532f38 <_ZN14__interception16real_swapcontextE>
ENDBR is missing here.
=> 0x00007ffff71fbef3 <+67>:	mov    0x8(%rsp),%rsi
   0x00007ffff71fbef8 <+72>:	mov    (%rsp),%rdi
   0x00007ffff71fbefc <+76>:	mov    %eax,%ebx
   0x00007ffff71fbefe <+78>:	callq  0x7ffff71fbd10 <ClearShadowMemoryForContextStack(__sanitizer::uptr, __sanitizer::uptr)>
   0x00007ffff71fbf03 <+83>:	add    $0x18,%rsp
   0x00007ffff71fbf07 <+87>:	mov    %ebx,%eax
   0x00007ffff71fbf09 <+89>:	pop    %rbx
   0x00007ffff71fbf0a <+90>:	pop    %rbp
   0x00007ffff71fbf0b <+91>:	retq   
   0x00007ffff71fbf0c <+92>:	nopl   0x0(%rax)
   0x00007ffff71fbf10 <+96>:	lea    0xf27b9(%rip),%rdi        # 0x7ffff72ee6d0
   0x00007ffff71fbf17 <+103>:	xor    %eax,%eax
   0x00007ffff71fbf19 <+105>:	

We need to enhance indirect_return attribute to accept function
pointer:

[hjl@gnu-cfl-1 pr85620]$ cat z.i
struct ucontext;
typedef struct ucontext ucontext_t;

extern int (*swapcontext) (ucontext_t *__restrict __oucp,
                           const ucontext_t *__restrict __ucp)
 __attribute__((__indirect_return__));

extern int res;

void
foo (ucontext_t *oucp, ucontext_t *ucp)
{
  res = swapcontext (oucp, ucp);
}
[hjl@gnu-cfl-1 pr85620]$ make z.s
/export/build/gnu/gcc-8-test/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-8-test/build-x86_64-linux/gcc/ -O2 -fcf-protection -S z.i
z.i:6:2: warning: \u2018indirect_return\u2019 attribute directive ignored [-Wattributes]
  __attribute__((__indirect_return__));
  ^~~~~~~~~~~~~
[hjl@gnu-cfl-1 pr85620]$
Comment 1 H.J. Lu 2018-07-18 04:26:46 UTC
GCC 9 error is:

[hjl@gnu-cfl-1 pr85620]$ make z.s
/export/build/gnu/gcc-test/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-test/build-x86_64-linux/gcc/ -O2 -fcf-protection -S z.i
z.i:6:2: warning: \u2018indirect_return\u2019 attribute only applies to functions [-Wattributes]
  __attribute__((__indirect_return__));
  ^~~~~~~~~~~~~
[hjl@gnu-cfl-1 pr85620]$
Comment 2 H.J. Lu 2018-07-18 13:12:12 UTC
LLVM bug is

https://bugs.llvm.org/show_bug.cgi?id=38207
Comment 3 hjl@gcc.gnu.org 2018-07-19 10:47:55 UTC
Author: hjl
Date: Thu Jul 19 10:47:23 2018
New Revision: 262877

URL: https://gcc.gnu.org/viewcvs?rev=262877&root=gcc&view=rev
Log:
i386: Change indirect_return to function type attribute

In

struct ucontext;
typedef struct ucontext ucontext_t;

extern int (*bar) (ucontext_t *__restrict __oucp,
                   const ucontext_t *__restrict __ucp)
  __attribute__((__indirect_return__));

extern int res;

void
foo (ucontext_t *oucp, ucontext_t *ucp)
{
  res = bar (oucp, ucp);
}

bar() may return via indirect branch.  This patch changes indirect_return
to type attribute to allow indirect_return attribute on variable or type
of function pointer so that ENDBR can be inserted after call to bar().

gcc/

	PR target/86560
	* config/i386/i386.c (rest_of_insert_endbranch): Lookup
	indirect_return as function type attribute.
	(ix86_attribute_table): Change indirect_return to function
	type attribute.
	* doc/extend.texi: Update indirect_return attribute.

gcc/testsuite/

	PR target/86560
	* gcc.target/i386/pr86560-1.c: New test.
	* gcc.target/i386/pr86560-2.c: Likewise.
	* gcc.target/i386/pr86560-3.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr86560-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr86560-2.c
    trunk/gcc/testsuite/gcc.target/i386/pr86560-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog
Comment 4 hjl@gcc.gnu.org 2018-07-19 12:01:33 UTC
Author: hjl
Date: Thu Jul 19 12:00:59 2018
New Revision: 262878

URL: https://gcc.gnu.org/viewcvs?rev=262878&root=gcc&view=rev
Log:
i386: Test __has_attribute (__indirect_return__)

The new indirect_return attribute is intended to mark swapcontext in
<ucontext.h>.  Test __has_attribute (__indirect_return__) so that it
can be backported to GCC 8.

	PR target/86560
	* gcc.target/i386/pr86560-4.c: New test.
	* gcc.target/i386/pr86560-5.c: Likewise.
---
 gcc/testsuite/gcc.target/i386/pr86560-4.c | 21 +++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr86560-5.c | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr86560-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr86560-5.c

diff --git a/gcc/testsuite/gcc.target/i386/pr86560-4.c b/gcc/testsuite/gcc.target/i386/pr86560-4.c
new file mode 100644
index 00000000000..a623e3dcbeb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr86560-4.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+struct ucontext;
+
+extern int (*bar) (struct ucontext *)
+#ifdef __has_attribute
+# if __has_attribute (indirect_return)
+  __attribute__((__indirect_return__))
+# endif
+#endif
+;
+
+extern int res;
+
+void
+foo (struct ucontext *oucp)
+{
+  res = bar (oucp);
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr86560-5.c b/gcc/testsuite/gcc.target/i386/pr86560-5.c
new file mode 100644
index 00000000000..33b0f6424c2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr86560-5.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+struct ucontext;
+
+extern int (*bar) (struct ucontext *)
+#ifdef __has_attribute
+# if __has_attribute (__indirect_return__)
+  __attribute__((__indirect_return__))
+# endif
+#endif
+;
+
+extern int res;
+
+void
+foo (struct ucontext *oucp)
+{
+  res = bar (oucp);
+}
Comment 5 hjl@gcc.gnu.org 2018-07-26 14:49:28 UTC
Author: hjl
Date: Thu Jul 26 14:48:55 2018
New Revision: 263009

URL: https://gcc.gnu.org/viewcvs?rev=263009&root=gcc&view=rev
Log:
libsanitizer: Mark REAL(swapcontext) with indirect_return attribute on x86

Cherry-pick compiler-rt revision 337603:

When shadow stack from Intel CET is enabled, the first instruction of all
indirect branch targets must be a special instruction, ENDBR.

lib/asan/asan_interceptors.cc has

...
  int res = REAL(swapcontext)(oucp, ucp);
...

REAL(swapcontext) is a function pointer to swapcontext in libc.  Since
swapcontext may return via indirect branch on x86 when shadow stack is
enabled, as in this case,

int res = REAL(swapcontext)(oucp, ucp);
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^  This function may be
returned via an indirect branch.

Here compiler must insert ENDBR after call, like

call *bar(%rip)
endbr64

I opened an LLVM bug:

https://bugs.llvm.org/show_bug.cgi?id=38207

to add the indirect_return attribute so that it can be used to inform
compiler to insert ENDBR after REAL(swapcontext) call.  We mark
REAL(swapcontext) with the indirect_return attribute if it is available.

This fixed:

https://bugs.llvm.org/show_bug.cgi?id=38249

Reviewed By: eugenis

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

	PR target/86560
	* asan/asan_interceptors.cc (swapcontext) Cherry-pick
	compiler-rt revision 337603.
	* sanitizer_common/sanitizer_internal_defs.h (__has_attribute):
	Likewise.

Modified:
    trunk/libsanitizer/ChangeLog
    trunk/libsanitizer/asan/asan_interceptors.cc
    trunk/libsanitizer/sanitizer_common/sanitizer_internal_defs.h
Comment 6 H.J. Lu 2018-07-26 14:50:25 UTC
Fixed for GCC 9.