Patchwork For alpha-vms, unset flag_jump_tables if flag_pic is nonzero

login
register
mail settings
Submitter Steven Bosscher
Date May 2, 2012, 7:12 p.m.
Message ID <CABu31nPzd9NpUdzXPRpcrhMiu7jUTt0-m-ZrQ2Fh_tjZV3ecZQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/156538/
State New
Headers show

Comments

Steven Bosscher - May 2, 2012, 7:12 p.m.
Hello,

alpha-vms is the only target that does not define
ASM_OUTPUT_ADDR_DIFF_ELT. That makes the code in stmt.c to handle this
case alpha-vms specific. But there is a better way to handle this:
Just mimic -fno-jump-tables if flag_pic is nonzero.

Tested by building a cross-compiler for alpha-dec-vms and verifying
that flag_jump_tables==0 in expand_case.
OK for trunk?

Ciao!
Steven


        * config/alpha/vms.h (SUBTARGET_OVERRIDE_OPTIONS): For pic code,
        unset flag_jump_tables.
        * stmt.c (expand_case): Remove special flag_pic case conditional
        on ASM_OUTPUT_ADDR_DIFF_ELT not being defined.
Steven Bosscher - May 2, 2012, 7:26 p.m.
On Wed, May 2, 2012 at 9:12 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> Hello,
>
> alpha-vms is the only target that does not define
> ASM_OUTPUT_ADDR_DIFF_ELT. That makes the code in stmt.c to handle this
> case alpha-vms specific. But there is a better way to handle this:
> Just mimic -fno-jump-tables if flag_pic is nonzero.
>
> Tested by building a cross-compiler for alpha-dec-vms and verifying
> that flag_jump_tables==0 in expand_case.
> OK for trunk?
>
> Ciao!
> Steven
>
>
>        * config/alpha/vms.h (SUBTARGET_OVERRIDE_OPTIONS): For pic code,
>        unset flag_jump_tables.
>        * stmt.c (expand_case): Remove special flag_pic case conditional
>        on ASM_OUTPUT_ADDR_DIFF_ELT not being defined.
>
> Index: config/alpha/vms.h
> ===================================================================
> --- config/alpha/vms.h  (revision 187046)
> +++ config/alpha/vms.h  (working copy)
> @@ -257,7 +257,15 @@ typedef struct {int num_args; enum avms_
>  #undef ASM_FINAL_SPEC
>
>  /* The VMS convention is to always provide minimal debug info
> -   for a traceback unless specifically overridden.  */
> +   for a traceback unless specifically overridden.
> +
> +   Because ASM_OUTPUT_ADDR_DIFF_ELT is not defined for alpha-vms,
> +   jump tables cannot be output for PIC code, because you can't put
> +   an absolute address in a readonly section.  Putting the table in
> +   a writable section is a security hole.  Therefore, we unset the
> +   flag_jump_tables flag, forcing switch statements to be expanded
> +   using decision trees.  There are probably other ways to address
> +   this issue, but using a decision tree is clearly safe.  */
>
>  #undef SUBTARGET_OVERRIDE_OPTIONS
>  #define SUBTARGET_OVERRIDE_OPTIONS                  \
> @@ -268,6 +276,8 @@ do {
>       write_symbols = VMS_DEBUG;                    \
>       debug_info_level = DINFO_LEVEL_TERSE;         \
>     }                                               \
> +  if (flag_pic == 1)                                \
> +      flag_jump_tables = 0;                         \
>  } while (0)

Obviously the above should instead be:

@@ -268,6 +276,8 @@ do {
       write_symbols = VMS_DEBUG;                    \
       debug_info_level = DINFO_LEVEL_TERSE;         \
     }                                               \
+  if (flag_pic)                                     \
+    flag_jump_tables = 0;                           \
 } while (0)

 #undef LINK_SPEC


