This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] enable -Wformat-length for dynamically allocated buffers (pr 78245)


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]