[PATCH] backends: ppc_abi_cfi reg1 use DW_CFA_val_offset not DW_CFA_val_expression.

[Mark Wielaard <mjw at redhat dot com>]

Register rules using expressions are stored using an offset from the
start of the .eh_frame or .debug_frame ELF section data. Since abi_cfi
rules aren't stored in those ELF sections they should use neither
DW_CFA_expression nor DW_CFA_val_expression. The only backend that used
DW_CFA_val_expression was ppc_cfi.c. It was easier to express the same
rule using DW_CFA_val_offset than to change the code to handle register
rules using expressions. On most architectures this did work by accident.
See the definition of struct dwarf_frame_register value in libdw/cfi.h to
see why. But on ia64 the abi_cfi data and actual frame data were placed
too far apart and caused a crash in tests/run-addrcfi.sh for ppc32.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 backends/ChangeLog   | 5 +++++
 backends/ppc_cfi.c   | 2 +-
 libdw/ChangeLog      | 5 +++++
 libdw/cfi.c          | 4 ++++
 libebl/ChangeLog     | 5 +++++
 libebl/libebl.h      | 5 +++++
 tests/ChangeLog      | 5 +++++
 tests/run-addrcfi.sh | 4 ++--
 8 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/backends/ChangeLog b/backends/ChangeLog
index 3409010..28c807a 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,8 @@
+2013-10-06  Mark Wielaard  <mjw@redhat.com>
+
+	* ppc_cfi.c (ppc_abi_cfi): Use DW_CFA_val_offset for reg1, not
+	DW_CFA_val_expression.
+
 2013-08-29  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (arm_SRCS): Add arm_cfi.c.
diff --git a/backends/ppc_cfi.c b/backends/ppc_cfi.c
index 6a4f461..55169ae 100644
--- a/backends/ppc_cfi.c
+++ b/backends/ppc_cfi.c
@@ -44,7 +44,7 @@ ppc_abi_cfi (Ebl *ebl __attribute__ ((unused)), Dwarf_CIE *abi_info)
 	 DW_CFA_def_cfa, ULEB128_7 (1), ULEB128_7 (0)  */
       /* r1 is assumed to be restored from cfa adress,
 	 r1 acts as a stack frame pointer.  */
-      DW_CFA_val_expression, ULEB128_7 (1), ULEB128_7 (1), DW_OP_nop,
+      DW_CFA_val_offset, ULEB128_7 (1), ULEB128_7 (0),
       /* lr is not callee-saved but it needs to be preserved as it is pre-set
 	 by the caller.  */
       DW_CFA_same_value, ULEB128_7 (65), /* lr */
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index e858096..960f5aa 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2013-10-06  Mark Wielaard  <mjw@redhat.com>
+
+	* cfi.c (execute_cfi): Make sure DW_CFA_expression and
+	DW_CFA_val_expression are not used with abi_cfi.
+
 2013-10-03  Josh Stone  <jistone@redhat.com>
 
 	* dwarf_formref_die.c (dwarf_formref_die): Don't hash the sig8 here.
diff --git a/libdw/cfi.c b/libdw/cfi.c
index b1e8d48..67e3dfb 100644
--- a/libdw/cfi.c
+++ b/libdw/cfi.c
@@ -252,6 +252,8 @@ execute_cfi (Dwarf_CFI *cache,
 	  continue;
 
 	case DW_CFA_expression:
+	  /* Expression rule relies on section data, abi_cfi cannot us it.  */
+	  assert (! abi_cfi);
 	  get_uleb128 (regno, program);
 	  offset = program - (const uint8_t *) cache->data->d.d_buf;
 	  /* DW_FORM_block is a ULEB128 length followed by that many bytes.  */
@@ -262,6 +264,8 @@ execute_cfi (Dwarf_CFI *cache,
 	  continue;
 
 	case DW_CFA_val_expression:
+	  /* Expression rule relies on section data, abi_cfi cannot us it.  */
+	  assert (! abi_cfi);
 	  get_uleb128 (regno, program);
 	  /* DW_FORM_block is a ULEB128 length followed by that many bytes.  */
 	  offset = program - (const uint8_t *) cache->data->d.d_buf;
diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index e81fae9..1bc0868 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,8 @@
+2013-10-06  Mark Wielaard  <mjw@redhat.com>
+
+	* libebl.h (ebl_abi_cfi): Document restrictions using register
+	rules.
+
 2013-09-26  Petr Machata  <pmachata@redhat.com>
 
 	* eblcorenotetypename.c: Handle NT_ARM_TLS, NT_ARM_HW_BREAK,
diff --git a/libebl/libebl.h b/libebl/libebl.h
index cae31c9..990167a 100644
--- a/libebl/libebl.h
+++ b/libebl/libebl.h
@@ -257,6 +257,11 @@ extern int ebl_syscall_abi (Ebl *ebl, int *sp, int *pc,
    before each CIE's initial instructions.  It should set the
    data_alignment_factor member if it affects the initial instructions.
 
+   The callback should not use the register rules DW_CFA_expression or
+   DW_CFA_val_expression.  Defining the CFA using DW_CFA_def_cfa_expression
+   is allowed.  This is an implementation detail since register rules
+   store expressions as offsets from the .eh_frame or .debug_frame data.
+
    As a shorthand for some common cases, for this instruction stream
    we overload some CFI instructions that cannot be used in a CIE:
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 71bcfc1..db248dc 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2013-10-06  Mark Wielaard  <mjw@redhat.com>
+
+	* run-addrcfi.sh: Remove nop from expected ppc and ppc64
+	location expression.
+
 2013-10-03  Josh Stone  <jistone@redhat.com>
 
 	* typeiter2.c: New file, reversing typeiter.c.
diff --git a/tests/run-addrcfi.sh b/tests/run-addrcfi.sh
index 70e85ed..c7ab71a 100755
--- a/tests/run-addrcfi.sh
+++ b/tests/run-addrcfi.sh
@@ -299,7 +299,7 @@ dwarf_cfi_addrframe (.eh_frame): no matching address range
 	return address in reg65
 	CFA location expression: bregx(1)
 	integer reg0 (r0): undefined
-	integer reg1 (r1): location expression: call_frame_cfa nop stack_value
+	integer reg1 (r1): location expression: call_frame_cfa stack_value
 	integer reg2 (r2): same_value
 	integer reg3 (r3): undefined
 	integer reg4 (r4): undefined
@@ -1326,7 +1326,7 @@ dwarf_cfi_addrframe (.eh_frame): no matching address range
 	return address in reg65
 	CFA location expression: bregx(1)
 	integer reg0 (r0): undefined
-	integer reg1 (r1): location expression: call_frame_cfa nop stack_value
+	integer reg1 (r1): location expression: call_frame_cfa stack_value
 	integer reg2 (r2): same_value
 	integer reg3 (r3): undefined
 	integer reg4 (r4): undefined
-- 
1.8.3.1