This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
- From: Jeff Law <law at redhat dot com>
- To: Manuel López-Ibáñez <lopezibanez at gmail dot com>, Martin Sebor <msebor at gmail dot com>
- Cc: Richard Biener <rguenther at suse dot de>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>, Bernd Schmidt <bschmidt at redhat dot com>, David Malcolm <dmalcolm at redhat dot com>, Florian Weimer <fweimer at redhat dot com>, Joseph Myers <joseph at codesourcery dot com>
- Date: Thu, 8 Sep 2016 13:19:52 -0600
- Subject: Re: [PATCH] - improve sprintf buffer overflow detection (middle-end/49905)
- Authentication-results: sourceware.org; auth=none
- References: <5776B33E.2080504@gmail.com> <alpine.LSU.2.11.1607041255060.29772@t29.fhfr.qr> <577A8D6A.3070902@gmail.com> <alpine.LSU.2.11.1607051208470.29772@t29.fhfr.qr> <578D512F.9050909@gmail.com> <9bb5ad66-4985-8c42-f800-4d84e0e18659@redhat.com> <57A3AFFF.7090109@gmail.com> <f3d09350-de5d-a0e4-8203-affac268ced2@redhat.com> <57AD30E5.3000801@gmail.com> <22a47656-c23c-4840-2e49-a59f4af513b1@redhat.com> <57B725F6.8000405@gmail.com> <110cfc6b-7856-9b51-885f-05402b14fc3e@redhat.com> <57BCC6A4.3050004@gmail.com> <CAESRpQBoxrFCzP657ZgfKrSZMgqPtUQmvq1AgSzvVT5uuXyTHg@mail.gmail.com> <57BE007E.5040600@gmail.com> <CAESRpQC9TMD0gQ0Tvf0CZ+OmUjv4sv6jDsMc2vUNgqmj0Y3sTQ@mail.gmail.com>
On 08/24/2016 05:14 PM, Manuel López-Ibáñez wrote:
I agree. The challenge is that not all the bits this depends on
(the g_string_concat_db and parse_in globals defined in the front
end) are available in the middle end. I've been talking to David
Malcolm about how best to factor things out of c-format.c and make
it available in both parts of the compiler under a convenient API.
Perhaps diagnostics_context could have pointers to those, forward
defined in the .h file and include the relevant libcpp headers in
diagnostics.c (or input.c). FEs that make use of those features could
initialize them (via some API) to some existing object. Otherwise,
they will work like in your patch (but within diagnostic.c). Similar
to how we initialize the FE-specific pretty-printers.
We already depend on libcpp for line-map.c, so internally depending on
other libcpp features is not so bad. The important thing is to hide
this from the clients, so that the clients do not need to be aware of
what diagnostics.c requires. That is, the middle-end and Ada should
not include headers that include libcpp headers, but diagnostics.c can
include whatever it needs.
Otherwise, the future will be again a mess and we get further away
from ever separating the FEs from the ME.
Warnings which rely on things like const/copy propagation, range
analysis, inherently belong in the the middle end. They're next to
useless in the front-end.
This implies that a goodly amount of what's in c-format needs to move
and likely bits of libcpp/line-map as well.
BTW, it would be nice to explain in comments why each header needs to
be included, besides obvious ones such as tree.h and gimple.h (it
would be great if we had guidelines on how to order included headers,
why not group together all gimple*, tree*, backend-stuff, diagnostics
stuff?). On the other hand, it is unfair to nitpick your patch
regarding this when other commits do the same.
I see this as busy work and work that easily gets out of date.
I'd rather just use Andrew's header file refactoring work to be able to
answer these kinds of questions, canonicalize include ordering and to
eliminate unnecessary/redundant includes. In fact, ISTM it ought to be
run before we hit stage1 close :-)
Jeff