| Message ID | 07b84003-4e73-8a7f-f949-4c3500e4ffc4@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 30, 2016 at 11:15:22AM +0000, Jiong Wang wrote: > > Hi GDB, Binutils maintainer: > > OK on this proposal and install this patch to binutils-gdb master? > This proposal is good to GDB, as long as you add a gdbarch hook and move the code handling DW_CFA_GNU_window_save in gdb/dwarf2-frame.c:execute_cfa_program to sparc-tdep.c or/and sparc64-tdep.c. -- Yao (齐尧)
Jiong Wang writes: > Jiong Wang writes: > >> On 16/11/16 14:02, Jakub Jelinek wrote: >>> On Wed, Nov 16, 2016 at 02:54:56PM +0100, Mark Wielaard wrote: >>>> On Wed, 2016-11-16 at 10:00 +0000, Jiong Wang wrote: >>>>> The two operations DW_OP_AARCH64_paciasp and DW_OP_AARCH64_paciasp_deref were >>>>> designed as shortcut operations when LR is signed with A key and using >>>>> function's CFA as salt. This is the default behaviour of return address >>>>> signing so is expected to be used for most of the time. DW_OP_AARCH64_pauth >>>>> is designed as a generic operation that allow describing pointer signing on >>>>> any value using any salt and key in case we can't use the shortcut operations >>>>> we can use this. >>>> >>>> I admit to not fully understand the salting/keying involved. But given >>>> that the DW_OP space is really tiny, so we would like to not eat up too >>>> many of them for new opcodes. And given that introducing any new DW_OPs >>>> using for CFI unwinding will break any unwinder anyway causing us to >>>> update them all for this new feature. Have you thought about using a new >>>> CIE augmentation string character for describing that the return >>>> address/link register used by a function/frame is salted/keyed? >>>> >>>> This seems a good description of CIE records and augmentation >>>> characters:http://www.airs.com/blog/archives/460 >>>> >>>> It obviously also involves updating all unwinders to understand the new >>>> augmentation character (and possible arguments). But it might be more >>>> generic and saves us from using up too many DW_OPs. >>> >>> From what I understood, the return address is not always scrambled, so >>> it doesn't apply to the whole function, just to most of it (except for >>> an insn in the prologue and some in the epilogue). So I think one op is >>> needed. But can't it be just a toggable flag whether the return address >>> is scrambled + some arguments to it? >>> Thus DW_OP_AARCH64_scramble .uleb128 0 would mean that the default >>> way of scrambling starts here (if not already active) or any kind of >>> scrambling ends here (if already active), and >>> DW_OP_AARCH64_scramble .uleb128 non-zero would be whatever encoding you need >>> to represent details of the less common variants with details what to do. >>> Then you'd just hook through some MD_* macro in the unwinder the >>> descrambling operation if the scrambling is active at the insns you unwind >>> on. >>> >>> Jakub >> >> Hi Mark, Jakub: >> >> Thanks very much for the suggestions. >> >> I have done some experiments on your ideas and am thinking it's good to >> combine them together. The use of DW_CFA instead of DW_OP can avoid building >> all information from scratch at each unwind location, while we can indicate >> the signing key index through new AArch64 CIE augmentation 'B'. This new >> approach reduce the unwind table size overhead from ~25% to ~5% when return >> address signing enabled, it also largely simplified dwarf generation code for >> return address signing. >> >> As one new DWARF call frame instruction is needed for AArch64, I want to reuse >> DW_CFA_GNU_window_save to save the space. It is in vendor extension space and >> used for Sparc only, I think it make sense to reuse it for AArch64. On >> AArch64, DW_CFA_GNU_window_save toggle return address sign status which kept >> in a new boolean type column in DWARF table, so DW_CFA_GNU_window_save takes >> no argument on AArch64, the same as on Sparc, this makes no difference to those >> existed encoding, length calculation code. >> >> Meanwhile one new DWARF expression operation number is still needed for >> AArch64, it's useful for describing those complex pointer signing scenarios >> and it will be used to multiplex some further extensions on AArch64. >> >> OK on this proposal and to install this patch to gcc trunk? >> >> Hi GDB, Binutils maintainer: >> >> OK on this proposal and install this patch to binutils-gdb master? >> >> include/ >> 2016-11-29 Richard Earnshaw <rearnsha@arm.com> >> Jiong Wang <jiong.wang@arm.com> >> >> * dwarf2.def (DW_OP_AARCH64_operation): Reserve the number 0xea. > > Ping~ Ping^2 -- Regards, Jiong
Jiong Wang writes: > Jiong Wang writes: > >> Jiong Wang writes: >> >>> On 16/11/16 14:02, Jakub Jelinek wrote: >>>> On Wed, Nov 16, 2016 at 02:54:56PM +0100, Mark Wielaard wrote: >>>>> On Wed, 2016-11-16 at 10:00 +0000, Jiong Wang wrote: >>>>>> The two operations DW_OP_AARCH64_paciasp and DW_OP_AARCH64_paciasp_deref were >>>>>> designed as shortcut operations when LR is signed with A key and using >>>>>> function's CFA as salt. This is the default behaviour of return address >>>>>> signing so is expected to be used for most of the time. DW_OP_AARCH64_pauth >>>>>> is designed as a generic operation that allow describing pointer signing on >>>>>> any value using any salt and key in case we can't use the shortcut operations >>>>>> we can use this. >>>>> >>>>> I admit to not fully understand the salting/keying involved. But given >>>>> that the DW_OP space is really tiny, so we would like to not eat up too >>>>> many of them for new opcodes. And given that introducing any new DW_OPs >>>>> using for CFI unwinding will break any unwinder anyway causing us to >>>>> update them all for this new feature. Have you thought about using a new >>>>> CIE augmentation string character for describing that the return >>>>> address/link register used by a function/frame is salted/keyed? >>>>> >>>>> This seems a good description of CIE records and augmentation >>>>> characters:http://www.airs.com/blog/archives/460 >>>>> >>>>> It obviously also involves updating all unwinders to understand the new >>>>> augmentation character (and possible arguments). But it might be more >>>>> generic and saves us from using up too many DW_OPs. >>>> >>>> From what I understood, the return address is not always scrambled, so >>>> it doesn't apply to the whole function, just to most of it (except for >>>> an insn in the prologue and some in the epilogue). So I think one op is >>>> needed. But can't it be just a toggable flag whether the return address >>>> is scrambled + some arguments to it? >>>> Thus DW_OP_AARCH64_scramble .uleb128 0 would mean that the default >>>> way of scrambling starts here (if not already active) or any kind of >>>> scrambling ends here (if already active), and >>>> DW_OP_AARCH64_scramble .uleb128 non-zero would be whatever encoding you need >>>> to represent details of the less common variants with details what to do. >>>> Then you'd just hook through some MD_* macro in the unwinder the >>>> descrambling operation if the scrambling is active at the insns you unwind >>>> on. >>>> >>>> Jakub >>> >>> Hi Mark, Jakub: >>> >>> Thanks very much for the suggestions. >>> >>> I have done some experiments on your ideas and am thinking it's good to >>> combine them together. The use of DW_CFA instead of DW_OP can avoid building >>> all information from scratch at each unwind location, while we can indicate >>> the signing key index through new AArch64 CIE augmentation 'B'. This new >>> approach reduce the unwind table size overhead from ~25% to ~5% when return >>> address signing enabled, it also largely simplified dwarf generation code for >>> return address signing. >>> >>> As one new DWARF call frame instruction is needed for AArch64, I want to reuse >>> DW_CFA_GNU_window_save to save the space. It is in vendor extension space and >>> used for Sparc only, I think it make sense to reuse it for AArch64. On >>> AArch64, DW_CFA_GNU_window_save toggle return address sign status which kept >>> in a new boolean type column in DWARF table, so DW_CFA_GNU_window_save takes >>> no argument on AArch64, the same as on Sparc, this makes no difference to those >>> existed encoding, length calculation code. >>> >>> Meanwhile one new DWARF expression operation number is still needed for >>> AArch64, it's useful for describing those complex pointer signing scenarios >>> and it will be used to multiplex some further extensions on AArch64. >>> >>> OK on this proposal and to install this patch to gcc trunk? >>> >>> Hi GDB, Binutils maintainer: >>> >>> OK on this proposal and install this patch to binutils-gdb master? >>> >>> include/ >>> 2016-11-29 Richard Earnshaw <rearnsha@arm.com> >>> Jiong Wang <jiong.wang@arm.com> >>> >>> * dwarf2.def (DW_OP_AARCH64_operation): Reserve the number 0xea. >> >> Ping~ > Ping^2 Ping^3 Can DWARF maintainers or global reviewers have a look at this? Thanks very much. -- Regards, Jiong
On 28/12/16 19:54, Cary Coutant wrote: >> OK on this proposal and to install this patch to gcc trunk? >> >> Hi GDB, Binutils maintainer: >> >> OK on this proposal and install this patch to binutils-gdb master? >> >> include/ >> 2016-11-29 Richard Earnshaw <rearnsha@arm.com> >> Jiong Wang <jiong.wang@arm.com> >> >> * dwarf2.def (DW_OP_AARCH64_operation): Reserve the number 0xea. > This is OK, but: > > +/* AARCH64 extensions. > + DW_OP_AARCH64_operation takes one mandatory unsigned LEB128 operand. > + Bits[6:0] of this operand is the action code, all others bits are > initialized > + to 0 except explicitly documented for one action. Please refer > AArch64 DWARF > + ABI documentation for details. */ > > Is it possible to include a stable URL that points to the ABI document? > > -cary As this patch has got approve from GCC, is it OK to install into binutils-gdb master? My understanding is "include/dwarf2.def" is shared between GDB and Binutils. This patch and the described AArch64 DWARF proposal has got GDB ack from https://sourceware.org/ml/gdb-patches/2016-11/msg01008.html which, from my understanding, is a conditional approval given there is follow up GDB patches on multiplexing DW_CFA_GNU_window_save. This patch still need Binutils approval. Thanks. Regards, Jiong
On Tue, Jan 3, 2017 at 3:21 PM, Nick Clifton <nickc@redhat.com> wrote: > I have now sync'ed the binutils copy with the current master copy in the gcc > repository. Thanks for doing this, Nick. -- Yao (齐尧)
diff --git a/include/dwarf2.def b/include/dwarf2.def index bb916ca238221151cf49359c25fd92643c7e60af..f3892a20da1fe13ddb419e5d7eda07f2c8d8b0c6 100644 --- a/include/dwarf2.def +++ b/include/dwarf2.def @@ -684,6 +684,12 @@ DW_OP (DW_OP_HP_unmod_range, 0xe5) DW_OP (DW_OP_HP_tls, 0xe6) /* PGI (STMicroelectronics) extensions. */ DW_OP (DW_OP_PGI_omp_thread_num, 0xf8) +/* AARCH64 extensions. + DW_OP_AARCH64_operation takes one mandatory unsigned LEB128 operand. + Bits[6:0] of this operand is the action code, all others bits are initialized + to 0 except explicitly documented for one action. Please refer AArch64 DWARF + ABI documentation for details. */ +DW_OP (DW_OP_AARCH64_operation, 0xea) DW_END_OP DW_FIRST_ATE (DW_ATE_void, 0x0) @@ -765,7 +771,8 @@ DW_CFA (DW_CFA_hi_user, 0x3f) /* SGI/MIPS specific. */ DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d) -/* GNU extensions. */ +/* GNU extensions. + NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64. */ DW_CFA (DW_CFA_GNU_window_save, 0x2d) DW_CFA (DW_CFA_GNU_args_size, 0x2e) DW_CFA (DW_CFA_GNU_negative_offset_extended, 0x2f)