| Message ID | CABu31nPzd9NpUdzXPRpcrhMiu7jUTt0-m-ZrQ2Fh_tjZV3ecZQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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~
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.
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
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
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.