RFA: ARM: Implement __builtin_trap

RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

This patch implements __builtin_trap for ARM.  The default, for
architectures that do not provide a "trap" instruction, is to call
abort.  Executing an invalid instruction is a faster, smaller way to
crash.

OK to apply?

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

2010-09-07  Mark Mitchell  <mark@codesourcery.com>

	* config/arm/arm.md (trap): New pattern.

2010-09-07  Mark Mitchell  <mark@codesourcery.com>

	* gcc.target/arm/builtin-trap.c: New.
	* gcc.target/arm/thumb-builtin-trap.c: Likewise.

# gcc diff
pushd /scratch/mitchell/builds/fsf-mainline/src/gcc-mainline
svn diff
Index: gcc/testsuite/gcc.target/arm/thumb-builtin-trap.c
===================================================================
--- gcc/testsuite/gcc.target/arm/thumb-builtin-trap.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/thumb-builtin-trap.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-mthumb" }  */
+
+void trap ()
+{
+  __builtin_trap ();
+}
+
+/* { dg-final { scan-assembler "0xdeff" } } */
Index: gcc/testsuite/gcc.target/arm/builtin-trap.c
===================================================================
--- gcc/testsuite/gcc.target/arm/builtin-trap.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/builtin-trap.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm32 } */
+
+void trap ()
+{
+  __builtin_trap ();
+}
+
+/* { dg-final { scan-assembler "0xe7ffffff" } } */
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 163924)
+++ gcc/config/arm/arm.md	(working copy)
@@ -8496,6 +8496,26 @@
 		      (const_int 4)))]
 )
 
+;; Generate an invalid instruction.  The instructions chosen are
+;; documented as permanently UNDEFINED, so we can rely on the fact
+;; that no future version of the architecture will use them.  The
+;; instructions used are chosen so as to be distinct from he
+;; instructions that the Linux kernel interprets as software
+;; breakpoints.
+(define_insn "trap"
+  [(trap_if (const_int 1) (const_int 0))]
+  ""
+  "*
+  if (TARGET_ARM)
+    return \".inst\\t0xe7ffffff\";
+  else
+    return \".inst\\t0xdeff\";
+  "
+  [(set (attr "length")
+	(if_then_else (eq_attr "is_thumb" "yes")
+		      (const_int 2)
+		      (const_int 4)))]
+)
 \f
 ;; Patterns to allow combination of arithmetic, cond code and shifts
 
popd

Re: RFA: ARM: Implement __builtin_trap

[Mike Stump]

On Sep 7, 2010, at 9:12 PM, Mark Mitchell wrote:
> This patch implements __builtin_trap for ARM.  The default, for
> architectures that do not provide a "trap" instruction, is to call
> abort.  Executing an invalid instruction is a faster, smaller way to
> crash.

Would be nice for arm to allocate (and document) some short sequence that will never be used in future ISA enhancements, as otherwise old binaries that should die, would then just do whatever new instruction was added.

Re: RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

On 9/8/2010 3:59 PM, Mike Stump wrote:
> On Sep 7, 2010, at 9:12 PM, Mark Mitchell wrote:
>> This patch implements __builtin_trap for ARM.  The default, for
>> architectures that do not provide a "trap" instruction, is to call
>> abort.  Executing an invalid instruction is a faster, smaller way to
>> crash.

> Would be nice for arm to allocate (and document) some short sequence that will never be used in future ISA enhancements, as otherwise old binaries that should die, would then just do whatever new instruction was added.

The instructions I chose are documented as such.  That's why I wrote:

;; Generate an invalid instruction.  The instructions chosen are
;; documented as permanently UNDEFINED, so we can rely on the fact
;; that no future version of the architecture will use them.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFA: ARM: Implement __builtin_trap

[David Daney]

On 09/08/2010 04:01 PM, Mark Mitchell wrote:
> On 9/8/2010 3:59 PM, Mike Stump wrote:
>> On Sep 7, 2010, at 9:12 PM, Mark Mitchell wrote:
>>> This patch implements __builtin_trap for ARM.  The default, for
>>> architectures that do not provide a "trap" instruction, is to call
>>> abort.  Executing an invalid instruction is a faster, smaller way to
>>> crash.
>
>> Would be nice for arm to allocate (and document) some short sequence that will never be used in future ISA enhancements, as otherwise old binaries that should die, would then just do whatever new instruction was added.
>
> The instructions I chose are documented as such.  That's why I wrote:
>
> ;; Generate an invalid instruction.  The instructions chosen are
> ;; documented as permanently UNDEFINED, so we can rely on the fact
> ;; that no future version of the architecture will use them.
>
> Thanks,
>

I saw that, and I have no objections to that patch.  But I was thinking 
that this essentially augments the ABI.  Could or should this be 
coordinated with the maintainers of any ARM ABIs?

