diff mbox

i386: Add some naked attribute tests

Message ID CAMe9rOqHptCFQ=OwJDHDYV0CBk2P=nzpFHoDptALnXNW5mdqvQ@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Aug. 1, 2017, 9:23 p.m. UTC 
On Tue, Aug 1, 2017 at 2:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 11:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Aug 1, 2017 at 1:49 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Tue, Aug 1, 2017 at 9:46 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>> Add some tests for implementing interrupt handlers with naked attribute.
>>>>
>>>> OK for trunk?
>>>>
>>>> H.J.
>>>> ---
>>>>         * gcc.dg/guality/pr25967-1.c: New test.
>>>>         * gcc.dg/guality/pr25967-2.c: Likewise.
>>>>         * gcc.dg/torture/pr25967-1.c: Likewise.
>>>>         * gcc.dg/torture/pr25967-2.c: Likewise.
>>>
>>> OK with a small change below.
>>>
>>
>>>> +void
>>>> +fn (void)
>>>> +{
>>>> +  struct interrupt_frame *frame;
>>>> +  uword_t error;
>>>> +  asm ("lea " WORD_SIZE "(%%" STACK_POINTER "), %0" : "=r" (frame) : );
>>>> +  asm ("mov (%%" STACK_POINTER "), %0" : "=r" (error) : );
>>>
>>> The above two asm needs to be volatile. They are not "simple" asm, and
>>> access stack pointer behind the compilers back. And please merge them
>>> to one multi-line volatile asm statement.
>>>
>>
>>
>> This is what I am checking in.
>
> OTOH, these asms can be avoided with something like:
>
> --cut here--
> typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>
> struct interrupt_frame
> {
>   uword_t ip;
>   uword_t cs;
>   uword_t flags;
>   uword_t sp;
>   uword_t ss;
> };
>
> void
> __attribute__((naked))
> test (void)
> {
>   register uword_t sp __asm__("sp");
>
>   long *error = (long *) sp;
>   struct interrupt_frame *frame
>     = (struct interrupt_frame *) (sp + sizeof (uword_t));
>
>   ...
>

How about this?  OK for trunk?

Comments

Uros Bizjak Aug. 1, 2017, 9:25 p.m. UTC | #1 
On Tue, Aug 1, 2017 at 11:23 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 2:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Tue, Aug 1, 2017 at 11:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Aug 1, 2017 at 1:49 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> On Tue, Aug 1, 2017 at 9:46 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>> Add some tests for implementing interrupt handlers with naked attribute.
>>>>>
>>>>> OK for trunk?
>>>>>
>>>>> H.J.
>>>>> ---
>>>>>         * gcc.dg/guality/pr25967-1.c: New test.
>>>>>         * gcc.dg/guality/pr25967-2.c: Likewise.
>>>>>         * gcc.dg/torture/pr25967-1.c: Likewise.
>>>>>         * gcc.dg/torture/pr25967-2.c: Likewise.
>>>>
>>>> OK with a small change below.
>>>>
>>>
>>>>> +void
>>>>> +fn (void)
>>>>> +{
>>>>> +  struct interrupt_frame *frame;
>>>>> +  uword_t error;
>>>>> +  asm ("lea " WORD_SIZE "(%%" STACK_POINTER "), %0" : "=r" (frame) : );
>>>>> +  asm ("mov (%%" STACK_POINTER "), %0" : "=r" (error) : );
>>>>
>>>> The above two asm needs to be volatile. They are not "simple" asm, and
>>>> access stack pointer behind the compilers back. And please merge them
>>>> to one multi-line volatile asm statement.
>>>>
>>>
>>>
>>> This is what I am checking in.
>>
>> OTOH, these asms can be avoided with something like:
>>
>> --cut here--
>> typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>>
>> struct interrupt_frame
>> {
>>   uword_t ip;
>>   uword_t cs;
>>   uword_t flags;
>>   uword_t sp;
>>   uword_t ss;
>> };
>>
>> void
>> __attribute__((naked))
>> test (void)
>> {
>>   register uword_t sp __asm__("sp");
>>
>>   long *error = (long *) sp;
>>   struct interrupt_frame *frame
>>     = (struct interrupt_frame *) (sp + sizeof (uword_t));
>>
>>   ...
>>
>
> How about this?  OK for trunk?

Even better.

Can we introduce asm_goto to the jmp in the main asm?

Uros.
H.J. Lu Aug. 1, 2017, 9:35 p.m. UTC | #2 
On Tue, Aug 1, 2017 at 2:25 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 11:23 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Aug 1, 2017 at 2:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Tue, Aug 1, 2017 at 11:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Tue, Aug 1, 2017 at 1:49 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>> On Tue, Aug 1, 2017 at 9:46 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>>> Add some tests for implementing interrupt handlers with naked attribute.
>>>>>>
>>>>>> OK for trunk?
>>>>>>
>>>>>> H.J.
>>>>>> ---
>>>>>>         * gcc.dg/guality/pr25967-1.c: New test.
>>>>>>         * gcc.dg/guality/pr25967-2.c: Likewise.
>>>>>>         * gcc.dg/torture/pr25967-1.c: Likewise.
>>>>>>         * gcc.dg/torture/pr25967-2.c: Likewise.
>>>>>
>>>>> OK with a small change below.
>>>>>
>>>>
>>>>>> +void
>>>>>> +fn (void)
>>>>>> +{
>>>>>> +  struct interrupt_frame *frame;
>>>>>> +  uword_t error;
>>>>>> +  asm ("lea " WORD_SIZE "(%%" STACK_POINTER "), %0" : "=r" (frame) : );
>>>>>> +  asm ("mov (%%" STACK_POINTER "), %0" : "=r" (error) : );
>>>>>
>>>>> The above two asm needs to be volatile. They are not "simple" asm, and
>>>>> access stack pointer behind the compilers back. And please merge them
>>>>> to one multi-line volatile asm statement.
>>>>>
>>>>
>>>>
>>>> This is what I am checking in.
>>>
>>> OTOH, these asms can be avoided with something like:
>>>
>>> --cut here--
>>> typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>>>
>>> struct interrupt_frame
>>> {
>>>   uword_t ip;
>>>   uword_t cs;
>>>   uword_t flags;
>>>   uword_t sp;
>>>   uword_t ss;
>>> };
>>>
>>> void
>>> __attribute__((naked))
>>> test (void)
>>> {
>>>   register uword_t sp __asm__("sp");
>>>
>>>   long *error = (long *) sp;
>>>   struct interrupt_frame *frame
>>>     = (struct interrupt_frame *) (sp + sizeof (uword_t));
>>>
>>>   ...
>>>
>>
>> How about this?  OK for trunk?
>
> Even better.
>
> Can we introduce asm_goto to the jmp in the main asm?
>

asm goto doesn't work since it only takes labels.  But interrupt handler
must be a function.
Uros Bizjak Aug. 1, 2017, 9:36 p.m. UTC | #3 
On Tue, Aug 1, 2017 at 11:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 2:25 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Tue, Aug 1, 2017 at 11:23 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Aug 1, 2017 at 2:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> On Tue, Aug 1, 2017 at 11:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Tue, Aug 1, 2017 at 1:49 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>>> On Tue, Aug 1, 2017 at 9:46 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>>>> Add some tests for implementing interrupt handlers with naked attribute.
>>>>>>>
>>>>>>> OK for trunk?
>>>>>>>
>>>>>>> H.J.
>>>>>>> ---
>>>>>>>         * gcc.dg/guality/pr25967-1.c: New test.
>>>>>>>         * gcc.dg/guality/pr25967-2.c: Likewise.
>>>>>>>         * gcc.dg/torture/pr25967-1.c: Likewise.
>>>>>>>         * gcc.dg/torture/pr25967-2.c: Likewise.
>>>>>>
>>>>>> OK with a small change below.
>>>>>>
>>>>>
>>>>>>> +void
>>>>>>> +fn (void)
>>>>>>> +{
>>>>>>> +  struct interrupt_frame *frame;
>>>>>>> +  uword_t error;
>>>>>>> +  asm ("lea " WORD_SIZE "(%%" STACK_POINTER "), %0" : "=r" (frame) : );
>>>>>>> +  asm ("mov (%%" STACK_POINTER "), %0" : "=r" (error) : );
>>>>>>
>>>>>> The above two asm needs to be volatile. They are not "simple" asm, and
>>>>>> access stack pointer behind the compilers back. And please merge them
>>>>>> to one multi-line volatile asm statement.
>>>>>>
>>>>>
>>>>>
>>>>> This is what I am checking in.
>>>>
>>>> OTOH, these asms can be avoided with something like:
>>>>
>>>> --cut here--
>>>> typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>>>>
>>>> struct interrupt_frame
>>>> {
>>>>   uword_t ip;
>>>>   uword_t cs;
>>>>   uword_t flags;
>>>>   uword_t sp;
>>>>   uword_t ss;
>>>> };
>>>>
>>>> void
>>>> __attribute__((naked))
>>>> test (void)
>>>> {
>>>>   register uword_t sp __asm__("sp");
>>>>
>>>>   long *error = (long *) sp;
>>>>   struct interrupt_frame *frame
>>>>     = (struct interrupt_frame *) (sp + sizeof (uword_t));
>>>>
>>>>   ...
>>>>
>>>
>>> How about this?  OK for trunk?
>>
>> Even better.
>>
>> Can we introduce asm_goto to the jmp in the main asm?
>>
>
> asm goto doesn't work since it only takes labels.  But interrupt handler
> must be a function.

Ah, indeed.

OK then.

Thanks,
Uros.
diff mbox

Patch

From b197de39efeac72489188dd2383abb87ff229048 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 1 Aug 2017 14:21:07 -0700
Subject: [PATCH] i386: Add more naked attribute tests

Add some tests for implementing interrupt handlers with naked attribute
and without asm statements.

	* gcc.dg/guality/pr25967-3.c: New test.
	* gcc.dg/guality/pr25967-4.c: Likewise.
	* gcc.dg/torture/pr25967-3.c: Likewise.
	* gcc.dg/torture/pr25967-4.c: Likewise.
---
 gcc/testsuite/gcc.dg/guality/pr25967-3.c | 70 ++++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/guality/pr25967-4.c | 64 +++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/torture/pr25967-3.c | 63 ++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/torture/pr25967-4.c | 58 ++++++++++++++++++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/guality/pr25967-3.c
 create mode 100644 gcc/testsuite/gcc.dg/guality/pr25967-4.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr25967-3.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr25967-4.c

diff --git a/gcc/testsuite/gcc.dg/guality/pr25967-3.c b/gcc/testsuite/gcc.dg/guality/pr25967-3.c
new file mode 100644
index 00000000000..0924d1c03c1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/guality/pr25967-3.c
@@ -0,0 +1,70 @@ 
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-g -mgeneral-regs-only" } */
+
+extern void exit (int);
+
+typedef unsigned int uword_t __attribute__ ((mode (__word__)));
+
+#define ERROR		0x12345670
+#define IP		0x12345671
+#define CS		0x12345672
+#define FLAGS		0x12345673
+#define SP		0x12345674
+#define SS		0x12345675
+
+#define STRING(x)	XSTRING(x)
+#define XSTRING(x)	#x
+#define ASMNAME(cname)  ASMNAME2 (__USER_LABEL_PREFIX__, cname)
+#define ASMNAME2(prefix, cname) XSTRING (prefix) cname
+
+struct interrupt_frame
+{
+  uword_t ip;
+  uword_t cs;
+  uword_t flags;
+  uword_t sp;
+  uword_t ss;
+};
+
+__attribute__((naked, used))
+void
+fn (void)
+{
+  register uword_t *sp __asm__("sp");
+  uword_t error = *sp;
+  struct interrupt_frame *frame = (struct interrupt_frame *) (sp + 1);
+  if (ERROR != error)		/* BREAK */
+    __builtin_abort ();
+  if (IP != frame->ip)
+    __builtin_abort ();
+  if (CS != frame->cs)
+    __builtin_abort ();
+  if (FLAGS != frame->flags)
+    __builtin_abort ();
+  if (SP != frame->sp)
+    __builtin_abort ();
+  if (SS != frame->ss)
+    __builtin_abort ();
+
+  exit (0);
+}
+
+int
+main ()
+{
+  asm ("push	$" STRING (SS) ";		\
+	push	$" STRING (SP) ";		\
+	push	$" STRING (FLAGS) ";		\
+	push	$" STRING (CS) ";		\
+	push	$" STRING (IP) ";		\
+	push	$" STRING (ERROR) ";		\
+	jmp	 " ASMNAME ("fn"));
+  return 0;
+}
+
+/* { dg-final { gdb-test 36 "error" "0x12345670" } } */
+/* { dg-final { gdb-test 36 "frame->ip" "0x12345671" } } */
+/* { dg-final { gdb-test 36 "frame->cs" "0x12345672" } } */
+/* { dg-final { gdb-test 36 "frame->flags" "0x12345673" } } */
+/* { dg-final { gdb-test 36 "frame->sp" "0x12345674" } } */
+/* { dg-final { gdb-test 36 "frame->ss" "0x12345675" } } */
diff --git a/gcc/testsuite/gcc.dg/guality/pr25967-4.c b/gcc/testsuite/gcc.dg/guality/pr25967-4.c
new file mode 100644
index 00000000000..c3b59e21251
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/guality/pr25967-4.c
@@ -0,0 +1,64 @@ 
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-g -mgeneral-regs-only" } */
+
+extern void exit (int);
+
+typedef unsigned int uword_t __attribute__ ((mode (__word__)));
+
+#define IP		0x12345671
+#define CS		0x12345672
+#define FLAGS		0x12345673
+#define SP		0x12345674
+#define SS		0x12345675
+
+#define STRING(x)	XSTRING(x)
+#define XSTRING(x)	#x
+#define ASMNAME(cname)  ASMNAME2 (__USER_LABEL_PREFIX__, cname)
+#define ASMNAME2(prefix, cname) XSTRING (prefix) cname
+
+struct interrupt_frame
+{
+  uword_t ip;
+  uword_t cs;
+  uword_t flags;
+  uword_t sp;
+  uword_t ss;
+};
+
+__attribute__((naked, used))
+void
+fn (void)
+{
+  register uword_t *sp __asm__("sp");
+  struct interrupt_frame *frame = (struct interrupt_frame *) sp;
+  if (IP != frame->ip)		/* BREAK */
+    __builtin_abort ();
+  if (CS != frame->cs)
+    __builtin_abort ();
+  if (FLAGS != frame->flags)
+    __builtin_abort ();
+  if (SP != frame->sp)
+    __builtin_abort ();
+  if (SS != frame->ss)
+    __builtin_abort ();
+
+  exit (0);
+}
+
+int
+main ()
+{
+  asm ("push	$" STRING (SS) ";		\
+	push	$" STRING (SP) ";		\
+	push	$" STRING (FLAGS) ";		\
+	push	$" STRING (CS) ";		\
+	push	$" STRING (IP) ";		\
+	jmp	 " ASMNAME ("fn"));
+  return 0;
+}
+
+/* { dg-final { gdb-test 34 "frame->ip" "0x12345671" } } */
+/* { dg-final { gdb-test 34 "frame->cs" "0x12345672" } } */
+/* { dg-final { gdb-test 34 "frame->flags" "0x12345673" } } */
+/* { dg-final { gdb-test 34 "frame->sp" "0x12345674" } } */
+/* { dg-final { gdb-test 34 "frame->ss" "0x12345675" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/pr25967-3.c b/gcc/testsuite/gcc.dg/torture/pr25967-3.c
new file mode 100644
index 00000000000..fd26a8b8ce3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr25967-3.c
@@ -0,0 +1,63 @@ 
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-mgeneral-regs-only" } */
+
+extern void exit (int);
+
+typedef unsigned int uword_t __attribute__ ((mode (__word__)));
+
+#define ERROR		0x12345670
+#define IP		0x12345671
+#define CS		0x12345672
+#define FLAGS		0x12345673
+#define SP		0x12345674
+#define SS		0x12345675
+
+#define STRING(x)	XSTRING(x)
+#define XSTRING(x)	#x
+#define ASMNAME(cname)  ASMNAME2 (__USER_LABEL_PREFIX__, cname)
+#define ASMNAME2(prefix, cname) XSTRING (prefix) cname
+
+struct interrupt_frame
+{
+  uword_t ip;
+  uword_t cs;
+  uword_t flags;
+  uword_t sp;
+  uword_t ss;
+};
+
+__attribute__((naked, used))
+void
+fn (void)
+{
+  register uword_t *sp __asm__("sp");
+  uword_t error = *sp;
+  struct interrupt_frame *frame = (struct interrupt_frame *) (sp + 1);
+  if (ERROR != error)		/* BREAK */
+    __builtin_abort ();
+  if (IP != frame->ip)
+    __builtin_abort ();
+  if (CS != frame->cs)
+    __builtin_abort ();
+  if (FLAGS != frame->flags)
+    __builtin_abort ();
+  if (SP != frame->sp)
+    __builtin_abort ();
+  if (SS != frame->ss)
+    __builtin_abort ();
+
+  exit (0);
+}
+
+int
+main ()
+{
+  asm ("push	$" STRING (SS) ";		\
+	push	$" STRING (SP) ";		\
+	push	$" STRING (FLAGS) ";		\
+	push	$" STRING (CS) ";		\
+	push	$" STRING (IP) ";		\
+	push	$" STRING (ERROR) ";		\
+	jmp	 " ASMNAME ("fn"));
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr25967-4.c b/gcc/testsuite/gcc.dg/torture/pr25967-4.c
new file mode 100644
index 00000000000..4a0dd78c0ad
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr25967-4.c
@@ -0,0 +1,58 @@ 
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-mgeneral-regs-only" } */
+
+extern void exit (int);
+
+typedef unsigned int uword_t __attribute__ ((mode (__word__)));
+
+#define IP		0x12345671
+#define CS		0x12345672
+#define FLAGS		0x12345673
+#define SP		0x12345674
+#define SS		0x12345675
+
+#define STRING(x)	XSTRING(x)
+#define XSTRING(x)	#x
+#define ASMNAME(cname)  ASMNAME2 (__USER_LABEL_PREFIX__, cname)
+#define ASMNAME2(prefix, cname) XSTRING (prefix) cname
+
+struct interrupt_frame
+{
+  uword_t ip;
+  uword_t cs;
+  uword_t flags;
+  uword_t sp;
+  uword_t ss;
+};
+
+__attribute__((naked, used))
+void
+fn (void)
+{
+  register uword_t *sp __asm__("sp");
+  struct interrupt_frame *frame = (struct interrupt_frame *) sp;
+  if (IP != frame->ip)		/* BREAK */
+    __builtin_abort ();
+  if (CS != frame->cs)
+    __builtin_abort ();
+  if (FLAGS != frame->flags)
+    __builtin_abort ();
+  if (SP != frame->sp)
+    __builtin_abort ();
+  if (SS != frame->ss)
+    __builtin_abort ();
+
+  exit (0);
+}
+
+int
+main ()
+{
+  asm ("push	$" STRING (SS) ";		\
+	push	$" STRING (SP) ";		\
+	push	$" STRING (FLAGS) ";		\
+	push	$" STRING (CS) ";		\
+	push	$" STRING (IP) ";		\
+	jmp	 " ASMNAME ("fn"));
+  return 0;
+}
-- 
2.13.3