| Message ID | 20100908041251.3586C5664F5@henry1.codesourcery.com |
|---|---|
| State | New |
| Headers | show |
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.
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,
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
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?
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
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.
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* ... :-)
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.
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.
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
> 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
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.
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
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,
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.
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)))] +) ;; Patterns to allow combination of arithmetic, cond code and shifts