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 08/11] Make conversion warnings work on NULL with -ftrack-macro-expansion


On Wed, Apr 11, 2012 at 3:46 AM, Dodji Seketeli <dodji@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");
> + ? ?}


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