David Daney

Re: RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

On 9/9/2010 11:20 AM, David Daney wrote:

> I saw that, and I have no objections to that patch.  But I was thinking
> that this essentially augments the ABI.  Could or should this be
> coordinated with the maintainers of any ARM ABIs?

Do other psABIs document how __builtin_trap is implemented?

Richard E. is certainly in a position to comment on this.  The only
issue I see is that we're "using up" an instruction in the undefined
instruction space, meaning that it can't be used in some other magic way
by an OS.  Linux presently uses an undefined instruction as a software
breakpoint.  (I'm not sure why it does that instead of using an SVC
instruction?)  But is there other stuff out there that might need this?

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFA: ARM: Implement __builtin_trap

[Chris Lattner]

On Sep 9, 2010, at 11:28 AM, Mark Mitchell wrote:

> On 9/9/2010 11:20 AM, David Daney wrote:
> 
>> I saw that, and I have no objections to that patch.  But I was thinking
>> that this essentially augments the ABI.  Could or should this be
>> coordinated with the maintainers of any ARM ABIs?
> 
> Do other psABIs document how __builtin_trap is implemented?
> 
> Richard E. is certainly in a position to comment on this.  The only
> issue I see is that we're "using up" an instruction in the undefined
> instruction space, meaning that it can't be used in some other magic way
> by an OS.  Linux presently uses an undefined instruction as a software
> breakpoint.  (I'm not sure why it does that instead of using an SVC
> instruction?)  But is there other stuff out there that might need this?

FWIW, the LLVM ARM backend compiles __builtin_trap into "trap" (aka .word 0xe7ffdefe).

-Chris

Re: RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

On 9/9/2010 11:34 AM, Chris Lattner wrote:

> FWIW, the LLVM ARM backend compiles __builtin_trap into "trap" (aka .word 0xe7ffdefe).

Is "trap" a standard pseudo-op?  I didn't see it in either the ARM ARM
or in ARM's assembler manual, but that doesn't mean it isn't there.
ARM's assembler has an "UND" pseudo-op, which takes an argument to say
exactly which undefined instruction you want.  From the documentation,
it looks like the default, if you don't specify an argument is 0xe7f000f0.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFA: ARM: Implement __builtin_trap

[Daniel Jacobowitz]

On Wed, Sep 08, 2010 at 04:01:53PM -0700, Mark Mitchell wrote:
> The instructions I chose are documented as such.  That's why I wrote:
> 
> ;; Generate an invalid instruction.  The instructions chosen are
> ;; documented as permanently UNDEFINED, so we can rely on the fact
> ;; that no future version of the architecture will use them.

FWIW, it occurred to me (belatedly) that there's no harm in using the
same instruction that GDB uses, anyway.  It would cause SIGTRAP rather
than SIGILL on Linux.  But then, it's __builtin_*trap* ... :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

On 9/9/2010 12:23 PM, Daniel Jacobowitz wrote:

>> ;; Generate an invalid instruction.  The instructions chosen are
>> ;; documented as permanently UNDEFINED, so we can rely on the fact
>> ;; that no future version of the architecture will use them.
> 
> FWIW, it occurred to me (belatedly) that there's no harm in using the
> same instruction that GDB uses, anyway.  It would cause SIGTRAP rather
> than SIGILL on Linux.  But then, it's __builtin_*trap* ... :-)

I certainly don't mind what bitpattern we use and am happy to make that
change if that's what the ARM maintainers think best.

But, would this cause gdbserver to report that the application had hit a
breakpoint?  If so, that seems bad; GDB won't know about the breakpoint.
 Also, FWIW, on x86 __builtin_trap causes SIGILL, and being consistent
seems useful.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFA: ARM: Implement __builtin_trap

[Daniel Jacobowitz]

On Thu, Sep 09, 2010 at 01:50:50PM -0700, Mark Mitchell wrote:
> But, would this cause gdbserver to report that the application had hit a
> breakpoint?  If so, that seems bad; GDB won't know about the breakpoint.
>  Also, FWIW, on x86 __builtin_trap causes SIGILL, and being consistent
> seems useful.

It works out; GDB knows how to handle SIGTRAPs that are not caused by
a breakpoint.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: ARM: Implement __builtin_trap

[Chris Lattner]

On Sep 9, 2010, at 11:44 AM, Mark Mitchell wrote:

> On 9/9/2010 11:34 AM, Chris Lattner wrote:
> 
>> FWIW, the LLVM ARM backend compiles __builtin_trap into "trap" (aka .word 0xe7ffdefe).
> 
> Is "trap" a standard pseudo-op?  I didn't see it in either the ARM ARM
> or in ARM's assembler manual, but that doesn't mean it isn't there.
> ARM's assembler has an "UND" pseudo-op, which takes an argument to say
> exactly which undefined instruction you want.  From the documentation,
> it looks like the default, if you don't specify an argument is 0xe7f000f0.

