[8/9] gas: Check for overflow on return column in version 1 CIE DWARF

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

Commit Message

Andrew Burgess Nov. 22, 2019, 12:10 p.m.
In version 1 of DWARF CIE format, the return register column is just a
single byte.  For targets with large numbers of DWARF registers, any
use of a register with a high number for the return column
will (currently) silently overflow giving incorrect DWARF.

This commit adds an error when the overflow occurs.

gas/ChangeLog:

	* dw2gencfi.c (output_cie): Error on return column overflow.
	* testsuite/gas/riscv/cie-rtn-col-1.d: New file.
	* testsuite/gas/riscv/cie-rtn-col-3.d: New file.
	* testsuite/gas/riscv/cie-rtn-col.s: New file.

Change-Id: I1809f739ba7771737ec012807f0260e1a3ed5e64
---
 gas/ChangeLog                           |  7 +++++++
 gas/dw2gencfi.c                         |  7 ++++++-
 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 +++
 5 files changed, 36 insertions(+), 1 deletion(-)
 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

-- 
2.14.5

Comments

Palmer Dabbelt via binutils Nov. 22, 2019, 10:33 p.m. | #1
On Fri, 22 Nov 2019 04:10:32 PST (-0800), andrew.burgess@embecosm.com wrote:
> In version 1 of DWARF CIE format, the return register column is just a

> single byte.  For targets with large numbers of DWARF registers, any

> use of a register with a high number for the return column

> will (currently) silently overflow giving incorrect DWARF.

>

> This commit adds an error when the overflow occurs.

>

> gas/ChangeLog:

>

> 	* dw2gencfi.c (output_cie): Error on return column overflow.

> 	* testsuite/gas/riscv/cie-rtn-col-1.d: New file.

> 	* testsuite/gas/riscv/cie-rtn-col-3.d: New file.

> 	* testsuite/gas/riscv/cie-rtn-col.s: New file.

>

> Change-Id: I1809f739ba7771737ec012807f0260e1a3ed5e64

> ---

>  gas/ChangeLog                           |  7 +++++++

>  gas/dw2gencfi.c                         |  7 ++++++-

>  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 +++

>  5 files changed, 36 insertions(+), 1 deletion(-)

>  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

>

> diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c

> index e27253db8ee..4c19987dfcf 100644

> --- a/gas/dw2gencfi.c

> +++ b/gas/dw2gencfi.c

> @@ -1887,7 +1887,12 @@ output_cie (struct cie_entry *cie, bfd_boolean eh_frame, int align)

>    out_uleb128 (DWARF2_LINE_MIN_INSN_LENGTH);	/* Code alignment.  */

>    out_sleb128 (DWARF2_CIE_DATA_ALIGNMENT);	/* Data alignment.  */

>    if (flag_dwarf_cie_version == 1)		/* Return column.  */

> -    out_one (cie->return_column);

> +    {

> +      if ((cie->return_column & 0xff) != cie->return_column)

> +        as_bad (_("return column number %d overflows in CIE version 1"),

> +                cie->return_column);

> +      out_one (cie->return_column);

> +    }

>    else

>      out_uleb128 (cie->return_column);

>    if (eh_frame)

> diff --git a/gas/testsuite/gas/riscv/cie-rtn-col-1.d b/gas/testsuite/gas/riscv/cie-rtn-col-1.d

> new file mode 100644

> index 00000000000..dba9c0d3811

> --- /dev/null

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

> @@ -0,0 +1,3 @@

> +#as: --gdwarf-cie-version=1

> +#source: cie-rtn-col.s

> +#error: return column number 4929 overflows in CIE version 1

> diff --git a/gas/testsuite/gas/riscv/cie-rtn-col-3.d b/gas/testsuite/gas/riscv/cie-rtn-col-3.d

> new file mode 100644

> index 00000000000..a1d71e1a940

> --- /dev/null

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

> @@ -0,0 +1,17 @@

> +#objdump: --dwarf=frames

> +#as: --gdwarf-cie-version=3

> +#source: cie-rtn-col.s

> +

