On 08/15/2018 12:02 AM, Jeff Law wrote:
On 08/13/2018 03:23 PM, Martin Sebor wrote:
To make reviewing the changes easier I've split up the patch
into a series:
[ ... ]
I'm about done for the night and thus won't get into the series (and as
you know Bernd has a competing patch in this space). But I did want to
chime in on two things...
There are many more string functions where unterminated (constant
or otherwise) should be diagnosed. I plan to continue to work on
those (with the constant ones first) but I want to post this
updated patch for review now, mainly so that the wrong code bug
(PR 86711) can be resolved and the basic detection infrastructure
agreed on.
Yes, I think we definitely want to focus on the wrong code bug first.
An open question in my mind is what should GCC do with such calls
after issuing a warning: replace them with traps? Fold them into
constants? Or continue to pass them through to the corresponding
library functions?
My personal preference is to turn them into traps. I don't think we
have to preserve the call itself in this case. I think the sequencing
is to insert the trap before the call point, split the block after the
trap, remove the outgoing edges, let DCE clean up the rest. At least I
think that's the sequencing.
That sounds fine to me. It would be close in its effects to
what _FORTIFY_SOURCE does.
It would be helpful to get a broader consensus on this and start
adopting the same consistent solution in all contexts. The question
has come up a few times, most recently also in PR 86519 (folding
memcmp(a, "a", 3)) where GCC ends up calling the library function.
FWIW, if there are other preferences it might be worthwhile to
consider providing an option to control the behavior in these
cases. There may also be interactions with or implications for
the sanitizers to consider.
Once there is agreement on what the solution should be I can look
into implementing it at some point in the future.
Martin
