Bug 106782 - dump_printf_loc has incorrect format attribute
Summary: dump_printf_loc has incorrect format attribute
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: ---
Assignee: Jakub Jelinek
URL:
Keywords: internal-improvement
Depends on:
Blocks:
 
Reported: 2022-08-31 08:42 UTC by Jakub Jelinek
Modified: 2022-09-02 08:23 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-08-31 00:00:00


Attachments
gcc13-pr106782.patch (4.49 KB, patch)
2022-08-31 16:22 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2022-08-31 08:42:40 UTC
We don't perform -Wformat checks on dump_printf_loc calls because it pretends it has a va_list argument instead of ... at the end:
--- gcc/dumpfile.h.jj	2022-01-18 11:58:59.501983213 +0100
+++ gcc/dumpfile.h	2022-08-31 10:36:25.789850981 +0200
@@ -574,7 +574,7 @@ extern void dump_printf (const dump_meta
 
 extern void dump_printf_loc (const dump_metadata_t &, const dump_user_location_t &,
 			     const char *, ...)
-  ATTRIBUTE_GCC_DUMP_PRINTF (3, 0);
+  ATTRIBUTE_GCC_DUMP_PRINTF (3, 4);
 extern void dump_function (int phase, tree fn);
 extern void dump_basic_block (dump_flags_t, basic_block, int);
 extern void dump_generic_expr_loc (const dump_metadata_t &,
Unfortunately this patch results in tons of warnings that should be fixed one way or another.  E.g.:
../../gcc/tree-vect-slp.cc: In function ‘void vect_print_slp_tree(dump_flags_t, dump_location_t, slp_tree)’:
../../gcc/tree-vect-slp.cc:2528:49: warning: format ‘%p’ expects argument of type ‘void*’, but argument 5 has type ‘slp_tree’ {aka ‘_slp_tree*’} [-Wformat=]
 2528 |   dump_printf_loc (metadata, user_loc, "node%s %p (max_nunits=%u, refcnt=%u)",
      |                                                ~^
      |                                                 |
      |                                                 void*
......
 2533 |                       : ""), node,
      |                              ~~~~                
      |                              |
      |                              slp_tree {aka _slp_tree*}
../../gcc/tree-vect-slp.cc:2528:64: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘long int’ [-Wformat=]
 2528 |   dump_printf_loc (metadata, user_loc, "node%s %p (max_nunits=%u, refcnt=%u)",
      |                                                               ~^
      |                                                                |
      |                                                                unsigned int
      |                                                               %lu
......
 2534 |                    estimated_poly_value (node->max_nunits),
      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~      
      |                                         |
      |                                         long int
Comment 1 Richard Biener 2022-08-31 12:16:18 UTC
Confirmed.
Comment 2 Jakub Jelinek 2022-08-31 16:22:56 UTC
Created attachment 53526 [details]
gcc13-pr106782.patch

I have actually fixed most of these warnings.
There is one remaining though and I have no idea what to do about that:
../../gcc/tree-vect-slp-patterns.cc: In function ‘bool vect_pattern_validate_optab(internal_fn, slp_tree)’:
../../gcc/tree-vect-slp-patterns.cc:99:72: warning: format ‘%T’ expects argument of type ‘tree_node*’, but argument 4 has type ‘int’ [-Wformat=]
   99 |                              "Target does not support vector type for %T\n",
      |                                                                       ~^
      |                                                                        |
      |                                                                        tree_node*
      |                                                                       %d
SLP_TREE_DEF_TYPE (node) is indeed an enum, while %T expects a tree.
Added in r11-5969-g3ed472af6bc9f83b7a8a
Comment 3 Jakub Jelinek 2022-08-31 16:24:08 UTC
Tamar, any thoughts on that?
Comment 4 Tamar Christina 2022-08-31 16:42:08 UTC
(In reply to Jakub Jelinek from comment #3)
> Tamar, any thoughts on that?

Apologies, didn't notice that earlier.

That should be "Target does not support vector type for %G\n"
with STMT_VINFO_STMT (SLP_TREE_REPRESENTATIVE (node))

to print the statement it can't vectorize.
Comment 5 GCC Commits 2022-09-01 09:08:20 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:953e08fde44a596e4ec2491efd15cd645e1ddc48

commit r13-2336-g953e08fde44a596e4ec2491efd15cd645e1ddc48
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Sep 1 11:07:44 2022 +0200

    Fix up dump_printf_loc format attribute and adjust uses [PR106782]
    
    As discussed on IRC, the r13-2299-g68c61c2daa1f bug only got missed
    because dump_printf_loc had incorrect format attribute and therefore
    almost no -Wformat=* checking was performed on it.
    3, 0 are suitable for function with (whatever, whatever, const char *, va_list)
    arguments, not for (whatever, whatever, const char *, ...), that one should
    use 3, 4.
    
    The following patch fixes that and adjusts all spots to fix warnings.
    In many cases it is just through an ugly cast (for %G casts to gimple *
    from gassign */gphi * and the like and for %p casts to void * from slp_node
    etc.).
    There are 3 spots where the mismatch was worse though, two using %u or %d
    for unsigned HOST_WIDE_INT argument and one %T for enum argument (promoted
    to int).
    
    2022-09-01  Jakub Jelinek  <jakub@redhat.com>
    
            PR other/106782
            * dumpfile.h (dump_printf_loc): Use ATTRIBUTE_GCC_DUMP_PRINTF (3, 4)
            instead of ATTRIBUTE_GCC_DUMP_PRINTF (3, 0).
            * tree-parloops.cc (parloops_is_slp_reduction): Cast pointers to
            derived types of gimple to gimple * to avoid -Wformat warnings.
            * tree-vect-loop-manip.cc (vect_set_loop_condition,
            vect_update_ivs_after_vectorizer): Likewise.
            * tree-vect-stmts.cc (vectorizable_load): Likewise.
            * tree-vect-patterns.cc (vect_split_statement,
            vect_recog_mulhs_pattern, vect_recog_average_pattern,
            vect_determine_precisions_from_range,
            vect_determine_precisions_from_users): Likewise.
            * gimple-loop-versioning.cc
            (loop_versioning::analyze_term_using_scevs): Likewise.
            * tree-vect-slp.cc (vect_build_slp_tree_1): Likewise.
            (vect_build_slp_tree): Cast slp_tree to void * to avoid
            -Wformat warnings.
            (optimize_load_redistribution_1, vect_match_slp_patterns,
            vect_build_slp_instance, vect_optimize_slp_pass::materialize,
            vect_optimize_slp_pass::dump, vect_slp_convert_to_external,
            vect_slp_analyze_node_operations, vect_bb_partition_graph): Likewise.
            (vect_print_slp_tree): Likewise.  Also use
            HOST_WIDE_INT_PRINT_UNSIGNED instead of %u.
            * tree-vect-loop.cc (vect_determine_vectorization_factor,
            vect_analyze_scalar_cycles_1, vect_analyze_loop_operations,
            vectorizable_induction, vect_transform_loop): Cast pointers to derived
            types of gimple to gimple * to avoid -Wformat warnings.
            (vect_analyze_loop_2): Cast slp_tree to void * to avoid
            -Wformat warnings.
            (vect_estimate_min_profitable_iters): Use HOST_WIDE_INT_PRINT_UNSIGNED
            instead of %d.
            * tree-vect-slp-patterns.cc (vect_pattern_validate_optab): Use %G
            instead of %T and STMT_VINFO_STMT (SLP_TREE_REPRESENTATIVE (node))
            instead of SLP_TREE_DEF_TYPE (node).
Comment 6 GCC Commits 2022-09-01 09:41:43 UTC
The releases/gcc-12 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:f9593025a290c68c0916dc6fa569eb38eda00535

commit r12-8734-gf9593025a290c68c0916dc6fa569eb38eda00535
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Sep 1 11:07:44 2022 +0200

    Fix up dump_printf_loc format attribute and adjust uses [PR106782]
    
    As discussed on IRC, the r13-2299-g68c61c2daa1f bug only got missed
    because dump_printf_loc had incorrect format attribute and therefore
    almost no -Wformat=* checking was performed on it.
    3, 0 are suitable for function with (whatever, whatever, const char *, va_list)
    arguments, not for (whatever, whatever, const char *, ...), that one should
    use 3, 4.
    
    There are 3 spots where the mismatch was worse though, two using %u or %d
    for unsigned HOST_WIDE_INT argument and one %T for enum argument (promoted
    to int) and this backport just fixes those spots.
    
    2022-09-01  Jakub Jelinek  <jakub@redhat.com>
    
            PR other/106782
            * tree-vect-slp.cc (vect_print_slp_tree): Use
            HOST_WIDE_INT_PRINT_UNSIGNED instead of %u.
            * tree-vect-loop.cc (vect_estimate_min_profitable_iters): Use
            HOST_WIDE_INT_PRINT_UNSIGNED instead of %d.
            * tree-vect-slp-patterns.cc (vect_pattern_validate_optab): Use %G
            instead of %T and STMT_VINFO_STMT (SLP_TREE_REPRESENTATIVE (node))
            instead of SLP_TREE_DEF_TYPE (node).
    
    (cherry picked from commit 953e08fde44a596e4ec2491efd15cd645e1ddc48)
Comment 7 Jakub Jelinek 2022-09-01 09:43:20 UTC
Fixed now.
Comment 8 GCC Commits 2022-09-01 21:05:33 UTC
The master branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:b98c5262d02c13cdbbf3b985859b436adec94d90

commit r13-2357-gb98c5262d02c13cdbbf3b985859b436adec94d90
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Sep 1 22:04:57 2022 +0100

    AArch64: Fix bootstrap failure due to dump_printf_loc format attribute uses [PR106782]
    
    This fixes the bootstrap failure on AArch64 following -Werror=format by
    correcting the print format modifiers in the backend.
    
    gcc/ChangeLog:
    
            PR other/106782
            * config/aarch64/aarch64.cc
            (aarch64_vector_costs::prefer_unrolled_loop): Replace %u with
            HOST_WIDE_INT_PRINT_UNSIGNED.
Comment 9 GCC Commits 2022-09-02 08:23:36 UTC
The releases/gcc-12 branch has been updated by Tamar Christina <tnfchris@gcc.gnu.org>:

https://gcc.gnu.org/g:e69134e12551a4289292e3955525f84d99773d31

commit r12-8736-ge69134e12551a4289292e3955525f84d99773d31
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Sep 1 22:04:57 2022 +0100

    AArch64: Fix bootstrap failure due to dump_printf_loc format attribute uses [PR106782]
    
    This fixes the bootstrap failure on AArch64 following -Werror=format by
    correcting the print format modifiers in the backend.
    
    gcc/ChangeLog:
    
            PR other/106782
            * config/aarch64/aarch64.cc
            (aarch64_vector_costs::prefer_unrolled_loop): Replace %u with
            HOST_WIDE_INT_PRINT_UNSIGNED.
    
    (cherry picked from commit b98c5262d02c13cdbbf3b985859b436adec94d90)