| Message ID | 20130826101509.GH4968@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 26, 2013 at 12:15 PM, Marek Polacek <polacek@redhat.com> wrote: > I noticed I forgot to apply this old patch, already acked by Jason. > It introduces new pointer_sized_int_node, thus we can get rid of > uptr_type function in ubsan, and it allows us to do some clean-up > in asan.c, too. > > Tested x86_64-linux, applying to ubsan branch. > > 2013-08-26 Marek Polacek <polacek@redhat.com> > > * tree.h (enum tree_index): Add TI_POINTER_SIZED_TYPE. > (pointer_sized_int_node): Define. > * tree.c (build_common_tree_nodes): Initialize > pointer_sized_int_node. > * ubsan.c (ubsan_encode_value): Use pointer_sized_int_node instead > calling uptr_type. > (uptr_type): Remove function. > * asan.c (asan_global_struct): Use pointer_sized_int_node instead > calling build_nonstandard_integer_type. > (initialize_sanitizer_builtins): Likewise. > (asan_finish_file): Likewise. > > --- gcc/tree.c.mp 2013-07-21 19:54:35.416986756 +0200 > +++ gcc/tree.c 2013-07-21 19:56:58.347562787 +0200 > @@ -9638,6 +9638,8 @@ build_common_tree_nodes (bool signed_cha > = build_pointer_type (build_type_variant (void_type_node, 1, 0)); > fileptr_type_node = ptr_type_node; > > + pointer_sized_int_node = build_nonstandard_integer_type (POINTER_SIZE, 1); > + > float_type_node = make_node (REAL_TYPE); > TYPE_PRECISION (float_type_node) = FLOAT_TYPE_SIZE; > layout_type (float_type_node); > --- gcc/ubsan.c.mp 2013-07-21 20:04:59.469653493 +0200 > +++ gcc/ubsan.c 2013-07-21 20:07:00.227178083 +0200 > @@ -123,14 +123,6 @@ ubsan_typedesc_new (tree type, tree decl > return desc; > } > > -/* Build the ubsan uptr type. */ > - > -static tree > -uptr_type (void) > -{ > - return build_nonstandard_integer_type (POINTER_SIZE, 1); > -} > - > /* Helper routine, which encodes a value in the uptr type. > Arguments with precision <= POINTER_SIZE are passed directly, > the rest is passed by reference. T is a value we are to encode. */ > @@ -143,7 +135,7 @@ ubsan_encode_value (tree t) > { > case INTEGER_TYPE: > if (TYPE_PRECISION (type) <= POINTER_SIZE) > - return fold_build1 (NOP_EXPR, uptr_type (), t); > + return fold_build1 (NOP_EXPR, pointer_sized_int_node, t); > else > return build_fold_addr_expr (t); > case REAL_TYPE: > @@ -153,7 +145,7 @@ ubsan_encode_value (tree t) > { > tree itype = build_nonstandard_integer_type (bitsize, true); > t = fold_build1 (VIEW_CONVERT_EXPR, itype, t); > - return fold_convert (uptr_type (), t); > + return fold_convert (pointer_sized_int_node, t); > } > else > { > --- gcc/tree.h.mp 2013-07-21 19:54:35.441986868 +0200 > +++ gcc/tree.h 2013-07-21 19:56:05.128353854 +0200 > @@ -4227,6 +4227,7 @@ enum tree_index > TI_VA_LIST_FPR_COUNTER_FIELD, > TI_BOOLEAN_TYPE, > TI_FILEPTR_TYPE, > + TI_POINTER_SIZED_TYPE, I'd rather see TI_UINTPTR_TYPE and TI_INTPTR_TYPE (note they might not be exactly of POINTER_SIZE but larger). TI_POINTER_SIZED_TYPE is ambiguous, too - what's its signedness? All around the compiler we use sizetype and ssizetype to munge pointers (well, not strictly correct as targets may define sizetype to be larger/smaller than actual pointers). Richard. > > TI_DFLOAT32_TYPE, > TI_DFLOAT64_TYPE, > @@ -4383,6 +4384,7 @@ extern GTY(()) tree global_trees[TI_MAX] > #define va_list_fpr_counter_field global_trees[TI_VA_LIST_FPR_COUNTER_FIELD] > /* The C type `FILE *'. */ > #define fileptr_type_node global_trees[TI_FILEPTR_TYPE] > +#define pointer_sized_int_node global_trees[TI_POINTER_SIZED_TYPE] > > #define boolean_type_node global_trees[TI_BOOLEAN_TYPE] > #define boolean_false_node global_trees[TI_BOOLEAN_FALSE] > --- gcc/asan.c.mp 2013-07-21 20:07:15.013237456 +0200 > +++ gcc/asan.c 2013-07-21 20:16:10.929376734 +0200 > @@ -1954,7 +1954,7 @@ asan_global_struct (void) > = build_decl (UNKNOWN_LOCATION, FIELD_DECL, > get_identifier (field_names[i]), > (i == 0 || i == 3) ? const_ptr_type_node > - : build_nonstandard_integer_type (POINTER_SIZE, 1)); > + : pointer_sized_int_node); > DECL_CONTEXT (fields[i]) = ret; > if (i) > DECL_CHAIN (fields[i - 1]) = fields[i]; > @@ -2039,8 +2039,7 @@ initialize_sanitizer_builtins (void) > ptr_type_node, ptr_type_node, NULL_TREE); > tree BT_FN_VOID_PTR_PTRMODE > = build_function_type_list (void_type_node, ptr_type_node, > - build_nonstandard_integer_type (POINTER_SIZE, > - 1), NULL_TREE); > + pointer_sized_int_node, NULL_TREE); > tree BT_FN_VOID_INT > = build_function_type_list (void_type_node, integer_type_node, NULL_TREE); > tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5]; > @@ -2197,7 +2196,6 @@ asan_finish_file (void) > if (gcount) > { > tree type = asan_global_struct (), var, ctor; > - tree uptr = build_nonstandard_integer_type (POINTER_SIZE, 1); > tree dtor_statements = NULL_TREE; > vec<constructor_elt, va_gc> *v; > char buf[20]; > @@ -2226,15 +2224,16 @@ asan_finish_file (void) > varpool_assemble_decl (varpool_node_for_decl (var)); > > fn = builtin_decl_implicit (BUILT_IN_ASAN_REGISTER_GLOBALS); > + tree gcount_tree = build_int_cst (pointer_sized_int_node, gcount); > append_to_statement_list (build_call_expr (fn, 2, > build_fold_addr_expr (var), > - build_int_cst (uptr, gcount)), > + gcount_tree), > &asan_ctor_statements); > > fn = builtin_decl_implicit (BUILT_IN_ASAN_UNREGISTER_GLOBALS); > append_to_statement_list (build_call_expr (fn, 2, > build_fold_addr_expr (var), > - build_int_cst (uptr, gcount)), > + gcount_tree), > &dtor_statements); > cgraph_build_static_cdtor ('D', dtor_statements, > MAX_RESERVED_INIT_PRIORITY - 1); > > Marek
On Tue, Aug 27, 2013 at 01:48:29PM +0200, Richard Biener wrote: > > + TI_POINTER_SIZED_TYPE, > > I'd rather see TI_UINTPTR_TYPE and TI_INTPTR_TYPE (note they might > not be exactly of POINTER_SIZE but larger). We already have [u]intptr_type_node -- but only in c-family/, thus ubsan.c/asan.c cannot use those nodes. I can create both TI_SIGNED_POINTER_SIZED_TYPE and TI_UNSIGNED_POINTER_SIZED_TYPE, but we currently need only the latter... > TI_POINTER_SIZED_TYPE is ambiguous, too - what's its signedness? Unsigned. But yeah, one can't tell by just looking at the name. > All around the compiler we use sizetype and ssizetype to munge pointers > (well, not strictly correct as targets may define sizetype to be larger/smaller > than actual pointers). I see. Marek
On Tue, Aug 27, 2013 at 2:56 PM, Marek Polacek <polacek@redhat.com> wrote: > On Tue, Aug 27, 2013 at 01:48:29PM +0200, Richard Biener wrote: >> > + TI_POINTER_SIZED_TYPE, >> >> I'd rather see TI_UINTPTR_TYPE and TI_INTPTR_TYPE (note they might >> not be exactly of POINTER_SIZE but larger). > > We already have [u]intptr_type_node -- but only in c-family/, thus > ubsan.c/asan.c cannot use those nodes. I can create both > TI_SIGNED_POINTER_SIZED_TYPE and TI_UNSIGNED_POINTER_SIZED_TYPE, > but we currently need only the latter... So simply move them to the middle-end. The set of C language types that define the target ABI should be constructed and maintained by the middle-end (you need it for various builtins anyway) Richard. >> TI_POINTER_SIZED_TYPE is ambiguous, too - what's its signedness? > > Unsigned. But yeah, one can't tell by just looking at the name. > >> All around the compiler we use sizetype and ssizetype to munge pointers >> (well, not strictly correct as targets may define sizetype to be larger/smaller >> than actual pointers). > > I see. > > Marek
On Wed, Aug 28, 2013 at 12:48:41PM +0200, Richard Biener wrote: > On Tue, Aug 27, 2013 at 2:56 PM, Marek Polacek <polacek@redhat.com> wrote: > > On Tue, Aug 27, 2013 at 01:48:29PM +0200, Richard Biener wrote: > >> > + TI_POINTER_SIZED_TYPE, > >> > >> I'd rather see TI_UINTPTR_TYPE and TI_INTPTR_TYPE (note they might > >> not be exactly of POINTER_SIZE but larger). > > > > We already have [u]intptr_type_node -- but only in c-family/, thus > > ubsan.c/asan.c cannot use those nodes. I can create both > > TI_SIGNED_POINTER_SIZED_TYPE and TI_UNSIGNED_POINTER_SIZED_TYPE, > > but we currently need only the latter... > > So simply move them to the middle-end. The set of C language types that > define the target ABI should be constructed and maintained by the middle-end > (you need it for various builtins anyway) That is not easily possible. The thing is, in the C FEs they are created from the C/C++ type name (for stdint purposes): if (INTPTR_TYPE) intptr_type_node = TREE_TYPE (identifier_global_value (c_get_ident (INTPTR_TYPE))); if (UINTPTR_TYPE) uintptr_type_node = TREE_TYPE (identifier_global_value (c_get_ident (UINTPTR_TYPE))); and are supposed to match the stdint.h previously used type, because it is part of ABI etc. (long vs. long long vs. int e.g. when two of these are the same precision). So the middle-end uintptr type needs to be something different, while it ideally should have the same precision, it is not the same language type. Jakub
On Wed, Aug 28, 2013 at 1:10 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Aug 28, 2013 at 12:48:41PM +0200, Richard Biener wrote: >> On Tue, Aug 27, 2013 at 2:56 PM, Marek Polacek <polacek@redhat.com> wrote: >> > On Tue, Aug 27, 2013 at 01:48:29PM +0200, Richard Biener wrote: >> >> > + TI_POINTER_SIZED_TYPE, >> >> >> >> I'd rather see TI_UINTPTR_TYPE and TI_INTPTR_TYPE (note they might >> >> not be exactly of POINTER_SIZE but larger). >> > >> > We already have [u]intptr_type_node -- but only in c-family/, thus >> > ubsan.c/asan.c cannot use those nodes. I can create both >> > TI_SIGNED_POINTER_SIZED_TYPE and TI_UNSIGNED_POINTER_SIZED_TYPE, >> > but we currently need only the latter... >> >> So simply move them to the middle-end. The set of C language types that >> define the target ABI should be constructed and maintained by the middle-end >> (you need it for various builtins anyway) > > That is not easily possible. The thing is, in the C FEs they are created > from the C/C++ type name (for stdint purposes): > if (INTPTR_TYPE) > intptr_type_node = > TREE_TYPE (identifier_global_value (c_get_ident (INTPTR_TYPE))); > if (UINTPTR_TYPE) > uintptr_type_node = > TREE_TYPE (identifier_global_value (c_get_ident (UINTPTR_TYPE))); > and are supposed to match the stdint.h previously used type, because > it is part of ABI etc. (long vs. long long vs. int e.g. when two of these > are the same precision). > So the middle-end uintptr type needs to be something different, while it > ideally should have the same precision, it is not the same language type. But it's the C ABI type - and we do need the C ABI types from within the middle-end. Joseph, any idea? Richard. > Jakub
On Wed, 28 Aug 2013, Richard Biener wrote: > > That is not easily possible. The thing is, in the C FEs they are created > > from the C/C++ type name (for stdint purposes): > > if (INTPTR_TYPE) > > intptr_type_node = > > TREE_TYPE (identifier_global_value (c_get_ident (INTPTR_TYPE))); > > if (UINTPTR_TYPE) > > uintptr_type_node = > > TREE_TYPE (identifier_global_value (c_get_ident (UINTPTR_TYPE))); > > and are supposed to match the stdint.h previously used type, because > > it is part of ABI etc. (long vs. long long vs. int e.g. when two of these > > are the same precision). > > So the middle-end uintptr type needs to be something different, while it > > ideally should have the same precision, it is not the same language type. > > But it's the C ABI type - and we do need the C ABI types from within > the middle-end. Joseph, any idea? The possible issues that I see are: * Types getting determined from strings (that are the values of macros such as INTPTR_TYPE) by the front ends. This is no real problem, in that the Fortran front end already emulates what's required in get_typenode_from_name. Of course it would be cleaner for all these macros to be defined to evaluate to enum values rather than strings, so the middle-end doesn't need such emulation of C type name parsing. See the thread starting at <http://gcc.gnu.org/ml/gcc-patches/2010-12/msg00964.html> for a conversion to using enum values for the stdint.h macros - that could be updated for current trunk. Of course the aim would be for other macros such as SIZE_TYPE to be converted as well, and indeed to get a conversion to hooks (probably a few separate hooks for different groups of types, taking appropriate enum values as arguments, rather than just one for all the typedefs or a separate hook for every typedef). * Targets still not having the type information in GCC. As I said in <http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00248.html>, listing those targets, it's probably time to deprecate them (and then freely check in changes that break them by requiring those types to be defined).
--- gcc/tree.c.mp 2013-07-21 19:54:35.416986756 +0200 +++ gcc/tree.c 2013-07-21 19:56:58.347562787 +0200 @@ -9638,6 +9638,8 @@ build_common_tree_nodes (bool signed_cha = build_pointer_type (build_type_variant (void_type_node, 1, 0)); fileptr_type_node = ptr_type_node; + pointer_sized_int_node = build_nonstandard_integer_type (POINTER_SIZE, 1); + float_type_node = make_node (REAL_TYPE); TYPE_PRECISION (float_type_node) = FLOAT_TYPE_SIZE; layout_type (float_type_node); --- gcc/ubsan.c.mp 2013-07-21 20:04:59.469653493 +0200 +++ gcc/ubsan.c 2013-07-21 20:07:00.227178083 +0200 @@ -123,14 +123,6 @@ ubsan_typedesc_new (tree type, tree decl return desc; } -/* Build the ubsan uptr type. */ - -static tree -uptr_type (void) -{ - return build_nonstandard_integer_type (POINTER_SIZE, 1); -} - /* Helper routine, which encodes a value in the uptr type. Arguments with precision <= POINTER_SIZE are passed directly, the rest is passed by reference. T is a value we are to encode. */ @@ -143,7 +135,7 @@ ubsan_encode_value (tree t) { case INTEGER_TYPE: if (TYPE_PRECISION (type) <= POINTER_SIZE) - return fold_build1 (NOP_EXPR, uptr_type (), t); + return fold_build1 (NOP_EXPR, pointer_sized_int_node, t); else return build_fold_addr_expr (t); case REAL_TYPE: @@ -153,7 +145,7 @@ ubsan_encode_value (tree t) { tree itype = build_nonstandard_integer_type (bitsize, true); t = fold_build1 (VIEW_CONVERT_EXPR, itype, t); - return fold_convert (uptr_type (), t); + return fold_convert (pointer_sized_int_node, t); } else { --- gcc/tree.h.mp 2013-07-21 19:54:35.441986868 +0200 +++ gcc/tree.h 2013-07-21 19:56:05.128353854 +0200 @@ -4227,6 +4227,7 @@ enum tree_index TI_VA_LIST_FPR_COUNTER_FIELD, TI_BOOLEAN_TYPE, TI_FILEPTR_TYPE, + TI_POINTER_SIZED_TYPE, TI_DFLOAT32_TYPE, TI_DFLOAT64_TYPE, @@ -4383,6 +4384,7 @@ extern GTY(()) tree global_trees[TI_MAX] #define va_list_fpr_counter_field global_trees[TI_VA_LIST_FPR_COUNTER_FIELD] /* The C type `FILE *'. */ #define fileptr_type_node global_trees[TI_FILEPTR_TYPE] +#define pointer_sized_int_node global_trees[TI_POINTER_SIZED_TYPE] #define boolean_type_node global_trees[TI_BOOLEAN_TYPE] #define boolean_false_node global_trees[TI_BOOLEAN_FALSE] --- gcc/asan.c.mp 2013-07-21 20:07:15.013237456 +0200 +++ gcc/asan.c 2013-07-21 20:16:10.929376734 +0200 @@ -1954,7 +1954,7 @@ asan_global_struct (void) = build_decl (UNKNOWN_LOCATION, FIELD_DECL, get_identifier (field_names[i]), (i == 0 || i == 3) ? const_ptr_type_node - : build_nonstandard_integer_type (POINTER_SIZE, 1)); + : pointer_sized_int_node); DECL_CONTEXT (fields[i]) = ret; if (i) DECL_CHAIN (fields[i - 1]) = fields[i]; @@ -2039,8 +2039,7 @@ initialize_sanitizer_builtins (void) ptr_type_node, ptr_type_node, NULL_TREE); tree BT_FN_VOID_PTR_PTRMODE = build_function_type_list (void_type_node, ptr_type_node, - build_nonstandard_integer_type (POINTER_SIZE, - 1), NULL_TREE); + pointer_sized_int_node, NULL_TREE); tree BT_FN_VOID_INT = build_function_type_list (void_type_node, integer_type_node, NULL_TREE); tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5]; @@ -2197,7 +2196,6 @@ asan_finish_file (void) if (gcount) { tree type = asan_global_struct (), var, ctor; - tree uptr = build_nonstandard_integer_type (POINTER_SIZE, 1); tree dtor_statements = NULL_TREE; vec<constructor_elt, va_gc> *v; char buf[20]; @@ -2226,15 +2224,16 @@ asan_finish_file (void) varpool_assemble_decl (varpool_node_for_decl (var)); fn = builtin_decl_implicit (BUILT_IN_ASAN_REGISTER_GLOBALS); + tree gcount_tree = build_int_cst (pointer_sized_int_node, gcount); append_to_statement_list (build_call_expr (fn, 2, build_fold_addr_expr (var), - build_int_cst (uptr, gcount)), + gcount_tree), &asan_ctor_statements); fn = builtin_decl_implicit (BUILT_IN_ASAN_UNREGISTER_GLOBALS); append_to_statement_list (build_call_expr (fn, 2, build_fold_addr_expr (var), - build_int_cst (uptr, gcount)), + gcount_tree), &dtor_statements); cgraph_build_static_cdtor ('D', dtor_statements, MAX_RESERVED_INIT_PRIORITY - 1);