[PATCH,V5 8/9] gas: aarch64: add experimental support for SCFI

Indu Bhagat indu.bhagat@oracle.com
Tue Jul 16 21:15:45 GMT 2024


On 7/16/24 9:21 AM, Richard Sandiford wrote:
> Indu Bhagat <indu.bhagat@oracle.com> writes:
>> [Changes in V5]
>>   - Use aarch64_zero_register_p () to detect zero reg.  Get rid of bool
>>     sp_allowed_p argument of ginsn_dw2_regnum ().  Also update the
>>     handling for case AARCH64_OPND_CLASS_MODIFIED_REG and return
>>     opnd->addr.base_regno.
>>   - Do not generate GINSN_TYPE_LOAD / GINSN_TYPE_STORE for memory
>>     operations tagged with subclass flag F_SUBCLASS_OTHER in both
>>     aarch64_ginsn_ldstp () and aarch64_ginsn_ldstr (). Examples of such
>>     insns include prfm, stg*, ldpsw.
>>   - Restructure code in aarch64_ginsn_ldstp () and aarch64_ginsn_ldstr ()
>>     to move the compensating action of ginsn_ind next to the creation of
>>     ginsn_ind itself. Update code comments.
>>   - Carve out two different functions for mov: aarch64_ginsn_mov_imm ()
>>     and aarch64_ginsn_mov_reg ().
>>   - Use conditionals instead of asserts to handle "b 4" and "cbz x0, 4",
>>     or even "b symbol+1" and "cbz x0, symbol+1" in
>>     aarch64_ginsn_branch_uncond () and aarch64_ginsn_branch_cond ().
>>   - Now that addg, subg are sub-classified with F_SUBCLASS_OTHER, add a
>>     comment in addsum_imm case block around the same.
>>   - In aarch64_ginsn_unhandled (), carve out the check for sve_misc into
>>     its own conditional block.
>>   - Use aarch64_opcode_subclass_p () consistently.
>>   - In aarch64_ginsn_safe_to_skip_p (), detect irg op (for whitelisting)
>>     by means of subclass to make the code more robust as in future, new
>>     ops may be added with destination Rd_SP.
>>   - Punt on W and S loads and stores, since they cannot implement a full
>>     save & restore for CFI purposes.
>>   - Include ginsn generation for ldstnapair_offs iclass.  This was
>>     ignored until now, but it seems these ops can be used for save /
>>     restore ops.
>>   - Other minor code and code comment fixes.
>> [End of changes in V5]
>>
>> [Changes in V4]
>>   - Use data types uniformly. Use 'unsigned int' instead of 'uint32_t'
>>     for DWARF register numbers.  Use offsetT where applicable.
>>   - Minor code restructuring in aarch64_ginsn_safe_to_skip_p (). Move
>>     common code out of switch case.
>>   - Add FP/Advanced SIMD registers to callee-saved registers too.
>>     Updated commit log to include some of the details.
>>   - Check for opnd type AARCH64_OPND_QLF_W or AARCH64_OPND_QLF_S_S in
>>     aarch64_ginsn_ldstp () to detect 32-bit word operations.
>>   - Skip generating ginsns for movk, movz, movn.  These do more work than
>>     just simple mov; Skip generating ginsn altogether for these.
>>   - ginsn_dw2_regnum () is now switch case on opnd_class.  Each case
>>     exposed by the current set of ginsn creation logic is handled.
>>   - Skip Z register usage altogether for now.  Skip sve_misc iclass but
>>     error out if callee-saved FP/Advanced SIMD registers or stack
>>     management are involved.
>> [End of changes in V4]
>>
>> [No changes in V3]
>>
>> [Changes in V2]
>> - Factored out the ginsn creation functionality from tc-aarch64.c into
>> tc-aarch64-ginsn.c.
>> - The switch case in aarch64_ginsn_new now is based on iclass rather
>> than (earlier) opcode.
>> - Rename aarch64_ginsn_jump / aarch64_ginsn_jump_cond to
>> aarch64_ginsn_branch_uncond / aarch64_ginsn_branch_cond respectively.
>> - Explicitly whitelist irg insn.
>> - Other minor code comment and readability fixes.
>> [End of changes in V2]
>>
>> For synthesizing CFI (SCFI) for hand-written asm, the SCFI machinery in
>> GAS works on the generic GAS insns (ginsns).  This patch adds support in
>> the aarch64 backend to create ginsns for a subset of the supported
>> machine instructions.  The subset includes the minimal necessary
>> instructions to ensure SCFI correctness:
>>
>> - Any potential register saves and unsaves.  Hence, process instructions
>>    belonging to a variety of iclasses involving str, ldr, stp, ldp.
>> - Any change of flow instructions.  This includes all conditional and
>>    unconditional branches, call (bl, blr, etc.) and return.
>> - Most importantly, any instruction that could affect the two registers
>>    of interest: REG_SP, REG_FP.  This set includes all pre-indexed and
>>    post-indexed memory operations, with writeback, on the stack.  This
>>    set must also include other instructions (e.g., arithmetic insns)
>>    where the destination register is one of the afore-mentioned registers.
>>
>> With respect to callee-saved registers in Aarch64, FP/Advanced SIMD
>> registers D8-D15 are included along with the relevant GPRs.  Calculating
>> offsets for loads and stores especially for Q registers needs special
>> attention here.
>>
>> As an example,
>>     str q8, [sp, #16]
>> On big-endian:
>>     STR Qn stores as a 128-bit integer (MSB first), hence, should record
>>     D8 as being saved at sp+24 rather than sp+16.
>> On little-endian:
>>     should record D8 as being saved at sp+16
>>
>> D8-D15 are the low 64 bits of Q8-Q15, and of Z8-Z15 if SVE is used;
>> hence, they remain "interesting" for SCFI purposes in such cases.  A CFI
>> save slot always represents the low 64 bits, regardless of whether a
>> save occurs on D, Q or Z registers.  Currently, the ginsn creation
>> machinery can handle D and Q registers on little-endian and big-endian.
>>
>> Apart from creating ginsn, another key responsibility of the backend is
>> to make sure there are safeguards in place to detect and alert if an
>> instruction of interest may have been skipped.  This is done via
>> aarch64_ginsn_unhandled () (similar to the x86 backend).  This function
>> , hence, is also intended to alert when future ISA changes may otherwise
>> render SCFI results incorrect, because of missing ginsns for the newly
>> added machine instructions.
>>
>> At this time, becuase of the complexities wrt endianness in handling Z
>> register usage, skip sve_misc opclass altogether for now.  The SCFI
>> machinery will error out (using the aarch64_ginsn_unhandled () code
>> path) though if Z register usage affects correctness.
>>
>> The current SCFI machinery does not currently synthesize the
>> PAC-related, aarch64-specific CFI directives: .cfi_b_key_frame.  The
>> support for this is planned for near future.
>>
>> SCFI is enabled for ELF targets only.
>>
>> gas/
>> 	* config/tc-aarch64-ginsn.c: New file.
>> 	* config/tc-aarch64.c (md_assemble): Include tc-aarch64-ginsn.c
>> 	file.  Invoke aarch64_ginsn_new.
>> 	* config/tc-aarch64.h (TARGET_USE_GINSN): Define for SCFI
>> 	enablement.
>> 	(TARGET_USE_SCFI): Likewise.
>> 	(SCFI_MAX_REG_ID): New definition.
>> 	(REG_FP): Likewise.
>> 	(REG_LR): Likewise.
>> 	(REG_SP): Likewise.
>> 	(SCFI_INIT_CFA_OFFSET): Likewise.
>> 	(SCFI_CALLEE_SAVED_REG_P): Likewise.
>> 	(aarch64_scfi_callee_saved_p): New declaration.
>> ---
>>   gas/config/tc-aarch64-ginsn.c | 874 ++++++++++++++++++++++++++++++++++
>>   gas/config/tc-aarch64.c       |  15 +
>>   gas/config/tc-aarch64.h       |  21 +
>>   3 files changed, 910 insertions(+)
>>   create mode 100644 gas/config/tc-aarch64-ginsn.c
>>
>> diff --git a/gas/config/tc-aarch64-ginsn.c b/gas/config/tc-aarch64-ginsn.c
>> new file mode 100644
>> index 00000000000..f6f6b2e4133
>> --- /dev/null
>> +++ b/gas/config/tc-aarch64-ginsn.c
>> @@ -0,0 +1,874 @@
>> +/* tc-aarch64-ginsn.c -- Ginsn generation for the AArch64 ISA
>> +
>> +   Copyright (C) 2024 Free Software Foundation, Inc.
>> +
>> +   This file is part of GAS.
>> +
>> +   GAS is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the license, or
>> +   (at your option) any later version.
>> +
>> +   GAS is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program; see the file COPYING3. If not,
>> +   see <http://www.gnu.org/licenses/>.  */
>> +
>> +/* This file contains the implementation of the ginsn creation for aarch64
>> +   instructions.  Most functions will read the aarch64_instruction inst
>> +   object, but none should need to modify it.  */
>> +
>> +#ifdef OBJ_ELF
>> +
>> +/* DWARF register number for R1.  Used as dummy value when WZR.  */
>> +#define GINSN_DW2_REGNUM_R1_DUMMY 1
> 
> I think we should use a more explicitly invalid number, like ~0U,
> and make callers of ginsn_dw2_regnum check for that and bail out.
> Using X1 for XZR seems dangerous, since if we do later track constants
> that feed stack adjustments, we could misinterpret daft code like:
> 
>          mov     x1, #100
>          mov     xzr, #200
>          sub     sp, sp, x1
> 

True. This should not be in the valid DWARF register number range, 
especially when supporting the above pattern is planned.

Hwoever, when we change to use a different invalid number (say, ~0U), I 
think we have another option though:  Let (~0U) appear as the regno in 
the ginsn, but make sure SCFI and other readers of ginsn work well with 
that.  ATM, gas/scfi.c is not that robust enough.  I have some patches 
in pipeline which improve the code in general in gas/scfi.c but I plan 
to post them only later.  I propose that we make this switch then ?

So, if you agree, I will make this change after the 2.43 release.

>> +
>> +/* Return whether the given register number is a callee-saved register for
>> +   SCFI purposes.
>> +
>> +   Apart from the callee-saved GPRs, SCFI always tracks SP, FP and LR
>> +   additionally.  As for the FP/Advanced SIMD registers, v8-v15 are
>> +   callee-saved.  */
>> +
>> +bool
>> +aarch64_scfi_callee_saved_p (unsigned int dw2reg_num)
>> +{
>> +  /* PS: Ensure SCFI_MAX_REG_ID is the max DWARF register number to cover
>> +     all the registers here.  */
>> +  if (dw2reg_num == REG_SP /* x31.  */
>> +      || dw2reg_num == REG_FP /* x29.  */
>> +      || dw2reg_num == REG_LR /* x30.  */
>> +      || (dw2reg_num >= 19 && dw2reg_num <= 28) /* x19 - x28.  */
>> +      || (dw2reg_num >= 72 && dw2reg_num <= 79) /* v8 - v15.  */)
>> +    return true;
>> +
>> +  return false;
>> +}
>> +
>> +/* Get the DWARF register number for the given OPND.  */
> 
> Might be worth adding something like:
> 
> /* Get the DWARF register number for the given OPND.  If OPND is an address,
>     the returned register is the base register.  If OPND spans multiple
>     registers, the returned register is the first of those registers.  */
> 

Sounds good. Added this comment.

>> +
>> +static unsigned int
>> +ginsn_dw2_regnum (aarch64_opnd_info *opnd)
>> +{
>> +  enum aarch64_operand_class opnd_class;
>> +  unsigned int dw2reg_num = 0;
>> +
>> +  opnd_class = aarch64_get_operand_class (opnd->type);
>> +
>> +  switch (opnd_class)
>> +    {
>> +    case AARCH64_OPND_CLASS_FP_REG:
>> +      dw2reg_num = opnd->reg.regno + 64;
>> +      break;
>> +    case AARCH64_OPND_CLASS_SVE_REGLIST:
>> +      dw2reg_num = opnd->reglist.first_regno + 64;
>> +      break;
>> +    case AARCH64_OPND_CLASS_MODIFIED_REG:
>> +      dw2reg_num = opnd->addr.base_regno;
>> +      break;
>> +    case AARCH64_OPND_CLASS_INT_REG:
>> +    case AARCH64_OPND_CLASS_ADDRESS:
> 
> The ADDRESS and MODIFIED_REG cases seem to be switched: ADDRESS should
> use addr.base_regno and MODIFIED_REG should use reg.regno.
> 

Oops. Corrected this.

>> +      /* Use a dummy register value in case of WZR, else this will be an
>> +	 incorrect dependency on REG_SP.  */
>> +      if (aarch64_zero_register_p (opnd))
>> +	dw2reg_num = GINSN_DW2_REGNUM_R1_DUMMY;
>> +      else
>> +	/* For GPRs of our interest (callee-saved regs, SP, FP, LR),
>> +	   DWARF register number is the same as AArch64 register number.  */
>> +	dw2reg_num = opnd->reg.regno;
>> +      break;
>> +    default:
>> +      as_bad ("Unexpected value in ginsn_dw2_regnum");
>> +      break;
>> +    }
>> +
>> +  return dw2reg_num;
>> +}
>> +
>> +/* Generate ginsn for addsub instructions with immediate opnd.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_addsub_imm (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  bool add_p, sub_p;
>> +  offsetT src_imm = 0;
>> +  unsigned int dst_reg, opnd_reg;
>> +  aarch64_opnd_info *dst, *opnd;
>> +  ginsnS *(*ginsn_func) (const symbolS *, bool,
>> +			 enum ginsn_src_type, unsigned int, offsetT,
>> +			 enum ginsn_src_type, unsigned int, offsetT,
>> +			 enum ginsn_dst_type, unsigned int, offsetT);
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  add_p = aarch64_opcode_subclass_p (opcode, F_ARITH_ADD);
>> +  sub_p = aarch64_opcode_subclass_p (opcode, F_ARITH_SUB);
>> +  gas_assert (add_p || sub_p);
>> +  ginsn_func = add_p ? ginsn_new_add : ginsn_new_sub;
>> +
>> +  gas_assert (aarch64_num_of_operands (opcode) == 3);
>> +  dst = &base->operands[0];
>> +  opnd = &base->operands[1];
>> +
>> +  dst_reg = ginsn_dw2_regnum (dst);
>> +
>> +  if (aarch64_gas_internal_fixup_p () && inst.reloc.exp.X_op == O_constant)
>> +    src_imm = inst.reloc.exp.X_add_number;
>> +  /* For any other relocation type, e.g., in add reg, reg, symbol, skip now
>> +     and handle via aarch64_ginsn_unhandled () code path.  */
>> +  else if (inst.reloc.type != BFD_RELOC_UNUSED)
>> +    return ginsn;
>> +  /* FIXME - verify the understanding and remove assert.  */
>> +  else
>> +    gas_assert (0);
>> +
>> +  opnd_reg = ginsn_dw2_regnum (opnd);
>> +
>> +  ginsn = ginsn_func (insn_end_sym, true,
>> +		      GINSN_SRC_REG, opnd_reg, 0,
>> +		      GINSN_SRC_IMM, 0, src_imm,
>> +		      GINSN_DST_REG, dst_reg, 0);
>> +  ginsn_set_where (ginsn);
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for addsub instructions with reg opnd.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_addsub_reg (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  bool add_p, sub_p;
>> +  unsigned int dst_reg, src1_reg, src2_reg;
>> +  aarch64_opnd_info *dst, *src1, *src2;
>> +  ginsnS *(*ginsn_func) (const symbolS *, bool,
>> +			 enum ginsn_src_type, unsigned int, offsetT,
>> +			 enum ginsn_src_type, unsigned int, offsetT,
>> +			 enum ginsn_dst_type, unsigned int, offsetT);
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  add_p = aarch64_opcode_subclass_p (opcode, F_ARITH_ADD);
>> +  sub_p = aarch64_opcode_subclass_p (opcode, F_ARITH_SUB);
>> +  gas_assert (add_p || sub_p);
>> +  ginsn_func = add_p ? ginsn_new_add : ginsn_new_sub;
>> +
>> +  gas_assert (aarch64_num_of_operands (opcode) == 3);
>> +  dst = &base->operands[0];
>> +  src1 = &base->operands[1];
>> +  src2 = &base->operands[2];
>> +
>> +  dst_reg = ginsn_dw2_regnum (dst);
>> +  src1_reg = ginsn_dw2_regnum (src1);
>> +  src2_reg = ginsn_dw2_regnum (src2);
>> +
>> +  /* Ignoring shift amount, if any,  does not affect SCFI correctness.
>> +     Note TBD_GINSN_INFO_LOSS.  */
> 
> Could we return null if:
> 
>    src2->shifter.kind != AARCH64_MOD_NONE
> 
> instead of ignoring?  It seems better not to create an "incorrect"
> description.  (Can keep the TBD comment at the same time.)
> 
>> +  ginsn = ginsn_func (insn_end_sym, true,
>> +		      GINSN_SRC_REG, src1_reg, 0,
>> +		      GINSN_SRC_REG, src2_reg, 0,
>> +		      GINSN_DST_REG, dst_reg, 0);
>> +  ginsn_set_where (ginsn);
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for the load pair and store pair instructions.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_ldstp (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  ginsnS *ginsn_ind = NULL;
>> +  ginsnS *ginsn_mem1 = NULL;
>> +  ginsnS *ginsn_mem2 = NULL;
>> +  unsigned int opnd_reg, addr_reg;
>> +  offsetT offset, mem_offset;
>> +  unsigned int width = 8;
>> +  bool store_p = false;
>> +  bool other_p = false;
>> +
>> +  aarch64_opnd_info *opnd1, *opnd2, *addr;
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  /* This function is for handling ldp / stp ops only.  */
>> +  gas_assert (opcode->iclass == ldstpair_indexed
>> +	      || opcode->iclass == ldstnapair_offs
>> +	      || opcode->iclass == ldstpair_off);
>> +  gas_assert (aarch64_num_of_operands (opcode) == 3);
>> +
>> +  opnd1 = &base->operands[0];
>> +  opnd2 = &base->operands[1];
>> +  addr = &base->operands[2];
>> +  store_p = aarch64_opcode_subclass_p (opcode, F_LDST_STORE);
>> +  other_p = aarch64_opcode_subclass_p (opcode, F_SUBCLASS_OTHER);
> 
> I think it'd be worth copying the approach taken for add/sub:
> have a load_p and assert that load_p, store_p or other_p are true.
> 

OK.  Added it.

>> +
>> +  addr_reg = ginsn_dw2_regnum (addr);
>> +  gas_assert (!addr->addr.offset.is_reg);
>> +  mem_offset = addr->addr.offset.imm;
>> +
>> +  offset = mem_offset;
>> +  /* Handle address calculation.  */
>> +  if ((addr->addr.preind || addr->addr.postind) && addr->addr.writeback)
>> +    {
>> +      /* Pre-indexed store, e.g., stp x29, x30, [sp, -128]!
>> +	 Pre-indexed addressing is like offset addressing, except that
>> +	 the base pointer is updated as a result of the instruction.
>> +
>> +	 Post-indexed store, e.g., stp     x29, x30, [sp],128
>> +	 Post-index addressing is useful for popping off the stack.  The
>> +	 instruction loads the value from the location pointed at by the stack
>> +	 pointer, and then moves the stack pointer on to the next full location
>> +	 in the stack.  */
>> +      ginsn_ind = ginsn_new_add (insn_end_sym, false,
>> +				 GINSN_SRC_REG, addr_reg, 0,
>> +				 GINSN_SRC_IMM, 0, mem_offset,
>> +				 GINSN_DST_REG, addr_reg, 0);
>> +      ginsn_set_where (ginsn_ind);
>> +
>> +      /* With post-index addressing, the value is loaded from the address in
>> +	 the base pointer, and then the pointer is updated.  With pre-index
>> +	 addressing, the addr computation has already been explicitly done.  */
>> +      offset = 0;
>> +    }
>> +
>> +  /* Save / restore of WZR is not of interest for SCFI.  Also, insns like ldpsw
>> +     (marked with subclass F_SUBCLASS_OTHER) do not need to generate any load
>> +     or store for SCFI purposes.  Lastly, for CFI puposes, the width of save /
>> +     restore operation has to be 8 bytes or more.  However, the address
>> +     processing component may have updated the stack pointer.  At least, emit
>> +     that ginsn and return.  Also note, TBD_GINSN_GEN_NOT_SCFI.  */
>> +  if (other_p || aarch64_zero_register_p (opnd1)
>> +      || aarch64_get_qualifier_esize (opnd1->qualifier) < 8)
>> +    return ginsn_ind;
> 
> Yeah, it feels odd that we treat, say:
> 
>          stp     xzr, xzr, [x0, #32]!
> 
> as "handled" (returning the add insn), but:
> 
>          stp     xzr, xzr, [x0, #32]
> 
> as unhandled (returning null).  But I suppose this is something that can
> be iterated on.  It doesn't need to hold up the initial commit.
> 
>> +
>> +  /* ldstp may load or store two 32-bit words or two 64-bit doublewords.  */
>> +  if (opnd1->qualifier == AARCH64_OPND_QLF_W
>> +      || opnd1->qualifier == AARCH64_OPND_QLF_S_S)
>> +    width = 4;
> 
> Is this condition ever true after the:
> 
>    aarch64_get_qualifier_esize (opnd1->qualifier) < 8
> 
> check above?
> 

No. It should be removed now.

>> +  else if (opnd1->qualifier == AARCH64_OPND_QLF_S_Q)
>> +    {
>> +      width = 16;
>> +      if (target_big_endian)
>> +	offset += 8;
>> +    }
>> +
>> +  opnd_reg = ginsn_dw2_regnum (opnd1);
>> +  if (store_p)
>> +    {
>> +      ginsn_mem1 = ginsn_new_store (insn_end_sym, false,
>> +				    GINSN_SRC_REG, opnd_reg,
>> +				    GINSN_DST_INDIRECT, addr_reg, offset);
>> +      ginsn_set_where (ginsn_mem1);
>> +
>> +      opnd_reg = ginsn_dw2_regnum (opnd2);
>> +      ginsn_mem2 = ginsn_new_store (insn_end_sym, false,
>> +				    GINSN_SRC_REG, opnd_reg,
>> +				    GINSN_DST_INDIRECT, addr_reg,
>> +				    offset + width);
>> +      ginsn_set_where (ginsn_mem2);
>> +    }
>> +  else
>> +    {
>> +      opnd_reg = ginsn_dw2_regnum (opnd1);
>> +      ginsn_mem1 = ginsn_new_load (insn_end_sym, false,
>> +				   GINSN_SRC_INDIRECT, addr_reg, offset,
>> +				   GINSN_DST_REG, opnd_reg);
>> +      ginsn_set_where (ginsn_mem1);
>> +
>> +      opnd_reg = ginsn_dw2_regnum (opnd2);
>> +      ginsn_mem2 = ginsn_new_load (insn_end_sym, false,
>> +				   GINSN_SRC_INDIRECT, addr_reg, offset + width,
>> +				   GINSN_DST_REG, opnd_reg);
>> +      ginsn_set_where (ginsn_mem2);
>> +    }
>> +
>> +  /* Link the list of ginsns created.  */
>> +  if (addr->addr.preind && addr->addr.writeback)
>> +    gas_assert (!ginsn_link_next (ginsn_ind, ginsn_mem1));
>> +
>> +  gas_assert (!ginsn_link_next (ginsn_mem1, ginsn_mem2));
>> +
>> +  if (addr->addr.postind && addr->addr.writeback)
>> +    gas_assert (!ginsn_link_next (ginsn_mem2, ginsn_ind));
>> +
>> +  /* Make note of the first instruction in the list.  */
>> +  ginsn = (addr->addr.preind && addr->addr.writeback) ? ginsn_ind : ginsn_mem1;
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for load and store instructions.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_ldstr (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  ginsnS *ginsn_ind = NULL;
>> +  ginsnS *ginsn_mem = NULL;
>> +  unsigned int opnd_reg, addr_reg;
>> +  offsetT offset, mem_offset;
>> +  bool store_p = false;
>> +  bool other_p = false;
>> +
>> +  aarch64_opnd_info *opnd1, *addr;
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  /* This function is for handling ldr, str ops only.  */
>> +  gas_assert (opcode->iclass == ldst_imm9 || opcode->iclass == ldst_pos);
>> +  gas_assert (aarch64_num_of_operands (opcode) == 2);
>> +
>> +  opnd1 = &base->operands[0];
>> +  addr = &base->operands[1];
>> +  store_p = aarch64_opcode_subclass_p (opcode, F_LDST_STORE);
>> +  other_p = aarch64_opcode_subclass_p (opcode, F_SUBCLASS_OTHER);
> 
> Same comment about load_p here as above.
> 

Okay.

>> +
>> +  addr_reg = ginsn_dw2_regnum (addr);
>> +
>> +  if (aarch64_gas_internal_fixup_p () && inst.reloc.exp.X_op == O_constant)
>> +    mem_offset = inst.reloc.exp.X_add_number;
>> +  else
>> +    {
>> +      gas_assert (!addr->addr.offset.is_reg);
>> +      mem_offset = addr->addr.offset.imm;
>> +    }
>> +
>> +  offset = mem_offset;
>> +  /* Handle address calculation.  */
>> +  if ((addr->addr.preind || addr->addr.postind) && addr->addr.writeback)
>> +    {
>> +      ginsn_ind = ginsn_new_add (insn_end_sym, false,
>> +				 GINSN_SRC_REG, addr_reg, 0,
>> +				 GINSN_SRC_IMM, 0, mem_offset,
>> +				 GINSN_DST_REG, addr_reg, 0);
>> +      ginsn_set_where (ginsn_ind);
>> +
>> +      /* With post-index addressing, the value is loaded from the address in
>> +	 the base pointer, and then the pointer is updated.  With pre-index
>> +	 addressing, the addr computation has already been explicitly done.  */
>> +      offset = 0;
>> +    }
>> +
>> +  /* Save / restore of WZR is not of interest for SCFI.  Also insns like
>> +     stg, prfm, ldrsw etc. (marked with subclass F_SUBCLASS_OTHER) do not need
>> +     to generate any load / store ginsns for SCFI purposes.  Lastly, for CFI
>> +     puposes, the width of save / restore operation has to be 8 bytes or more.
>> +     That said, the address processing component may have updated the stack
>> +     pointer.  At least, emit that ginsn and return.
>> +     Also note, TBD_GINSN_GEN_NOT_SCFI.  */
>> +  if (other_p || aarch64_zero_register_p (opnd1)
>> +      || aarch64_get_qualifier_esize (opnd1->qualifier) < 8)
>> +    return ginsn_ind;
>> +
>> +  if (target_big_endian && opnd1->qualifier == AARCH64_OPND_QLF_S_Q)
>> +    offset += 8;
>> +
>> +  /* STR <Xt>, [<Xn|SP>, (<Wm>|<Xm>){, <extend> {<amount>}}].
> 
> AIUI, the function only handles immediate offsets.
> 

I should update the comment and remove that line.

>> +     LDR <Xt>, [<Xn|SP>], #<simm>.  */
>> +  opnd_reg = ginsn_dw2_regnum (opnd1);
>> +
>> +  if (store_p)
>> +    ginsn_mem = ginsn_new_store (insn_end_sym, false,
>> +				 GINSN_SRC_REG, opnd_reg,
>> +				 GINSN_DST_INDIRECT, addr_reg, offset);
>> +  else
>> +    ginsn_mem = ginsn_new_load (insn_end_sym, false,
>> +				GINSN_SRC_INDIRECT, addr_reg, offset,
>> +				GINSN_DST_REG, opnd_reg);
>> +  ginsn_set_where (ginsn_mem);
>> +
>> +  if (addr->addr.preind && addr->addr.writeback)
>> +    gas_assert (!ginsn_link_next (ginsn_ind, ginsn_mem));
>> +  else if (addr->addr.postind && addr->addr.writeback)
>> +    gas_assert (!ginsn_link_next (ginsn_mem, ginsn_ind));
>> +
>> +  /* Make note of the first instruction in the list.  */
>> +  ginsn = (addr->addr.preind && addr->addr.writeback) ? ginsn_ind : ginsn_mem;
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for unconditional branch instructions.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_branch_uncond (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  const symbolS *src_symbol = NULL;
>> +  enum ginsn_src_type src_type = GINSN_SRC_UNKNOWN;
>> +  unsigned int src_reg = 0;
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  if (opcode->iclass == branch_imm
>> +      && (inst.reloc.type == BFD_RELOC_AARCH64_CALL26
>> +	  || inst.reloc.type == BFD_RELOC_AARCH64_JUMP26))
>> +    {
>> +      if (inst.reloc.exp.X_add_number)
>> +	{
>> +	  /* A non-zero addend in b/bl target makes control-flow tracking
>> +	     difficult.  Skip SCFI for now.  */
>> +	  as_bad (_("SCFI: %#x op with non-zero addend to sym not supported"),
>> +		  opcode->opcode);
>> +	  return ginsn;
>> +	}
>> +      /* b or bl.  */
>> +      src_symbol = inst.reloc.exp.X_add_symbol;
>> +      src_type = GINSN_SRC_SYMBOL;
>> +    }
>> +  else if (opcode->iclass == branch_reg
>> +	   && aarch64_num_of_operands (opcode) >= 1)
>> +    {
>> +      /* Some insns (e.g., braa, blraa etc.) may have > 1 operands.  For
>> +	 current SCFI implementation, it suffices however to simply pass
>> +	 the information about the first source.  Although, strictly speaking,
>> +	 (if reg) the source info is currently of no material use either.  */
>> +      src_type = GINSN_SRC_REG;
>> +      src_reg = ginsn_dw2_regnum (&base->operands[0]);
>> +    }
>> +  else
>> +    /* Skip insns like branch imm.  */
>> +    return ginsn;
>> +
>> +  if (aarch64_opcode_subclass_p (opcode, F_BRANCH_CALL))
>> +    {
>> +      gas_assert (src_type != GINSN_SRC_UNKNOWN);
>> +      ginsn = ginsn_new_call (insn_end_sym, true,
>> +			      src_type, src_reg, src_symbol);
>> +    }
>> +  else if (aarch64_opcode_subclass_p (opcode, F_BRANCH_RET))
>> +    /* TBD_GINSN_REPRESENTATION_LIMIT.  The following function to create a
>> +       GINSN_TYPE_RETURN does not allow src info ATM.  */
>> +    ginsn = ginsn_new_return (insn_end_sym, true);
>> +  else
>> +    ginsn = ginsn_new_jump (insn_end_sym, true,
>> +			    src_type, src_reg, src_symbol);
>> +
>> +  ginsn_set_where (ginsn);
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for conditional branch instructions.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_branch_cond (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  const symbolS *src_symbol;
>> +  enum ginsn_src_type src_type;
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  if (inst.reloc.type == BFD_RELOC_AARCH64_BRANCH19
>> +      || inst.reloc.type == BFD_RELOC_AARCH64_TSTBR14)
>> +    {
>> +      if (inst.reloc.exp.X_add_number)
>> +	{
>> +	  /* A non-zero addend in target makes control-flow tracking
>> +	     difficult.  Skip SCFI for now.  */
>> +	  as_bad (_("SCFI: %#x op with non-zero addend to sym not supported"),
>> +		  opcode->opcode);
>> +	  return ginsn;
>> +	}
>> +
>> +      src_symbol = inst.reloc.exp.X_add_symbol;
>> +      src_type = GINSN_SRC_SYMBOL;
>> +
>> +      ginsn = ginsn_new_jump_cond (insn_end_sym, true, src_type, 0, src_symbol);
>> +      ginsn_set_where (ginsn);
>> +    }
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for mov instructions with reg opnd.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_mov_reg (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  unsigned int src_reg = 0, dst_reg;
>> +  aarch64_opnd_info *src, *dst;
>> +  offsetT src_imm = 0;
>> +  enum ginsn_src_type src_type;
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  gas_assert (aarch64_num_of_operands (opcode) == 2);
>> +
>> +  dst = &base->operands[0];
>> +  src = &base->operands[1];
>> +
>> +  dst_reg = ginsn_dw2_regnum (dst);
>> +  src_reg = ginsn_dw2_regnum (src);
>> +  src_type = GINSN_SRC_REG;
>> +
>> +  ginsn = ginsn_new_mov (insn_end_sym, false,
>> +			 src_type, src_reg, src_imm,
>> +			 GINSN_DST_REG, dst_reg, 0);
>> +  ginsn_set_where (ginsn);
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Generate ginsn for mov instructions with imm opnd.  */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_mov_imm (const symbolS *insn_end_sym)
>> +{
>> +  ginsnS *ginsn = NULL;
>> +  unsigned int src_reg = 0, dst_reg;
>> +  aarch64_opnd_info *src, *dst;
>> +  offsetT src_imm = 0;
>> +  enum ginsn_src_type src_type;
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  gas_assert (aarch64_num_of_operands (opcode) == 2);
>> +
>> +  dst = &base->operands[0];
>> +  src = &base->operands[1];
>> +
>> +  dst_reg = ginsn_dw2_regnum (dst);
>> +
>> +  /* For some mov ops, e.g., movn, movk, or movz, there may optionally be more
>> +     work than just a simple mov.  Skip handling these mov altogether and let
>> +     the aarch64_ginsn_unhandled () alert if these insns affect SCFI
>> +     correctness.  TBD_GINSN_GEN_NOT_SCFI.  */
>> +  if (src->type == AARCH64_OPND_HALF)
>> +    return ginsn;
>> +
>> +  if (src->type == AARCH64_OPND_IMM_MOV
>> +      && aarch64_gas_internal_fixup_p () && inst.reloc.exp.X_op == O_constant)
>> +    {
>> +      src_imm = inst.reloc.exp.X_add_number;
>> +      src_type = GINSN_SRC_IMM;
>> +    }
>> +  else
>> +    /* Skip now and handle via aarch64_ginsn_unhandled () code path.  */
>> +    return ginsn;
>> +
>> +  ginsn = ginsn_new_mov (insn_end_sym, false,
>> +			 src_type, src_reg, src_imm,
>> +			 GINSN_DST_REG, dst_reg, 0);
>> +  ginsn_set_where (ginsn);
>> +
>> +  return ginsn;
>> +}
>> +
>> +/* Check if an instruction is whitelisted.
>> +
>> +   An instruction is a candidate for whitelisting if not generating ginsn for
>> +   it, does not affect SCFI correctness.
>> +
>> +   TBD_GINSN_GEN_NOT_SCFI.  This function assumes GINSN_GEN_SCFI is in effect.
>> +   When other ginsn_gen_mode are added, this will need fixing.  */
>> +
>> +static bool
>> +aarch64_ginsn_safe_to_skip_p (void)
>> +{
>> +  bool skip_p = false;
>> +  aarch64_opnd_info *opnd = NULL;
>> +  unsigned int opnd_reg;
>> +  int num_opnds = 0;
>> +  bool dp_tag_only_p = false;
>> +
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  /* ATM, whitelisting operations with no operands does not seem to be
>> +     necessary.  In fact, whitelisting insns like ERET will be dangerous for
>> +     SCFI.  So, return false now and bar any such insns from being whitelisted
>> +     altogether.  */
>> +  num_opnds = aarch64_num_of_operands (opcode);
>> +  if (!num_opnds)
>> +    return false;
>> +
>> +  opnd = &base->operands[0];
>> +
>> +  switch (opcode->iclass)
>> +    {
>> +    case ldst_regoff:
>> +      /* It is not expected to have reg offset based ld/st ops to be used
>> +	 for reg save and restore operations.  Warn the user though.  */
>> +      opnd_reg = ginsn_dw2_regnum (opnd);
>> +      if (aarch64_scfi_callee_saved_p (opnd_reg))
>> +	{
>> +	  skip_p = true;
>> +	  as_warn ("SCFI: ignored probable save/restore op with reg offset");
>> +	}
>> +      break;
>> +
>> +    case dp_2src:
>> +      /* irg insn needs to be explicitly whitelisted.  This is because the
>> +	 dest is Rd_SP, but irg insn affects the tag only.  To detect irg
>> +	 insn, avoid an opcode-based check, however.  */
>> +      dp_tag_only_p = aarch64_opcode_subclass_p (opcode, F_DP_TAG_ONLY);
>> +      if (dp_tag_only_p)
>> +	skip_p = true;
>> +      break;
>> +
>> +    default:
>> +      break;
>> +    }
>> +
>> +  return skip_p;
>> +}
>> +
>> +#define AARCH64_GINSN_UNHANDLED_NONE        0
>> +#define AARCH64_GINSN_UNHANDLED_DEST_REG    1
>> +#define AARCH64_GINSN_UNHANDLED_CFG         2
>> +#define AARCH64_GINSN_UNHANDLED_STACKOP     3
>> +#define AARCH64_GINSN_UNHANDLED_UNEXPECTED  4
> 
> Could this be an enum instead?
> 

Yes.  Swtiched to enum for V6.

>> +
>> +/* Check the input insn for its impact on the correctness of the synthesized
>> +   CFI.  Returns an error code to the caller.  */
>> +
>> +static int
>> +aarch64_ginsn_unhandled (void)
>> +{
>> +  int err = AARCH64_GINSN_UNHANDLED_NONE;
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +  aarch64_opnd_info *dest = &base->operands[0];
>> +  int num_opnds = aarch64_num_of_operands (opcode);
>> +  aarch64_opnd_info *addr;
>> +  unsigned int dw2_regnum;
>> +  unsigned int addr_reg;
>> +  aarch64_opnd_info *opnd = NULL;
>> +  unsigned int opnd_reg;
>> +
>> +  /* All change of flow instructions (COFI) are important for SCFI.
>> +     N.B. New iclasses for COFI when defined must be added here too.  */
>> +  if (opcode->iclass == condbranch
>> +      || opcode->iclass == compbranch
>> +      || opcode->iclass == testbranch
>> +      || opcode->iclass == branch_imm
>> +      || opcode->iclass == branch_reg)
>> +    err = AARCH64_GINSN_UNHANDLED_CFG;
>> +  /* Also, any memory instructions that may involve an update to the stack
>> +     pointer or save/restore of callee-saved registers must not be skipped.
>> +     Note that, some iclasses cannot be used to push or pop stack because of
>> +     disallowed writeback: ldst_unscaled, ldst_regoff, ldst_unpriv, ldstexcl,
>> +     loadlit, ldstnapair_offs.  Except ldstnapair_offs from the afore-mentioned
>> +     list, these iclasses do not seem to be amenable to being used for
>> +     save/restore ops either.  */
>> +  else if (opcode->iclass == ldstpair_off
>> +	   || opcode->iclass == ldstnapair_offs
>> +	   || opcode->iclass == ldstpair_indexed
>> +	   || opcode->iclass == ldst_imm9
>> +	   || opcode->iclass == ldst_imm10
>> +	   || opcode->iclass == ldst_pos)
>> +    {
>> +      addr = &base->operands[num_opnds - 1];
>> +      addr_reg = ginsn_dw2_regnum (addr);
>> +      if (addr_reg == REG_SP || addr_reg == REG_FP)
>> +	{
>> +	  /* For all skipped memory operations, check if an update to REG_SP or
>> +	     REG_FP is involved.  */
>> +	  if ((addr->addr.postind || addr->addr.preind) && addr->addr.writeback)
>> +	    err = AARCH64_GINSN_UNHANDLED_STACKOP;
>> +	  /* Also check if a save / restore of a callee-saved register has been
>> +	     missed.  */
>> +	  else if (!aarch64_opcode_subclass_p (opcode, F_SUBCLASS_OTHER))
> 
> Why's it correct to skip F_SUBCLASS_OTHER opcodes?
> 

Only F_SUBLASS_OTHER of the load/store iclass are being skipped.

Load / store operations marked with F_SUBCLASS_OTHER are not viable 
candidates for register save and restore.

If no ginsn is created far for a load/store op:
   - first we check if there is any writeback involved to REG_SP / REG_FP.
   - next, check if these instructions cannot be used for register save 
/ restore operations, its safe to not signal error.

>> +	    {
>> +	      opnd = &base->operands[0];
>> +	      opnd_reg = ginsn_dw2_regnum (opnd);
>> +	      if (aarch64_scfi_callee_saved_p (opnd_reg)
>> +		  && aarch64_get_qualifier_esize (opnd->qualifier) >= 8)
>> +		err = AARCH64_GINSN_UNHANDLED_STACKOP;
>> +	    }
>> +	}
>> +    }
>> +  /* STR Zn are especially complicated as they do not store in the same byte
>> +     order for big-endian: STR Qn stores as a 128-bit integer (MSB first),
>> +     whereas STR Zn stores as a stream of bytes (LSB first).  FIXME Simply punt
>> +     on the big-endian and little-endian SVE PCS case for now.  */
>> +  else if (opcode->iclass == sve_misc)
>> +    {
>> +      opnd = &base->operands[0];
>> +      addr = &base->operands[num_opnds - 1];
>> +      addr_reg = ginsn_dw2_regnum (addr);
>> +      opnd_reg = ginsn_dw2_regnum (opnd);
>> +      /* For all skipped memory operations, check if an update to REG_SP or
>> +	 REG_FP is involved.  */
>> +      if ((addr_reg == REG_SP || addr_reg == REG_FP)
> 
> I think this should check the type of addr as well, since sve_misc is
> used for more than just loads and stores.
> 

Okay. I will add a (aarch64_get_operand_class (addr->type) == 
AARCH64_OPND_CLASS_ADDRESS)

>> +	  && (((addr->addr.postind || addr->addr.preind) && addr->addr.writeback)
>> +	      || aarch64_scfi_callee_saved_p (opnd_reg)))
>> +	err = AARCH64_GINSN_UNHANDLED_STACKOP;
>> +    }
>> +
>> +  /* Finally, irrespective of the iclass, check if the missed instructions are
>> +     affecting REG_SP or REG_FP.  */
>> +  else if (dest && (dest->type == AARCH64_OPND_Rd
>> +		    || dest->type == AARCH64_OPND_Rd_SP))
>> +    {
>> +      dw2_regnum = ginsn_dw2_regnum (dest);
>> +
>> +      if (dw2_regnum == REG_SP || dw2_regnum == REG_FP)
>> +	err = AARCH64_GINSN_UNHANDLED_DEST_REG;
>> +    }
>> +
>> +  return err;
>> +}
>> +
>> +/* Generate one or more generic GAS instructions, a.k.a, ginsns for the
>> +   current machine instruction.
>> +
>> +   Returns the head of linked list of ginsn(s) added, if success; Returns NULL
>> +   if failure.
>> +
>> +   The input ginsn_gen_mode GMODE determines the set of minimal necessary
>> +   ginsns necessary for correctness of any passes applicable for that mode.
>> +   For supporting the GINSN_GEN_SCFI generation mode, following is the list of
>> +   machine instructions that must be translated into the corresponding ginsns
>> +   to ensure correctness of SCFI:
>> +     - All instructions affecting the two registers that could potentially
>> +       be used as the base register for CFA tracking.  For SCFI, the base
>> +       register for CFA tracking is limited to REG_SP and REG_FP only.
>> +     - All change of flow instructions: conditional and unconditional
>> +       branches, call and return from functions.
>> +     - All instructions that can potentially be a register save / restore
>> +       operations.
>> +     - All instructions that may update the stack pointer: pre-indexed and
>> +     post-indexed stack operations with writeback.
> 
> Nit: the indentation of this line doesn't seem to match the others.
> 

Done.

Thanks for reviewing.

> Thanks,
> Richard
> 
>> +
>> +   The function currently supports GINSN_GEN_SCFI ginsn generation mode only.
>> +   To support other generation modes will require work on this target-specific
>> +   process of creation of ginsns:
>> +     - Some of such places are tagged with TBD_GINSN_GEN_NOT_SCFI to serve as
>> +       possible starting points.
>> +     - Also note that ginsn representation may need enhancements.  Specifically,
>> +       note some TBD_GINSN_INFO_LOSS and TBD_GINSN_REPRESENTATION_LIMIT markers.
>> +   */
>> +
>> +static ginsnS *
>> +aarch64_ginsn_new (const symbolS *insn_end_sym, enum ginsn_gen_mode gmode)
>> +{
>> +  int err = 0;
>> +  ginsnS *ginsn = NULL;
>> +  unsigned int dw2_regnum;
>> +  aarch64_opnd_info *dest = NULL;
>> +  aarch64_inst *base = &inst.base;
>> +  const aarch64_opcode *opcode = base->opcode;
>> +
>> +  /* Currently supports generation of selected ginsns, sufficient for
>> +     the use-case of SCFI only.  To remove this condition will require
>> +     work on this target-specific process of creation of ginsns.  Some
>> +     of such places are tagged with TBD_GINSN_GEN_NOT_SCFI to serve as
>> +     examples.  */
>> +  if (gmode != GINSN_GEN_SCFI)
>> +    return ginsn;
>> +
>> +  switch (opcode->iclass)
>> +    {
>> +    case addsub_ext:
>> +      /* TBD_GINSN_GEN_NOT_SCFI: other insns are not of interest for SCFI.  */
>> +      if (aarch64_opcode_subclass_p (opcode, F_ARITH_ADD)
>> +	   || aarch64_opcode_subclass_p (opcode, F_ARITH_SUB))
>> +	ginsn = aarch64_ginsn_addsub_reg (insn_end_sym);
>> +      break;
>> +
>> +    case addsub_imm:
>> +      if (aarch64_opcode_subclass_p (opcode, F_ARITH_MOV))
>> +	ginsn = aarch64_ginsn_mov_reg (insn_end_sym);
>> +      else if (aarch64_opcode_subclass_p (opcode, F_ARITH_ADD)
>> +	       || aarch64_opcode_subclass_p (opcode, F_ARITH_SUB))
>> +	ginsn = aarch64_ginsn_addsub_imm (insn_end_sym);
>> +      /* Note how addg, subg involving tags have F_SUBCLASS_OTHER flag.  These
>> +	 insns will see a GINSN_TYPE_OTHER created for them if the destination
>> +	 register is of interest via the aarch64_ginsn_unhandled ()
>> +	 codepath.  */
>> +      break;
>> +
>> +    case movewide:
>> +      ginsn = aarch64_ginsn_mov_imm (insn_end_sym);
>> +      break;
>> +
>> +    case ldst_imm9:
>> +    case ldst_pos:
>> +      ginsn = aarch64_ginsn_ldstr (insn_end_sym);
>> +      break;
>> +
>> +    case ldstpair_indexed:
>> +    case ldstpair_off:
>> +    case ldstnapair_offs:
>> +      ginsn = aarch64_ginsn_ldstp (insn_end_sym);
>> +      break;
>> +
>> +    case branch_imm:
>> +    case branch_reg:
>> +      ginsn = aarch64_ginsn_branch_uncond (insn_end_sym);
>> +      break;
>> +
>> +    case compbranch:
>> +      /* Although cbz/cbnz has an additional operand and are functionally
>> +	 distinct from conditional branches, it is fine to use the same ginsn
>> +	 type for both from the perspective of SCFI.  */
>> +    case testbranch:
>> +    case condbranch:
>> +      ginsn = aarch64_ginsn_branch_cond (insn_end_sym);
>> +      break;
>> +
>> +    default:
>> +      /* TBD_GINSN_GEN_NOT_SCFI: Skip all other opcodes uninteresting for
>> +	 GINSN_GEN_SCFI mode.  */
>> +      break;
>> +    }
>> +
>> +  if (!ginsn && !aarch64_ginsn_safe_to_skip_p ())
>> +    {
>> +      /* For all unhandled insns, check that they no not impact SCFI
>> +	 correctness.  */
>> +      err = aarch64_ginsn_unhandled ();
>> +      switch (err)
>> +	{
>> +	case AARCH64_GINSN_UNHANDLED_NONE:
>> +	  break;
>> +	case AARCH64_GINSN_UNHANDLED_DEST_REG:
>> +	  /* Not all writes to REG_FP are harmful in context of SCFI.  Simply
>> +	     generate a GINSN_TYPE_OTHER with destination set to the
>> +	     appropriate register.  The SCFI machinery will bail out if this
>> +	     ginsn affects SCFI correctness.  */
>> +	  dest = &base->operands[0];
>> +	  dw2_regnum = ginsn_dw2_regnum (dest);
>> +	  ginsn = ginsn_new_other (insn_end_sym, true,
>> +				   GINSN_SRC_IMM, 0,
>> +				   GINSN_SRC_IMM, 0,
>> +				   GINSN_DST_REG, dw2_regnum);
>> +	  ginsn_set_where (ginsn);
>> +	  break;
>> +	case AARCH64_GINSN_UNHANDLED_CFG:
>> +	case AARCH64_GINSN_UNHANDLED_STACKOP:
>> +	  as_bad (_("SCFI: unhandled op %#x may cause incorrect CFI"),
>> +		  opcode->opcode);
>> +	  break;
>> +	case AARCH64_GINSN_UNHANDLED_UNEXPECTED:
>> +	  as_bad (_("SCFI: unexpected op %#x may cause incorrect CFI"),
>> +		  opcode->opcode);
>> +	  break;
>> +	default:
>> +	  abort ();
>> +	  break;
>> +	}
>> +    }
>> +
>> +  return ginsn;
>> +}
>> +
>> +#endif /* OBJ_ELF.  */
>> +
>> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
>> index ed5ab762bc9..1c60595f941 100644
>> --- a/gas/config/tc-aarch64.c
>> +++ b/gas/config/tc-aarch64.c
>> @@ -33,6 +33,7 @@
>>   #include "dw2gencfi.h"
>>   #include "sframe.h"
>>   #include "gen-sframe.h"
>> +#include "scfi.h"
>>   #endif
>>   
>>   #include "dw2gencfi.h"
>> @@ -8613,6 +8614,10 @@ dump_opcode_operands (const aarch64_opcode *opcode)
>>   }
>>   #endif /* DEBUG_AARCH64 */
>>   
>> +#ifdef OBJ_ELF
>> +# include "tc-aarch64-ginsn.c"
>> +#endif
>> +
>>   /* This is the guts of the machine-dependent assembler.  STR points to a
>>      machine dependent instruction.  This function is supposed to emit
>>      the frags/bytes it assembles to.  */
>> @@ -8750,6 +8755,16 @@ md_assemble (char *str)
>>   	      output_inst (copy);
>>   	    }
>>   
>> +#ifdef OBJ_ELF
>> +	  if (flag_synth_cfi)
>> +	    {
>> +	      ginsnS *ginsn;
>> +	      ginsn = aarch64_ginsn_new (symbol_temp_new_now (),
>> +					 frch_ginsn_gen_mode ());
>> +	      frch_ginsn_data_append (ginsn);
>> +	    }
>> +#endif
>> +
>>   	  /* Issue non-fatal messages if any.  */
>>   	  output_operand_error_report (str, true);
>>   	  return;
>> diff --git a/gas/config/tc-aarch64.h b/gas/config/tc-aarch64.h
>> index 0063e85a7f1..15e22436bf7 100644
>> --- a/gas/config/tc-aarch64.h
>> +++ b/gas/config/tc-aarch64.h
>> @@ -263,6 +263,27 @@ extern void aarch64_after_parse_args (void);
>>   
>>   #ifdef OBJ_ELF
>>   
>> +#define TARGET_USE_GINSN 1
>> +/* Allow GAS to synthesize DWARF CFI for hand-written asm.
>> +   PS: TARGET_USE_CFIPOP is a pre-condition.  */
>> +#define TARGET_USE_SCFI 1
>> +/* Identify the maximum DWARF register number of all the registers being
>> +   tracked for SCFI.  This is the last DWARF register number of the set
>> +   of SP, FP, and all callee-saved registers.  For Aarch64, this means 79
>> +   because FP/Advanced SIMD v8-v15 are also callee-saved registers.  */
>> +# define SCFI_MAX_REG_ID 79
>> +/* Identify the DWARF register number of the frame-pointer register.  */
>> +# define REG_FP 29
>> +/* Identify the DWARF register number of the link register.  */
>> +# define REG_LR 30
>> +/* Identify the DWARF register number of the stack-pointer register.  */
>> +# define REG_SP 31
>> +
>> +#define SCFI_INIT_CFA_OFFSET 0
>> +
>> +#define SCFI_CALLEE_SAVED_REG_P(dw2reg)  aarch64_scfi_callee_saved_p (dw2reg)
>> +extern bool aarch64_scfi_callee_saved_p (uint32_t dw2reg_num);
>> +
>>   /* Whether SFrame stack trace info is supported.  */
>>   extern bool aarch64_support_sframe_p (void);
>>   #define support_sframe_p aarch64_support_sframe_p



More information about the Binutils mailing list