Patchwork [ARM] Fix PR target/60504

login
register
mail settings
Submitter Eric Botcazou
Date March 25, 2014, 12:21 p.m.
Message ID <3601706.JId0R1sFEV@polaris>
Download mbox | patch
Permalink /patch/333460/
State New
Headers show

Comments

Eric Botcazou - March 25, 2014, 12:21 p.m.
Hi,

because of popular demand we switched the Ada compiler to ZCX, i.e. table-
driven EH scheme, on ARM/Linux, only to discover that GCC doesn't generate 
correct EH tables, which means that the Ada compiler cannot catch exceptions.

See also the earlier message posted by Tristan:
  http://gcc.gnu.org/ml/gcc/2013-12/msg00157.html

The problem is the discrepancy between the declared encoding and the actual 
encoding of the TType.  The latter is specified by the EABI: symbols must be 
relocated with an R_ARM_TARGET2 relocation, as visible in the assembly:

.LLSDACSE0:
	.byte	0x1	@ Action record table
	.byte	0
	.align	2
	.word	_ZTIi(TARGET2)

R_ARM_TARGET2 means DW_EH_PE_pcrel | DW_EH_PE_indirect in DWARF parlance on 
Linux, but the declared TType encoding is different:

.LLSDA0:
	.byte	0xff	@ @LPStart format (omit)
	.byte	0	@ @TType format (absolute)
	.uleb128 .LLSDATT0-.LLSDATTD0	@ @TType base offset

i.e. it's the DW_EH_PE_absptr default.

This works in C++ because of the _GLIBCXX_OVERRIDE_TTYPE_ENCODING kludge, 
which overrides the declared encoding during the parsing of the EH table:

  // Find @TType, the base of the handler and exception spec type data.
  info->ttype_encoding = *p++;
  if (info->ttype_encoding != DW_EH_PE_omit)
    {
#if _GLIBCXX_OVERRIDE_TTYPE_ENCODING
      /* Older ARM EABI toolchains set this value incorrectly, so use a
	 hardcoded OS-specific format.  */
      info->ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING;
#endif
      p = read_uleb128 (p, &tmp);
      info->TType = p + tmp;
    }
  else
    info->TType = 0;

This comes from http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00765.html

Note that the above comment suggests that Paul's intent was to fix the bogus 
declared encoding in newer toolchains, but he didn't because of a misplaced 
#endif as spotted by Tristan.  Hence the attached patch, which aligns the 
declared encoding with the actual encoding and yields the correct:

.LLSDA0:
	.byte	0xff	@ @LPStart format (omit)
	.byte	0x90	@ @TType format (indirect pcrel)
	.uleb128 .LLSDATT0-.LLSDATTD0	@ @TType base offset

which fixes the Ada failures in the process.  It was kindly tested by Bernd 
Edlinger on armv7l-unknown-linux-gnueabihf (all languages) with no regressions 
so we would like to have it for 4.9.x.


2014-04-25  Douglas B Rupp  <rupp@adacore.com>

	PR target/60504
	* config/arm/arm.h (ASM_PREFERRED_EH_DATA_FORMAT): Expose from
	ARM_TARGET2_DWARF_FORMAT.



2014-04-25  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/test_raise_from_pure.adb: UnXFAIL for ARM.
Ramana Radhakrishnan - April 7, 2014, 2:52 p.m.
Sorry about the delayed review. I seem to have missed this earlier.

On Tue, Mar 25, 2014 at 12:21 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> because of popular demand we switched the Ada compiler to ZCX, i.e. table-
> driven EH scheme, on ARM/Linux, only to discover that GCC doesn't generate
> correct EH tables, which means that the Ada compiler cannot catch exceptions.

