On 12/16/2016 09:41 AM, Jakub Jelinek wrote:
On Fri, Dec 16, 2016 at 11:08:00AM +0100, Jakub Jelinek wrote:
Here is an untested proof of concept for:
1) keeping the warning in the FEs no matter what optimization level is on,
just making sure TREE_NO_WARNING is set on the CALL_EXPR if we've warned
2) moving the rest of the warning shortly post IPA, when we have performed
inlining already and some constant propagation afterwards, but where
hopefully the IL still isn't too much different from the original source
3) as the nonnull attribute is a type property, it warns about the function
type of the call, doesn't require a fndecl
The tree-ssa-ccp.c location is just randomly chosen, the pass could go
into its own file, or some other file. And I think e.g. the -Walloc-zero
warning should move there as well.
If you think warning later can be still useful to some users at the expense
of higher false positive rate, we could have -Wmaybe-nonnull warning that
would guard that and set the gimple no warning flag when we warn in the
pass.
If needed, there is always the option on the table to turn
TREE_NO_WARNING/gimple_no_warning_p into a bit that says on the side hash
table contains bitmap of disabled warnings for the expression or statement.
IMHO we want to do that in any case, just not sure if it is urgent to do for
GCC 7.
Now successfully bootstrapped/regtested on x86_64-linux and i686-linux, so I
wrote ChangeLog for it as well:
2016-12-16 Jakub Jelinek <ja...@redhat.com>
PR bootstrap/78817
* tree-pass.h (make_pass_post_ipa_warn): Declare.
* builtins.c (validate_arglist): Adjust get_nonnull_args call.
Check for NULL pointer argument to nonnull arg here.
(validate_arg): Revert 2016-12-14 changes.
* calls.h (get_nonnull_args): Remove declaration.
* tree-ssa-ccp.c: Include diagnostic-core.h.
(pass_data_post_ipa_warn): New variable.
(pass_post_ipa_warn): New class.
(pass_post_ipa_warn::execute): New method.
(make_pass_post_ipa_warn): New function.
* tree.h (get_nonnull_args): Declare.
* tree.c (get_nonnull_args): New function.
* calls.c (maybe_warn_null_arg): Removed.
(maybe_warn_null_arg): Removed.
(initialize_argument_information): Revert 2016-12-14 changes.
* passes.def: Add pass_post_ipa_warn after first ccp after IPA.
c-family/
* c-common.c (struct nonnull_arg_ctx): New type.
(check_function_nonnull): Return bool instead of void. Use
nonnull_arg_ctx as context rather than just location_t.
(check_nonnull_arg): Adjust for the new context type, set
warned_p to true if a warning has been diagnosed.
(check_function_arguments): Return bool instead of void.
* c-common.h (check_function_arguments): Adjust prototype.
c/
* c-typeck.c (build_function_call_vec): If check_function_arguments
returns true, set TREE_NO_WARNING on CALL_EXPR.
cp/
* typeck.c (cp_build_function_call_vec): If check_function_arguments
returns true, set TREE_NO_WARNING on CALL_EXPR.
* call.c (build_over_call): Likewise.
So I spoke with Martin yesterday and have been convinced that we ought
to go forward now rather than waiting for gcc-8. Essentially the
argument is that Jakub's patch is a significant improvement over where
the warnings were in prior GCC releases, even if they don't go as far as
Martin's work.
We can (and should) evaluate whether or not to push things further in gcc-8.
So with that in mind...
It looks like you could avoid a lot of work in
pass_post_ipa_warn::execute by checking if warnings were asked for
outside the main loop. Presumably you wrote this with the check inside
the loop with the expectation that other warnings might move into this
routine, right?
Also in pass_post_ipa_warn::execute, the BITMAP_FREE call is technically
in a correct position, but it might be more maintainable long term if
the allocation/deallocation occur at the same nesting level.
OK as-is or with the BITMAP_FREE call moved to the same scoping level as
get_nonnull_args.
Jeff
