On Thu, Jul 18, 2013 at 03:47:28PM -0400, Jason Merrill wrote:
> Please describe the hash table more up here. What are you tracking?
>
> >+ hashval_t h = iterative_hash_object (data->type, 0);
> >+ h = iterative_hash_object (data->decl, h);
>
> If you hash the decl as well as the type, the find_slot in
> ubsan_type_descriptor will almost never find an existing entry.
Yeah.
> >I have yet to handle
> >freeing the hash table, but I think I'll need the GTY machinery for
> >this (ubsan is not a pass, so I can't just call it at the end of the
> >pas). Or maybe just create a destructor and use append_to_statement_list.
>
> That won't work; append_to_statement_list is for things that happen
> at runtime, but freeing the hash table is something that needs to
> happen in the compiler.
Also, I wonder if we need to free the hash table ever, the only meaningful
place would be when we are done with compilation, if all uses of this (even
in the future) are from the FEs, perhaps freeing the hash table somewhere
after parsing of the whole CU finished would be fine, but if we need it even
from the middle-end, that might be too early.
> >+/* This routine returns a magic number for TYPE.
> >+ ??? This is probably too ugly. Tweak it. */
> >+
> >+static unsigned short
> >+get_tinfo_for_type (tree type)
>
> Why map from size to some magic number rather than use the size
> directly? Also, "tinfo" sounds to me like something to do with C++
> type_info.
Yeah, get_ubsan_type_info_for_type would be better, plus
The docs say that it should be log2 of the bit width, so I'd expect that
you return
(exact_log2 (TYPE_PRECISION (type)) << 1) | !TYPE_UNSIGNED (type)
(perhaps with error if exact_log2 returns -1) and thus handle all possible
type bitsizes. Or if you prefer to use GET_MODE_BITSIZE, that.
Have you checked what clang does if you use typedefs? Does it use
the names appearing in the typedefs rather than their underlying types,
or is it TYPE_MAIN_VARIANT (type) actually?
Also:
ASM_FORMAT_PRIVATE_NAME (tmp_name, ".Lubsan_type", type_var_id_num++);
I don't think the . at the beginning is going to work everywhere
(on targets that don't support dots in labels),
I think you want ASM_GENERATE_INTERNAL_LABEL instead and just use
"Lubsan_type" as the second argument (note, this macro needs a buffer,
doesn't alloca it).
Jakub