>
> See also the earlier message posted by Tristan:
>   http://gcc.gnu.org/ml/gcc/2013-12/msg00157.html
>
> The problem is the discrepancy between the declared encoding and the actual
> encoding of the TType.  The latter is specified by the EABI: symbols must be
> relocated with an R_ARM_TARGET2 relocation, as visible in the assembly:
>
> .LLSDACSE0:
>         .byte   0x1     @ Action record table
>         .byte   0
>         .align  2
>         .word   _ZTIi(TARGET2)
>
> R_ARM_TARGET2 means DW_EH_PE_pcrel | DW_EH_PE_indirect in DWARF parlance on
> Linux, but the declared TType encoding is different:
>
> .LLSDA0:
>         .byte   0xff    @ @LPStart format (omit)
>         .byte   0       @ @TType format (absolute)
>         .uleb128 .LLSDATT0-.LLSDATTD0   @ @TType base offset
>
> i.e. it's the DW_EH_PE_absptr default.
>
> This works in C++ because of the _GLIBCXX_OVERRIDE_TTYPE_ENCODING kludge,
> which overrides the declared encoding during the parsing of the EH table:

Ok and I suspect the kludge has to continue for some more time.

>
>   // Find @TType, the base of the handler and exception spec type data.
>   info->ttype_encoding = *p++;
>   if (info->ttype_encoding != DW_EH_PE_omit)
>     {
> #if _GLIBCXX_OVERRIDE_TTYPE_ENCODING
>       /* Older ARM EABI toolchains set this value incorrectly, so use a
>          hardcoded OS-specific format.  */
>       info->ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING;
> #endif
>       p = read_uleb128 (p, &tmp);
>       info->TType = p + tmp;
>     }
>   else
>     info->TType = 0;
>
> This comes from http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00765.html

>
> Note that the above comment suggests that Paul's intent was to fix the bogus
> declared encoding in newer toolchains, but he didn't because of a misplaced
> #endif as spotted by Tristan.  Hence the attached patch, which aligns the
> declared encoding with the actual encoding and yields the correct:
>
> .LLSDA0:
>         .byte   0xff    @ @LPStart format (omit)
>         .byte   0x90    @ @TType format (indirect pcrel)
>         .uleb128 .LLSDATT0-.LLSDATTD0   @ @TType base offset
>
> which fixes the Ada failures in the process.  It was kindly tested by Bernd
> Edlinger on armv7l-unknown-linux-gnueabihf (all languages) with no regressions
> so we would like to have it for 4.9.x.

Ok if no regressions.

Ramana


>
>
> 2014-04-25  Douglas B Rupp  <rupp@adacore.com>
>
>         PR target/60504
>         * config/arm/arm.h (ASM_PREFERRED_EH_DATA_FORMAT): Expose from
>         ARM_TARGET2_DWARF_FORMAT.
>
>
>
> 2014-04-25  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/test_raise_from_pure.adb: UnXFAIL for ARM.
>
>
> --
> Eric Botcazou

Patch

Index: config/arm/arm.h
===================================================================
--- config/arm/arm.h	(revision 208763)
+++ config/arm/arm.h	(working copy)
@@ -937,13 +937,13 @@  extern int arm_arch_crc;
 
 #ifndef ARM_TARGET2_DWARF_FORMAT
 #define ARM_TARGET2_DWARF_FORMAT DW_EH_PE_pcrel
+#endif
 
 /* ttype entries (the only interesting data references used)
    use TARGET2 relocations.  */
 #define ASM_PREFERRED_EH_DATA_FORMAT(code, data) \
   (((code) == 0 && (data) == 1 && ARM_UNWIND_INFO) ? ARM_TARGET2_DWARF_FORMAT \
 			       : DW_EH_PE_absptr)
-#endif
 
 /* The native (Norcroft) Pascal compiler for the ARM passes the static chain
    as an invisible last argument (possible since varargs don't exist in
Index: testsuite/gnat.dg/test_raise_from_pure.adb
===================================================================
--- testsuite/gnat.dg/test_raise_from_pure.adb	(revision 208763)
+++ testsuite/gnat.dg/test_raise_from_pure.adb	(working copy)
@@ -1,4 +1,4 @@ 
--- { dg-do run { xfail arm*-*-* } }
+-- { dg-do run }
 -- { dg-options "-O2" }
 
 -- This is an optimization test and its failure is only a missed optimization.