Message ID | d33218bb-df7d-420c-18ce-2e3aa1fe5ebe@netcologne.de |
---|---|
State | New |
Headers | show |
Series | [fortran] Optionally improve debugging of auxiliary variables | expand |
Thomas Koenig via Fortran <fortran@gcc.gnu.org> wrote: > Hello world, > > I have struggled with debugging the GENERIC generated by the > Fortran front end because it is only possible to look at the > code via -fdump-tree-original, but it is not possible to > inspect the values; additionally, the D.3456 form makes it > hard to read which variable is which. > > This patch adds a flag which gives all variables generated by > Fortran a name and makes them visible to debuggers. As an > example, compiler-generated variables now look like this > (for a "Hello, world" program): > > { > struct __st_parameter_dt dt_parm:0; > > dt_parm:0.common.filename = &"hello.f90"[1]{lb: 1 sz: 1}; > dt_parm:0.common.line = 2; > dt_parm:0.common.flags = 128; > dt_parm:0.common.unit = 6; > _gfortran_st_write (&dt_parm:0); > _gfortran_transfer_character_write (&dt_parm:0, &"Hello, world"[1]{lb: 1 sz: 1}, 12); > _gfortran_st_write_done (&dt_parm:0); > } > > Note the colon in the variable name, which I chose because it is > not in the user's namespace, and gdb does not choke on it. If it’s part of a symbol used by the rest of the toolchain (assembler, linker debugger) then it’s also important to note that some OS/tool pairs might be more constrained than the one you’ve tested. In particular, some assemblers will not accept all characters in an identifier. > In order to inspect the variables, you usually have to step > a bit through the generated assembly code, but you can then > print the values, manipulate them etc (and sometimes also hit > an internal error in gdb). > --- a/gcc/fortran/trans.c > +++ b/gcc/fortran/trans.c > @@ -73,6 +73,40 @@ gfc_advance_chain (tree t, int n) > return t; > + /* We want debug info for it. */ > + DECL_IGNORED_P (t) = 0; > + /* It should not be nameless. */ > + DECL_NAMELESS (t) = 0; > tree > @@ -80,6 +114,9 @@ gfc_create_var_np (tree type, const char *prefix) > { > tree t; > > + if (flag_debug_aux_vars) > + return create_var_debug_raw (type, prefix); > + > t = create_tmp_var_raw (type, prefix); > You could take advantage of the understanding of assembler identifier rules built into create_var_debug_raw() .. perhaps (totally untested)…. if (flag_debug_aux_vars) prefix = prefix ? prefix : “gfc”; t = create_tmp_var_raw (type, prefix); if (flag_debug_aux_vars) { /* We want debug info for it. */ DECL_IGNORED_P (t) = false; /* It should not be nameless. */ DECL_NAMELESS (t) = false; } return t; … or doens’t this approach work for some reason? cheers Iain
Hi Iain, > If it’s part of a symbol used by the rest of the toolchain (assembler, > linker > debugger) then it’s also important to note that some OS/tool pairs might > be more constrained than the one you’ve tested. In particular, some > assemblers will not accept all characters in an identifier. That is, of course, true. What I have works for the usual Linux toolchain. Since I do not have access to other systems where I can compile gcc (see the recent *BSD desaster when I tried this), the most I could do is to make the character configurable - use : by default, use something else if specified. > You could take advantage of the understanding of assembler identifier rules > built into create_var_debug_raw() > > .. perhaps (totally untested)…. > > if (flag_debug_aux_vars) > prefix = prefix ? prefix : “gfc”; > > t = create_tmp_var_raw (type, prefix); > if (flag_debug_aux_vars) > { > /* We want debug info for it. */ > DECL_IGNORED_P (t) = false; > /* It should not be nameless. */ > DECL_NAMELESS (t) = false; > } > > return t; > > > … or doens’t this approach work for some reason? This doesn't work for gdb because gdb searches for a variable called "S" when trying to access "S.0". I also tried out @ as a separation character; that is also interpreted by gdb in a special way. And I wanted to avoid anything in the namespace of Fortran identifiers, even with options, so _ and $ were out. Do you have any other suggestions? Best regards Thomas
Hi Thomas, Thomas Koenig <tkoenig@netcologne.de> wrote: >> If it’s part of a symbol used by the rest of the toolchain (assembler, >> linker >> debugger) then it’s also important to note that some OS/tool pairs might >> be more constrained than the one you’ve tested. In particular, some >> assemblers will not accept all characters in an identifier. > > That is, of course, true. What I have works for the usual Linux > toolchain. Since I do not have access to other systems where > I can compile gcc (see the recent *BSD desaster when I tried this), > the most I could do is to make the character configurable - use > : by default, use something else if specified. yes a configure option is a possible way around a conflict. >> You could take advantage of the understanding of assembler identifier >> rules >> built into create_var_debug_raw() >> .. perhaps (totally untested)…. >> if (flag_debug_aux_vars) >> prefix = prefix ? prefix : “gfc”; >> t = create_tmp_var_raw (type, prefix); >> if (flag_debug_aux_vars) >> { >> /* We want debug info for it. */ >> DECL_IGNORED_P (t) = false; >> /* It should not be nameless. */ >> DECL_NAMELESS (t) = false; >> } >> return t; >> … or doens’t this approach work for some reason? > > This doesn't work for gdb because gdb searches for > a variable called "S" when trying to access "S.0". > I also tried out @ as a separation character; that > is also interpreted by gdb in a special way. and there are other debug consumers in common use - at least lldb. > And I wanted to avoid anything in the namespace of > Fortran identifiers, even with options, so _ and $ were > out. > > Do you have any other suggestions? At the moment none other than the obvious.. .. test the current proposal on a wider range of systems - I expect someone would volunteer to test on Darwin (Dominique or me) .. you could use the cfarm for AIX, Solaris and others (I mention AIX, Solaris and Darwin as three known to use non-GNU assemblers and linkers). Iain
Hi Iain,
> yes a configure option is a possible way around a conflict.
I was thinking more along making it a run-time option.
This is an option which will only be used by a developer,
who should know what the source code contains and what
the tool chain supports (and who can try out an underscore
if nothing else works).
So, something along the lines of -fdebug-aux-vars=: would be
equivalent to -fdebug-aux-vars, -fdebug-aux-vars=_ should work
on any system, with the (small) risk of colliding with user code.
How does that sound?
Best regards
Thomas
Thomas Koenig via Fortran <fortran@gcc.gnu.org> wrote: >> yes a configure option is a possible way around a conflict. > > I was thinking more along making it a run-time option. > > This is an option which will only be used by a developer, > who should know what the source code contains and what > the tool chain supports (and who can try out an underscore > if nothing else works). I guess the hard thing is for the developer to know that (s)he wants the option and what to do when a conflict occurs. > So, something along the lines of -fdebug-aux-vars=: would be > equivalent to -fdebug-aux-vars, -fdebug-aux-vars=_ should work > on any system, with the (small) risk of colliding with user code. naming is ever hard - although I suppose, if this is very infrequent in use, having a long name isn’t so terrible maybe: -fdebug-aux-vars-with-separator=x and -fdebug-aux-vars as an alias to '-fdebug-aux-vars-with-separator=:' (so that, unless one needs something different, the shorter version is available). FAOD, this is ‘thinking aloud’ about what might make it easier to understand; your version is equivalent other than the names. > How does that sound? A reasonable compromise (at this point I will defer to those folks who are actually likely to use the facility). cheers Iain
Hi Iain, > I guess the hard thing is for the developer to know that (s)he wants > the option and what to do when a conflict occurs. Actually, I just hit on a much simpler solution. We translate all symbols to lower case (except for those with BIND(C), which do not matter in this context). So, prefixing everything with GFC_ (upper case) should work on any toolchain that can handle C, which is all we care about. So, here is a patch which implements this method. The tree dump now looks like this: struct __st_parameter_dt GFC_dt_parm_0; GFC_dt_parm_0.common.filename = &"hello.f90"[1]{lb: 1 sz: 1}; GFC_dt_parm_0.common.line = 2; GFC_dt_parm_0.common.flags = 128; GFC_dt_parm_0.common.unit = 6; _gfortran_st_write (&GFC_dt_parm_0); _gfortran_transfer_character_write (&GFC_dt_parm_0, &"Hello, world"[1]{lb: 1 sz: 1}, 12); _gfortran_st_write_done (&GFC_dt_parm_0); So, thanks for raising your concerns, I like this much better now :-) Regression-tested. Also tested with "make dvi" and "make pdf". If there are no other comments, I'd like to commit this as having no user impact and kind of obvious (now :-) tomorrow or so. Best regards Thomas
Hi Thomas, Thomas Koenig via Fortran <fortran@gcc.gnu.org> wrote: >> I guess the hard thing is for the developer to know that (s)he wants >> the option and what to do when a conflict occurs. > > Actually, I just hit on a much simpler solution. :) .. I’m all in favour of simplicity. > We translate all symbols to lower case (except for those > with BIND(C), which do not matter in this context). So, > prefixing everything with GFC_ (upper case) should work on > any toolchain that can handle C, which is all we care about. For bonus points you can prefix with _GFC_ which puts the symbols in the implementation’s namespace (at least for C-Family purposes) and therefore means that a clash with a user’s symbols is the user’s problem … > So, here is a patch which implements this method. The tree > dump now looks like this: > > struct __st_parameter_dt GFC_dt_parm_0; > > GFC_dt_parm_0.common.filename = &"hello.f90"[1]{lb: 1 sz: 1}; > GFC_dt_parm_0.common.line = 2; > GFC_dt_parm_0.common.flags = 128; > GFC_dt_parm_0.common.unit = 6; > _gfortran_st_write (&GFC_dt_parm_0); > _gfortran_transfer_character_write (&GFC_dt_parm_0, &"Hello, world"[1]{lb: 1 sz: 1}, 12); > _gfortran_st_write_done (&GFC_dt_parm_0); > > So, thanks for raising your concerns, I like this much better now :-) > > Regression-tested. Also tested with "make dvi" and "make pdf". > If there are no other comments, I'd like to commit this as having > no user impact and kind of obvious (now :-) tomorrow or so. > > Best regards > > Thomas > <p3.diff>
Am 13.12.20 um 20:59 schrieb Iain Sandoe via Fortran: > For bonus points you can prefix with _GFC_ which puts the symbols > in the implementation’s namespace (at least for C-Family purposes) > and therefore means that a clash with a user’s symbols is the user’s > problem … I don't think this is necessary. Specifying uppercase letters is already a technical impossibility, we don't need to add convention to it as well :-) After reading some tree dumps, I have also now come to the conclusion that adding GFC_ to everything makes for poor reading. In the version I have committed as r11-6087, I have now capitalized all prefixes, if present, and used GFC_ otherweise. Just one more remark: This version lets you set a watchpoint on num_var so you can easily stop if a certain variable is generated. That also helps :-) Best regards, and thanks again for the comments. Thomas Add the -fdebug-aux-vars flag to debug variables generated by Fortran. gcc/fortran/ChangeLog: PR fortran/90207 * invoke.texi: Document -fdebug-aux-vars. * lang.opt: Add -fdebug-aux-vars. * trans.c (MAX_PREFIX_LEN): New macro. (create_var_debug_raw): New function. (gfc_create_var_np): Call create_var_debug_raw if flag_debug_aux_vars is set.
diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi index 8bdc8a6b038..58e4e50b315 100644 --- a/gcc/fortran/invoke.texi +++ b/gcc/fortran/invoke.texi @@ -1227,6 +1227,17 @@ compiler itself. The output generated by this option might change between releases. This option may also generate internal compiler errors for features which have only recently been added. +@item -fdebug-aux-vars +@opindex @code{debug-aux-vars} +Make internal variables generated by the gfortran front end visible to a +debugger. This option also renames these internal variables shown by +@code{-fdump-tree-original} to a form @code{string:number} and enables +compiler warnings on them. + +This option is only really useful when debugging the gfortran compiler +itself together with @code{-g} or @code{-ggdb3} and +@code{-fdump-tree-original}. + @item -ffpe-trap=@var{list} @opindex @code{ffpe-trap=}@var{list} Specify a list of floating point exception traps to enable. On most diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt index 96ed208cb85..57b0264458e 100644 --- a/gcc/fortran/lang.opt +++ b/gcc/fortran/lang.opt @@ -452,6 +452,10 @@ fd-lines-as-comments Fortran RejectNegative Treat lines with 'D' in column one as comments. +fdebug-aux-vars +Fortran Var(flag_debug_aux_vars) +Issue debug information for compiler-generated auxiliary variables. + fdec Fortran Var(flag_dec) Enable all DEC language extensions. diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index 025abe38985..1d5f6c28c7a 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -73,6 +73,40 @@ gfc_advance_chain (tree t, int n) return t; } +static int num_var; + +#define MAX_PREFIX_LEN 20 + +static tree +create_var_debug_raw (tree type, const char *prefix) +{ + /* Space for prefix + @ + 9-digit-number + \0. */ + char name_buf[MAX_PREFIX_LEN + 1 + 9 + 1]; + tree t; + + if (prefix == NULL) + prefix = "gfc"; + else + gcc_assert (strlen(prefix) <= MAX_PREFIX_LEN); + + snprintf (name_buf, sizeof(name_buf), "%s:%d", prefix, num_var++); + t = build_decl (input_location, VAR_DECL, get_identifier (name_buf), type); + + /* We want debug info for it. */ + DECL_IGNORED_P (t) = 0; + /* It should not be nameless. */ + DECL_NAMELESS (t) = 0; + + /* Make the variable writable. */ + TREE_READONLY (t) = 0; + + DECL_EXTERNAL (t) = 0; + TREE_STATIC (t) = 0; + TREE_USED (t) = 1; + + return t; +} + /* Creates a variable declaration with a given TYPE. */ tree @@ -80,6 +114,9 @@ gfc_create_var_np (tree type, const char *prefix) { tree t; + if (flag_debug_aux_vars) + return create_var_debug_raw (type, prefix); + t = create_tmp_var_raw (type, prefix); /* No warnings for anonymous variables. */