[0/9] DWARF Register Number Handling, Including RISC-V CSRs

Message ID cover.1574423265.git.andrew.burgess@embecosm.com
Headers show
Series
  • DWARF Register Number Handling, Including RISC-V CSRs
Related show

Message

Andrew Burgess Nov. 22, 2019, 12:10 p.m.
The aim of this series is to provide support for using RISC-V CSRs
(Control Status Registers) in gas .cfi directives.  The problem with
this is that the DWARF register numbers for these registers are rather
large, in the range 4096 - 8191, and the number space is only sparsely
populated.  The existing code for mapping DWARF register numbers to
names is not really setup to handle this situation, and additionally,
the default DWARF CIE version that gas uses only has a single byte
return address column, which is overflowed by these CSRs.

This series starts with some general code clean up (I think) in the
area around DWARF register number handling, introduces a new mechanism
for mapping from DWARF register numbers to names, and then adds
support for RISC-V CSRs.  Finally I make DWARF CIE version 3 the
default for RISC-V, this should be largely invisible to consumers,
except the return address column is now uleb128 instead of ubyte.

---

Andrew Burgess (9):
  gas/riscv: Remove unneeded structure
  binutils: Be more forgiving of targets with large numbers of registers
  binutils: Rename init_dwarf_regnames
  binutils: Add a new function to initialise DWARF register name state
  binutils: Make some functions static in dwarf.c
  binutils: Make DWARF register name lookup be via a function pointer
  binutils/gas/riscv: Add DWARF register numbers for CSRs
  gas: Check for overflow on return column in version 1 CIE DWARF
  gas/riscv: Produce version 3 DWARF CIE by default

 binutils/ChangeLog                            |  59 ++++++
 binutils/dwarf.c                              | 143 ++++++++++++--
 binutils/dwarf.h                              |  10 +-
 binutils/objdump.c                            |  40 +---
 binutils/readelf.c                            |   2 +-
 gas/ChangeLog                                 |  30 +++
 gas/as.c                                      |  10 +-
 gas/config/tc-riscv.c                         |  18 +-
 gas/dw2gencfi.c                               |   7 +-
 gas/dwarf2dbg.c                               |  11 ++
 gas/testsuite/gas/riscv/cie-rtn-col-1.d       |   3 +
 gas/testsuite/gas/riscv/cie-rtn-col-3.d       |  17 ++
 gas/testsuite/gas/riscv/cie-rtn-col.s         |   3 +
 gas/testsuite/gas/riscv/csr-dw-regnums.d      | 265 ++++++++++++++++++++++++++
 gas/testsuite/gas/riscv/csr-dw-regnums.s      | 255 +++++++++++++++++++++++++
 gas/testsuite/gas/riscv/default-cie-version.d |  15 ++
 gas/testsuite/gas/riscv/default-cie-version.s |   2 +
 17 files changed, 815 insertions(+), 75 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/cie-rtn-col-1.d
 create mode 100644 gas/testsuite/gas/riscv/cie-rtn-col-3.d
 create mode 100644 gas/testsuite/gas/riscv/cie-rtn-col.s
 create mode 100644 gas/testsuite/gas/riscv/csr-dw-regnums.d
 create mode 100644 gas/testsuite/gas/riscv/csr-dw-regnums.s
 create mode 100644 gas/testsuite/gas/riscv/default-cie-version.d
 create mode 100644 gas/testsuite/gas/riscv/default-cie-version.s

-- 
2.14.5

Comments

Alan Modra via Binutils Nov. 22, 2019, 10:33 p.m. | #1
On Fri, 22 Nov 2019 04:10:24 PST (-0800), andrew.burgess@embecosm.com wrote:
> The aim of this series is to provide support for using RISC-V CSRs

> (Control Status Registers) in gas .cfi directives.  The problem with

> this is that the DWARF register numbers for these registers are rather

> large, in the range 4096 - 8191, and the number space is only sparsely

> populated.  The existing code for mapping DWARF register numbers to

> names is not really setup to handle this situation, and additionally,

> the default DWARF CIE version that gas uses only has a single byte

> return address column, which is overflowed by these CSRs.

>

> This series starts with some general code clean up (I think) in the

> area around DWARF register number handling, introduces a new mechanism

> for mapping from DWARF register numbers to names, and then adds

> support for RISC-V CSRs.  Finally I make DWARF CIE version 3 the

> default for RISC-V, this should be largely invisible to consumers,

> except the return address column is now uleb128 instead of ubyte.


Thanks!  Aside from that minor nit about naming unknown CSRs this looks good to
me.

>

> ---

>

> Andrew Burgess (9):

>   gas/riscv: Remove unneeded structure

>   binutils: Be more forgiving of targets with large numbers of registers

>   binutils: Rename init_dwarf_regnames

>   binutils: Add a new function to initialise DWARF register name state

>   binutils: Make some functions static in dwarf.c

>   binutils: Make DWARF register name lookup be via a function pointer

>   binutils/gas/riscv: Add DWARF register numbers for CSRs

>   gas: Check for overflow on return column in version 1 CIE DWARF

