Re: [PATCH] Invalid gimple __BB# accepted due to usage of atoi -> replace atoi with stroul in c_parser_gimple_parse_bb_spec [PR114541]

[Richard Biener]

On Sat, Dec 7, 2024 at 12:28 AM Heiko Eißfeldt <he...@hexco.de> wrote:
>
> and here is the forgotten patch (it is late...)

Hmm.  While this might seem to be "good", the failure mode after fixing
the issue with atoi (or the alternate failure mode when using __BBB1234)
shows that we are missing handling in the GIMPLE frontend for
labels used that are not declared - lookup_label_for_goto tentatively
records a label.  I couldn't quickly find how the C frontend later discovers
used but not bound labels - in principle the GIMPLE FE would need sth
similar.

There's also missing support for indirect gotos it seems.

void foo(int x)
{
  void *l = &&lab;
lab:
  goto *l;
}

is, when in SSA form

void __GIMPLE (ssa)
foo (int x)
{
  void * gotovar.0;
  void * l;
  void * _2;

  __BB(2):
  l_1 = &lab;
  goto __BB3;

  __BB(3):
lab:
  _2 = l_1;
  goto __BB4;

  __BB(4):
  goto _2;

}

so you can see we have to support a non-__BB(N) goto even on GIMPLE
(labels shouldn't appear there - those are resolved to __BB(N)).

With your patch applied the following still ICEs in calc_dfs_tree for me
and no error is emitted:

void __GIMPLE (cfg)
foo (int x)
{
  void * gotovar;
  void * l;

  __BB(2):
lab:
  goto __BBB3;

  __BB(3):
  if (x != 0)
    goto __BB5;
  else
    goto __BB6;

  __BB(4):
  goto gotovar;

  __BB(5):
  gotovar = l;
  goto __BB4;

  __BB(6):
  return;

}

Note you picked a difficult patch - as said, the atoi fix looks OK to
me, even if
it doesn't solve downstream issues.  Can you split that out so I can
approve that
separately?

The GIMPLE FE is really not meant to parse arbitrary errorneous input - the ICEs
you run into should be viewed as error reporting (even if not exactly
helpful in this case).

That said, we should emit helpful errors for feeding
-fdump-tree-X-gimple output into
the GIMPLE FE since the dump output you get is not yet parseable without manual
editing.  "Fuzzing" attempts shouldn't be considered priority when fixing.

Thanks,
Richard.