> +.*:     file format elf.*-.*riscv

> +

> +Contents of the .* section:

> +

> +

> +00000000 [a-zA-Z0-9]+ [a-zA-Z0-9]+ CIE

> +  Version:               3

> +  Augmentation:          .*

> +  Code alignment factor: .*

> +  Data alignment factor: .*

> +  Return address column: 4929

> +  Augmentation data:     .*

> +#...

> diff --git a/gas/testsuite/gas/riscv/cie-rtn-col.s b/gas/testsuite/gas/riscv/cie-rtn-col.s

> new file mode 100644

> index 00000000000..ca8774f1bcc

> --- /dev/null

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

> @@ -0,0 +1,3 @@

> +        .cfi_startproc

> +        .cfi_return_column mepc

> +        .cfi_endproc


Have you tried backtracing through a trap handler?  I guess in theory that
would work, assuming everything was sufficiently decorated and the whole system
was a single ELF.

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Andrew Burgess Nov. 22, 2019, 10:51 p.m. | #2
* Palmer Dabbelt <palmerdabbelt@google.com> [2019-11-22 14:33:03 -0800]:

> On Fri, 22 Nov 2019 04:10:32 PST (-0800), andrew.burgess@embecosm.com wrote:

> > In version 1 of DWARF CIE format, the return register column is just a

> > single byte.  For targets with large numbers of DWARF registers, any

> > use of a register with a high number for the return column

> > will (currently) silently overflow giving incorrect DWARF.

> >

> > This commit adds an error when the overflow occurs.

> >

> > gas/ChangeLog:

> >

> > 	* dw2gencfi.c (output_cie): Error on return column overflow.

> > 	* testsuite/gas/riscv/cie-rtn-col-1.d: New file.

> > 	* testsuite/gas/riscv/cie-rtn-col-3.d: New file.

> > 	* testsuite/gas/riscv/cie-rtn-col.s: New file.

> >

> > Change-Id: I1809f739ba7771737ec012807f0260e1a3ed5e64

> > ---

> >  gas/ChangeLog                           |  7 +++++++

> >  gas/dw2gencfi.c                         |  7 ++++++-

> >  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 +++

> >  5 files changed, 36 insertions(+), 1 deletion(-)

> >  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

> >

> > diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c

> > index e27253db8ee..4c19987dfcf 100644

> > --- a/gas/dw2gencfi.c

> > +++ b/gas/dw2gencfi.c

> > @@ -1887,7 +1887,12 @@ output_cie (struct cie_entry *cie, bfd_boolean eh_frame, int align)

> >    out_uleb128 (DWARF2_LINE_MIN_INSN_LENGTH);	/* Code alignment.  */

> >    out_sleb128 (DWARF2_CIE_DATA_ALIGNMENT);	/* Data alignment.  */

> >    if (flag_dwarf_cie_version == 1)		/* Return column.  */

> > -    out_one (cie->return_column);

> > +    {

> > +      if ((cie->return_column & 0xff) != cie->return_column)

> > +        as_bad (_("return column number %d overflows in CIE version 1"),

> > +                cie->return_column);

> > +      out_one (cie->return_column);

> > +    }

> >    else

> >      out_uleb128 (cie->return_column);

> >    if (eh_frame)

> > diff --git a/gas/testsuite/gas/riscv/cie-rtn-col-1.d b/gas/testsuite/gas/riscv/cie-rtn-col-1.d

> > new file mode 100644

> > index 00000000000..dba9c0d3811

> > --- /dev/null

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

> > @@ -0,0 +1,3 @@

> > +#as: --gdwarf-cie-version=1

> > +#source: cie-rtn-col.s

> > +#error: return column number 4929 overflows in CIE version 1

> > diff --git a/gas/testsuite/gas/riscv/cie-rtn-col-3.d b/gas/testsuite/gas/riscv/cie-rtn-col-3.d

> > new file mode 100644

> > index 00000000000..a1d71e1a940

> > --- /dev/null

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

> > @@ -0,0 +1,17 @@

> > +#objdump: --dwarf=frames

