Re: Sv: [PATCH] jit : Generate debug info for variables

[David Malcolm <dmalcolm@redhat.com>]

On Tue, 2021-08-31 at 00:23 +0000, Petter Tomner via Gcc-patches wrote:
> Well I seemed to have attached the wrong testcase. Here is the proper
> one attached.
> 
> Regards,
> 
> -----Ursprungligt meddelande-----
> Från: Petter Tomner 
> Skickat: den 31 augusti 2021 02:14
> Till: gcc-patches@gcc.gnu.org; jit@gcc.gnu.org
> Ämne: [PATCH] jit : Generate debug info for variables
> 
> Hi,
> 
> This is a patch to generate debug info for local variables as well as
> globals. 
> With this, "ptype foo", "info variables", "info locals" etc works when
> debugging in GDB.
> 
> Finalizing of global variable declares are moved to after locations are
> handled and done
> as Fortran, C, Go etc do it. Also, primitive types have their TYPE_NAME
> set for debug info
> on types to work.
> 
> Below are the patch, and I attached a testcase. Since it requires GDB
> to run it might
> not be suitable? Make check-jit runs fine on Debian x64.
> 
> Regards,

> From 6a5d24cbe80429d19042e643bd4c23940cd185fa Mon Sep 17 00:00:00 2001
> From: Petter Tomner <tomner@kth.se>
> Date: Mon, 30 Aug 2021 01:45:11 +0200
> Subject: [PATCH 2/2] libgccjit: Test cases for debug info
> 
> Assure that debug info is available for variables.
> 
> gcc/testsuite/jit.dg/
> 	jit.exp: Helper function
> 	test-debuginfo.c

Again, please provided non-empty ChangeLog entries.

You can use contrib/gcc-changelog/git_check_commit.py to validate them.

I don't see "Signed-off-by" tags in the patches.  Have you either filed
a copyright assignment with the FSF, or can you please add the tags to
certify that you wrote the patches and can contribute them, see:
  https://gcc.gnu.org/contribute.html#legal
  https://gcc.gnu.org/dco.html

[...snip...]

> +proc jit-check-debug-info { obj_file cmds match } {
> +    verbose "Checking debug info for $obj_file with match: $match"
> +
> +    if { [catch {exec gdb -v} fid] } {
> +        verbose "No gdb seems to be in path. Can't check debug info.
Reporting expected fail."
> +        xfail "No gdb seems to be in path. Can't check debug info"

I think this should be "unsupported" rather than "xfail".

[...snip...]

> diff --git a/gcc/testsuite/jit.dg/test-debuginfo.c
b/gcc/testsuite/jit.dg/test-debuginfo.c
> new file mode 100644
> index 00000000000..0af5779fdd1
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-debuginfo.c
> @@ -0,0 +1,72 @@
> +/* Essentially this test checks that debug info are generated for
globals
> +   locals and functions, including type info.  The comment bellow is
used
> +   as fake code (does not affect the test, use for manual
debugging). */
> +/*
> +int a_global_for_test_debuginfo;
> +int main (int argc, char **argv)
> +{
> +    int a_local_for_test_debuginfo = 2;
> +    return a_global_for_test_debuginfo + a_local_for_test_debuginfo;
> +}
> +*/

This is OK, but maybe using gcc_jit_context_dump_to_file with
update_locations == 1 might be more sustainable in the long run?  See:
  https://gcc.gnu.org/onlinedocs/jit/topics/locations.html#faking-it

OK otherwise.

Thanks
Dave