siddhesh / fortify-metrics

Get build time statistics of _FORTIFY_SOURCE coverage for programs
Other
8 stars 3 forks source link

ICE with builtin-dynamic-object-size-0.c #3

Open thesamesam opened 10 hours ago

thesamesam commented 10 hours ago

Hit this when running make check:

Starting program: /usr/libexec/gcc/x86_64-pc-linux-gnu/15/cc1 -quiet -iplugindir=/usr/lib/gcc/x86_64-pc-linux-gnu/15/plugin builtin-dynamic-object-size-0.c -iplugindir=/usr/lib/gcc/x86_64-pc-linux-gnu/15/plugin -quiet -dumpbase builtin-dynamic-object-size-0.c -dumpbase-ext .c -mtune=generic -march=x86-64 -O2 -fplugin=../fmetrics.so -fplugin-arg-fmetrics-project=fmetrics -o /dev/null
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib64/libthread_db.so.1".
builtin-dynamic-object-size-0.c: In function ‘main’:
builtin-dynamic-object-size-0.c:646:7: warning: ‘test_parmsz_scaled’ accessing 176 bytes in a region of size 168 [-Wstringop-overflow=]
  646 |   if (test_parmsz_scaled (arr, 44) != 44 * sizeof (int)) /* { dg-warning "-Wstringop-overflow=" } */
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
builtin-dynamic-object-size-0.c:411:1: note: in a call to function ‘test_parmsz_scaled’ declared with attribute ‘access (read_write, 1, 2)’
  411 | test_parmsz_scaled (int *obj, size_t sz)
      | ^~~~~~~~~~~~~~~~~~

Breakpoint 3, internal_error (gmsgid=gmsgid@entry=0x555557fb11f7 "in %s, at %s:%d") at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/diagnostic-global-context.cc:512
512     {
(gdb) bt
#0  internal_error (gmsgid=gmsgid@entry=0x555557fb11f7 "in %s, at %s:%d") at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/diagnostic-global-context.cc:512
#1  0x0000555556d86b7e in fancy_abort (file=0x555557fbb6a8 "/var/tmp/portage/sys-devel/gcc-15.0.9999/work/gcc-15.0.9999/gcc/passes.cc", line=2150,
    function=0x555557f0d3cb "execute_todo") at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/diagnostic.cc:1657
#2  0x00005555555d0c9a in execute_todo (flags=64) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2150
#3  0x00005555555cf43a in execute_one_pass (pass=warning: RTTI symbol not found for class 'pass_fmetrics'
warning: RTTI symbol not found for class 'pass_fmetrics'
<opt_pass* 0x55555880a4c0 "fmetrics"(369)>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2705
#4  0x0000555557070e4c in execute_pass_list_1 (pass=warning: RTTI symbol not found for class 'pass_fmetrics'
warning: RTTI symbol not found for class 'pass_fmetrics'
<opt_pass* 0x55555880a4c0 "fmetrics"(369)>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2768
#5  0x0000555557070e6b in execute_pass_list_1 (pass=<opt_pass* 0x555558829b40 "*all_optimizations"(-1)>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2769
#6  0x0000555557070bee in execute_pass_list (fn=0x7ffff6c050c0, pass=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2779
#7  0x0000555556fe1626 in cgraph_node::expand (this=<cgraph_node * const 0x7ffff6c08000 "test_strdup"/43>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/context.h:48
#8  0x0000555556f60ed6 in expand_all_functions () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2028
#9  symbol_table::compile (this=0x7ffff7206000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2404
#10 0x00005555577915ee in symbol_table::finalize_compilation_unit (this=0x7ffff7206000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2589
#11 0x00005555577328e1 in compile_file () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:478
#12 0x00005555576e222f in do_compile () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:2213
#13 toplev::main (this=this@entry=0x7fffffffd566, argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:2373
#14 0x00005555576e12fc in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/main.cc:39
(gdb) frame 2
#2  0x00005555555d0c9a in execute_todo (flags=64) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2150
2150        gcc_assert (flags & TODO_update_ssa_any);

Needs checking to hit it, I guess.

thesamesam commented 10 hours ago

