On Wed, Apr 11, 2012 at 3:46 AM, Dodji Seketeli <do...@redhat.com> wrote:
> There are various conversion related warnings that trigger on
> potentially dangerous uses of NULL (or __null). NULL is defined as a
> macro in a system header, so calling warning or warning_at on a virtual
> location of NULL yields no diagnostic. So the test accompanying this
> patch (as well as others), was failling when run with
> -ftrack-macro-expansion.
>
> I think it's necessary to use the location of NULL that is in the main
> source code (instead of, e.g, the spelling location that is in the
> system header where the macro is defined) in those cases. Note that for
> __null, we don't have the issue.
I like the idea. However, you should write a separate function
(get_null_ptr_cst_location?) that computes the location of the null
pointer constant token and call it from where you need the location.
> @@ -5538,12 +5538,26 @@ conversion_null_warnings (tree totype, tree expr,
> tree fn, int argnum)
> if (expr == null_node && TREE_CODE (totype) != BOOLEAN_TYPE
> && ARITHMETIC_TYPE_P (totype))
> {
> + source_location loc = input_location;
> + /* The location of the warning here is most certainly the one for
> + the token that represented null_node in the source code.
> + That is either NULL or __null. If it is NULL, then that
> + macro is defined in a system header and, as a consequence,
> + warning_at won't emit any diagnostic for it. In this case,
> + we are going to resolve that location to the point in the
> + source code where the expression that triggered this error
> + message is, rather than the point where the NULL macro is
> + defined. */
> + if (in_system_header_at (input_location))
> + loc = linemap_resolve_location (line_table, loc,
> + LRK_MACRO_EXPANSION_POINT,
> + NULL);
> if (fn)
> - warning_at (input_location, OPT_Wconversion_null,
> + warning_at (loc, OPT_Wconversion_null,
> "passing NULL to non-pointer argument %P of %qD",
> argnum, fn);
> else
> - warning_at (input_location, OPT_Wconversion_null,
> + warning_at (loc, OPT_Wconversion_null,
> "converting to non-pointer type %qT from NULL", totype);
> }
>
> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> index c411a47..73bdf71 100644
> --- a/gcc/cp/cvt.c
> +++ b/gcc/cp/cvt.c
> @@ -1468,8 +1468,22 @@ build_expr_type_conversion (int desires, tree expr,
> bool complain)
> if (expr == null_node
> && (desires & WANT_INT)
> && !(desires & WANT_NULL))
> - warning_at (input_location, OPT_Wconversion_null,
> - "converting NULL to non-pointer type");
> + {
> + source_location loc = input_location;
> + /* The location of the warning here is the one for the
> + (expansion of the) NULL macro, or for __null. If it is for
> + NULL, then, as that that macro is defined in a system header,
> + warning_at won't emit any diagnostic. In this case, we are
> + going to resolve that location to the point in the source
> + code where the expression that triggered this warning is,
> + rather than the point where the NULL macro is defined. */
> + if (in_system_header_at (loc))
> + loc = linemap_resolve_location (line_table, loc,
> + LRK_MACRO_EXPANSION_POINT,
> + NULL);
> + warning_at (loc, OPT_Wconversion_null,
> + "converting NULL to non-pointer type");
> + }
>
> basetype = TREE_TYPE (expr);
>
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index d2ed940..d096f1e 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -3798,9 +3798,23 @@ cp_build_binary_op (location_t location,
> || (!null_ptr_cst_p (orig_op1)
> && !TYPE_PTR_P (type1) && !TYPE_PTR_TO_MEMBER_P (type1)))
> && (complain & tf_warning))
> - /* Some sort of arithmetic operation involving NULL was
> - performed. */
> - warning (OPT_Wpointer_arith, "NULL used in arithmetic");
> + {
> + source_location loc = input_location;
> + /* Some sort of arithmetic operation involving NULL was
> + performed. The location of the warning here is the one for
> + the (expansion of the) NULL macro, or for __null. If it is
> + for NULL, then, as that that macro is defined in a system
> + header, warning_at won't emit any diagnostic. In this case,
> + we are going to resolve that location to the point in the
> + source code where the expression that triggered this warning
> + is, rather than the point where the NULL macro is
> + defined. */
> + if (in_system_header_at (loc))
> + loc = linemap_resolve_location (line_table, loc,
> + LRK_MACRO_EXPANSION_POINT,
> + NULL);
> + warning_at (loc, OPT_Wpointer_arith, "NULL used in arithmetic");
> + }