> > +#as: --gdwarf-cie-version=3

> > +#source: cie-rtn-col.s

> > +

> > +.*:     file format elf.*-.*riscv

> > +

> > +Contents of the .* section:

> > +

> > +

> > +00000000 [a-zA-Z0-9]+ [a-zA-Z0-9]+ CIE

> > +  Version:               3

> > +  Augmentation:          .*

> > +  Code alignment factor: .*

> > +  Data alignment factor: .*

> > +  Return address column: 4929

> > +  Augmentation data:     .*

> > +#...

> > diff --git a/gas/testsuite/gas/riscv/cie-rtn-col.s b/gas/testsuite/gas/riscv/cie-rtn-col.s

> > new file mode 100644

> > index 00000000000..ca8774f1bcc

> > --- /dev/null

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

> > @@ -0,0 +1,3 @@

> > +        .cfi_startproc

> > +        .cfi_return_column mepc

> > +        .cfi_endproc

> 

> Have you tried backtracing through a trap handler?  I guess in theory that

> would work, assuming everything was sufficiently decorated and the whole system

> was a single ELF.


Absolutely! I have that working locally but using a slightly modified
GDB.  Once these patches land in binutils I'm going to take a look to
see what GDB might still be outstanding to support this - but it
shouldn't be much, if anything.

Thanks,
Andrew

> 

> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>

Patch

diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
index e27253db8ee..4c19987dfcf 100644
--- a/gas/dw2gencfi.c
+++ b/gas/dw2gencfi.c
@@ -1887,7 +1887,12 @@  output_cie (struct cie_entry *cie, bfd_boolean eh_frame, int align)
   out_uleb128 (DWARF2_LINE_MIN_INSN_LENGTH);	/* Code alignment.  */
   out_sleb128 (DWARF2_CIE_DATA_ALIGNMENT);	/* Data alignment.  */
   if (flag_dwarf_cie_version == 1)		/* Return column.  */
-    out_one (cie->return_column);
+    {
+      if ((cie->return_column & 0xff) != cie->return_column)
+        as_bad (_("return column number %d overflows in CIE version 1"),
+                cie->return_column);
+      out_one (cie->return_column);
+    }
   else
     out_uleb128 (cie->return_column);
   if (eh_frame)
diff --git a/gas/testsuite/gas/riscv/cie-rtn-col-1.d b/gas/testsuite/gas/riscv/cie-rtn-col-1.d
new file mode 100644
index 00000000000..dba9c0d3811
--- /dev/null
+++ b/gas/testsuite/gas/riscv/cie-rtn-col-1.d
@@ -0,0 +1,3 @@ 
+#as: --gdwarf-cie-version=1
+#source: cie-rtn-col.s
+#error: return column number 4929 overflows in CIE version 1
diff --git a/gas/testsuite/gas/riscv/cie-rtn-col-3.d b/gas/testsuite/gas/riscv/cie-rtn-col-3.d
new file mode 100644
index 00000000000..a1d71e1a940
--- /dev/null
+++ b/gas/testsuite/gas/riscv/cie-rtn-col-3.d
@@ -0,0 +1,17 @@ 
+#objdump: --dwarf=frames
+#as: --gdwarf-cie-version=3
+#source: cie-rtn-col.s
+
+.*:     file format elf.*-.*riscv
+
+Contents of the .* section:
+
+
+00000000 [a-zA-Z0-9]+ [a-zA-Z0-9]+ CIE
+  Version:               3
+  Augmentation:          .*
+  Code alignment factor: .*
+  Data alignment factor: .*
+  Return address column: 4929
+  Augmentation data:     .*
+#...
diff --git a/gas/testsuite/gas/riscv/cie-rtn-col.s b/gas/testsuite/gas/riscv/cie-rtn-col.s
new file mode 100644
index 00000000000..ca8774f1bcc
--- /dev/null
+++ b/gas/testsuite/gas/riscv/cie-rtn-col.s
@@ -0,0 +1,3 @@ 
+        .cfi_startproc
+        .cfi_return_column mepc
+        .cfi_endproc