| Message ID | 2fa72d08-0fac-28b3-4fcf-c70f83b18c8d@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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. Isn't the goal here to be able to get format-length warnings when there aren't explicit calls to _b_o_s in the IL? Can't you initialize the object-size framework at the start of your pass and tear it down when your pass is complete? You could do that by exporting the init/fini routines and calling them directly, or by wrapping that in a class and instantiating the class when you need it. That would avoid having to worry about the GC system entirely since you wouldn't have stuff living across passes. Jeff
On 11/23/2016 12:10 PM, Jeff Law wrote: > 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. Ah, right, that might explain the ICE I just noticed during Ada bootstrap. Is there some other way to uniquely identify a function? A DECL_UID maybe? > > Isn't the goal here to be able to get format-length warnings when there > aren't explicit calls to _b_o_s in the IL? Can't you initialize the > object-size framework at the start of your pass and tear it down when > your pass is complete? You could do that by exporting the init/fini > routines and calling them directly, or by wrapping that in a class and > instantiating the class when you need it. > > That would avoid having to worry about the GC system entirely since you > wouldn't have stuff living across passes. Yes, that is the immediate goal of this patch. Beyond it, though, I would like to call this function from anywhere, including during expansion (as is done in my patch for bug 53562 and related). Martin https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00896.html
On 11/23/2016 12:32 PM, Martin Sebor wrote: >> My worry here would be a hash collision. Then we'd be using object >> sizes from the wrong function. > > Ah, right, that might explain the ICE I just noticed during Ada > bootstrap. Is there some other way to uniquely identify a function? > A DECL_UID maybe? DECL_UID would be your best bet. But ISTM that trying to avoid invocations by reusing data from prior passes is likely to be more fragile than recomputing on a per-pass basis -- as long as the number of times we need this stuff is small (as I suspect it is). > >> >> Isn't the goal here to be able to get format-length warnings when there >> aren't explicit calls to _b_o_s in the IL? Can't you initialize the >> object-size framework at the start of your pass and tear it down when >> your pass is complete? You could do that by exporting the init/fini >> routines and calling them directly, or by wrapping that in a class and >> instantiating the class when you need it. >> >> That would avoid having to worry about the GC system entirely since you >> wouldn't have stuff living across passes. > > Yes, that is the immediate goal of this patch. Beyond it, though, > I would like to call this function from anywhere, including during > expansion (as is done in my patch for bug 53562 and related). But why not detect the builtins during your pass and check there. ie, I don't see why we necessarily need to have checking and expansion intertwined together. Maybe I'm missing something. Is there something that makes it inherently easier or better to implement checking during builtin expansion? Jeff
On 11/23/2016 12:47 PM, Jeff Law wrote: > On 11/23/2016 12:32 PM, Martin Sebor wrote: > >>> My worry here would be a hash collision. Then we'd be using object >>> sizes from the wrong function. >> >> Ah, right, that might explain the ICE I just noticed during Ada >> bootstrap. Is there some other way to uniquely identify a function? >> A DECL_UID maybe? > DECL_UID would be your best bet. But ISTM that trying to avoid > invocations by reusing data from prior passes is likely to be more > fragile than recomputing on a per-pass basis -- as long as the number of > times we need this stuff is small (as I suspect it is). > >> >>> >>> Isn't the goal here to be able to get format-length warnings when there >>> aren't explicit calls to _b_o_s in the IL? Can't you initialize the >>> object-size framework at the start of your pass and tear it down when >>> your pass is complete? You could do that by exporting the init/fini >>> routines and calling them directly, or by wrapping that in a class and >>> instantiating the class when you need it. >>> >>> That would avoid having to worry about the GC system entirely since you >>> wouldn't have stuff living across passes. >> >> Yes, that is the immediate goal of this patch. Beyond it, though, >> I would like to call this function from anywhere, including during >> expansion (as is done in my patch for bug 53562 and related). > But why not detect the builtins during your pass and check there. ie, I > don't see why we necessarily need to have checking and expansion > intertwined together. Maybe I'm missing something. Is there something > that makes it inherently easier or better to implement checking during > builtin expansion? I hadn't thought of extending the gimple-ssa-sprintf pass to all the memxxx and strxxx builtins. The _chk functions are already being handled in builtins.c so calling compute_builtin_object_size for the non-checking ones there and detecting overflow in those was an easy and, I had hoped, non-controversial enhancement to make. In a discussion of bug 77784 (handled in the patch for bug 53562) Jakub also expressed a preference for some of the diagnostics staying in builtins.c. I also suspect that the answer to your question is yes. Range information is pretty bad in the gimple-ssa-sprintf pass (it looks like it runs after EVRP but before VRP). Maybe the pass should run after VRP? That said, I defer to you on how to proceed here. I'm prepared to do the work(*) but I do worry about jeopardizing the chances of this patch and the others making it into 7.0. Martin PS If I understand what you are suggesting this would mean extending the gimple-ssa-sprintf pass to the memxxx and strxxx functions and running the pass later, after VRP.
On 11/23/2016 01:09 PM, Martin Sebor wrote: > > I hadn't thought of extending the gimple-ssa-sprintf pass to all > the memxxx and strxxx builtins. The _chk functions are already > being handled in builtins.c so calling compute_builtin_object_size > for the non-checking ones there and detecting overflow in those > was an easy and, I had hoped, non-controversial enhancement to make. > In a discussion of bug 77784 (handled in the patch for bug 53562) > Jakub also expressed a preference for some of the diagnostics > staying in builtins.c. I'm just trying to understand how the pieces fit together. I wasn't aware of Jakub's desire to keep them in builtins.c. > > I also suspect that the answer to your question is yes. Range > information is pretty bad in the gimple-ssa-sprintf pass (it looks > like it runs after EVRP but before VRP). Maybe the pass should run > after VRP? Let's investigate this separately rather than draw in additional potential issues. But I do think this is worth investigation. > > That said, I defer to you on how to proceed here. I'm prepared > to do the work(*) but I do worry about jeopardizing the chances > of this patch and the others making it into 7.0. So would it make sense to just init/fini the b_o_s framework in your pass and for builtin expansion? > PS If I understand what you are suggesting this would mean > extending the gimple-ssa-sprintf pass to the memxxx and strxxx > functions and running the pass later, after VRP. That was my original thought, but I'm certainly not deeply vested in it -- it was primarily to avoid initializing b_o_s an extra time. Jeff
On 11/23/2016 01:30 PM, Jeff Law wrote: > On 11/23/2016 01:09 PM, Martin Sebor wrote: >> >> I hadn't thought of extending the gimple-ssa-sprintf pass to all >> the memxxx and strxxx builtins. The _chk functions are already >> being handled in builtins.c so calling compute_builtin_object_size >> for the non-checking ones there and detecting overflow in those >> was an easy and, I had hoped, non-controversial enhancement to make. >> In a discussion of bug 77784 (handled in the patch for bug 53562) >> Jakub also expressed a preference for some of the diagnostics >> staying in builtins.c. > I'm just trying to understand how the pieces fit together. I wasn't > aware of Jakub's desire to keep them in builtins.c. After thinking about it a bit it does seem that having all the size and buffer overflow checking (though not necessarily BOS itself) in the same place or pass would make sense. >> I also suspect that the answer to your question is yes. Range >> information is pretty bad in the gimple-ssa-sprintf pass (it looks >> like it runs after EVRP but before VRP). Maybe the pass should run >> after VRP? > Let's investigate this separately rather than draw in additional > potential issues. But I do think this is worth investigation. Sounds good. > >> >> That said, I defer to you on how to proceed here. I'm prepared >> to do the work(*) but I do worry about jeopardizing the chances >> of this patch and the others making it into 7.0. > So would it make sense to just init/fini the b_o_s framework in your > pass and for builtin expansion? I think that should work for the sprintf checking. Let me test it. We can deal with the memxxx and strxxx patch (53562) independently if you prefer. Thanks Martin
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/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index ead8b0e..ea56570 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -2468,7 +2468,7 @@ get_destination_size (tree dest) object (the function fails without optimization in this type). */ int ost = optimize > 0; unsigned HOST_WIDE_INT size; - if (compute_builtin_object_size (dest, ost, &size)) + if (compute_object_size (dest, ost, &size)) return size; return HOST_WIDE_INT_M1U; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c index 8d97fa8..9874332 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c @@ -1,5 +1,10 @@ /* { dg-do compile } */ -/* { dg-options "-std=c99 -O2 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */ +/* { dg-options "-O2 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */ +/* Verify that all sprintf built-ins detect overflow involving directives + with non-constant arguments known to be constrained by some range of + values, and even when writing into dynamically allocated buffers. + -O2 (-ftree-vrp) is necessary for the tests involving ranges to pass, + otherwise -O1 is sufficient. */ #ifndef LINE # define LINE 0 @@ -7,18 +12,26 @@ #define bos(x) __builtin_object_size (x, 0) -#define T(bufsize, fmt, ...) \ - do { \ - if (!LINE || __LINE__ == LINE) \ - { \ - char *d = (char *)__builtin_malloc (bufsize); \ - __builtin___sprintf_chk (d, 0, bos (d), fmt, __VA_ARGS__); \ - sink (d); \ - } \ - } while (0) +/* Defined (and redefined) to the allocation function to use, either + malloc, or alloca, or a VLA. */ +#define ALLOC(p, n) (p) = __builtin_malloc (n) -void -sink (void*); +/* Defined (and redefined) to the sprintf function to exercise. */ +#define TEST_SPRINTF(d, maxsize, objsize, fmt, ...) \ + __builtin___sprintf_chk (d, 0, objsize, fmt, __VA_ARGS__) + +#define T(bufsize, fmt, ...) \ + do { \ + if (!LINE || __LINE__ == LINE) \ + { \ + char *d; \ + ALLOC (d, bufsize); \ + TEST_SPRINTF (d, 0, bos (d), fmt, __VA_ARGS__); \ + sink (d); \ + } \ + } while (0) + +void sink (void*); /* Identity function to verify that the checker figures out the value of the operand even when it's not constant (i.e., makes use of @@ -232,3 +245,88 @@ void test_sprintf_chk_range_sshort (signed short *a, signed short *b) T ( 4, "%i", Ra (998, 999)); T ( 4, "%i", Ra (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */ } + +/* Exercise ordinary sprintf with malloc. */ +#undef TEST_SPRINTF +#define TEST_SPRINTF(d, maxsize, objsize, fmt, ...) \ + __builtin_sprintf (d, fmt, __VA_ARGS__) + +void test_sprintf_malloc (const char *s, const char *t) +{ +#define x x () + + T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? s : t); + + T (2, "%-s", x ? "" : "1"); + T (2, "%-s", x ? "" : s); + T (2, "%-s", x ? "1" : ""); + T (2, "%-s", x ? s : ""); + T (2, "%-s", x ? "1" : "2"); + T (2, "%-s", x ? "" : "12"); /* { dg-warning "nul past the end" } */ + T (2, "%-s", x ? "12" : ""); /* { dg-warning "nul past the end" } */ + + T (2, "%-s", x ? "" : "123"); /* { dg-warning "into a region" } */ + T (2, "%-s", x ? "123" : ""); /* { dg-warning "into a region" } */ + +#undef x +} + +/* Exercise ordinary sprintf with alloca. */ +#undef ALLOC +#define ALLOC(p, n) (p) = __builtin_alloca (n) + +void test_sprintf_alloca (const char *s, const char *t) +{ +#define x x () + + T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? s : t); + + T (2, "%-s", x ? "" : "1"); + T (2, "%-s", x ? "" : s); + T (2, "%-s", x ? "1" : ""); + T (2, "%-s", x ? s : ""); + T (2, "%-s", x ? "1" : "2"); + T (2, "%-s", x ? "" : "12"); /* { dg-warning "nul past the end" } */ + T (2, "%-s", x ? "12" : ""); /* { dg-warning "nul past the end" } */ + + T (2, "%-s", x ? "" : "123"); /* { dg-warning "into a region" } */ + T (2, "%-s", x ? "123" : ""); /* { dg-warning "into a region" } */ + +#undef x +} + +/* Exercise ordinary sprintf with a VLA. */ +#undef ALLOC +#define ALLOC(p, n) char vla [i (n)]; (p) = vla + +void test_sprintf_vla (const char *s, const char *t) +{ +#define x x () + + T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */ + T (1, "%-s", x ? s : t); + + T (2, "%-s", x ? "" : "1"); + T (2, "%-s", x ? "" : s); + T (2, "%-s", x ? "1" : ""); + T (2, "%-s", x ? s : ""); + T (2, "%-s", x ? "1" : "2"); + T (2, "%-s", x ? "" : "12"); /* { dg-warning "nul past the end" } */ + T (2, "%-s", x ? "12" : ""); /* { dg-warning "nul past the end" } */ + + T (2, "%-s", x ? "" : "123"); /* { dg-warning "into a region" } */ + T (2, "%-s", x ? "123" : ""); /* { dg-warning "into a region" } */ + +#undef x +} 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 @@ -50,6 +50,8 @@ static const unsigned HOST_WIDE_INT unknown[4] = { 0 }; +static bool internal_object_size (tree, int, unsigned HOST_WIDE_INT *); +static void fini_object_sizes (void); static tree compute_object_offset (const_tree, const_tree); static bool addr_object_size (struct object_size_info *, const_tree, int, unsigned HOST_WIDE_INT *); @@ -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]; /* Bitmaps what object sizes have been computed already. */ -static bitmap computed[4]; +static GTY (()) bitmap computed[4]; /* Maximum value of offset we consider to be addition. */ static unsigned HOST_WIDE_INT offset_limit; @@ -187,7 +189,7 @@ addr_object_size (struct object_size_info *osi, const_tree ptr, if (!osi || (object_size_type & 1) != 0 || TREE_CODE (TREE_OPERAND (pt_var, 0)) != SSA_NAME) { - compute_builtin_object_size (TREE_OPERAND (pt_var, 0), + internal_object_size (TREE_OPERAND (pt_var, 0), object_size_type & ~1, &sz); } else @@ -491,14 +493,14 @@ pass_through_call (const gcall *call) } -/* Compute __builtin_object_size value for PTR and set *PSIZE to - the resulting value. OBJECT_SIZE_TYPE is the second argument - to __builtin_object_size. Return true on success and false - when the object size could not be determined. */ +/* Compute the size of the object pointed to by PTR and set *PSIZE + to the resulting value. OBJECT_SIZE_TYPE is the second argument + to __builtin_object_size. Return true on success and false when + the object size could not be determined. */ bool -compute_builtin_object_size (tree ptr, int object_size_type, - unsigned HOST_WIDE_INT *psize) +internal_object_size (tree ptr, int object_size_type, + unsigned HOST_WIDE_INT *psize) { gcc_assert (object_size_type >= 0 && object_size_type <= 3); @@ -534,7 +536,7 @@ compute_builtin_object_size (tree ptr, int object_size_type, ptr = gimple_assign_rhs1 (def); if (cst_and_fits_in_hwi (offset) - && compute_builtin_object_size (ptr, object_size_type, psize)) + && internal_object_size (ptr, object_size_type, psize)) { /* Return zero when the offset is out of bounds. */ unsigned HOST_WIDE_INT off = tree_to_shwi (offset); @@ -664,6 +666,45 @@ compute_builtin_object_size (tree ptr, int object_size_type, return *psize != unknown[object_size_type]; } +/* Compute __builtin_object_size value for PTR and set *PSIZE to + the resulting value. OBJECT_SIZE_TYPE is the second argument + to __builtin_object_size. Return true on success and false + when the object size could not be determined. */ + +bool +compute_builtin_object_size (tree ptr, int object_size_type, + unsigned HOST_WIDE_INT *psize) +{ + return internal_object_size (ptr, object_size_type, psize); +} + + +/* 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) + { + lastfunchash = curfunchash; + fini_object_sizes (); + init_object_sizes (); + } + + bool retval = internal_object_size (ptr, object_size_type, psize); + + return retval; +} + /* Compute object_sizes for PTR, defined to VALUE, which is not an SSA_NAME. */ static void @@ -1221,7 +1262,7 @@ init_object_sizes (void) for (object_size_type = 0; object_size_type <= 3; object_size_type++) { object_sizes[object_size_type].safe_grow (num_ssa_names); - computed[object_size_type] = BITMAP_ALLOC (NULL); + computed[object_size_type] = BITMAP_GGC_ALLOC (); } init_offset_limit (); @@ -1238,7 +1279,11 @@ fini_object_sizes (void) for (object_size_type = 0; object_size_type <= 3; object_size_type++) { object_sizes[object_size_type].release (); - BITMAP_FREE (computed[object_size_type]); + if (computed[object_size_type]) + { + bitmap_clear (computed[object_size_type]); + computed[object_size_type] = NULL; + } } } @@ -1325,7 +1370,7 @@ pass_object_sizes::execute (function *fun) { tree type = TREE_TYPE (lhs); unsigned HOST_WIDE_INT bytes; - if (compute_builtin_object_size (ptr, object_size_type, + if (internal_object_size (ptr, object_size_type, &bytes) && wi::fits_to_tree_p (bytes, type)) { diff --git a/gcc/tree-object-size.h b/gcc/tree-object-size.h index 38c3e07..60256e6 100644 --- a/gcc/tree-object-size.h +++ b/gcc/tree-object-size.h @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #define GCC_TREE_OBJECT_SIZE_H extern void init_object_sizes (void); +extern bool compute_object_size (tree, int, unsigned HOST_WIDE_INT *); extern bool compute_builtin_object_size (tree, int, unsigned HOST_WIDE_INT *); #endif // GCC_TREE_OBJECT_SIZE_H