OK (keeping in mind I'm very new to this):

static void
execute_todo (unsigned int flags)
{
  if (flag_checking
      && cfun
      && need_ssa_update_p (cfun))
    gcc_assert (flags & TODO_update_ssa_any);
[...]
/* Return true if there is any work to be done by update_ssa
   for function FN.  */

bool
need_ssa_update_p (struct function *fn)
{
  gcc_assert (fn != NULL);
  return (update_ssa_initialized_fn == fn
          || (fn->gimple_df && fn->gimple_df->ssa_renaming_needed));
}

We only have PROP_ssa in properties_required (input?), not properties_provided (output?) so I don't get why it's expecting it to update SSA?

--

If I set a breakpoint for mark_virtual_operands_for_renaming, I see it gets called twice, both by tail_merge_optimize:

Breakpoint 9.1, mark_virtual_operands_for_renaming (fn=0x7ffff73f00c0) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-into-ssa.cc:3124
3124      fn->gimple_df->ssa_renaming_needed = 1;
(gdb) bt
#0  mark_virtual_operands_for_renaming (fn=0x7ffff73f00c0) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-into-ssa.cc:3124
#1  0x00005555573075fd in tail_merge_optimize (need_crit_edge_split=need_crit_edge_split@entry=false)
    at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-ssa-tail-merge.cc:1873
#2  0x00005555572e7d68 in (anonymous namespace)::pass_pre::execute (this=<optimized out>, fun=0x7ffff73f00c0)
    at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-ssa-pre.cc:4553
#3  0x00005555555cf08c in execute_one_pass (pass=0x55555882af50) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2659
#4  0x0000555557070e4c in execute_pass_list_1 (pass=0x55555882af50) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2768
#5  0x0000555557070e6b in execute_pass_list_1 (pass=0x555558829b40) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2769
#6  0x0000555557070bee in execute_pass_list (fn=0x7ffff73f00c0, pass=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2779
#7  0x0000555556fe1626 in cgraph_node::expand (this=0x7ffff73e9990) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/context.h:48
#8  0x0000555556f60ed6 in expand_all_functions () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2028
#9  symbol_table::compile (this=0x7ffff7206000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2404
#10 0x00005555577915ee in symbol_table::finalize_compilation_unit (this=0x7ffff7206000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2589
#11 0x00005555577328e1 in compile_file () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:478
#12 0x00005555576e222f in do_compile () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:2213
#13 toplev::main (this=this@entry=0x7fffffffd566, argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:2373
#14 0x00005555576e12fc in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/main.cc:39
(gdb) c
Continuing.

Breakpoint 9.1, mark_virtual_operands_for_renaming (fn=0x7ffff6c05000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-into-ssa.cc:3124
3124      fn->gimple_df->ssa_renaming_needed = 1;
(gdb) bt
#0  mark_virtual_operands_for_renaming (fn=0x7ffff6c05000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-into-ssa.cc:3124
#1  0x00005555573075fd in tail_merge_optimize (need_crit_edge_split=need_crit_edge_split@entry=false)
    at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/tree-ssa-tail-merge.cc:1873
#2  0x00005555572e7d68 in (anonymous namespace)::pass_pre::execute (this=<optimized out>, fun=0x7ffff6c05000)
    at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/`tree-ssa-pre`.cc:4553
#3  0x00005555555cf08c in execute_one_pass (pass=0x55555882af50) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2659
#4  0x0000555557070e4c in execute_pass_list_1 (pass=0x55555882af50) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2768
#5  0x0000555557070e6b in execute_pass_list_1 (pass=0x555558829b40) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2769
#6  0x0000555557070bee in execute_pass_list (fn=0x7ffff6c05000, pass=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/passes.cc:2779
#7  0x0000555556fe1626 in cgraph_node::expand (this=0x7ffff73f9ee0) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/context.h:48
#8  0x0000555556f60ed6 in expand_all_functions () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2028
#9  symbol_table::compile (this=0x7ffff7206000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2404
#10 0x00005555577915ee in symbol_table::finalize_compilation_unit (this=0x7ffff7206000) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/cgraphunit.cc:2589
#11 0x00005555577328e1 in compile_file () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:478
#12 0x00005555576e222f in do_compile () at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:2213
#13 toplev::main (this=this@entry=0x7fffffffd566, argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/toplev.cc:2373
#14 0x00005555576e12fc in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/sys-devel/gcc-15.0.9999/gcc-15.0.9999/gcc/main.cc:39
(gdb) c
Continuing.

Then when we die later on, the cfunc address is the same as the first fn. I'm not sure if this means that tree-ssa-pre or tree-ssa-tail-merge is forgetting a cleanup at some point?

thesamesam commented 10 hours ago

This "fixes" it but I'd like to understand why it's right (if it is) before submitting it as a PR:

diff --git a/fmetrics.cc b/fmetrics.cc
index 5653981..7079589 100644
--- a/fmetrics.cc
+++ b/fmetrics.cc
@@ -6,6 +6,7 @@
 #include <builtins.h>
 #include <context.h>
 #include <tree.h>
+#include <tree-cfgcleanup.h>
 #include <tree-object-size.h>
 #include <tree-pass.h>
 #include <tree-pretty-print.h>
@@ -107,6 +108,8 @@ unsigned int
 pass_fmetrics :: execute (function *fun)
 {
   basic_block bb;
+  unsigned int todo = 0;
+
   FOR_EACH_BB_FN (bb, fun)
     {
       gimple_stmt_iterator i;
@@ -146,5 +149,8 @@ pass_fmetrics :: execute (function *fun)

   fini_object_sizes ();

-  return 0;
+  if (cleanup_tree_cfg (TODO_update_ssa))
+    todo |= TODO_update_ssa;
+
+  return todo;
 }

I think we could instead add it to todo_flags_finish.

siddhesh commented 1 hour ago

We do end up having to update SSA for a couple of cases (strdup/strndup and the newer __counted_by__ stuff) but given that it's builtin-dynamic-object-size-0.c, it's probably strdup/strndup. TODO_update_ssa_only_virtuals should be sufficient I think, that's what the objsz pass uses. Returning todo like you do there is better since it'll only attempt to update SSA when actually needed, not in the bulk of the other cases.