[PATCH] Accept functions with DW_AT_linkage_name present

[PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

[Public]

Hi all,

    Currently GDB is not able to debug (Binary generated with Clang) variables
    present in shared/private clause of OpenMP Task construct. Please note that
    LLVM debugger LLDB is able to debug.
    # make check RUNTESTFLAGS='CC_FOR_TARGET="clang" ' TESTS="gdb.threads/omp-task.exp"

    In case of OpenMP, compilers generate artificial functions which are not
    present in actual program. This is done to apply parallelism to block of
    code.

    For non-artifical functions, DW_AT_name attribute should contains the name
    exactly as present in actual program.
    (Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
    Since artificial functions are not present in actual program they not having
    DW_AT_name and having DW_AT_linkage_name instead should be fine. This
    Is what Clang chooses to do.

    Currently GDB is invalidating any function not havnig DW_AT_name that is why
    it is not able to debug OpenMP (Clang).

    It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
    is absent. The current patch applies this and enables GDB to debug Clang generated
    binaries correctly.

    Please review the patch and let me know your thoughts.

Regards,
Alok

[-- Attachment #2: 0001-PATCH-Accept-functions-with-DW_AT_linkage_name-prese.patch --]
[-- Type: application/octet-stream, Size: 6097 bytes --]

From 208630ac735e4cd1cfc368f8e60a09269a18edc1 Mon Sep 17 00:00:00 2001
From: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date: Sun, 10 Apr 2022 18:50:01 +0530
Subject: [PATCH] [PATCH] Accept functions with DW_AT_linkage_name present

Currently GDB is not able to debug (Binary generated with Clang) variables
present in shared/private clause of OpenMP Task construct. Please note that
LLVM debugger LLDB is able to debug.

In case of OpenMP, compilers generate artificial functions which are not
present in actual program. This is done to apply parallelism to block of
code.

For non-artifical functions, DW_AT_name attribute should contains the name
exactly as present in actual program.
(Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
Since artificial functions are not present in actual program they not having
DW_AT_name and having DW_AT_linkage_name instead should be fine.

Currently GDB is invalidating any function not havnig DW_AT_name which is why
it is not able to debug OpenMP (Clang).

It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
is absent.

Change-Id: I5048af6c0d07c227be762241da02582918ab66b8
---
 gdb/dwarf2/read.c                      | 18 +++++++--
 gdb/testsuite/gdb.threads/omp-task.c   | 28 ++++++++++++++
 gdb/testsuite/gdb.threads/omp-task.exp | 51 ++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.c
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d8268de8031..d32895f55d9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -13045,7 +13045,7 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
   CORE_ADDR highpc;
   struct die_info *child_die;
   struct attribute *attr, *call_line, *call_file;
-  const char *name;
+  const char *name = NULL;
   CORE_ADDR baseaddr;
   struct block *block;
   int inlined_func = (die->tag == DW_TAG_inlined_subroutine);
@@ -13068,7 +13068,10 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
 
   baseaddr = objfile->text_section_offset ();
 
-  name = dwarf2_name (die, cu);
+  if (dwarf2_name (die, cu))
+    name = dwarf2_name (die, cu);
+  else
+    name = dw2_linkage_name (die, cu);
 
   /* Ignore functions with missing or empty names.  These are actually
      illegal according to the DWARF standard.  */
@@ -21753,7 +21756,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   struct objfile *objfile = per_objfile->objfile;
   struct gdbarch *gdbarch = objfile->arch ();
   struct symbol *sym = NULL;
-  const char *name;
+  const char *name = NULL;
   struct attribute *attr = NULL;
   struct attribute *attr2 = NULL;
   CORE_ADDR baseaddr;
@@ -21763,7 +21766,14 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 
   baseaddr = objfile->text_section_offset ();
 
-  name = dwarf2_name (die, cu);
+  if (dwarf2_name (die, cu))
+    name = dwarf2_name (die, cu);
+  else if (die->tag == DW_TAG_subprogram
+           || die->tag == DW_TAG_subroutine_type
+           || die->tag == DW_TAG_inlined_subroutine
+           || die->tag == DW_TAG_entry_point)
+    name = dw2_linkage_name (die, cu);
+
   if (name)
     {
       int suppress_add = 0;
diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
new file mode 100644
index 00000000000..27aa3e805dc
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.c
@@ -0,0 +1,28 @@
+#include <stdio.h>
+#include <omp.h>
+
+int foo(int n) {
+  int share1 = 9, share2 = 11, share3 = 13, priv1, priv2, fpriv;
+  fpriv = n + 4;
+
+  if (n < 2)
+    return n;
+  else {
+#pragma omp task shared(share1, share2) private(priv1, priv2) firstprivate(fpriv) shared(share3)
+    {
+      priv1 = n;
+      priv2 = n + 2;
+      share2 += share3;
+      printf("share1 = %d, share2 = %d, share3 = %d\n", share1, share2, share3);
+      share1 = priv1 + priv2 + fpriv + foo(n - 1) + share2 + share3;
+    }
+#pragma omp taskwait
+    return share1 + share2 + share3;
+  }
+}
+
+int main() {
+  int n = 10;
+  printf("foo(%d) = %d\n", n, foo(n));
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/omp-task.exp b/gdb/testsuite/gdb.threads/omp-task.exp
new file mode 100644
index 00000000000..d3b7a7ed0a0
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.exp
@@ -0,0 +1,51 @@
+# Copyright 2017-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Tests which verify (or not) that GDB can access shared and private
+# clauses of OpenMP task construct.
+
+standard_testfile
+
+set have_nested_function_support 0
+set opts {openmp debug}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $opts]} {
+    return -1
+}
+
+if {[info procs gdb_openmp_setup] != ""} {
+    if {[gdb_openmp_setup $binfile] != ""} {
+	untested "could not set up OpenMP environment"
+	return -1
+    }
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "omp task shared"]
+gdb_test "continue" ".*Breakpoint 2.*" "continue 1"
+gdb_test "print share1" "= 9"
+gdb_test "print share2" "= 11"
+gdb_test "print share3" "= 13"
+gdb_test "disable 2" ".*"
+gdb_breakpoint [gdb_get_line_number "share1 = priv1"]
+gdb_test "continue" ".*Breakpoint 3.*" "continue 2"
+gdb_test "print priv1" "= 10"
+gdb_test "print priv2" "= 12"
+gdb_test "print fpriv" "= 14"
-- 
2.25.1

Re: [PATCH] Accept functions with DW_AT_linkage_name present

[Tom Tromey]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

> Currently GDB is invalidating any function not havnig DW_AT_name which is why
> it is not able to debug OpenMP (Clang).

> It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
> is absent.

Thank you for the patch.

> -  name = dwarf2_name (die, cu);
> +  if (dwarf2_name (die, cu))
> +    name = dwarf2_name (die, cu);
> +  else
> +    name = dw2_linkage_name (die, cu);

Nowadays we are preferring checks against nullptr rather than implicit
decays to bool.

Also I think it's better to call dwarf2_name less often, so I'd write
this as:

   name = dwarf2_name (die, cu);
   if (name == nullptr)
     name = dw2_linkage_name (die, cu);

> -  name = dwarf2_name (die, cu);
> +  if (dwarf2_name (die, cu))
> +    name = dwarf2_name (die, cu);
> +  else if (die->tag == DW_TAG_subprogram
> +           || die->tag == DW_TAG_subroutine_type
> +           || die->tag == DW_TAG_inlined_subroutine
> +           || die->tag == DW_TAG_entry_point)
> +    name = dw2_linkage_name (die, cu);

Similar change here.
Though ... is this tag check really needed?

Tom

RE: [PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

Thanks Tom for your comments.

Please find the updated patch which include your suggestions.

> is this tag check really needed?
I want to keep this limited to functions.

Regards,
Alok

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Friday, April 15, 2022 10:00 PM
To: Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Accept functions with DW_AT_linkage_name present

[CAUTION: External Email]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

> Currently GDB is invalidating any function not havnig DW_AT_name which 
> is why it is not able to debug OpenMP (Clang).

> It should be fair to fallback to check DW_AT_linkage_name in case 
> DW_AT_name is absent.

Thank you for the patch.

> -  name = dwarf2_name (die, cu);
> +  if (dwarf2_name (die, cu))
> +    name = dwarf2_name (die, cu);
> +  else
> +    name = dw2_linkage_name (die, cu);

Nowadays we are preferring checks against nullptr rather than implicit decays to bool.

Also I think it's better to call dwarf2_name less often, so I'd write this as:

   name = dwarf2_name (die, cu);
   if (name == nullptr)
     name = dw2_linkage_name (die, cu);

> -  name = dwarf2_name (die, cu);
> +  if (dwarf2_name (die, cu))
> +    name = dwarf2_name (die, cu);
> +  else if (die->tag == DW_TAG_subprogram
> +           || die->tag == DW_TAG_subroutine_type
> +           || die->tag == DW_TAG_inlined_subroutine
> +           || die->tag == DW_TAG_entry_point)
> +    name = dw2_linkage_name (die, cu);

Similar change here.
Though ... is this tag check really needed?

Tom

[-- Attachment #2: 0001-PATCH-Accept-functions-with-DW_AT_linkage_name-prese.patch --]
[-- Type: application/octet-stream, Size: 5303 bytes --]

From 10dd28d6d5e07efd6afe34d4a522971611933efb Mon Sep 17 00:00:00 2001
From: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date: Sun, 10 Apr 2022 18:50:01 +0530
Subject: [PATCH] [PATCH] Accept functions with DW_AT_linkage_name present

Currently GDB is not able to debug (Binary generated with Clang) variables
present in shared/private clause of OpenMP Task construct. Please note that
LLVM debugger LLDB is able to debug.

In case of OpenMP, compilers generate artificial functions which are not
present in actual program. This is done to apply parallelism to block of
code.

For non-artifical functions, DW_AT_name attribute should contains the name
exactly as present in actual program.
(Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
Since artificial functions are not present in actual program they not having
DW_AT_name and having DW_AT_linkage_name instead should be fine.

Currently GDB is invalidating any function not havnig DW_AT_name which is why
it is not able to debug OpenMP (Clang).

It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
is absent.

Change-Id: I5048af6c0d07c227be762241da02582918ab66b8
---
 gdb/dwarf2/read.c                      |  8 ++++
 gdb/testsuite/gdb.threads/omp-task.c   | 28 ++++++++++++++
 gdb/testsuite/gdb.threads/omp-task.exp | 51 ++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.c
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6dcd446e5f4..10709b17179 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12000,6 +12000,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr)
+    name = dw2_linkage_name (die, cu);
 
   /* Ignore functions with missing or empty names.  These are actually
      illegal according to the DWARF standard.  */
@@ -20657,6 +20659,12 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr && (die->tag == DW_TAG_subprogram
+                          || die->tag == DW_TAG_subroutine_type
+                          || die->tag == DW_TAG_inlined_subroutine
+                          || die->tag == DW_TAG_entry_point))
+    name = dw2_linkage_name (die, cu);
+
   if (name)
     {
       int suppress_add = 0;
diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
new file mode 100644
index 00000000000..27aa3e805dc
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.c
@@ -0,0 +1,28 @@
+#include <stdio.h>
+#include <omp.h>
+
+int foo(int n) {
+  int share1 = 9, share2 = 11, share3 = 13, priv1, priv2, fpriv;
+  fpriv = n + 4;
+
+  if (n < 2)
+    return n;
+  else {
+#pragma omp task shared(share1, share2) private(priv1, priv2) firstprivate(fpriv) shared(share3)
+    {
+      priv1 = n;
+      priv2 = n + 2;
+      share2 += share3;
+      printf("share1 = %d, share2 = %d, share3 = %d\n", share1, share2, share3);
+      share1 = priv1 + priv2 + fpriv + foo(n - 1) + share2 + share3;
+    }
+#pragma omp taskwait
+    return share1 + share2 + share3;
+  }
+}
+
+int main() {
+  int n = 10;
+  printf("foo(%d) = %d\n", n, foo(n));
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/omp-task.exp b/gdb/testsuite/gdb.threads/omp-task.exp
new file mode 100644
index 00000000000..d3b7a7ed0a0
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.exp
@@ -0,0 +1,51 @@
+# Copyright 2017-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Tests which verify (or not) that GDB can access shared and private
+# clauses of OpenMP task construct.
+
+standard_testfile
+
+set have_nested_function_support 0
+set opts {openmp debug}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $opts]} {
+    return -1
+}
+
+if {[info procs gdb_openmp_setup] != ""} {
+    if {[gdb_openmp_setup $binfile] != ""} {
+	untested "could not set up OpenMP environment"
+	return -1
+    }
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "omp task shared"]
+gdb_test "continue" ".*Breakpoint 2.*" "continue 1"
+gdb_test "print share1" "= 9"
+gdb_test "print share2" "= 11"
+gdb_test "print share3" "= 13"
+gdb_test "disable 2" ".*"
+gdb_breakpoint [gdb_get_line_number "share1 = priv1"]
+gdb_test "continue" ".*Breakpoint 3.*" "continue 2"
+gdb_test "print priv1" "= 10"
+gdb_test "print priv2" "= 12"
+gdb_test "print fpriv" "= 14"
-- 
2.25.1

Re: [PATCH] Accept functions with DW_AT_linkage_name present

[Tom Tromey]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

>> is this tag check really needed?

> I want to keep this limited to functions.

Yes, but why?  Would there be something bad about using the linkage name
for a variable?

>    name = dwarf2_name (die, cu);
>    if (name == nullptr)
>      name = dw2_linkage_name (die, cu);

I guess if we avoided the tag check, this could just be done in
dwarf2_name itself.

> +  if (name == nullptr && (die->tag == DW_TAG_subprogram
> +                          || die->tag == DW_TAG_subroutine_type
> +                          || die->tag == DW_TAG_inlined_subroutine
> +                          || die->tag == DW_TAG_entry_point))

If there is a reason to only check functions then you should remove
DW_TAG_subroutine_type here, because that is a type, not a function.

Tom

RE: [PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]

Thanks Tom for your review comments. Attached is the updated patch to exclude DW_TAG_subroutine_type.

> Yes, but why?  Would there be something bad about using the linkage name for a variable?

Actually initially I tried without this conditions but that failed few gdb tests. Let me share the short testcase to explain.
-----------
#cat test.cxx
typedef enum
{
 DD, EE, FF
} anon_enum_t;

typedef anon_enum_t nested_anon_enum_t;
volatile nested_anon_enum_t var_s;

int
main ()
{
  asm ("" ::: "memory");
  return 0;
}
# g++ -g test.cxx -o test
---------------
I tried below command with the tag check and output matches what was there without this complete patch.
----------
# gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum : unsigned int {DD, EE, FF}
All defined types:

File test.cxx:
4:      typedef enum {...} anon_enum_t;
        int
6:      typedef enum {...} nested_anon_enum_t;
        unsigned int
-----------
When we remove that tag condition it looks like below
----------- 
/gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum 11anon_enum_t : unsigned int {DD, EE, FF}
All defined types:

File test.cxx:
2:      11anon_enum_t;
4:      typedef 11anon_enum_t anon_enum_t;
        int
6:      typedef 11anon_enum_t nested_anon_enum_t;
        unsigned int
---------- 

Please note the name "11anon_enum_t", which is linkage name (internal name) not exposed to user and showing that probably is not good. This linkage name is present only in case for C++ compilation only, in case the same code is in c and compile with gcc this linkage name is not present. So probably it is better to keep the output same as we get in C and not expose this linkage name for enums and keep it exposed only for functions. Please let me know if you differ, I shall change the testcases to suite the new output.

Regards,
Alok 

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Monday, April 18, 2022 7:15 PM
To: Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Accept functions with DW_AT_linkage_name present

[CAUTION: External Email]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

>> is this tag check really needed?

> I want to keep this limited to functions.

Yes, but why?  Would there be something bad about using the linkage name for a variable?

>    name = dwarf2_name (die, cu);
>    if (name == nullptr)
>      name = dw2_linkage_name (die, cu);

I guess if we avoided the tag check, this could just be done in dwarf2_name itself.

> +  if (name == nullptr && (die->tag == DW_TAG_subprogram
> +                          || die->tag == DW_TAG_subroutine_type
> +                          || die->tag == DW_TAG_inlined_subroutine
> +                          || die->tag == DW_TAG_entry_point))

If there is a reason to only check functions then you should remove DW_TAG_subroutine_type here, because that is a type, not a function.

Tom

[-- Attachment #2: 0001-PATCH-Accept-functions-with-DW_AT_linkage_name-prese.patch --]
[-- Type: application/octet-stream, Size: 5233 bytes --]

From 3887e4f9fa0097a3e58725d187305c6e4d281095 Mon Sep 17 00:00:00 2001
From: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date: Sun, 10 Apr 2022 18:50:01 +0530
Subject: [PATCH] [PATCH] Accept functions with DW_AT_linkage_name present

Currently GDB is not able to debug (Binary generated with Clang) variables
present in shared/private clause of OpenMP Task construct. Please note that
LLVM debugger LLDB is able to debug.

In case of OpenMP, compilers generate artificial functions which are not
present in actual program. This is done to apply parallelism to block of
code.

For non-artifical functions, DW_AT_name attribute should contains the name
exactly as present in actual program.
(Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
Since artificial functions are not present in actual program they not having
DW_AT_name and having DW_AT_linkage_name instead should be fine.

Currently GDB is invalidating any function not havnig DW_AT_name which is why
it is not able to debug OpenMP (Clang).

It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
is absent.

Change-Id: I5048af6c0d07c227be762241da02582918ab66b8
---
 gdb/dwarf2/read.c                      |  7 ++++
 gdb/testsuite/gdb.threads/omp-task.c   | 28 ++++++++++++++
 gdb/testsuite/gdb.threads/omp-task.exp | 51 ++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.c
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6dcd446e5f4..4014aa770e8 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12000,6 +12000,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr)
+    name = dw2_linkage_name (die, cu);
 
   /* Ignore functions with missing or empty names.  These are actually
      illegal according to the DWARF standard.  */
@@ -20657,6 +20659,11 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr && (die->tag == DW_TAG_subprogram
+                          || die->tag == DW_TAG_inlined_subroutine
+                          || die->tag == DW_TAG_entry_point))
+    name = dw2_linkage_name (die, cu);
+
   if (name)
     {
       int suppress_add = 0;
diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
new file mode 100644
index 00000000000..27aa3e805dc
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.c
@@ -0,0 +1,28 @@
+#include <stdio.h>
+#include <omp.h>
+
+int foo(int n) {
+  int share1 = 9, share2 = 11, share3 = 13, priv1, priv2, fpriv;
+  fpriv = n + 4;
+
+  if (n < 2)
+    return n;
+  else {
+#pragma omp task shared(share1, share2) private(priv1, priv2) firstprivate(fpriv) shared(share3)
+    {
+      priv1 = n;
+      priv2 = n + 2;
+      share2 += share3;
+      printf("share1 = %d, share2 = %d, share3 = %d\n", share1, share2, share3);
+      share1 = priv1 + priv2 + fpriv + foo(n - 1) + share2 + share3;
+    }
+#pragma omp taskwait
+    return share1 + share2 + share3;
+  }
+}
+
+int main() {
+  int n = 10;
+  printf("foo(%d) = %d\n", n, foo(n));
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/omp-task.exp b/gdb/testsuite/gdb.threads/omp-task.exp
new file mode 100644
index 00000000000..8f46658fe0d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.exp
@@ -0,0 +1,51 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Tests which verify (or not) that GDB can access shared and private
+# clauses of OpenMP task construct.
+
+standard_testfile
+
+set have_nested_function_support 0
+set opts {openmp debug}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $opts]} {
+    return -1
+}
+
+if {[info procs gdb_openmp_setup] != ""} {
+    if {[gdb_openmp_setup $binfile] != ""} {
+	untested "could not set up OpenMP environment"
+	return -1
+    }
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "omp task shared"]
+gdb_test "continue" ".*Breakpoint 2.*" "continue 1"
+gdb_test "print share1" "= 9"
+gdb_test "print share2" "= 11"
+gdb_test "print share3" "= 13"
+gdb_test "disable 2" ".*"
+gdb_breakpoint [gdb_get_line_number "share1 = priv1"]
+gdb_test "continue" ".*Breakpoint 3.*" "continue 2"
+gdb_test "print priv1" "= 10"
+gdb_test "print priv2" "= 12"
+gdb_test "print fpriv" "= 14"
-- 
2.25.1

RE: [PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

[-- Attachment #1: Type: text/plain, Size: 3739 bytes --]

Hi Tom,

Please find the attached updated patch, which includes required changes to work with your patch https://sourceware.org/pipermail/gdb-patches/2022-April/188285.html

Regards,
Alok

-----Original Message-----
From: Sharma, Alok Kumar 
Sent: Tuesday, April 19, 2022 11:17 AM
To: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Accept functions with DW_AT_linkage_name present

Thanks Tom for your review comments. Attached is the updated patch to exclude DW_TAG_subroutine_type.

> Yes, but why?  Would there be something bad about using the linkage name for a variable?

Actually initially I tried without this conditions but that failed few gdb tests. Let me share the short testcase to explain.
-----------
#cat test.cxx
typedef enum
{
 DD, EE, FF
} anon_enum_t;

typedef anon_enum_t nested_anon_enum_t;
volatile nested_anon_enum_t var_s;

int
main ()
{
  asm ("" ::: "memory");
  return 0;
}
# g++ -g test.cxx -o test
---------------
I tried below command with the tag check and output matches what was there without this complete patch.
----------
# gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum : unsigned int {DD, EE, FF} All defined types:

File test.cxx:
4:      typedef enum {...} anon_enum_t;
        int
6:      typedef enum {...} nested_anon_enum_t;
        unsigned int
-----------
When we remove that tag condition it looks like below
-----------
/gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum 11anon_enum_t : unsigned int {DD, EE, FF} All defined types:

File test.cxx:
2:      11anon_enum_t;
4:      typedef 11anon_enum_t anon_enum_t;
        int
6:      typedef 11anon_enum_t nested_anon_enum_t;
        unsigned int
---------- 

Please note the name "11anon_enum_t", which is linkage name (internal name) not exposed to user and showing that probably is not good. This linkage name is present only in case for C++ compilation only, in case the same code is in c and compile with gcc this linkage name is not present. So probably it is better to keep the output same as we get in C and not expose this linkage name for enums and keep it exposed only for functions. Please let me know if you differ, I shall change the testcases to suite the new output.

Regards,
Alok 

-----Original Message-----
From: Tom Tromey <tom@tromey.com>
Sent: Monday, April 18, 2022 7:15 PM
To: Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Accept functions with DW_AT_linkage_name present

[CAUTION: External Email]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

>> is this tag check really needed?

> I want to keep this limited to functions.

Yes, but why?  Would there be something bad about using the linkage name for a variable?

>    name = dwarf2_name (die, cu);
>    if (name == nullptr)
>      name = dw2_linkage_name (die, cu);

I guess if we avoided the tag check, this could just be done in dwarf2_name itself.

> +  if (name == nullptr && (die->tag == DW_TAG_subprogram
> +                          || die->tag == DW_TAG_subroutine_type
> +                          || die->tag == DW_TAG_inlined_subroutine
> +                          || die->tag == DW_TAG_entry_point))

If there is a reason to only check functions then you should remove DW_TAG_subroutine_type here, because that is a type, not a function.

Tom

[-- Attachment #2: 0001-PATCH-Accept-functions-with-DW_AT_linkage_name-prese.patch --]
[-- Type: application/octet-stream, Size: 5923 bytes --]

From b19dcc00ffd64ab73f7a02a19250dfeecfc96ea5 Mon Sep 17 00:00:00 2001
From: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date: Sun, 10 Apr 2022 18:50:01 +0530
Subject: [PATCH] [PATCH] Accept functions with DW_AT_linkage_name present

Currently GDB is not able to debug (Binary generated with Clang) variables
present in shared/private clause of OpenMP Task construct. Please note that
LLVM debugger LLDB is able to debug.

In case of OpenMP, compilers generate artificial functions which are not
present in actual program. This is done to apply parallelism to block of
code.

For non-artifical functions, DW_AT_name attribute should contains the name
exactly as present in actual program.
(Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
Since artificial functions are not present in actual program they not having
DW_AT_name and having DW_AT_linkage_name instead should be fine.

Currently GDB is invalidating any function not havnig DW_AT_name which is why
it is not able to debug OpenMP (Clang).

It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
is absent.

Change-Id: I5048af6c0d07c227be762241da02582918ab66b8
---
 gdb/dwarf2/read.c                      | 16 +++++++-
 gdb/testsuite/gdb.threads/omp-task.c   | 28 ++++++++++++++
 gdb/testsuite/gdb.threads/omp-task.exp | 51 ++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.c
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d146d525066..b7ad75e3a29 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -11926,6 +11926,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr)
+    name = dw2_linkage_name (die, cu);
 
   /* Ignore functions with missing or empty names.  These are actually
      illegal according to the DWARF standard.  */
@@ -18320,7 +18322,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
 	     but not an ordinary name.  */
 	  if (name != nullptr)
 	    flags = flags & ~IS_MAIN;
-	  m_index_storage->add (this_die, abbrev->tag, flags | IS_LINKAGE,
+	  /* Set the IS_LINKAGE on for everything except when functions
+	     have linkage name present but name is absent.  */
+	  if (name != nullptr
+	      || (abbrev->tag != DW_TAG_subprogram
+	          && abbrev->tag != DW_TAG_inlined_subroutine
+	          && abbrev->tag != DW_TAG_entry_point))
+	    flags = flags | IS_LINKAGE;
+	  m_index_storage->add (this_die, abbrev->tag, flags,
 				linkage_name, nullptr, m_per_cu);
 	}
 
@@ -20597,6 +20606,11 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr && (die->tag == DW_TAG_subprogram
+                          || die->tag == DW_TAG_inlined_subroutine
+                          || die->tag == DW_TAG_entry_point))
+    name = dw2_linkage_name (die, cu);
+
   if (name)
     {
       int suppress_add = 0;
diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
new file mode 100644
index 00000000000..27aa3e805dc
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.c
@@ -0,0 +1,28 @@
+#include <stdio.h>
+#include <omp.h>
+
+int foo(int n) {
+  int share1 = 9, share2 = 11, share3 = 13, priv1, priv2, fpriv;
+  fpriv = n + 4;
+
+  if (n < 2)
+    return n;
+  else {
+#pragma omp task shared(share1, share2) private(priv1, priv2) firstprivate(fpriv) shared(share3)
+    {
+      priv1 = n;
+      priv2 = n + 2;
+      share2 += share3;
+      printf("share1 = %d, share2 = %d, share3 = %d\n", share1, share2, share3);
+      share1 = priv1 + priv2 + fpriv + foo(n - 1) + share2 + share3;
+    }
+#pragma omp taskwait
+    return share1 + share2 + share3;
+  }
+}
+
+int main() {
+  int n = 10;
+  printf("foo(%d) = %d\n", n, foo(n));
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/omp-task.exp b/gdb/testsuite/gdb.threads/omp-task.exp
new file mode 100644
index 00000000000..8f46658fe0d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.exp
@@ -0,0 +1,51 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Tests which verify (or not) that GDB can access shared and private
+# clauses of OpenMP task construct.
+
+standard_testfile
+
+set have_nested_function_support 0
+set opts {openmp debug}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $opts]} {
+    return -1
+}
+
+if {[info procs gdb_openmp_setup] != ""} {
+    if {[gdb_openmp_setup $binfile] != ""} {
+	untested "could not set up OpenMP environment"
+	return -1
+    }
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "omp task shared"]
+gdb_test "continue" ".*Breakpoint 2.*" "continue 1"
+gdb_test "print share1" "= 9"
+gdb_test "print share2" "= 11"
+gdb_test "print share3" "= 13"
+gdb_test "disable 2" ".*"
+gdb_breakpoint [gdb_get_line_number "share1 = priv1"]
+gdb_test "continue" ".*Breakpoint 3.*" "continue 2"
+gdb_test "print priv1" "= 10"
+gdb_test "print priv2" "= 12"
+gdb_test "print fpriv" "= 14"
-- 
2.25.1

RE: [PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

Gentle Reminder.

Regards,
Alok

-----Original Message-----
From: Sharma, Alok Kumar 
Sent: Wednesday, May 4, 2022 2:26 PM
To: 'Tom Tromey' <tom@tromey.com>; 'Sharma, Alok Kumar via Gdb-patches' <gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Accept functions with DW_AT_linkage_name present

Hi Tom,

Please find the attached updated patch, which includes required changes to work with your patch https://sourceware.org/pipermail/gdb-patches/2022-April/188285.html

Regards,
Alok

-----Original Message-----
From: Sharma, Alok Kumar 
Sent: Tuesday, April 19, 2022 11:17 AM
To: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Accept functions with DW_AT_linkage_name present

Thanks Tom for your review comments. Attached is the updated patch to exclude DW_TAG_subroutine_type.

> Yes, but why?  Would there be something bad about using the linkage name for a variable?

Actually initially I tried without this conditions but that failed few gdb tests. Let me share the short testcase to explain.
-----------
#cat test.cxx
typedef enum
{
 DD, EE, FF
} anon_enum_t;

typedef anon_enum_t nested_anon_enum_t;
volatile nested_anon_enum_t var_s;

int
main ()
{
  asm ("" ::: "memory");
  return 0;
}
# g++ -g test.cxx -o test
---------------
I tried below command with the tag check and output matches what was there without this complete patch.
----------
# gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum : unsigned int {DD, EE, FF} All defined types:

File test.cxx:
4:      typedef enum {...} anon_enum_t;
        int
6:      typedef enum {...} nested_anon_enum_t;
        unsigned int
-----------
When we remove that tag condition it looks like below
-----------
/gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum 11anon_enum_t : unsigned int {DD, EE, FF} All defined types:

File test.cxx:
2:      11anon_enum_t;
4:      typedef 11anon_enum_t anon_enum_t;
        int
6:      typedef 11anon_enum_t nested_anon_enum_t;
        unsigned int
---------- 

Please note the name "11anon_enum_t", which is linkage name (internal name) not exposed to user and showing that probably is not good. This linkage name is present only in case for C++ compilation only, in case the same code is in c and compile with gcc this linkage name is not present. So probably it is better to keep the output same as we get in C and not expose this linkage name for enums and keep it exposed only for functions. Please let me know if you differ, I shall change the testcases to suite the new output.

Regards,
Alok 

-----Original Message-----
From: Tom Tromey <tom@tromey.com>
Sent: Monday, April 18, 2022 7:15 PM
To: Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Accept functions with DW_AT_linkage_name present

[CAUTION: External Email]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

>> is this tag check really needed?

> I want to keep this limited to functions.

Yes, but why?  Would there be something bad about using the linkage name for a variable?

>    name = dwarf2_name (die, cu);
>    if (name == nullptr)
>      name = dw2_linkage_name (die, cu);

I guess if we avoided the tag check, this could just be done in dwarf2_name itself.

> +  if (name == nullptr && (die->tag == DW_TAG_subprogram
> +                          || die->tag == DW_TAG_subroutine_type
> +                          || die->tag == DW_TAG_inlined_subroutine
> +                          || die->tag == DW_TAG_entry_point))

If there is a reason to only check functions then you should remove DW_TAG_subroutine_type here, because that is a type, not a function.

Tom

RE: [PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

PING 2 !

Regards,
Alok

-----Original Message-----
From: Sharma, Alok Kumar 
Sent: Thursday, May 12, 2022 1:36 PM
To: 'Tom Tromey' <tom@tromey.com>; 'Sharma, Alok Kumar via Gdb-patches' <gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Accept functions with DW_AT_linkage_name present

Gentle Reminder.

Regards,
Alok

-----Original Message-----
From: Sharma, Alok Kumar 
Sent: Wednesday, May 4, 2022 2:26 PM
To: 'Tom Tromey' <tom@tromey.com>; 'Sharma, Alok Kumar via Gdb-patches' <gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Accept functions with DW_AT_linkage_name present

Hi Tom,

Please find the attached updated patch, which includes required changes to work with your patch https://sourceware.org/pipermail/gdb-patches/2022-April/188285.html

Regards,
Alok

-----Original Message-----
From: Sharma, Alok Kumar 
Sent: Tuesday, April 19, 2022 11:17 AM
To: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Accept functions with DW_AT_linkage_name present

Thanks Tom for your review comments. Attached is the updated patch to exclude DW_TAG_subroutine_type.

> Yes, but why?  Would there be something bad about using the linkage name for a variable?

Actually initially I tried without this conditions but that failed few gdb tests. Let me share the short testcase to explain.
-----------
#cat test.cxx
typedef enum
{
 DD, EE, FF
} anon_enum_t;

typedef anon_enum_t nested_anon_enum_t;
volatile nested_anon_enum_t var_s;

int
main ()
{
  asm ("" ::: "memory");
  return 0;
}
# g++ -g test.cxx -o test
---------------
I tried below command with the tag check and output matches what was there without this complete patch.
----------
# gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum : unsigned int {DD, EE, FF} All defined types:

File test.cxx:
4:      typedef enum {...} anon_enum_t;
        int
6:      typedef enum {...} nested_anon_enum_t;
        unsigned int
-----------
When we remove that tag condition it looks like below
-----------
/gdb -q test -ex "pt var_s" -ex "info types"
Reading symbols from test...
type = volatile enum 11anon_enum_t : unsigned int {DD, EE, FF} All defined types:

File test.cxx:
2:      11anon_enum_t;
4:      typedef 11anon_enum_t anon_enum_t;
        int
6:      typedef 11anon_enum_t nested_anon_enum_t;
        unsigned int
---------- 

Please note the name "11anon_enum_t", which is linkage name (internal name) not exposed to user and showing that probably is not good. This linkage name is present only in case for C++ compilation only, in case the same code is in c and compile with gcc this linkage name is not present. So probably it is better to keep the output same as we get in C and not expose this linkage name for enums and keep it exposed only for functions. Please let me know if you differ, I shall change the testcases to suite the new output.

Regards,
Alok 

-----Original Message-----
From: Tom Tromey <tom@tromey.com>
Sent: Monday, April 18, 2022 7:15 PM
To: Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Accept functions with DW_AT_linkage_name present

[CAUTION: External Email]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

>> is this tag check really needed?

> I want to keep this limited to functions.

Yes, but why?  Would there be something bad about using the linkage name for a variable?

>    name = dwarf2_name (die, cu);
>    if (name == nullptr)
>      name = dw2_linkage_name (die, cu);

I guess if we avoided the tag check, this could just be done in dwarf2_name itself.

> +  if (name == nullptr && (die->tag == DW_TAG_subprogram
> +                          || die->tag == DW_TAG_subroutine_type
> +                          || die->tag == DW_TAG_inlined_subroutine
> +                          || die->tag == DW_TAG_entry_point))

If there is a reason to only check functions then you should remove DW_TAG_subroutine_type here, because that is a type, not a function.

Tom

Re: [PATCH] Accept functions with DW_AT_linkage_name present

[Tom Tromey]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
> new file mode 100644
> index 00000000000..27aa3e805dc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/omp-task.c
> @@ -0,0 +1,28 @@
> +#include <stdio.h>
> +#include <omp.h>

This file should have a copyright header.

This is ok with this change.

Tom

RE: [PATCH] Accept functions with DW_AT_linkage_name present

[Sharma, Alok Kumar]

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

Thanks a lot.

Attached is the updated patch which I shall commit.

Regards,
Alok

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Friday, May 20, 2022 9:43 PM
To: Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Accept functions with DW_AT_linkage_name present

[CAUTION: External Email]

>>>>> Sharma, Alok Kumar via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
> new file mode 100644
> index 00000000000..27aa3e805dc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/omp-task.c
> @@ -0,0 +1,28 @@
> +#include <stdio.h>
> +#include <omp.h>

This file should have a copyright header.

This is ok with this change.

Tom

[-- Attachment #2: 0001-Accept-functions-with-DW_AT_linkage_name-present.patch --]
[-- Type: application/octet-stream, Size: 6804 bytes --]

From 00c95d7bdbd02b26dc22253917da876d07062dcc Mon Sep 17 00:00:00 2001
From: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date: Sun, 10 Apr 2022 18:50:01 +0530
Subject: [PATCH] Accept functions with DW_AT_linkage_name present

Currently GDB is not able to debug (Binary generated with Clang) variables
present in shared/private clause of OpenMP Task construct. Please note that
LLVM debugger LLDB is able to debug.

In case of OpenMP, compilers generate artificial functions which are not
present in actual program. This is done to apply parallelism to block of
code.

For non-artifical functions, DW_AT_name attribute should contains the name
exactly as present in actual program.
(Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
Since artificial functions are not present in actual program they not having
DW_AT_name and having DW_AT_linkage_name instead should be fine.

Currently GDB is invalidating any function not havnig DW_AT_name which is why
it is not able to debug OpenMP (Clang).

It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
is absent.

Change-Id: I408af9be22c82409fb9e27a11639680673e2f7ef
---
 gdb/dwarf2/read.c                      | 16 +++++++-
 gdb/testsuite/gdb.threads/omp-task.c   | 49 +++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/omp-task.exp | 51 ++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.c
 create mode 100644 gdb/testsuite/gdb.threads/omp-task.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d146d525066..b7ad75e3a29 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -11926,6 +11926,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr)
+    name = dw2_linkage_name (die, cu);
 
   /* Ignore functions with missing or empty names.  These are actually
      illegal according to the DWARF standard.  */
@@ -18320,7 +18322,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
 	     but not an ordinary name.  */
 	  if (name != nullptr)
 	    flags = flags & ~IS_MAIN;
-	  m_index_storage->add (this_die, abbrev->tag, flags | IS_LINKAGE,
+	  /* Set the IS_LINKAGE on for everything except when functions
+	     have linkage name present but name is absent.  */
+	  if (name != nullptr
+	      || (abbrev->tag != DW_TAG_subprogram
+	          && abbrev->tag != DW_TAG_inlined_subroutine
+	          && abbrev->tag != DW_TAG_entry_point))
+	    flags = flags | IS_LINKAGE;
+	  m_index_storage->add (this_die, abbrev->tag, flags,
 				linkage_name, nullptr, m_per_cu);
 	}
 
@@ -20597,6 +20606,11 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   baseaddr = objfile->text_section_offset ();
 
   name = dwarf2_name (die, cu);
+  if (name == nullptr && (die->tag == DW_TAG_subprogram
+                          || die->tag == DW_TAG_inlined_subroutine
+                          || die->tag == DW_TAG_entry_point))
+    name = dw2_linkage_name (die, cu);
+
   if (name)
     {
       int suppress_add = 0;
diff --git a/gdb/testsuite/gdb.threads/omp-task.c b/gdb/testsuite/gdb.threads/omp-task.c
new file mode 100644
index 00000000000..bdeeae501cb
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.c
@@ -0,0 +1,49 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Tests which verify (or not) that GDB can access shared and private
+   clauses of OpenMP task construct.
+*/
+
+#include <stdio.h>
+#include <omp.h>
+
+int foo(int n) {
+  int share1 = 9, share2 = 11, share3 = 13, priv1, priv2, fpriv;
+  fpriv = n + 4;
+
+  if (n < 2)
+    return n;
+  else {
+#pragma omp task shared(share1, share2) private(priv1, priv2) firstprivate(fpriv) shared(share3)
+    {
+      priv1 = n;
+      priv2 = n + 2;
+      share2 += share3;
+      printf("share1 = %d, share2 = %d, share3 = %d\n", share1, share2, share3);
+      share1 = priv1 + priv2 + fpriv + foo(n - 1) + share2 + share3;
+    }
+#pragma omp taskwait
+    return share1 + share2 + share3;
+  }
+}
+
+int main() {
+  int n = 10;
+  printf("foo(%d) = %d\n", n, foo(n));
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/omp-task.exp b/gdb/testsuite/gdb.threads/omp-task.exp
new file mode 100644
index 00000000000..8f46658fe0d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/omp-task.exp
@@ -0,0 +1,51 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Tests which verify (or not) that GDB can access shared and private
+# clauses of OpenMP task construct.
+
+standard_testfile
+
+set have_nested_function_support 0
+set opts {openmp debug}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $opts]} {
+    return -1
+}
+
+if {[info procs gdb_openmp_setup] != ""} {
+    if {[gdb_openmp_setup $binfile] != ""} {
+	untested "could not set up OpenMP environment"
+	return -1
+    }
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "omp task shared"]
+gdb_test "continue" ".*Breakpoint 2.*" "continue 1"
+gdb_test "print share1" "= 9"
+gdb_test "print share2" "= 11"
+gdb_test "print share3" "= 13"
+gdb_test "disable 2" ".*"
+gdb_breakpoint [gdb_get_line_number "share1 = priv1"]
+gdb_test "continue" ".*Breakpoint 3.*" "continue 2"
+gdb_test "print priv1" "= 10"
+gdb_test "print priv2" "= 12"
+gdb_test "print fpriv" "= 14"
-- 
2.25.1