Specifically, the LLVM backend emits:

_t:                                     @ @t
	.long 0xe7ffdefe @ trap


So no, I don't think that trap is a standard mnemonic.  I have a vague memory of ARM not having an official trap instruction, but then retroactively deciding that some invalid encodings would never be defined.

-Chris

Re: RFA: ARM: Implement __builtin_trap

[Anton Korobeynikov]

> Specifically, the LLVM backend emits:
>
> _t:                                     @ @t
>        .long 0xe7ffdefe @ trap
>
>
> So no, I don't think that trap is a standard mnemonic.
iirc "trap" is supported as a separate mnemonic in darwin as. gas does
not support it, thus we emit .word / .long here


-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

Re: RFA: ARM: Implement __builtin_trap

[Richard Earnshaw]

On Thu, 2010-09-09 at 14:20 -0700, Chris Lattner wrote:
> On Sep 9, 2010, at 11:44 AM, Mark Mitchell wrote:
> 
> > On 9/9/2010 11:34 AM, Chris Lattner wrote:
> > 
> >> FWIW, the LLVM ARM backend compiles __builtin_trap into "trap" (aka .word 0xe7ffdefe).
> > 
> > Is "trap" a standard pseudo-op?  I didn't see it in either the ARM ARM
> > or in ARM's assembler manual, but that doesn't mean it isn't there.
> > ARM's assembler has an "UND" pseudo-op, which takes an argument to say
> > exactly which undefined instruction you want.  From the documentation,
> > it looks like the default, if you don't specify an argument is 0xe7f000f0.
> 
> Specifically, the LLVM backend emits:
> 
> _t:                                     @ @t
> 	.long 0xe7ffdefe @ trap
> 
> 
> So no, I don't think that trap is a standard mnemonic.  I have a vague memory of ARM not having an official trap instruction, but then retroactively deciding that some invalid encodings would never be defined.
> 
> -Chris

You shouldn't use ".long" for instructions, use ".inst".  ".long"
implies a word of data while ".inst" implies an instruction: in
big-endian mode the difference is significant.

R.

Re: RFA: ARM: Implement __builtin_trap

[Mikael Pettersson]

Richard Earnshaw writes:
 > 
 > On Thu, 2010-09-09 at 14:20 -0700, Chris Lattner wrote:
 > > On Sep 9, 2010, at 11:44 AM, Mark Mitchell wrote:
 > > 
 > > > On 9/9/2010 11:34 AM, Chris Lattner wrote:
 > > > 
 > > >> FWIW, the LLVM ARM backend compiles __builtin_trap into "trap" (aka .word 0xe7ffdefe).
 > > > 
 > > > Is "trap" a standard pseudo-op?  I didn't see it in either the ARM ARM
 > > > or in ARM's assembler manual, but that doesn't mean it isn't there.
 > > > ARM's assembler has an "UND" pseudo-op, which takes an argument to say
 > > > exactly which undefined instruction you want.  From the documentation,
 > > > it looks like the default, if you don't specify an argument is 0xe7f000f0.
 > > 
 > > Specifically, the LLVM backend emits:
 > > 
 > > _t:                                     @ @t
 > > 	.long 0xe7ffdefe @ trap
 > > 
 > > 
 > > So no, I don't think that trap is a standard mnemonic.  I have a vague memory of ARM not having an official trap instruction, but then retroactively deciding that some invalid encodings would never be defined.
 > > 
 > > -Chris
 > 
 > You shouldn't use ".long" for instructions, use ".inst".  ".long"
 > implies a word of data while ".inst" implies an instruction: in
 > big-endian mode the difference is significant.

It also affects objdump, which in recent binutils won't disassemble
code in .text entered with pseudo-ops like .long or .word. ".inst" works.

/Mikael

Re: RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

On 9/10/2010 2:45 AM, Richard Earnshaw wrote:

> You shouldn't use ".long" for instructions, use ".inst".  ".long"
> implies a word of data while ".inst" implies an instruction: in
> big-endian mode the difference is significant.

Just for avoidance of doubt, my patch doesn't use ".long"; it uses
".inst".  May I apply it (possibly with some change to the choice of
constants) or is there something else that needs doing?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFA: ARM: Implement __builtin_trap

[Mark Mitchell]

On 9/10/2010 6:55 AM, Mark Mitchell wrote:

>> You shouldn't use ".long" for instructions, use ".inst".  ".long"
>> implies a word of data while ".inst" implies an instruction: in
>> big-endian mode the difference is significant.
> 
> Just for avoidance of doubt, my patch doesn't use ".long"; it uses
> ".inst".  May I apply it (possibly with some change to the choice of
> constants) or is there something else that needs doing?

Ping.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713