On 11/23/2016 11:26 AM, Martin Sebor wrote:
My only real concern here is that if we call compute_builtin_object_size
without having initialized the passes, then we initialize, compute, then
finalize. Subsequent calls will go through the same process -- the key
being each one re-computes the internal state which might get expensive.
Wouldn't it just make more sense to pull up the init/fini calls, either
explicitly (which likely means externalizing the init/fini routines) or
by wrapping all this stuff in a class and instantiating a suitable
object?
I think the answer to your memory management question is that internal
state is likely not marked as a GC root and thus if you get a GC call
pieces of internal state are not seen as reachable, but you still can
reference them. ie, you end up with dangling pointers.
Usually all you'd have to do is mark them so that gengtype will see
them. Bitmaps, trees, rtl, are all good examples. So marking the
bitmap would look like:
static GTY (()) bitmap computed[4];
Or something like that.
You might try --enable-checking=yes,extra,gc,gcac
That will be slow, but is often helpful for tracking down cases where
someone has an object expected to be live across passes, but it isn't
reachable because someone failed to register a GC root.
Thanks. Attached is an updated patch that avoids flushing the computed
data until the current function changes, and tells the garbage collector
not to collect it. I haven't finished bootstrapping and regtesting it
yet but running it through Valgrind doesn't show any errors. Please
let me know if this is what you had in mind.
Thanks
Martin
gcc-78245.diff
PR middle-end/78245 - missing -Wformat-length on an overflow of a
dynamically allocated buffer
gcc/testsuite/ChangeLog:
PR middle-end/78245
* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: Add tests.
gcc/ChangeLog:
PR middle-end/78245
* gimple-ssa-sprintf.c (object_sizes, computed): Declared GTY.
(get_destination_size): Call compute_object_size.
* tree-object-size.c (addr_object_size): Adjust.
(pass_through_call): Adjust.
(compute_object_size, internal_object_size): New functions.
(compute_builtin_object_size): Call internal_object_size.
(init_object_sizes): Initialize computed bitmap so the garbage
collector knows about it.
(fini_object_sizes): Clear the computed bitmap when non-null.
(pass_object_sizes::execute): Adjust.
* tree-object-size.h (compute_object_size): Declare.
diff --git a/gcc/tree-object-size.c b/gcc/tree-object-size.c
index 1317ad7..39c32e3 100644
--- a/gcc/tree-object-size.c
+++ b/gcc/tree-object-size.c
@@ -72,10 +74,10 @@ static void check_for_plus_in_loops_1 (struct
object_size_info *, tree,
the subobject (innermost array or field with address taken).
object_sizes[2] is lower bound for number of bytes till the end of
the object and object_sizes[3] lower bound for subobject. */
-static vec<unsigned HOST_WIDE_INT> object_sizes[4];
+static GTY (()) vec<unsigned HOST_WIDE_INT> object_sizes[4];
I don't think this needs a GTY marker.
/* Bitmaps what object sizes have been computed already. */
-static bitmap computed[4];
+static GTY (()) bitmap computed[4];
This is the one you probably needed :-)
+/* Like compute_builtin_object_size but intended to be called
+ without a corresponding __builtin_object_size in the program. */
+
+bool
+compute_object_size (tree ptr, int object_size_type,
+ unsigned HOST_WIDE_INT *psize)
+{
+ static unsigned lastfunchash;
+ unsigned curfunchash
+ = IDENTIFIER_HASH_VALUE (DECL_NAME (current_function_decl));
+
+ /* Initialize the internal data structures for each new function
+ and keep the computed data around for any subsequent calls to
+ compute_object_size. */
+ if (curfunchash != lastfunchash)
My worry here would be a hash collision. Then we'd be using object
sizes from the wrong function.