| Message ID | CAOyqgcVLk_pBFm8_yn1zPbhrmA0TZzZxrgJea63xmZDTzbs0MQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Dec 21, 2015 at 4:10 PM, Ian Lance Taylor <iant@golang.org> wrote: > The GNU linker doesn't support a zero-sized global variable that is > dynamically exported, so gccgo generates those global variables with a > size of 1 byte. Unfortunately, that means that passing a global > variable to a function that takes an argument of a zero-sized type > will actually pass 1 byte, taking up an argument slot, rather than a > zero-sized argument that should be skipped. This patch fixes the > problem in the Go frontend -> GCC interface by adding a conversion to > the real type for any such global variables, and undoing the > conversion where necessary. Bootstrapped and ran Go testsuite on > x86_64-pc-linux-gnu. Committed to mainline and gccgo branch. VIEW_CONVERT_EXPR on different size types is invalid for GCC's IR. So this patch cause other issues. Thanks, Andrew Pinski > > Ian > > 2015-12-21 Ian Lance Taylor <iant@google.com> > > * go-gcc.cc (Gcc_backend::global_variable): If type is zero-sized, > add a VIEW_CONVERT_EXPR to the tree. > (Gcc_backend::global_variable_set_init): Remove any > VIEW_CONVERT_EXPR. > (Gcc_backend::write_global_definitions): Likewise.
On Mon, Dec 21, 2015 at 4:19 PM, Andrew Pinski <pinskia@gmail.com> wrote: > On Mon, Dec 21, 2015 at 4:10 PM, Ian Lance Taylor <iant@golang.org> wrote: >> The GNU linker doesn't support a zero-sized global variable that is >> dynamically exported, so gccgo generates those global variables with a >> size of 1 byte. Unfortunately, that means that passing a global >> variable to a function that takes an argument of a zero-sized type >> will actually pass 1 byte, taking up an argument slot, rather than a >> zero-sized argument that should be skipped. This patch fixes the >> problem in the Go frontend -> GCC interface by adding a conversion to >> the real type for any such global variables, and undoing the >> conversion where necessary. Bootstrapped and ran Go testsuite on >> x86_64-pc-linux-gnu. Committed to mainline and gccgo branch. > > VIEW_CONVERT_EXPR on different size types is invalid for GCC's IR. > So this patch cause other issues. Hmmm, thanks for the pointer. Any suggestions? The constraints are that the variable has to be non-zero sized (or the GNU linker complains) and I can't entirely drop the reference to the variable (or the linker might fail to pull in an object from an archive) and the resulting expression has to be zero-sized (or the function call is miscompiled). ian
Index: go-gcc.cc =================================================================== --- go-gcc.cc (revision 231887) +++ go-gcc.cc (working copy) @@ -2390,6 +2390,7 @@ Gcc_backend::global_variable(const std:: return this->error_variable(); // The GNU linker does not like dynamic variables with zero size. + tree orig_type_tree = type_tree; if ((is_external || !is_hidden) && int_size_in_bytes(type_tree) == 0) type_tree = this->non_zero_size_type(type_tree); @@ -2419,6 +2420,10 @@ Gcc_backend::global_variable(const std:: go_preserve_from_gc(decl); + if (orig_type_tree != type_tree) + decl = fold_build1_loc(location.gcc_location(), VIEW_CONVERT_EXPR, + orig_type_tree, decl); + return new Bvariable(decl); } @@ -2434,6 +2439,10 @@ Gcc_backend::global_variable_set_init(Bv tree var_decl = var->get_tree(); if (var_decl == error_mark_node) return; + // Undo the VIEW_CONVERT_EXPR that may have been added by + // global_variable. + if (TREE_CODE(var_decl) == VIEW_CONVERT_EXPR) + var_decl = TREE_OPERAND(var_decl, 0); DECL_INITIAL(var_decl) = expr_tree; // If this variable goes in a unique section, it may need to go into @@ -3030,7 +3039,12 @@ Gcc_backend::write_global_definitions( { if ((*p)->get_tree() != error_mark_node) { - defs[i] = (*p)->get_tree(); + tree t = (*p)->get_tree(); + // Undo the VIEW_CONVERT_EXPR that may have been added by + // global_variable. + if (TREE_CODE(t) == VIEW_CONVERT_EXPR) + t = TREE_OPERAND(t, 0); + defs[i] = t; go_preserve_from_gc(defs[i]); ++i; }