* Go patch committed: Fix passing zero-sized global variable to function
@ 2015-12-22 0:10 Ian Lance Taylor
2015-12-22 0:19 ` Andrew Pinski
0 siblings, 1 reply; 4+ messages in thread
From: Ian Lance Taylor @ 2015-12-22 0:10 UTC (permalink / raw)
To: gcc-patches, gofrontend-dev
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
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.
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.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1642 bytes --]
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Go patch committed: Fix passing zero-sized global variable to function
2015-12-22 0:10 Go patch committed: Fix passing zero-sized global variable to function Ian Lance Taylor
@ 2015-12-22 0:19 ` Andrew Pinski
2015-12-22 0:23 ` Ian Lance Taylor
2015-12-22 1:59 ` Ian Lance Taylor
0 siblings, 2 replies; 4+ messages in thread
From: Andrew Pinski @ 2015-12-22 0:19 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: gcc-patches, gofrontend-dev
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Go patch committed: Fix passing zero-sized global variable to function
2015-12-22 0:19 ` Andrew Pinski
@ 2015-12-22 0:23 ` Ian Lance Taylor
2015-12-22 1:59 ` Ian Lance Taylor
1 sibling, 0 replies; 4+ messages in thread
From: Ian Lance Taylor @ 2015-12-22 0:23 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches, gofrontend-dev
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Go patch committed: Fix passing zero-sized global variable to function
2015-12-22 0:19 ` Andrew Pinski
2015-12-22 0:23 ` Ian Lance Taylor
@ 2015-12-22 1:59 ` Ian Lance Taylor
1 sibling, 0 replies; 4+ messages in thread
From: Ian Lance Taylor @ 2015-12-22 1:59 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches, gofrontend-dev
[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]
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.
Thanks again. This patch should fix that by taking a somewhat
different approach. Instead of a VIEW_CONVERT_EXPR it generates
*(orig_type*)&decl. Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu. Committed to mainline and gccgo branch.
Ian
2015-12-21 Ian Lance Taylor <iant@google.com>
* go-gcc.cc (class Bvariable): Remove Gcc_tree parent class. Add
t_ and orig_type_ fields. Add new two parameter constructor. Add
get_tree and get_decl methods.
(Gcc_backend::var_expression): Pass location to var get_tree.
(Gcc_backend::global_variable): Don't add VIEW_CONVERT_EXPR. Use
two parameter constructor for Bvariable.
(Gcc_backend::global_variable_set_init): Don't remove
VIEW_CONVERT_EXPR. Use var get_decl, not get_tree.
(Gcc_backend::write_global_definitions): Likewise.
(Gcc_backend::init_statement): Call var get_decl, not get_tree.
(Gcc_backend::block): Likewise.
(Gcc_backend::implicit_variable_set_init): Likewise.
(Gcc_backend::immutable_struct_set_init): Likewise.
(Gcc_backend::function_set_parameters): Likewise.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 5540 bytes --]
Index: go-gcc.cc
===================================================================
--- go-gcc.cc (revision 231888)
+++ go-gcc.cc (working copy)
@@ -109,22 +109,65 @@ class Bblock : public Gcc_tree
{ }
};
-class Bvariable : public Gcc_tree
+class Blabel : public Gcc_tree
{
public:
- Bvariable(tree t)
+ Blabel(tree t)
: Gcc_tree(t)
{ }
};
-class Blabel : public Gcc_tree
+// Bvariable is a bit more complicated, because of zero-sized types.
+// The GNU linker does not permit dynamic variables with zero size.
+// When we see such a variable, we generate a version of the type with
+// non-zero size. However, when referring to the global variable, we
+// want an expression of zero size; otherwise, if, say, the global
+// variable is passed to a function, we will be passing a
+// non-zero-sized value to a zero-sized value, which can lead to a
+// miscompilation.
+
+class Bvariable
{
public:
- Blabel(tree t)
- : Gcc_tree(t)
+ Bvariable(tree t)
+ : t_(t), orig_type_(NULL)
{ }
+
+ Bvariable(tree t, tree orig_type)
+ : t_(t), orig_type_(orig_type)
+ { }
+
+ // Get the tree for use as an expression.
+ tree
+ get_tree(Location) const;
+
+ // Get the actual decl;
+ tree
+ get_decl() const
+ { return this->t_; }
+
+ private:
+ tree t_;
+ tree orig_type_;
};
+// Get the tree of a variable for use as an expression. If this is a
+// zero-sized global, create an expression that refers to the decl but
+// has zero size.
+tree
+Bvariable::get_tree(Location location) const
+{
+ if (this->orig_type_ == NULL
+ || this->t_ == error_mark_node
+ || TREE_TYPE(this->t_) == this->orig_type_)
+ return this->t_;
+ // Return *(orig_type*)&decl. */
+ tree t = build_fold_addr_expr_loc(location.gcc_location(), this->t_);
+ t = fold_build1_loc(location.gcc_location(), NOP_EXPR,
+ build_pointer_type(this->orig_type_), t);
+ return build_fold_indirect_ref_loc(location.gcc_location(), t);
+}
+
// This file implements the interface between the Go frontend proper
// and the gcc IR. This implements specific instantiations of
// abstract classes defined by the Go frontend proper. The Go
@@ -1158,9 +1201,9 @@ Gcc_backend::zero_expression(Btype* btyp
// An expression that references a variable.
Bexpression*
-Gcc_backend::var_expression(Bvariable* var, Location)
+Gcc_backend::var_expression(Bvariable* var, Location location)
{
- tree ret = var->get_tree();
+ tree ret = var->get_tree(location);
if (ret == error_mark_node)
return this->error_expression();
return this->make_expression(ret);
@@ -1894,7 +1937,7 @@ Gcc_backend::expression_statement(Bexpre
Bstatement*
Gcc_backend::init_statement(Bvariable* var, Bexpression* init)
{
- tree var_tree = var->get_tree();
+ tree var_tree = var->get_decl();
tree init_tree = init->get_tree();
if (var_tree == error_mark_node || init_tree == error_mark_node)
return this->error_statement();
@@ -2264,7 +2307,7 @@ Gcc_backend::block(Bfunction* function,
pv != vars.end();
++pv)
{
- *pp = (*pv)->get_tree();
+ *pp = (*pv)->get_decl();
if (*pp != error_mark_node)
pp = &DECL_CHAIN(*pp);
}
@@ -2420,11 +2463,7 @@ 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);
+ return new Bvariable(decl, orig_type_tree);
}
// Set the initial value of a global variable.
@@ -2436,13 +2475,9 @@ Gcc_backend::global_variable_set_init(Bv
if (expr_tree == error_mark_node)
return;
gcc_assert(TREE_CONSTANT(expr_tree));
- tree var_decl = var->get_tree();
+ tree var_decl = var->get_decl();
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
@@ -2668,7 +2703,7 @@ Gcc_backend::implicit_variable_set_init(
Btype*, bool, bool, bool is_common,
Bexpression* init)
{
- tree decl = var->get_tree();
+ tree decl = var->get_decl();
tree init_tree;
if (init == NULL)
init_tree = NULL_TREE;
@@ -2762,7 +2797,7 @@ Gcc_backend::immutable_struct_set_init(B
bool, bool is_common, Btype*, Location,
Bexpression* initializer)
{
- tree decl = var->get_tree();
+ tree decl = var->get_decl();
tree init_tree = initializer->get_tree();
if (decl == error_mark_node || init_tree == error_mark_node)
return;
@@ -2981,7 +3016,7 @@ Gcc_backend::function_set_parameters(Bfu
pv != param_vars.end();
++pv)
{
- *pp = (*pv)->get_tree();
+ *pp = (*pv)->get_decl();
gcc_assert(*pp != error_mark_node);
pp = &DECL_CHAIN(*pp);
}
@@ -3037,14 +3072,10 @@ Gcc_backend::write_global_definitions(
p != variable_decls.end();
++p)
{
- if ((*p)->get_tree() != error_mark_node)
+ tree v = (*p)->get_decl();
+ if (v != error_mark_node)
{
- 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;
+ defs[i] = v;
go_preserve_from_gc(defs[i]);
++i;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-22 1:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-22 0:10 Go patch committed: Fix passing zero-sized global variable to function Ian Lance Taylor
2015-12-22 0:19 ` Andrew Pinski
2015-12-22 0:23 ` Ian Lance Taylor
2015-12-22 1:59 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).