[V3,00/13] Synthesize CFI for hand-written asm

Message ID 20231211060359.3561062-1-indu.bhagat@oracle.com
Headers
Series Synthesize CFI for hand-written asm |

Message

Indu Bhagat Dec. 11, 2023, 6:03 a.m. UTC
  Hello,

This patch series adds support in GAS to synthesize CFI for hand-written
asm, acronym'd as SCFI.

Previous postings:
  - RFC patch series (https://sourceware.org/pipermail/binutils/2023-September/129560.html).
  - V1 (https://sourceware.org/pipermail/binutils/2023-October/130163.html)
  - V2 (https://sourceware.org/pipermail/binutils/2023-October/130210.html)

I have addressed many of the review comments and created the V3 series here.

Each patch in this series has notes on how it has changed from the V2
posting, but here is a brief summary:
  - x86 translation to ginsn has better handling of existing and new
    opcodes.
  - Added more testcases to the testsuite.
  - Code comments and coding style/structure improvements; bug fixes.
  - Three minor additional patches as compared to V2:
    + gas: dw2gencfi: expose dot_cfi_sections for scfidw2gen
    + gas: dw2gencfi: externalize the all_cfi_sections
    + i386-reg.tbl: Add a comment to reflect dependency on ordering

Motivation for the patch series is to alleviate users from manually
adding the CFI directives in hand-written asm. Manually adding CFI
annotations to user input asm needs additional expertise; human-errors
are possible and indeed occur more often than one may like.  These
errors, if present at the time of virtual stack unwind, lead to
unfavorable outcomes: incorrect stacktraces, program state corruption
etc.

For synthesis of CFI to work, the user input must adhere to the ABI and
the appropriate calling conventions, as GAS derives the set of
callee-saved registers based on that contract.  The SCFI implementation
is based on some heuristics/rules, please see the patch "gas: synthesize
CFI for hand-written asm" for more details.  Further, at the moment,
SCFI does not help in cases when the control flow graph of the input asm
cannot be generated (e.g., in case of indirect jumps, jump tables).

Thanks,
Indu Bhagat (13):
  gas: dw2gencfi: minor rejig for cfi_sections_set and all_cfi_sections
  gas: dw2gencfi: use all_cfi_sections instead of cfi_sections
  gas: dw2gencfi: expose a new cfi_set_last_fde API
  gas: dw2gencfi: move some tc_* defines to the header file
  gas: dw2gencfi: expose dot_cfi_sections for scfidw2gen
  gas: dw2gencfi: externalize the all_cfi_sections
  gas: add new command line option --scfi[=all,none]
  gas: scfidw2gen: new functionality to prepare for SCFI
  gas: synthesize CFI for hand-written asm
  gas: doc: update documentation for the new listing option
  i386-reg.tbl: Add a comment to reflect dependency on ordering
  gas: testsuite: add a x86_64 testsuite for SCFI
  gas/NEWS: announce the new command line option

 gas/Makefile.am                               |    6 +
 gas/Makefile.in                               |   18 +-
 gas/NEWS                                      |    2 +
 gas/as.c                                      |   27 +-
 gas/as.h                                      |    8 +
 gas/config/obj-elf.c                          |   16 +
 gas/config/tc-i386.c                          |  966 +++++++++++++
 gas/config/tc-i386.h                          |   21 +
 gas/doc/as.texi                               |   28 +-
 gas/dw2gencfi.c                               |   45 +-
 gas/dw2gencfi.h                               |   20 +
 gas/ginsn.c                                   | 1227 +++++++++++++++++
 gas/ginsn.h                                   |  382 +++++
 gas/listing.h                                 |    1 +
 gas/read.c                                    |   23 +-
 gas/scfi.c                                    | 1218 ++++++++++++++++
 gas/scfi.h                                    |   38 +
 gas/scfidw2gen.c                              |  272 ++++
 gas/scfidw2gen.h                              |   35 +
 gas/subsegs.c                                 |    1 +
 gas/subsegs.h                                 |    2 +
 gas/symbols.c                                 |    3 +
 gas/testsuite/gas/scfi/README                 |   17 +
 .../gas/scfi/x86_64/ginsn-dw2-regnum-1.l      |   55 +
 .../gas/scfi/x86_64/ginsn-dw2-regnum-1.s      |   28 +
 gas/testsuite/gas/scfi/x86_64/ginsn-pop-1.l   |   30 +
 gas/testsuite/gas/scfi/x86_64/ginsn-pop-1.s   |   13 +
 gas/testsuite/gas/scfi/x86_64/ginsn-push-1.l  |   33 +
 gas/testsuite/gas/scfi/x86_64/ginsn-push-1.s  |   14 +
 gas/testsuite/gas/scfi/x86_64/scfi-add-1.d    |   26 +
 gas/testsuite/gas/scfi/x86_64/scfi-add-1.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-add-1.s    |   13 +
 gas/testsuite/gas/scfi/x86_64/scfi-add-2.d    |   37 +
 gas/testsuite/gas/scfi/x86_64/scfi-add-2.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-add-2.s    |   48 +
 .../gas/scfi/x86_64/scfi-asm-marker-1.d       |   29 +
 .../gas/scfi/x86_64/scfi-asm-marker-1.l       |    3 +
 .../gas/scfi/x86_64/scfi-asm-marker-1.s       |   27 +
 .../gas/scfi/x86_64/scfi-asm-marker-2.d       |   25 +
 .../gas/scfi/x86_64/scfi-asm-marker-2.l       |    3 +
 .../gas/scfi/x86_64/scfi-asm-marker-2.s       |   11 +
 .../gas/scfi/x86_64/scfi-asm-marker-3.d       |   32 +
 .../gas/scfi/x86_64/scfi-asm-marker-3.l       |    2 +
 .../gas/scfi/x86_64/scfi-asm-marker-3.s       |   38 +
 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-1.d  |   32 +
 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-1.l  |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-1.s  |   21 +
 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.d  |   58 +
 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.l  |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.s  |   52 +
 .../gas/scfi/x86_64/scfi-callee-saved-1.d     |   41 +
 .../gas/scfi/x86_64/scfi-callee-saved-1.l     |    2 +
 .../gas/scfi/x86_64/scfi-callee-saved-1.s     |   25 +
 .../gas/scfi/x86_64/scfi-callee-saved-2.d     |   42 +
 .../gas/scfi/x86_64/scfi-callee-saved-2.l     |    2 +
 .../gas/scfi/x86_64/scfi-callee-saved-2.s     |   41 +
 .../gas/scfi/x86_64/scfi-callee-saved-3.d     |   43 +
 .../gas/scfi/x86_64/scfi-callee-saved-3.l     |    3 +
 .../gas/scfi/x86_64/scfi-callee-saved-3.s     |   39 +
 .../gas/scfi/x86_64/scfi-callee-saved-4.d     |   41 +
 .../gas/scfi/x86_64/scfi-callee-saved-4.l     |    3 +
 .../gas/scfi/x86_64/scfi-callee-saved-4.s     |   55 +
 gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.d    |   37 +
 gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.s    |   47 +
 gas/testsuite/gas/scfi/x86_64/scfi-cfg-2.d    |   29 +
 gas/testsuite/gas/scfi/x86_64/scfi-cfg-2.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-cfg-2.s    |   21 +
 .../gas/scfi/x86_64/scfi-cfi-label-1.d        |   38 +
 .../gas/scfi/x86_64/scfi-cfi-label-1.l        |    2 +
 .../gas/scfi/x86_64/scfi-cfi-label-1.s        |   19 +
 .../gas/scfi/x86_64/scfi-cfi-sections-1.d     |   24 +
 .../gas/scfi/x86_64/scfi-cfi-sections-1.l     |    2 +
 .../gas/scfi/x86_64/scfi-cfi-sections-1.s     |   22 +
 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d   |    5 +
 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l   |    3 +
 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.s   |   24 +
 gas/testsuite/gas/scfi/x86_64/scfi-diag-1.l   |    4 +
 gas/testsuite/gas/scfi/x86_64/scfi-diag-1.s   |   22 +
 gas/testsuite/gas/scfi/x86_64/scfi-diag-2.l   |    4 +
 gas/testsuite/gas/scfi/x86_64/scfi-diag-2.s   |   28 +
 .../gas/scfi/x86_64/scfi-dyn-stack-1.d        |   24 +
 .../gas/scfi/x86_64/scfi-dyn-stack-1.l        |    2 +
 .../gas/scfi/x86_64/scfi-dyn-stack-1.s        |   50 +
 gas/testsuite/gas/scfi/x86_64/scfi-enter-1.d  |   36 +
 gas/testsuite/gas/scfi/x86_64/scfi-enter-1.l  |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-enter-1.s  |   24 +
 .../gas/scfi/x86_64/scfi-fp-diag-2.l          |    3 +
 .../gas/scfi/x86_64/scfi-fp-diag-2.s          |   55 +
 .../gas/scfi/x86_64/scfi-indirect-mov-1.d     |   52 +
 .../gas/scfi/x86_64/scfi-indirect-mov-1.l     |    2 +
 .../gas/scfi/x86_64/scfi-indirect-mov-1.s     |   48 +
 .../gas/scfi/x86_64/scfi-indirect-mov-2.d     |   42 +
 .../gas/scfi/x86_64/scfi-indirect-mov-2.l     |    2 +
 .../gas/scfi/x86_64/scfi-indirect-mov-2.s     |   38 +
 .../gas/scfi/x86_64/scfi-indirect-mov-3.d     |   42 +
 .../gas/scfi/x86_64/scfi-indirect-mov-3.l     |    2 +
 .../gas/scfi/x86_64/scfi-indirect-mov-3.s     |   38 +
 .../gas/scfi/x86_64/scfi-indirect-mov-4.d     |   64 +
 .../gas/scfi/x86_64/scfi-indirect-mov-4.l     |    3 +
 .../gas/scfi/x86_64/scfi-indirect-mov-4.s     |   68 +
 .../gas/scfi/x86_64/scfi-indirect-mov-5.s     |   35 +
 gas/testsuite/gas/scfi/x86_64/scfi-lea-1.d    |   38 +
 gas/testsuite/gas/scfi/x86_64/scfi-lea-1.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-lea-1.s    |   39 +
 gas/testsuite/gas/scfi/x86_64/scfi-leave-1.d  |   37 +
 gas/testsuite/gas/scfi/x86_64/scfi-leave-1.l  |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-leave-1.s  |   25 +
 gas/testsuite/gas/scfi/x86_64/scfi-pushq-1.d  |   36 +
 gas/testsuite/gas/scfi/x86_64/scfi-pushq-1.l  |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-pushq-1.s  |   23 +
 .../gas/scfi/x86_64/scfi-pushsection-1.d      |   43 +
 .../gas/scfi/x86_64/scfi-pushsection-1.l      |    2 +
 .../gas/scfi/x86_64/scfi-pushsection-1.s      |   40 +
 .../gas/scfi/x86_64/scfi-pushsection-2.d      |   40 +
 .../gas/scfi/x86_64/scfi-pushsection-2.l      |    2 +
 .../gas/scfi/x86_64/scfi-pushsection-2.s      |   41 +
 .../gas/scfi/x86_64/scfi-selfalign-func-1.d   |   32 +
 .../gas/scfi/x86_64/scfi-selfalign-func-1.l   |    2 +
 .../gas/scfi/x86_64/scfi-selfalign-func-1.s   |   36 +
 gas/testsuite/gas/scfi/x86_64/scfi-simple-1.d |   27 +
 gas/testsuite/gas/scfi/x86_64/scfi-simple-1.l |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-simple-1.s |   15 +
 gas/testsuite/gas/scfi/x86_64/scfi-simple-2.d |   31 +
 gas/testsuite/gas/scfi/x86_64/scfi-simple-2.l |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-simple-2.s |   16 +
 gas/testsuite/gas/scfi/x86_64/scfi-sub-1.d    |   26 +
 gas/testsuite/gas/scfi/x86_64/scfi-sub-1.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-sub-1.s    |   12 +
 gas/testsuite/gas/scfi/x86_64/scfi-sub-2.d    |   32 +
 gas/testsuite/gas/scfi/x86_64/scfi-sub-2.l    |    2 +
 gas/testsuite/gas/scfi/x86_64/scfi-sub-2.s    |   29 +
 .../gas/scfi/x86_64/scfi-unsupported-1.l      |    2 +
 .../gas/scfi/x86_64/scfi-unsupported-1.s      |    9 +
 .../gas/scfi/x86_64/scfi-unsupported-2.l      |    3 +
 .../gas/scfi/x86_64/scfi-unsupported-2.s      |   13 +
 .../gas/scfi/x86_64/scfi-unsupported-3.l      |    3 +
 .../gas/scfi/x86_64/scfi-unsupported-3.s      |   13 +
 .../gas/scfi/x86_64/scfi-unsupported-4.l      |    4 +
 .../gas/scfi/x86_64/scfi-unsupported-4.s      |   22 +
 .../gas/scfi/x86_64/scfi-unsupported-cfg-1.l  |    3 +
 .../gas/scfi/x86_64/scfi-unsupported-cfg-1.s  |   52 +
 .../gas/scfi/x86_64/scfi-unsupported-cfg-2.l  |    4 +
 .../gas/scfi/x86_64/scfi-unsupported-cfg-2.s  |   14 +
 .../gas/scfi/x86_64/scfi-unsupported-drap-1.l |    4 +
 .../gas/scfi/x86_64/scfi-unsupported-drap-1.s |   75 +
 gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp |  111 ++
 opcodes/i386-reg.tbl                          |    3 +
 148 files changed, 7251 insertions(+), 40 deletions(-)
 create mode 100644 gas/ginsn.c
 create mode 100644 gas/ginsn.h
 create mode 100644 gas/scfi.c
 create mode 100644 gas/scfi.h
 create mode 100644 gas/scfidw2gen.c
 create mode 100644 gas/scfidw2gen.h
 create mode 100644 gas/testsuite/gas/scfi/README
 create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-dw2-regnum-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-pop-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-pop-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-push-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/ginsn-push-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-add-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-add-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-add-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-add-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-add-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-add-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-3.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-3.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-3.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-3.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-3.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-3.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-4.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-4.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-callee-saved-4.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfg-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfi-label-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfi-label-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfi-label-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cfi-sections-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-cofi-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-diag-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-diag-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-diag-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-diag-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-dyn-stack-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-enter-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-enter-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-enter-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-fp-diag-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-fp-diag-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-3.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-3.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-3.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-4.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-4.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-4.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-indirect-mov-5.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-lea-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-lea-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-lea-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-leave-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-leave-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-leave-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushq-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushq-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushq-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushsection-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushsection-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushsection-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushsection-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushsection-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-pushsection-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-selfalign-func-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-selfalign-func-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-selfalign-func-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-simple-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-simple-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-simple-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-simple-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-simple-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-simple-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-sub-1.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-sub-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-sub-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-sub-2.d
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-sub-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-sub-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-3.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-3.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-4.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-4.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-2.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-cfg-2.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-drap-1.l
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-unsupported-drap-1.s
 create mode 100644 gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp
  

Comments

Nick Clifton Dec. 15, 2023, 9:13 a.m. UTC | #1
Hi Indu,

> Hello,
> 
> This patch series adds support in GAS to synthesize CFI for hand-written
> asm, acronym'd as SCFI.
> 
> Previous postings:
>    - RFC patch series (https://sourceware.org/pipermail/binutils/2023-September/129560.html).
>    - V1 (https://sourceware.org/pipermail/binutils/2023-October/130163.html)
>    - V2 (https://sourceware.org/pipermail/binutils/2023-October/130210.html)
> 
The patch series looks great to me.  Series approved - please apply (with a
small update to the 09 patch as already mentioned).

Cheers
   Nick
  
Jan Beulich Dec. 18, 2023, 8:47 a.m. UTC | #2
On 15.12.2023 10:13, Nick Clifton wrote:
>> This patch series adds support in GAS to synthesize CFI for hand-written
>> asm, acronym'd as SCFI.
>>
>> Previous postings:
>>    - RFC patch series (https://sourceware.org/pipermail/binutils/2023-September/129560.html).
>>    - V1 (https://sourceware.org/pipermail/binutils/2023-October/130163.html)
>>    - V2 (https://sourceware.org/pipermail/binutils/2023-October/130210.html)
>>
> The patch series looks great to me.  Series approved - please apply (with a
> small update to the 09 patch as already mentioned).

Please can you hold off committing the x86 part(s) of this, until I got a
chance to look over them again? Furthermore, as expressed before, I'm wary
of these additions going stale the minute the APX patches are committed on
top, if your changes went in first. Despite APX work still being in flight,
I would much prefer if that went in first, and then you re-based your work
on top, such that the new MOV and ALU insns are covered right away.

While this is an unusual situation - new very general purpose insns aren't
introduced frequently -, I'd also like to see more formally addressed the
idea of ongoing support: From the original review I recall that you need
to minimally track insns altering GPRs, in order to avoid silently
generating bad CFI. Remember that I haven't looked at v3 yet, but as long
as that tracking is based on specific insns rather than a generalized
pattern, any ISA addition allowing GPRs to be altered would be at risk of
rendering the CFI generator code stale. Yet people, once they've started
to detect availability of this functionality, may validly expect that
their use of the functionality won't silently break behind their backs. In
this respect, did you consider constraining under what conditions the
generator code may actually come into play (at least for the time being)?

Jan
  
Indu Bhagat Dec. 19, 2023, 9:02 p.m. UTC | #3
On 12/18/23 00:47, Jan Beulich wrote:
> On 15.12.2023 10:13, Nick Clifton wrote:
>>> This patch series adds support in GAS to synthesize CFI for hand-written
>>> asm, acronym'd as SCFI.
>>>
>>> Previous postings:
>>>     - RFC patch series (https://sourceware.org/pipermail/binutils/2023-September/129560.html).
>>>     - V1 (https://sourceware.org/pipermail/binutils/2023-October/130163.html)
>>>     - V2 (https://sourceware.org/pipermail/binutils/2023-October/130210.html)
>>>
>> The patch series looks great to me.  Series approved - please apply (with a
>> small update to the 09 patch as already mentioned).
> 
> Please can you hold off committing the x86 part(s) of this, until I got a
> chance to look over them again? Furthermore, as expressed before, I'm wary
> of these additions going stale the minute the APX patches are committed on
> top, if your changes went in first. Despite APX work still being in flight,
> I would much prefer if that went in first, and then you re-based your work
> on top, such that the new MOV and ALU insns are covered right away.
> 

There will still remain handling the newly added APX Push/Pop 
instructions as well.

> While this is an unusual situation - new very general purpose insns aren't
> introduced frequently -, I'd also like to see more formally addressed the
> idea of ongoing support: From the original review I recall that you need
> to minimally track insns altering GPRs, in order to avoid silently
> generating bad CFI. Remember that I haven't looked at v3 yet, but as long
> as that tracking is based on specific insns rather than a generalized
> pattern, any ISA addition allowing GPRs to be altered would be at risk of
> rendering the CFI generator code stale. Yet people, once they've started
> to detect availability of this functionality, may validly expect that
> their use of the functionality won't silently break behind their backs. In
> this respect, did you consider constraining under what conditions the
> generator code may actually come into play (at least for the time being)?
> 

(Step 1) I propose that, for now, we add a check such that if any APX 
insn is seen for --scfi invocation, we bail out.  IIUC, we could check 
using the is_any_apx_rex2_encoding ().

(Step 2a) We can remove this check once there is support for all APX 
instructions for SCFI. I can add support for ginsns for APX instructions 
once the APX work is pushed.

(Step 2b) Orthogonal to supporting the APX instruction set: For SCFI, it 
is ideal to add a way to raise alert if new instructions are added in 
the three categories (For APX, I see we have additions in #2, and #3):

1. Control flow instructions
    We can detect them by checking for insn.tm.opcode_modifier.jump.

2. Operations altering GPRs
    We can detect them by checking for:
    if (insn.operands && insn.reg_operands)
      {
        reg_op = i.op[insn.operands - 1].regs;
        if (reg_op)
          check if destination reg is REG_SP/REG_FP
      }

3. Operations with implicit update to stack pointer.
    Currently we have no marker, but how about adding something like 
unsigned int implicitstackop:1 in i386_opcode_modifier ? We will also 
need to ensure this property is correctly conveyed for all existing 
instructions in i386-opc.tbl.  When new instructions are added, the SCFI 
machinery will be able to warn the user of missing functionality (and 
not generate wrong CFI).

After this support for #3 is added to the backend, we will be in good 
stead for future ISA additions wrt SCFI.

I can _try_ to accommodate 2b before the 2.42 release is cut. However, I 
think its best to plan for both 2a and 2b for 2.43 release (given the 
2.42 is around the corner), if there is agreement.

What do you think about this plan ?

Thanks
  
Jan Beulich Dec. 20, 2023, 7:51 a.m. UTC | #4
On 19.12.2023 22:02, Indu Bhagat wrote:
> On 12/18/23 00:47, Jan Beulich wrote:
>> While this is an unusual situation - new very general purpose insns aren't
>> introduced frequently -, I'd also like to see more formally addressed the
>> idea of ongoing support: From the original review I recall that you need
>> to minimally track insns altering GPRs, in order to avoid silently
>> generating bad CFI. Remember that I haven't looked at v3 yet, but as long
>> as that tracking is based on specific insns rather than a generalized
>> pattern, any ISA addition allowing GPRs to be altered would be at risk of
>> rendering the CFI generator code stale. Yet people, once they've started
>> to detect availability of this functionality, may validly expect that
>> their use of the functionality won't silently break behind their backs. In
>> this respect, did you consider constraining under what conditions the
>> generator code may actually come into play (at least for the time being)?
> 
> (Step 1) I propose that, for now, we add a check such that if any APX 
> insn is seen for --scfi invocation, we bail out.  IIUC, we could check 
> using the is_any_apx_rex2_encoding ().

That would cover only the REX2 subset of additions. The promoted-to-EVEX
insns would need covering as well.

> (Step 2a) We can remove this check once there is support for all APX 
> instructions for SCFI. I can add support for ginsns for APX instructions 
> once the APX work is pushed.
> 
> (Step 2b) Orthogonal to supporting the APX instruction set: For SCFI, it 
> is ideal to add a way to raise alert if new instructions are added in 
> the three categories (For APX, I see we have additions in #2, and #3):
> 
> 1. Control flow instructions
>     We can detect them by checking for insn.tm.opcode_modifier.jump.

We may certainly hope that any new control flow insns would also have
this attribute set.

This reminds me of another guard you may need, though: Use of .byte to
hand-craft insns.

> 2. Operations altering GPRs
>     We can detect them by checking for:
>     if (insn.operands && insn.reg_operands)
>       {
>         reg_op = i.op[insn.operands - 1].regs;
>         if (reg_op)
>           check if destination reg is REG_SP/REG_FP
>       }

The condition here isn't sufficient (the destination could also be a
memory operand), but something along these lines would certainly
address one of my fundamental concerns.

> 3. Operations with implicit update to stack pointer.
>     Currently we have no marker, but how about adding something like 
> unsigned int implicitstackop:1 in i386_opcode_modifier ? We will also 
> need to ensure this property is correctly conveyed for all existing 
> instructions in i386-opc.tbl.  When new instructions are added, the SCFI 
> machinery will be able to warn the user of missing functionality (and 
> not generate wrong CFI).

While I'm generally hesitant to see new attributes added, if one's
needed for a clear purpose (like looks to be the case here), that's
certainly fine. Much will depend on people (at least one of submitter
or reviewer) remembering to (ask to) add such an attribute whenever
needed.

> After this support for #3 is added to the backend, we will be in good 
> stead for future ISA additions wrt SCFI.
> 
> I can _try_ to accommodate 2b before the 2.42 release is cut. However, I 
> think its best to plan for both 2a and 2b for 2.43 release (given the 
> 2.42 is around the corner), if there is agreement.

If 2b was properly in place, I could probably be convinced to agree with
your work going in ahead of the APX stuff. It's in particular unclear to
me in how far APX is really going to make 2.42, considering how much of
a change it is, and how hard is has been so far to review the changes. It
would nevertheless feel better to me if your work was given some more
time, to go in only after 2.42 was branched off. I certainly can't
promise I will be able get around to reviewing either SCFI or APX again
before the end of the year (more precisely: before the holidays).

Jan
  
Indu Bhagat Jan. 4, 2024, 6:08 p.m. UTC | #5
On 12/19/23 23:51, Jan Beulich wrote:
> On 19.12.2023 22:02, Indu Bhagat wrote:
>> On 12/18/23 00:47, Jan Beulich wrote:
>>> While this is an unusual situation - new very general purpose insns aren't
>>> introduced frequently -, I'd also like to see more formally addressed the
>>> idea of ongoing support: From the original review I recall that you need
>>> to minimally track insns altering GPRs, in order to avoid silently
>>> generating bad CFI. Remember that I haven't looked at v3 yet, but as long
>>> as that tracking is based on specific insns rather than a generalized
>>> pattern, any ISA addition allowing GPRs to be altered would be at risk of
>>> rendering the CFI generator code stale. Yet people, once they've started
>>> to detect availability of this functionality, may validly expect that
>>> their use of the functionality won't silently break behind their backs. In
>>> this respect, did you consider constraining under what conditions the
>>> generator code may actually come into play (at least for the time being)?
>>
>> (Step 1) I propose that, for now, we add a check such that if any APX
>> insn is seen for --scfi invocation, we bail out.  IIUC, we could check
>> using the is_any_apx_rex2_encoding ().
> 
> That would cover only the REX2 subset of additions. The promoted-to-EVEX
> insns would need covering as well.
> 

Since I worked out 2b, I have omitted these checks from V4.

>> (Step 2a) We can remove this check once there is support for all APX
>> instructions for SCFI. I can add support for ginsns for APX instructions
>> once the APX work is pushed.
>>
>> (Step 2b) Orthogonal to supporting the APX instruction set: For SCFI, it
>> is ideal to add a way to raise alert if new instructions are added in
>> the three categories (For APX, I see we have additions in #2, and #3):
>>
>> 1. Control flow instructions
>>      We can detect them by checking for insn.tm.opcode_modifier.jump.
> 
> We may certainly hope that any new control flow insns would also have
> this attribute set.
> 
> This reminds me of another guard you may need, though: Use of .byte to
> hand-craft insns.
> 

I have added a thunk in V4 to not allow .byte usage with --scfi.

>> 2. Operations altering GPRs
>>      We can detect them by checking for:
>>      if (insn.operands && insn.reg_operands)
>>        {
>>          reg_op = i.op[insn.operands - 1].regs;
>>          if (reg_op)
>>            check if destination reg is REG_SP/REG_FP
>>        }
> 
> The condition here isn't sufficient (the destination could also be a
> memory operand), but something along these lines would certainly
> address one of my fundamental concerns.
> 
>> 3. Operations with implicit update to stack pointer.
>>      Currently we have no marker, but how about adding something like
>> unsigned int implicitstackop:1 in i386_opcode_modifier ? We will also
>> need to ensure this property is correctly conveyed for all existing
>> instructions in i386-opc.tbl.  When new instructions are added, the SCFI
>> machinery will be able to warn the user of missing functionality (and
>> not generate wrong CFI).
> 
> While I'm generally hesitant to see new attributes added, if one's
> needed for a clear purpose (like looks to be the case here), that's
> certainly fine. Much will depend on people (at least one of submitter
> or reviewer) remembering to (ask to) add such an attribute whenever
> needed.
> 
>> After this support for #3 is added to the backend, we will be in good
>> stead for future ISA additions wrt SCFI.
>>
>> I can _try_ to accommodate 2b before the 2.42 release is cut. However, I
>> think its best to plan for both 2a and 2b for 2.43 release (given the
>> 2.42 is around the corner), if there is agreement.
> 
> If 2b was properly in place, I could probably be convinced to agree with
> your work going in ahead of the APX stuff. It's in particular unclear to
> me in how far APX is really going to make 2.42, considering how much of
> a change it is, and how hard is has been so far to review the changes. It
> would nevertheless feel better to me if your work was given some more
> time, to go in only after 2.42 was branched off. I certainly can't
> promise I will be able get around to reviewing either SCFI or APX again
> before the end of the year (more precisely: before the holidays).
> 

I have addressed 2b in the SCFI V4 series sent earlier to the mailing 
list.  I hope that the SCFI series can make it to 2.42.

Thanks for reviewing,
Indu