Message ID | 20130719184530.GE3169@redhat.com |
---|---|
State | New |
Headers | show |
On Fri, Jul 19, 2013 at 08:45:30PM +0200, Marek Polacek wrote: > > >+uptr_type (void) > > >+{ > > >+ return build_nonstandard_integer_type (POINTER_SIZE, 1); > > > > Why not use uintptr_type_node? > > I suppose I could. I just followed suit what asan.c does. I didn't > address this in this patch, but I can, if you want to. uintptr_type_node is a C/C++/ObjC/ObjC++ FE tree. So, if you use it just in c-family/c-ubsan.c, that is just fine, but you can't use it in ubsan.c. > @@ -67,8 +68,8 @@ inline bool > ubsan_typedesc_hasher::equal (const ubsan_typedesc *d1, > const ubsan_typedesc *d2) > { > - /* ??? Here, the types should have identical __typekind, > - _typeinfo and __typename. Is this enough? */ > + /* Here, the types should have identical __typekind, > + _typeinfo and __typename. */ > return d1->type == d2->type; > } Only one underscore for _typeinfo ? Jakub
On Fri, Jul 19, 2013 at 08:50:42PM +0200, Jakub Jelinek wrote: > On Fri, Jul 19, 2013 at 08:45:30PM +0200, Marek Polacek wrote: > > > >+uptr_type (void) > > > >+{ > > > >+ return build_nonstandard_integer_type (POINTER_SIZE, 1); > > > > > > Why not use uintptr_type_node? > > > > I suppose I could. I just followed suit what asan.c does. I didn't > > address this in this patch, but I can, if you want to. > > uintptr_type_node is a C/C++/ObjC/ObjC++ FE tree. So, if you use it just > in c-family/c-ubsan.c, that is just fine, but you can't use it in ubsan.c. In that case I prefer to keep uptr_type around. Even though I like uintptr_type_node more. > > @@ -67,8 +68,8 @@ inline bool > > ubsan_typedesc_hasher::equal (const ubsan_typedesc *d1, > > const ubsan_typedesc *d2) > > { > > - /* ??? Here, the types should have identical __typekind, > > - _typeinfo and __typename. Is this enough? */ > > + /* Here, the types should have identical __typekind, > > + _typeinfo and __typename. */ > > return d1->type == d2->type; > > } > > Only one underscore for _typeinfo ? I wonder where the _ disappeared. Will fix. Marek
On 07/19/2013 03:01 PM, Marek Polacek wrote: > On Fri, Jul 19, 2013 at 08:50:42PM +0200, Jakub Jelinek wrote: >> uintptr_type_node is a C/C++/ObjC/ObjC++ FE tree. So, if you use it just >> in c-family/c-ubsan.c, that is just fine, but you can't use it in ubsan.c. Any reason not to move it into the middle-end? Jason
On Sat, Jul 20, 2013 at 02:04:24AM -0400, Jason Merrill wrote: > On 07/19/2013 03:01 PM, Marek Polacek wrote: > >On Fri, Jul 19, 2013 at 08:50:42PM +0200, Jakub Jelinek wrote: > >>uintptr_type_node is a C/C++/ObjC/ObjC++ FE tree. So, if you use it just > >>in c-family/c-ubsan.c, that is just fine, but you can't use it in ubsan.c. > > Any reason not to move it into the middle-end? I don't know, but it seems iffy to pluck out one particular type of those supplemented when adding the support of stdint.h types. We'd have to move all of them into the middle-end (from c_common_nodes_and_builtins). I'm not sure if it's worth it. Marek
On Sat, Jul 20, 2013 at 10:19:55AM +0200, Marek Polacek wrote: > On Sat, Jul 20, 2013 at 02:04:24AM -0400, Jason Merrill wrote: > > On 07/19/2013 03:01 PM, Marek Polacek wrote: > > >On Fri, Jul 19, 2013 at 08:50:42PM +0200, Jakub Jelinek wrote: > > >>uintptr_type_node is a C/C++/ObjC/ObjC++ FE tree. So, if you use it just > > >>in c-family/c-ubsan.c, that is just fine, but you can't use it in ubsan.c. > > > > Any reason not to move it into the middle-end? > > I don't know, but it seems iffy to pluck out one particular type of > those supplemented when adding the support of stdint.h types. > We'd have to move all of them into the middle-end (from > c_common_nodes_and_builtins). I'm not sure if it's worth it. The way uintptr_type_node is constructed relies on C/C++ FE to transform the string "unsigned long int" etc. defined in the target headers to a type, but you can't do that in the middle-end. So, the middle-end can only use some other type, say build_nonstandard_integer_type (POINTER_SIZE, 1); which often will be the same type as uintptr_type_node, but perhaps not on all targets. It could be worth to create such a type in tree.c and stick it into global trees and then just use, but right now all the uses are just in asan.c and ubsan.c. Jakub
On 07/20/2013 10:27 AM, Jakub Jelinek wrote: > So, the middle-end can only use > some other type, say build_nonstandard_integer_type (POINTER_SIZE, 1); > which often will be the same type as uintptr_type_node, but perhaps not on > all targets. It could be worth to create such a type in tree.c and stick it > into global trees and then just use, but right now all the uses are just in > asan.c and ubsan.c. Let's do that; I expect it will be useful in other situations, too. Jason
On 07/19/2013 02:45 PM, Marek Polacek wrote: > /* This type represents an entry in the hash table. */ > struct ubsan_typedesc > { > + /* This represents the type of a variable. */ > tree type; > + > + /* This is the VAR_DECL of the type. */ > tree decl; > }; What I was looking for was something along the lines of "this hash table maps from a TYPE to a ubsan type descriptor VAR_DECL for that type". Jason
--- gcc/ubsan.c.mp 2013-07-19 18:42:42.896249415 +0200 +++ gcc/ubsan.c 2013-07-19 20:34:11.459792189 +0200 @@ -34,7 +34,10 @@ along with GCC; see the file COPYING3. /* This type represents an entry in the hash table. */ struct ubsan_typedesc { + /* This represents the type of a variable. */ tree type; + + /* This is the VAR_DECL of the type. */ tree decl; }; @@ -56,9 +59,7 @@ struct ubsan_typedesc_hasher inline hashval_t ubsan_typedesc_hasher::hash (const ubsan_typedesc *data) { - hashval_t h = iterative_hash_object (data->type, 0); - h = iterative_hash_object (data->decl, h); - return h; + return iterative_hash_object (data->type, 0); } /* Compare two data types. */ @@ -67,8 +68,8 @@ inline bool ubsan_typedesc_hasher::equal (const ubsan_typedesc *d1, const ubsan_typedesc *d2) { - /* ??? Here, the types should have identical __typekind, - _typeinfo and __typename. Is this enough? */ + /* Here, the types should have identical __typekind, + _typeinfo and __typename. */ return d1->type == d2->type; } @@ -93,7 +94,6 @@ ubsan_typedesc_get_alloc_pool () ubsan_typedesc_alloc_pool = create_alloc_pool ("ubsan_typedesc", sizeof (ubsan_typedesc), 10); - // XXX But where do we free this? We'll need GTY machinery. return ubsan_typedesc_alloc_pool; } @@ -123,18 +123,6 @@ ubsan_typedesc_new (tree type, tree decl return desc; } -/* Clear all entries from the type descriptor hash table. */ - -#if 0 -static void -empty_ubsan_typedesc_hash_table () -{ - // XXX But when do we call this? - if (ubsan_typedesc_ht.is_created ()) - ubsan_typedesc_ht.empty (); -} -#endif - /* Build the ubsan uptr type. */ static tree @@ -245,7 +233,6 @@ ubsan_source_location_type (void) { fields[i] = build_decl (UNKNOWN_LOCATION, FIELD_DECL, get_identifier (field_names[i]), - //(i == 0) ? const_string_type_node (i == 0) ? build_pointer_type (const_char_type) : unsigned_type_node); DECL_CONTEXT (fields[i]) = ret; @@ -291,34 +278,16 @@ ubsan_source_location (location_t loc) return ctor; } -/* This routine returns a magic number for TYPE. - ??? This is probably too ugly. Tweak it. */ +/* This routine returns a magic number for TYPE. */ static unsigned short -get_tinfo_for_type (tree type) +get_ubsan_type_info_for_type (tree type) { - unsigned short tinfo; + int prec = exact_log2 (TYPE_PRECISION (type)); + if (prec == -1) + error ("unexpected size of type %qT", type); - switch (GET_MODE_SIZE (TYPE_MODE (type))) - { - case 4: - tinfo = 5; - break; - case 8: - tinfo = 6; - break; - case 16: - tinfo = 7; - break; - default: - error ("unexpected size of type %qT", type); - } - - tinfo <<= 1; - - /* The MSB here says whether the value is signed or not. */ - tinfo |= !TYPE_UNSIGNED (type); - return tinfo; + return (prec << 1) | !TYPE_UNSIGNED (type); } /* Helper routine that returns ADDR_EXPR of a VAR_DECL of a type @@ -333,6 +302,9 @@ ubsan_type_descriptor (tree type) ubsan_typedesc d; ubsan_typedesc_init (&d, type, NULL); + /* See through any typedefs. */ + type = TYPE_MAIN_VARIANT (type); + ubsan_typedesc **slot = ht.find_slot (&d, INSERT); if (*slot != NULL) /* We have the VAR_DECL in the table. Return it. */ @@ -353,7 +325,7 @@ ubsan_type_descriptor (tree type) { /* For INTEGER_TYPE, this is 0x0000. */ tkind = 0x000; - tinfo = get_tinfo_for_type (type); + tinfo = get_ubsan_type_info_for_type (type); } else if (TREE_CODE (type) == REAL_TYPE) /* We don't have float support yet. */ @@ -362,9 +334,9 @@ ubsan_type_descriptor (tree type) gcc_unreachable (); /* Create a new VAR_DECL of type descriptor. */ - char *tmp_name; + char tmp_name[32]; static unsigned int type_var_id_num; - ASM_FORMAT_PRIVATE_NAME (tmp_name, ".Lubsan_type", type_var_id_num++); + ASM_GENERATE_INTERNAL_LABEL (tmp_name, "Lubsan_type", type_var_id_num++); tree decl = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier (tmp_name), dtype); TREE_STATIC (decl) = 1; @@ -446,9 +418,9 @@ ubsan_create_data (const char *name, loc va_end (args); /* Now, fill in the type. */ - char *tmp_name; + char tmp_name[32]; static unsigned int ubsan_var_id_num; - ASM_FORMAT_PRIVATE_NAME (tmp_name, ".Lubsan_data", ubsan_var_id_num++); + ASM_GENERATE_INTERNAL_LABEL (tmp_name, "Lubsan_data", ubsan_var_id_num++); tree var = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier (tmp_name), ret); TREE_STATIC (var) = 1;