>   gas/riscv: Produce version 3 DWARF CIE by default

>

>  binutils/ChangeLog                            |  59 ++++++

>  binutils/dwarf.c                              | 143 ++++++++++++--

>  binutils/dwarf.h                              |  10 +-

>  binutils/objdump.c                            |  40 +---

>  binutils/readelf.c                            |   2 +-

>  gas/ChangeLog                                 |  30 +++

>  gas/as.c                                      |  10 +-

>  gas/config/tc-riscv.c                         |  18 +-

>  gas/dw2gencfi.c                               |   7 +-

>  gas/dwarf2dbg.c                               |  11 ++

>  gas/testsuite/gas/riscv/cie-rtn-col-1.d       |   3 +

>  gas/testsuite/gas/riscv/cie-rtn-col-3.d       |  17 ++

>  gas/testsuite/gas/riscv/cie-rtn-col.s         |   3 +

>  gas/testsuite/gas/riscv/csr-dw-regnums.d      | 265 ++++++++++++++++++++++++++

>  gas/testsuite/gas/riscv/csr-dw-regnums.s      | 255 +++++++++++++++++++++++++

>  gas/testsuite/gas/riscv/default-cie-version.d |  15 ++

>  gas/testsuite/gas/riscv/default-cie-version.s |   2 +

>  17 files changed, 815 insertions(+), 75 deletions(-)

>  create mode 100644 gas/testsuite/gas/riscv/cie-rtn-col-1.d

>  create mode 100644 gas/testsuite/gas/riscv/cie-rtn-col-3.d

>  create mode 100644 gas/testsuite/gas/riscv/cie-rtn-col.s

>  create mode 100644 gas/testsuite/gas/riscv/csr-dw-regnums.d

>  create mode 100644 gas/testsuite/gas/riscv/csr-dw-regnums.s

>  create mode 100644 gas/testsuite/gas/riscv/default-cie-version.d

>  create mode 100644 gas/testsuite/gas/riscv/default-cie-version.s
Nelson Chu Nov. 25, 2019, 10:10 a.m. | #2
Dear Andrew,

Thanks for your contribution, these patches looks good to me :)

There are three things after applying these patches.
1. I get the conflict when applying the fourth patch (binutils: Add a
new function to initialise DWARF register name state), this might be
caused by the GNU coding standards.
2. After running the binutils testsuite, the ld-elf/eh5 of ld-elf
testsuite failed, but it passes originally.
3. I agree with palmer's suggestion that spit out "csr%d" is more
clear to understand.

Thanks again, and also thanks for palmer's review :)
Best regards
Nelson

On Fri, Nov 22, 2019 at 8:10 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>

> The aim of this series is to provide support for using RISC-V CSRs

> (Control Status Registers) in gas .cfi directives.  The problem with

> this is that the DWARF register numbers for these registers are rather

> large, in the range 4096 - 8191, and the number space is only sparsely

> populated.  The existing code for mapping DWARF register numbers to

> names is not really setup to handle this situation, and additionally,

> the default DWARF CIE version that gas uses only has a single byte

> return address column, which is overflowed by these CSRs.

>

> This series starts with some general code clean up (I think) in the

> area around DWARF register number handling, introduces a new mechanism

> for mapping from DWARF register numbers to names, and then adds

> support for RISC-V CSRs.  Finally I make DWARF CIE version 3 the

> default for RISC-V, this should be largely invisible to consumers,

> except the return address column is now uleb128 instead of ubyte.
Alan Modra Nov. 25, 2019, 11:53 a.m. | #3
On Fri, Nov 22, 2019 at 12:10:24PM +0000, Andrew Burgess wrote:
> The aim of this series is to provide support for using RISC-V CSRs

> (Control Status Registers) in gas .cfi directives.  The problem with

> this is that the DWARF register numbers for these registers are rather

> large, in the range 4096 - 8191, and the number space is only sparsely

> populated.  The existing code for mapping DWARF register numbers to

> names is not really setup to handle this situation, and additionally,

> the default DWARF CIE version that gas uses only has a single byte

> return address column, which is overflowed by these CSRs.

> 

> This series starts with some general code clean up (I think) in the

> area around DWARF register number handling, introduces a new mechanism

> for mapping from DWARF register numbers to names, and then adds

> support for RISC-V CSRs.  Finally I make DWARF CIE version 3 the

> default for RISC-V, this should be largely invisible to consumers,

> except the return address column is now uleb128 instead of ubyte.


This all looks reasonable to me.  One nit: binutils generally uses
tabs to indent.  Please fix that before committing.  (Of course,
testsuite .d and similar can use spaces to match output!)

I find the following in my .git/config helps me avoid whitespace
errors:

[core]
	whitespace = indent-with-non-tab,space-before-tab,trailing-space

-- 
Alan Modra
Australia Development Lab, IBM
Andrew Burgess Nov. 28, 2019, 12:06 a.m. | #4
Thanks for all the feedback.

I addressed all of the issues that were raised and went ahead and
pushed this series.

Thanks,

Andrew