>  #undef LINK_SPEC
> Index: stmt.c
> ===================================================================
> --- stmt.c      (revision 187046)
> +++ stmt.c      (working copy)
> @@ -2198,9 +2198,6 @@ expand_case (gimple stmt)
>               /* RANGE may be signed, and really large ranges will show up
>                  as negative numbers.  */
>               || compare_tree_int (range, 0) < 0
> -#ifndef ASM_OUTPUT_ADDR_DIFF_ELT
> -              || flag_pic
> -#endif
>               || !flag_jump_tables
>               || TREE_CONSTANT (index_expr)
>               /* If neither casesi or tablejump is available, we can
Richard Henderson - May 2, 2012, 9:45 p.m.
On 05/02/2012 12:26 PM, Steven Bosscher wrote:
>>          * config/alpha/vms.h (SUBTARGET_OVERRIDE_OPTIONS): For pic code,
>>          unset flag_jump_tables.
>>          * stmt.c (expand_case): Remove special flag_pic case conditional
>>          on ASM_OUTPUT_ADDR_DIFF_ELT not being defined.

Ok.

I had a brief look at the vms file format.  It looks to me as if the lack
of A_O_A_D_E is a defect in gas; it ought to be representable in the object
file as a difference of two psect+offset addresses.


r~
Tristan Gingold - May 3, 2012, 7:42 a.m.
On May 2, 2012, at 11:45 PM, Richard Henderson wrote:

> On 05/02/2012 12:26 PM, Steven Bosscher wrote:
>>>         * config/alpha/vms.h (SUBTARGET_OVERRIDE_OPTIONS): For pic code,
>>>         unset flag_jump_tables.
>>>         * stmt.c (expand_case): Remove special flag_pic case conditional
>>>         on ASM_OUTPUT_ADDR_DIFF_ELT not being defined.
> 
> Ok.
> 
> I had a brief look at the vms file format.  It looks to me as if the lack
> of A_O_A_D_E is a defect in gas; it ought to be representable in the object
> file as a difference of two psect+offset addresses.

Agreed.  VMS relocations are much more expressive than standard ELF, so a difference should be supported.

Tristan.
Tristan Gingold - May 3, 2012, 7:48 a.m.
On May 2, 2012, at 9:12 PM, Steven Bosscher wrote:

> Hello,
> 
> alpha-vms is the only target that does not define
> ASM_OUTPUT_ADDR_DIFF_ELT. That makes the code in stmt.c to handle this
> case alpha-vms specific. But there is a better way to handle this:
> Just mimic -fno-jump-tables if flag_pic is nonzero.
> 
> Tested by building a cross-compiler for alpha-dec-vms and verifying
> that flag_jump_tables==0 in expand_case.
> OK for trunk?

-fpic is never used on VMS as the 'calling standard' makes it useless: code
is always position independent.  In the jump table cases, at worst relocations
entries will be generated by the linker for them.

Tristan.

> 
> Ciao!
> Steven
> 
> 
>        * config/alpha/vms.h (SUBTARGET_OVERRIDE_OPTIONS): For pic code,
>        unset flag_jump_tables.
>        * stmt.c (expand_case): Remove special flag_pic case conditional
>        on ASM_OUTPUT_ADDR_DIFF_ELT not being defined.
> 
> Index: config/alpha/vms.h
> ===================================================================
> --- config/alpha/vms.h  (revision 187046)
> +++ config/alpha/vms.h  (working copy)
> @@ -257,7 +257,15 @@ typedef struct {int num_args; enum avms_
> #undef ASM_FINAL_SPEC
> 
> /* The VMS convention is to always provide minimal debug info
> -   for a traceback unless specifically overridden.  */
> +   for a traceback unless specifically overridden.
> +
> +   Because ASM_OUTPUT_ADDR_DIFF_ELT is not defined for alpha-vms,
> +   jump tables cannot be output for PIC code, because you can't put
> +   an absolute address in a readonly section.  Putting the table in
> +   a writable section is a security hole.  Therefore, we unset the
> +   flag_jump_tables flag, forcing switch statements to be expanded
> +   using decision trees.  There are probably other ways to address
> +   this issue, but using a decision tree is clearly safe.  */
> 
> #undef SUBTARGET_OVERRIDE_OPTIONS
> #define SUBTARGET_OVERRIDE_OPTIONS                  \
> @@ -268,6 +276,8 @@ do {
>       write_symbols = VMS_DEBUG;                    \
>       debug_info_level = DINFO_LEVEL_TERSE;         \
>     }                                               \
> +  if (flag_pic == 1)                                \
> +      flag_jump_tables = 0;                         \
> } while (0)
> 
> #undef LINK_SPEC
> Index: stmt.c
> ===================================================================
> --- stmt.c      (revision 187046)
> +++ stmt.c      (working copy)
> @@ -2198,9 +2198,6 @@ expand_case (gimple stmt)
>               /* RANGE may be signed, and really large ranges will show up
>                  as negative numbers.  */
>               || compare_tree_int (range, 0) < 0
> -#ifndef ASM_OUTPUT_ADDR_DIFF_ELT
> -              || flag_pic
> -#endif
>               || !flag_jump_tables
>               || TREE_CONSTANT (index_expr)
>               /* If neither casesi or tablejump is available, we can

Patch

Index: config/alpha/vms.h
===================================================================
--- config/alpha/vms.h  (revision 187046)
+++ config/alpha/vms.h  (working copy)
@@ -257,7 +257,15 @@  typedef struct {int num_args; enum avms_
 #undef ASM_FINAL_SPEC

 /* The VMS convention is to always provide minimal debug info
-   for a traceback unless specifically overridden.  */
+   for a traceback unless specifically overridden.
+
+   Because ASM_OUTPUT_ADDR_DIFF_ELT is not defined for alpha-vms,
+   jump tables cannot be output for PIC code, because you can't put
+   an absolute address in a readonly section.  Putting the table in
+   a writable section is a security hole.  Therefore, we unset the
+   flag_jump_tables flag, forcing switch statements to be expanded
+   using decision trees.  There are probably other ways to address
+   this issue, but using a decision tree is clearly safe.  */

 #undef SUBTARGET_OVERRIDE_OPTIONS
 #define SUBTARGET_OVERRIDE_OPTIONS                  \
@@ -268,6 +276,8 @@  do {
       write_symbols = VMS_DEBUG;                    \
       debug_info_level = DINFO_LEVEL_TERSE;         \
     }                                               \
+  if (flag_pic == 1)                                \
+      flag_jump_tables = 0;                         \
 } while (0)

 #undef LINK_SPEC
Index: stmt.c
===================================================================
--- stmt.c      (revision 187046)
+++ stmt.c      (working copy)
@@ -2198,9 +2198,6 @@  expand_case (gimple stmt)
               /* RANGE may be signed, and really large ranges will show up
                  as negative numbers.  */
               || compare_tree_int (range, 0) < 0
-#ifndef ASM_OUTPUT_ADDR_DIFF_ELT
-              || flag_pic
-#endif
               || !flag_jump_tables
               || TREE_CONSTANT (index_expr)
               /* If neither casesi or tablejump is available, we can