| Message ID | 0be9bd9b-4efb-47da-e4ce-2c8ccb6f7f9a@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix Fortran size_t parameter passing | expand |
On Wed, May 22, 2019 at 1:54 PM Andrew Stubbs <ams@codesourcery.com> wrote: > > This patch fixes a bug observed on amdgcn in which the Fortran frontend > creates function calls using the 32-bit parameters where they ought to > be 64-bit, resulting in UB. > > The issue is caused by the use of "integer_zero_node" where the > definition of the function calls for "size_zero_node". I presume this > works on other architectures because the types are the same size, or > else because parameters are always 64-bit wide. > > OK to commit? > > Andrew Thanks for the catch. Though for size_t you should use build_zero_cst (size_type_node). size_zero_node is a zero constant of type sizetype, which is not the same as size_type_node (size_t in C). Ok with that change.
On 22/05/2019 12:35, Janne Blomqvist wrote: > Thanks for the catch. Though for size_t you should use build_zero_cst > (size_type_node). size_zero_node is a zero constant of type sizetype, > which is not the same as size_type_node (size_t in C). Ok with that > change. So, integer_zero_node is compatible with integer_type_node, but size_zero_node is not (necessarily) compatible with size_type_node? Well, that's just asking for trouble. :-( Just to confirm, is the attached what you mean? Thanks Andrew
On Wed, May 22, 2019 at 3:20 PM Andrew Stubbs <ams@codesourcery.com> wrote: > > On 22/05/2019 12:35, Janne Blomqvist wrote: > > Thanks for the catch. Though for size_t you should use build_zero_cst > > (size_type_node). size_zero_node is a zero constant of type sizetype, > > which is not the same as size_type_node (size_t in C). Ok with that > > change. > > So, integer_zero_node is compatible with integer_type_node, but > size_zero_node is not (necessarily) compatible with size_type_node? > Well, that's just asking for trouble. :-( Indeed it is. IIRC the main difference is that while both are unsigned types, sizetype has undefined behavior on overflow whereas size_type_node wraps around. And sizetype is an internal implementation detail, so should not be used in ABI-visible places. > Just to confirm, is the attached what you mean? Yes, looks good.
On 22/05/2019 13:28, Janne Blomqvist wrote: >> Just to confirm, is the attached what you mean? > > Yes, looks good. Thanks, now committed. Andrew
Fix fortran size_t parameter passing. 2019-05-22 Andrew Stubbs <ams@codesourcery.com> gcc/fortran/ * trans-stmt.c (gfc_trans_critical): Use size_zero_node for gfor_fndecl_caf_lock and gfor_fndecl_caf_unlock calls. (gfc_trans_allocate): Use size_zero_node for gfor_fndecl_caf_sync_all call. diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 5fa182bf05a..4314a1edb90 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -1579,9 +1579,9 @@ gfc_trans_critical (gfc_code *code) token = gfc_get_symbol_decl (code->resolved_sym); token = GFC_TYPE_ARRAY_CAF_TOKEN (TREE_TYPE (token)); tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_lock, 7, - token, integer_zero_node, integer_one_node, + token, size_zero_node, integer_one_node, null_pointer_node, null_pointer_node, - null_pointer_node, integer_zero_node); + null_pointer_node, size_zero_node); gfc_add_expr_to_block (&block, tmp); /* It guarantees memory consistency within the same segment */ @@ -1602,9 +1602,9 @@ gfc_trans_critical (gfc_code *code) if (flag_coarray == GFC_FCOARRAY_LIB) { tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_unlock, 6, - token, integer_zero_node, integer_one_node, + token, size_zero_node, integer_one_node, null_pointer_node, null_pointer_node, - integer_zero_node); + size_zero_node); gfc_add_expr_to_block (&block, tmp); /* It guarantees memory consistency within the same segment */ @@ -6774,7 +6774,7 @@ gfc_trans_allocate (gfc_code * code) /* Add a sync all after the allocation has been executed. */ tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all, 3, null_pointer_node, null_pointer_node, - integer_zero_node); + size_zero_node); gfc_add_expr_to_block (&post, tmp); }