public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
@ 2021-04-15 19:37 Kumar N, Bhuvanendra
  2021-04-30  8:59 ` Kumar N, Bhuvanendra
  2021-05-29  2:20 ` Simon Marchi
  0 siblings, 2 replies; 7+ messages in thread
From: Kumar N, Bhuvanendra @ 2021-04-15 19:37 UTC (permalink / raw)
  To: gdb-patches
  Cc: George, Jini Susan, Sharma, Alok Kumar, Achra, Nitika, Tomar,
	Sourabh Singh, E, Nagajyothi

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

[AMD Official Use Only - Internal Distribution Only]

Hello All,

I request all of you to please review this patch. Below are the details.

Problem Description:
Following 8 test points started to fail after the clang patch(https://reviews.llvm.org/D91734).

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

Resolution:


These comments in the clang patch(https://reviews.llvm.org/D91734) explain/address the issue :

". . .the test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location. And when the parameters span multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters.

This is not ideal IMO; I'd rather be more principled about (a) stop at every parameter, or (b) stop at no parameters. But without reworking how we do is_stmt, I think fiddling the test to do enough single-steps to actually get past the function call is okay."

After this clang patch, we can see additional .debug_line entries created while handling the function call and parameter evaluation as mentioned in the commit message. Hence to suit the new clang behavior, its suggested to modify the test case for clang-12 and above.

Thanks and Regards,
Bhuvan

Patch content inlined:

From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
Date: Fri, 16 Apr 2021 00:18:07 +0530
Subject: [PATCH] [gdb.base] Additional next commands added for
clang-12 and above.

After this clang patch(https://reviews.llvm.org/D91734), 8 test points
started to FAIL in this test case. As mentioned in this PR,
"...this test is trying to "next" over a function call; gcc attributes
all parameter evaluation to the function name, while clang will attribute
each parameter to its own location. And when the parameters span across
multiple source lines, the is_stmt heuristic kicks in, so we stop on each
line with actual parameters...".
We can see additional .debug_line entries getting created after this clang
patch, hence the test case is modified accordingly to suit the new clang
behavior. This test case modification is required for clang-12 and above.

Line table: (before clang patch for the below code snippet) :
0x000000b0: 84 address += 8,  line += 2
            0x000000000020196a     42      3      1   0             0
0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000b2: 41 address += 3,  line += 5
            0x000000000020197e     47      3      1   0             0

Line table: (after clang patch for the below code snippet) :
0x000000b5: 84 address += 8,  line += 2
            0x0000000000201958     42     11      1   0             0
0x000000b6: 05 DW_LNS_set_column (4)
0x000000b8: 75 address += 7,  line += 1
            0x000000000020195f     43      4      1   0             0
0x000000b9: 05 DW_LNS_set_column (3)
0x000000bb: 73 address += 7,  line += -1
            0x0000000000201966     42      3      1   0             0
0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000bd: 4f address += 4,  line += 5
            0x000000000020197b     47      3      1   0             0

gdb.base/foll-exec.c test file snippet :
. . .
42   execlp (prog, /* tbreak-execlp */
43           prog,
44           "execlp arg1 from foll-exec",
45           (char *) 0);
46
47   printf ("foll-exec is about to execl(execd-prog)...\n");

Following 8 test points started to fail after the above clang patch.

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

gdb/testsuite/ChangeLog:
        * gdb.base/foll-exec.exp: Additional next commands added for
        clang-12 and above.
---
gdb/testsuite/ChangeLog              |  5 +++++
gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 52b0752276..ad289c135d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-16  Bhuvanendra Kumar  Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
+
+        * gdb.base/foll-exec.exp: Additional next commands added for
+     clang-12 and above.
+
2021-02-12  Andrew Burgess  andrew.burgess@embecosm.com<mailto:andrew.burgess@embecosm.com>
      * gdb.fortran/allocated.exp: New file.
diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp
index 6d5e21f54d..8b1fa42db9 100644
--- a/gdb/testsuite/gdb.base/foll-exec.exp
+++ b/gdb/testsuite/gdb.base/foll-exec.exp
@@ -118,7 +118,14 @@ proc do_exec_tests {} {
    # We should stop in execd-program, at its first statement.
    #
    set execd_line [gdb_get_line_number "after-exec" $srcfile2]
-   send_gdb "next\n"
+   # Clang-12 and above will emit extra .debug_line entries when
+   # parameters span across multiple source lines, hence additional
+   # next commands were added.
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execlp call"}
@@ -269,7 +276,11 @@ proc do_exec_tests {} {
    # the newly-exec'd program, not after the remaining step-count
    # reaches zero.
    #
-   send_gdb "next 2\n"
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next 2\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execl call"}
--
2.17.1





[-- Attachment #2: gdb.base-Additional-next-commands-added.patch --]
[-- Type: application/octet-stream, Size: 4834 bytes --]

From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= <Bhuvanendra.KumarN@amd.com>
Date: Fri, 16 Apr 2021 00:18:07 +0530
Subject: [PATCH] [gdb.base] Additional next commands added for
 clang-12 and above.

After this clang patch(https://reviews.llvm.org/D91734), 8 test points
started to FAIL in this test case. As mentioned in this PR,
"...this test is trying to "next" over a function call; gcc attributes
all parameter evaluation to the function name, while clang will attribute
each parameter to its own location. And when the parameters span across
multiple source lines, the is_stmt heuristic kicks in, so we stop on each
line with actual parameters...".
We can see additional .debug_line entries getting created after this clang
patch, hence the test case is modified accordingly to suit the new clang
behavior. This test case modification is required for clang-12 and above.

Line table: (before clang patch for the below code snippet) :
0x000000b0: 84 address += 8,  line += 2
            0x000000000020196a     42      3      1   0             0
0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000b2: 41 address += 3,  line += 5
            0x000000000020197e     47      3      1   0             0

Line table: (after clang patch for the below code snippet) :
0x000000b5: 84 address += 8,  line += 2
            0x0000000000201958     42     11      1   0             0
0x000000b6: 05 DW_LNS_set_column (4)
0x000000b8: 75 address += 7,  line += 1
            0x000000000020195f     43      4      1   0             0
0x000000b9: 05 DW_LNS_set_column (3)
0x000000bb: 73 address += 7,  line += -1
            0x0000000000201966     42      3      1   0             0
0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000bd: 4f address += 4,  line += 5
            0x000000000020197b     47      3      1   0             0

gdb.base/foll-exec.c test file snippet :
. . .
 42   execlp (prog, /* tbreak-execlp */
 43           prog,
 44           "execlp arg1 from foll-exec",
 45           (char *) 0);
 46
 47   printf ("foll-exec is about to execl(execd-prog)...\n");

Following 8 test points started to fail after the above clang patch.

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

gdb/testsuite/ChangeLog:
        * gdb.base/foll-exec.exp: Additional next commands added for
        clang-12 and above.
---
 gdb/testsuite/ChangeLog              |  5 +++++
 gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 52b0752276..ad289c135d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-16  Bhuvanendra Kumar  <Bhuvanendra.KumarN@amd.com>
+
+        * gdb.base/foll-exec.exp: Additional next commands added for
+	clang-12 and above.
+
 2021-02-12  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.fortran/allocated.exp: New file.
diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp
index 6d5e21f54d..8b1fa42db9 100644
--- a/gdb/testsuite/gdb.base/foll-exec.exp
+++ b/gdb/testsuite/gdb.base/foll-exec.exp
@@ -118,7 +118,14 @@ proc do_exec_tests {} {
    # We should stop in execd-program, at its first statement.
    #
    set execd_line [gdb_get_line_number "after-exec" $srcfile2]
-   send_gdb "next\n"
+   # Clang-12 and above will emit extra .debug_line entries when
+   # parameters span across multiple source lines, hence additional
+   # next commands were added.
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execlp call"}
@@ -269,7 +276,11 @@ proc do_exec_tests {} {
    # the newly-exec'd program, not after the remaining step-count
    # reaches zero.
    #
-   send_gdb "next 2\n"
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next 2\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execl call"}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
  2021-04-15 19:37 [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp Kumar N, Bhuvanendra
@ 2021-04-30  8:59 ` Kumar N, Bhuvanendra
  2021-05-26  8:00   ` Kumar N, Bhuvanendra
  2021-05-29  2:20 ` Simon Marchi
  1 sibling, 1 reply; 7+ messages in thread
From: Kumar N, Bhuvanendra @ 2021-04-30  8:59 UTC (permalink / raw)
  To: gdb-patches
  Cc: George, Jini Susan, Sharma, Alok Kumar, Achra, Nitika, Tomar,
	Sourabh Singh, E, Nagajyothi

[AMD Official Use Only - Internal Distribution Only]

Hello all,

Gentle Ping!

Please let me know if any additional information or discussion is required, thanks in advance

This test case change is important for clang.

Regards,
bhuvan

From: Kumar N, Bhuvanendra
Sent: Friday, April 16, 2021 1:07 AM
To: gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>
Subject: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp


[AMD Official Use Only - Internal Distribution Only]

Hello All,

I request all of you to please review this patch. Below are the details.

Problem Description:
Following 8 test points started to fail after the clang patch(https://reviews.llvm.org/D91734).

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

Resolution:


These comments in the clang patch(https://reviews.llvm.org/D91734) explain/address the issue :

". . .the test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location. And when the parameters span multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters.

This is not ideal IMO; I'd rather be more principled about (a) stop at every parameter, or (b) stop at no parameters. But without reworking how we do is_stmt, I think fiddling the test to do enough single-steps to actually get past the function call is okay."

After this clang patch, we can see additional .debug_line entries created while handling the function call and parameter evaluation as mentioned in the commit message. Hence to suit the new clang behavior, its suggested to modify the test case for clang-12 and above.

Thanks and Regards,
Bhuvan

Patch content inlined:

From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
Date: Fri, 16 Apr 2021 00:18:07 +0530
Subject: [PATCH] [gdb.base] Additional next commands added for
clang-12 and above.

After this clang patch(https://reviews.llvm.org/D91734), 8 test points
started to FAIL in this test case. As mentioned in this PR,
"...this test is trying to "next" over a function call; gcc attributes
all parameter evaluation to the function name, while clang will attribute
each parameter to its own location. And when the parameters span across
multiple source lines, the is_stmt heuristic kicks in, so we stop on each
line with actual parameters...".
We can see additional .debug_line entries getting created after this clang
patch, hence the test case is modified accordingly to suit the new clang
behavior. This test case modification is required for clang-12 and above.

Line table: (before clang patch for the below code snippet) :
0x000000b0: 84 address += 8,  line += 2
            0x000000000020196a     42      3      1   0             0
0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000b2: 41 address += 3,  line += 5
            0x000000000020197e     47      3      1   0             0

Line table: (after clang patch for the below code snippet) :
0x000000b5: 84 address += 8,  line += 2
            0x0000000000201958     42     11      1   0             0
0x000000b6: 05 DW_LNS_set_column (4)
0x000000b8: 75 address += 7,  line += 1
            0x000000000020195f     43      4      1   0             0
0x000000b9: 05 DW_LNS_set_column (3)
0x000000bb: 73 address += 7,  line += -1
            0x0000000000201966     42      3      1   0             0
0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000bd: 4f address += 4,  line += 5
            0x000000000020197b     47      3      1   0             0

gdb.base/foll-exec.c test file snippet :
. . .
42   execlp (prog, /* tbreak-execlp */
43           prog,
44           "execlp arg1 from foll-exec",
45           (char *) 0);
46
47   printf ("foll-exec is about to execl(execd-prog)...\n");

Following 8 test points started to fail after the above clang patch.

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

gdb/testsuite/ChangeLog:
        * gdb.base/foll-exec.exp: Additional next commands added for
        clang-12 and above.
---
gdb/testsuite/ChangeLog              |  5 +++++
gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 52b0752276..ad289c135d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-16  Bhuvanendra Kumar  Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
+
+        * gdb.base/foll-exec.exp: Additional next commands added for
+     clang-12 and above.
+
2021-02-12  Andrew Burgess  andrew.burgess@embecosm.com<mailto:andrew.burgess@embecosm.com>

      * gdb.fortran/allocated.exp: New file.
diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp
index 6d5e21f54d..8b1fa42db9 100644
--- a/gdb/testsuite/gdb.base/foll-exec.exp
+++ b/gdb/testsuite/gdb.base/foll-exec.exp
@@ -118,7 +118,14 @@ proc do_exec_tests {} {
    # We should stop in execd-program, at its first statement.
    #
    set execd_line [gdb_get_line_number "after-exec" $srcfile2]
-   send_gdb "next\n"
+   # Clang-12 and above will emit extra .debug_line entries when
+   # parameters span across multiple source lines, hence additional
+   # next commands were added.
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execlp call"}
@@ -269,7 +276,11 @@ proc do_exec_tests {} {
    # the newly-exec'd program, not after the remaining step-count
    # reaches zero.
    #
-   send_gdb "next 2\n"
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next 2\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execl call"}
--
2.17.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
  2021-04-30  8:59 ` Kumar N, Bhuvanendra
@ 2021-05-26  8:00   ` Kumar N, Bhuvanendra
  0 siblings, 0 replies; 7+ messages in thread
From: Kumar N, Bhuvanendra @ 2021-05-26  8:00 UTC (permalink / raw)
  To: gdb-patches
  Cc: George, Jini Susan, Sharma, Alok Kumar, Achra, Nitika, Tomar,
	Sourabh Singh, E, Nagajyothi

[AMD Official Use Only - Internal Distribution Only]

Hello all,

Gentle Ping 2!

Please let me know if any additional information or discussion is required, thanks in advance

This test case change is important for clang.

regards,
bhuvan

From: Kumar N, Bhuvanendra
Sent: Friday, April 30, 2021 2:30 PM
To: gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>
Subject: RE: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp


[AMD Official Use Only - Internal Distribution Only]

Hello all,

Gentle Ping!

Please let me know if any additional information or discussion is required, thanks in advance

This test case change is important for clang.

Regards,
bhuvan

From: Kumar N, Bhuvanendra
Sent: Friday, April 16, 2021 1:07 AM
To: gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com<mailto:JiniSusan.George@amd.com>>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com<mailto:AlokKumar.Sharma@amd.com>>; Achra, Nitika <Nitika.Achra@amd.com<mailto:Nitika.Achra@amd.com>>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com<mailto:SourabhSingh.Tomar@amd.com>>; E, Nagajyothi <Nagajyothi.E@amd.com<mailto:Nagajyothi.E@amd.com>>
Subject: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp


[AMD Official Use Only - Internal Distribution Only]

Hello All,

I request all of you to please review this patch. Below are the details.

Problem Description:
Following 8 test points started to fail after the clang patch(https://reviews.llvm.org/D91734).

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

Resolution:


These comments in the clang patch(https://reviews.llvm.org/D91734) explain/address the issue :

". . .the test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location. And when the parameters span multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters.

This is not ideal IMO; I'd rather be more principled about (a) stop at every parameter, or (b) stop at no parameters. But without reworking how we do is_stmt, I think fiddling the test to do enough single-steps to actually get past the function call is okay."

After this clang patch, we can see additional .debug_line entries created while handling the function call and parameter evaluation as mentioned in the commit message. Hence to suit the new clang behavior, its suggested to modify the test case for clang-12 and above.

Thanks and Regards,
Bhuvan

Patch content inlined:

From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
Date: Fri, 16 Apr 2021 00:18:07 +0530
Subject: [PATCH] [gdb.base] Additional next commands added for
clang-12 and above.

After this clang patch(https://reviews.llvm.org/D91734), 8 test points
started to FAIL in this test case. As mentioned in this PR,
"...this test is trying to "next" over a function call; gcc attributes
all parameter evaluation to the function name, while clang will attribute
each parameter to its own location. And when the parameters span across
multiple source lines, the is_stmt heuristic kicks in, so we stop on each
line with actual parameters...".
We can see additional .debug_line entries getting created after this clang
patch, hence the test case is modified accordingly to suit the new clang
behavior. This test case modification is required for clang-12 and above.

Line table: (before clang patch for the below code snippet) :
0x000000b0: 84 address += 8,  line += 2
            0x000000000020196a     42      3      1   0             0
0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000b2: 41 address += 3,  line += 5
            0x000000000020197e     47      3      1   0             0

Line table: (after clang patch for the below code snippet) :
0x000000b5: 84 address += 8,  line += 2
            0x0000000000201958     42     11      1   0             0
0x000000b6: 05 DW_LNS_set_column (4)
0x000000b8: 75 address += 7,  line += 1
            0x000000000020195f     43      4      1   0             0
0x000000b9: 05 DW_LNS_set_column (3)
0x000000bb: 73 address += 7,  line += -1
            0x0000000000201966     42      3      1   0             0
0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000bd: 4f address += 4,  line += 5
            0x000000000020197b     47      3      1   0             0

gdb.base/foll-exec.c test file snippet :
. . .
42   execlp (prog, /* tbreak-execlp */
43           prog,
44           "execlp arg1 from foll-exec",
45           (char *) 0);
46
47   printf ("foll-exec is about to execl(execd-prog)...\n");

Following 8 test points started to fail after the above clang patch.

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

gdb/testsuite/ChangeLog:
        * gdb.base/foll-exec.exp: Additional next commands added for
        clang-12 and above.
---
gdb/testsuite/ChangeLog              |  5 +++++
gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 52b0752276..ad289c135d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-16  Bhuvanendra Kumar  Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
+
+        * gdb.base/foll-exec.exp: Additional next commands added for
+     clang-12 and above.
+
2021-02-12  Andrew Burgess  andrew.burgess@embecosm.com<mailto:andrew.burgess@embecosm.com>

      * gdb.fortran/allocated.exp: New file.
diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp
index 6d5e21f54d..8b1fa42db9 100644
--- a/gdb/testsuite/gdb.base/foll-exec.exp
+++ b/gdb/testsuite/gdb.base/foll-exec.exp
@@ -118,7 +118,14 @@ proc do_exec_tests {} {
    # We should stop in execd-program, at its first statement.
    #
    set execd_line [gdb_get_line_number "after-exec" $srcfile2]
-   send_gdb "next\n"
+   # Clang-12 and above will emit extra .debug_line entries when
+   # parameters span across multiple source lines, hence additional
+   # next commands were added.
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execlp call"}
@@ -269,7 +276,11 @@ proc do_exec_tests {} {
    # the newly-exec'd program, not after the remaining step-count
    # reaches zero.
    #
-   send_gdb "next 2\n"
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next 2\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execl call"}
--
2.17.1





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
  2021-04-15 19:37 [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp Kumar N, Bhuvanendra
  2021-04-30  8:59 ` Kumar N, Bhuvanendra
@ 2021-05-29  2:20 ` Simon Marchi
  2021-06-01 11:10   ` Kumar N, Bhuvanendra
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2021-05-29  2:20 UTC (permalink / raw)
  To: Kumar N, Bhuvanendra, gdb-patches
  Cc: George, Jini Susan, Achra, Nitika, Sharma, Alok Kumar, E,
	Nagajyothi, Tomar, Sourabh Singh

On 2021-04-15 3:37 p.m., Kumar N, Bhuvanendra via Gdb-patches wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> Hello All,
> 
> I request all of you to please review this patch. Below are the details.
> 
> Problem Description:
> Following 8 test points started to fail after the clang patch(https://reviews.llvm.org/D91734).
> 
> FAIL: gdb.base/foll-exec.exp: step through execlp call
> FAIL: gdb.base/foll-exec.exp: step after execlp call
> FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
> FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
> FAIL: gdb.base/foll-exec.exp: step through execl call
> FAIL: gdb.base/foll-exec.exp: step after execl call
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)
> 
> Resolution:
> 
> 
> These comments in the clang patch(https://reviews.llvm.org/D91734) explain/address the issue :
> 
> ". . .the test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location. And when the parameters span multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters.
> 
> This is not ideal IMO; I'd rather be more principled about (a) stop at every parameter, or (b) stop at no parameters. But without reworking how we do is_stmt, I think fiddling the test to do enough single-steps to actually get past the function call is okay."
> 
> After this clang patch, we can see additional .debug_line entries created while handling the function call and parameter evaluation as mentioned in the commit message. Hence to suit the new clang behavior, its suggested to modify the test case for clang-12 and above.
> 
> Thanks and Regards,
> Bhuvan
> 
> Patch content inlined:
> 
> From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
> Date: Fri, 16 Apr 2021 00:18:07 +0530
> Subject: [PATCH] [gdb.base] Additional next commands added for
> clang-12 and above.
> 
> After this clang patch(https://reviews.llvm.org/D91734), 8 test points
> started to FAIL in this test case. As mentioned in this PR,
> "...this test is trying to "next" over a function call; gcc attributes
> all parameter evaluation to the function name, while clang will attribute
> each parameter to its own location. And when the parameters span across
> multiple source lines, the is_stmt heuristic kicks in, so we stop on each
> line with actual parameters...".
> We can see additional .debug_line entries getting created after this clang
> patch, hence the test case is modified accordingly to suit the new clang
> behavior. This test case modification is required for clang-12 and above.
> 
> Line table: (before clang patch for the below code snippet) :
> 0x000000b0: 84 address += 8,  line += 2
>             0x000000000020196a     42      3      1   0             0
> 0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
> 0x000000b2: 41 address += 3,  line += 5
>             0x000000000020197e     47      3      1   0             0
> 
> Line table: (after clang patch for the below code snippet) :
> 0x000000b5: 84 address += 8,  line += 2
>             0x0000000000201958     42     11      1   0             0
> 0x000000b6: 05 DW_LNS_set_column (4)
> 0x000000b8: 75 address += 7,  line += 1
>             0x000000000020195f     43      4      1   0             0
> 0x000000b9: 05 DW_LNS_set_column (3)
> 0x000000bb: 73 address += 7,  line += -1
>             0x0000000000201966     42      3      1   0             0
> 0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
> 0x000000bd: 4f address += 4,  line += 5
>             0x000000000020197b     47      3      1   0             0
> 
> gdb.base/foll-exec.c test file snippet :
> . . .
> 42   execlp (prog, /* tbreak-execlp */
> 43           prog,
> 44           "execlp arg1 from foll-exec",
> 45           (char *) 0);
> 46
> 47   printf ("foll-exec is about to execl(execd-prog)...\n");
> 
> Following 8 test points started to fail after the above clang patch.
> 
> FAIL: gdb.base/foll-exec.exp: step through execlp call
> FAIL: gdb.base/foll-exec.exp: step after execlp call
> FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
> FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
> FAIL: gdb.base/foll-exec.exp: step through execl call
> FAIL: gdb.base/foll-exec.exp: step after execl call
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)
> 
> gdb/testsuite/ChangeLog:
>         * gdb.base/foll-exec.exp: Additional next commands added for
>         clang-12 and above.
> ---
> gdb/testsuite/ChangeLog              |  5 +++++
> gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)

For some reason I can't apply your patch, git tells me it's corrupt.
Did you use git-send-email to send it?

> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 52b0752276..ad289c135d 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2021-04-16  Bhuvanendra Kumar  Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
> +
> +        * gdb.base/foll-exec.exp: Additional next commands added for
> +     clang-12 and above.
> +
> 2021-02-12  Andrew Burgess  andrew.burgess@embecosm.com<mailto:andrew.burgess@embecosm.com>
>       * gdb.fortran/allocated.exp: New file.
> diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp
> index 6d5e21f54d..8b1fa42db9 100644
> --- a/gdb/testsuite/gdb.base/foll-exec.exp
> +++ b/gdb/testsuite/gdb.base/foll-exec.exp
> @@ -118,7 +118,14 @@ proc do_exec_tests {} {
>     # We should stop in execd-program, at its first statement.
>     #
>     set execd_line [gdb_get_line_number "after-exec" $srcfile2]
> -   send_gdb "next\n"
> +   # Clang-12 and above will emit extra .debug_line entries when
> +   # parameters span across multiple source lines, hence additional
> +   # next commands were added.
> +   if {[test_compiler_info {clang-1[2-9]-*}]} {
> +       send_gdb "next 3\n"
> +   } else {
> +       send_gdb "next\n"
> +   }
>     gdb_expect {
>       -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
>                       {pass "step through execlp call"}
> @@ -269,7 +276,11 @@ proc do_exec_tests {} {
>     # the newly-exec'd program, not after the remaining step-count
>     # reaches zero.
>     #
> -   send_gdb "next 2\n"
> +   if {[test_compiler_info {clang-1[2-9]-*}]} {
> +       send_gdb "next 3\n"
> +   } else {
> +       send_gdb "next 2\n"
> +   }
>     gdb_expect {
>       -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
>                       {pass "step through execl call"}
> --
> 2.17.1

Does it work if we just put the whole execlp call on a single line
instead?

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
  2021-05-29  2:20 ` Simon Marchi
@ 2021-06-01 11:10   ` Kumar N, Bhuvanendra
  2021-06-01 13:50     ` Simon Marchi
  0 siblings, 1 reply; 7+ messages in thread
From: Kumar N, Bhuvanendra @ 2021-06-01 11:10 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches
  Cc: George, Jini Susan, Achra, Nitika, Sharma, Alok Kumar, E,
	Nagajyothi, Tomar, Sourabh Singh

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

[AMD Official Use Only - Internal Distribution Only]

Hi,

Thanks for your comments on this.

>For some reason I can't apply your patch, git tells me it's corrupt.
>Did you use git-send-email to send it?

I used my email client (which is Microsoft outlook) for submitting the patch for review. I am reattaching the same old patch(sent on 2021-04-15) as an attachment this time, in case if it helps for you

>Does it work if we just put the whole execlp call on a single line instead?

Yes, it works fine. if we are putting everything under one line as shred below, then it works fine with existing foll-exec.exp itself and no further change is required to foll-exec.exp.

42   execlp (prog, /* tbreak-execlp */ prog, "execlp arg1 from foll-exec", (char *) 0);
. . .
46   execl (prog,  /* tbreak-execl */ prog, "execl arg1 from foll-exec", "execl arg2 from foll-exec", (char *) 0);


-----Original Message-----
From: Simon Marchi <simon.marchi@polymtl.ca> 
Sent: Saturday, May 29, 2021 7:51 AM
To: Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>; gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>
Subject: Re: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp

[CAUTION: External Email]

On 2021-04-15 3:37 p.m., Kumar N, Bhuvanendra via Gdb-patches wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hello All,
>
> I request all of you to please review this patch. Below are the details.
>
> Problem Description:
> Following 8 test points started to fail after the clang patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD91734&amp;data=04%7C01%7CBhuvanendra.KumarN%40amd.com%7Cbdc30f93800b4f6c032d08d92248612a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637578516557363689%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=FAPo1TaCGE%2FImUyN3Fhex96GdMVqercM6HnzS46bNQ0%3D&amp;reserved=0).
>
> FAIL: gdb.base/foll-exec.exp: step through execlp call
> FAIL: gdb.base/foll-exec.exp: step after execlp call
> FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after 
> execlp)
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after 
> execlp)
> FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
> FAIL: gdb.base/foll-exec.exp: step through execl call
> FAIL: gdb.base/foll-exec.exp: step after execl call
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after 
> execl)
>
> Resolution:
>
>
> These comments in the clang patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD91734&amp;data=04%7C01%7CBhuvanendra.KumarN%40amd.com%7Cbdc30f93800b4f6c032d08d92248612a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637578516557373684%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=TW7QjCio73C%2B8cqsa56Gd4nh8qxVe04u2gjXPHDDmxg%3D&amp;reserved=0) explain/address the issue :
>
> ". . .the test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location. And when the parameters span multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters.
>
> This is not ideal IMO; I'd rather be more principled about (a) stop at every parameter, or (b) stop at no parameters. But without reworking how we do is_stmt, I think fiddling the test to do enough single-steps to actually get past the function call is okay."
>
> After this clang patch, we can see additional .debug_line entries created while handling the function call and parameter evaluation as mentioned in the commit message. Hence to suit the new clang behavior, its suggested to modify the test case for clang-12 and above.
>
> Thanks and Regards,
> Bhuvan
>
> Patch content inlined:
>
> From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= 
> Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
> Date: Fri, 16 Apr 2021 00:18:07 +0530
> Subject: [PATCH] [gdb.base] Additional next commands added for
> clang-12 and above.
>
> After this clang 
> patch(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%
> 2Freviews.llvm.org%2FD91734&amp;data=04%7C01%7CBhuvanendra.KumarN%40am
> d.com%7Cbdc30f93800b4f6c032d08d92248612a%7C3dd8961fe4884e608e11a82d994
> e183d%7C0%7C0%7C637578516557373684%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sda
> ta=TW7QjCio73C%2B8cqsa56Gd4nh8qxVe04u2gjXPHDDmxg%3D&amp;reserved=0), 8 test points started to FAIL in this test case. As mentioned in this PR, "...this test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location. And when the parameters span across multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters...".
> We can see additional .debug_line entries getting created after this 
> clang patch, hence the test case is modified accordingly to suit the 
> new clang behavior. This test case modification is required for clang-12 and above.
>
> Line table: (before clang patch for the below code snippet) :
> 0x000000b0: 84 address += 8,  line += 2
>             0x000000000020196a     42      3      1   0             0
> 0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
> 0x000000b2: 41 address += 3,  line += 5
>             0x000000000020197e     47      3      1   0             0
>
> Line table: (after clang patch for the below code snippet) :
> 0x000000b5: 84 address += 8,  line += 2
>             0x0000000000201958     42     11      1   0             0
> 0x000000b6: 05 DW_LNS_set_column (4)
> 0x000000b8: 75 address += 7,  line += 1
>             0x000000000020195f     43      4      1   0             0
> 0x000000b9: 05 DW_LNS_set_column (3)
> 0x000000bb: 73 address += 7,  line += -1
>             0x0000000000201966     42      3      1   0             0
> 0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
> 0x000000bd: 4f address += 4,  line += 5
>             0x000000000020197b     47      3      1   0             0
>
> gdb.base/foll-exec.c test file snippet :
> . . .
> 42   execlp (prog, /* tbreak-execlp */
> 43           prog,
> 44           "execlp arg1 from foll-exec",
> 45           (char *) 0);
> 46
> 47   printf ("foll-exec is about to execl(execd-prog)...\n");
>
> Following 8 test points started to fail after the above clang patch.
>
> FAIL: gdb.base/foll-exec.exp: step through execlp call
> FAIL: gdb.base/foll-exec.exp: step after execlp call
> FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after 
> execlp)
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after 
> execlp)
> FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
> FAIL: gdb.base/foll-exec.exp: step through execl call
> FAIL: gdb.base/foll-exec.exp: step after execl call
> FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after 
> execl)
>
> gdb/testsuite/ChangeLog:
>         * gdb.base/foll-exec.exp: Additional next commands added for
>         clang-12 and above.
> ---
> gdb/testsuite/ChangeLog              |  5 +++++
> gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)

For some reason I can't apply your patch, git tells me it's corrupt.
Did you use git-send-email to send it?

> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 
> 52b0752276..ad289c135d 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2021-04-16  Bhuvanendra Kumar  
> +Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
> +
> +        * gdb.base/foll-exec.exp: Additional next commands added for
> +     clang-12 and above.
> +
> 2021-02-12  Andrew Burgess  andrew.burgess@embecosm.com<mailto:andrew.burgess@embecosm.com>
>       * gdb.fortran/allocated.exp: New file.
> diff --git a/gdb/testsuite/gdb.base/foll-exec.exp 
> b/gdb/testsuite/gdb.base/foll-exec.exp
> index 6d5e21f54d..8b1fa42db9 100644
> --- a/gdb/testsuite/gdb.base/foll-exec.exp
> +++ b/gdb/testsuite/gdb.base/foll-exec.exp
> @@ -118,7 +118,14 @@ proc do_exec_tests {} {
>     # We should stop in execd-program, at its first statement.
>     #
>     set execd_line [gdb_get_line_number "after-exec" $srcfile2]
> -   send_gdb "next\n"
> +   # Clang-12 and above will emit extra .debug_line entries when
> +   # parameters span across multiple source lines, hence additional
> +   # next commands were added.
> +   if {[test_compiler_info {clang-1[2-9]-*}]} {
> +       send_gdb "next 3\n"
> +   } else {
> +       send_gdb "next\n"
> +   }
>     gdb_expect {
>       -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
>                       {pass "step through execlp call"} @@ -269,7 
> +276,11 @@ proc do_exec_tests {} {
>     # the newly-exec'd program, not after the remaining step-count
>     # reaches zero.
>     #
> -   send_gdb "next 2\n"
> +   if {[test_compiler_info {clang-1[2-9]-*}]} {
> +       send_gdb "next 3\n"
> +   } else {
> +       send_gdb "next 2\n"
> +   }
>     gdb_expect {
>       -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
>                       {pass "step through execl call"}
> --
> 2.17.1

Does it work if we just put the whole execlp call on a single line instead?

Simon

[-- Attachment #2: gdb.base-Additional-next-commands-added.patch --]
[-- Type: application/octet-stream, Size: 4834 bytes --]

From b6c3646cdd40fae679131107a0b2be4ff5b9eae5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= <Bhuvanendra.KumarN@amd.com>
Date: Fri, 16 Apr 2021 00:18:07 +0530
Subject: [PATCH] [gdb.base] Additional next commands added for
 clang-12 and above.

After this clang patch(https://reviews.llvm.org/D91734), 8 test points
started to FAIL in this test case. As mentioned in this PR,
"...this test is trying to "next" over a function call; gcc attributes
all parameter evaluation to the function name, while clang will attribute
each parameter to its own location. And when the parameters span across
multiple source lines, the is_stmt heuristic kicks in, so we stop on each
line with actual parameters...".
We can see additional .debug_line entries getting created after this clang
patch, hence the test case is modified accordingly to suit the new clang
behavior. This test case modification is required for clang-12 and above.

Line table: (before clang patch for the below code snippet) :
0x000000b0: 84 address += 8,  line += 2
            0x000000000020196a     42      3      1   0             0
0x000000b1: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000b2: 41 address += 3,  line += 5
            0x000000000020197e     47      3      1   0             0

Line table: (after clang patch for the below code snippet) :
0x000000b5: 84 address += 8,  line += 2
            0x0000000000201958     42     11      1   0             0
0x000000b6: 05 DW_LNS_set_column (4)
0x000000b8: 75 address += 7,  line += 1
            0x000000000020195f     43      4      1   0             0
0x000000b9: 05 DW_LNS_set_column (3)
0x000000bb: 73 address += 7,  line += -1
            0x0000000000201966     42      3      1   0             0
0x000000bc: 08 DW_LNS_const_add_pc (0x0000000000000011)
0x000000bd: 4f address += 4,  line += 5
            0x000000000020197b     47      3      1   0             0

gdb.base/foll-exec.c test file snippet :
. . .
 42   execlp (prog, /* tbreak-execlp */
 43           prog,
 44           "execlp arg1 from foll-exec",
 45           (char *) 0);
 46
 47   printf ("foll-exec is about to execl(execd-prog)...\n");

Following 8 test points started to fail after the above clang patch.

FAIL: gdb.base/foll-exec.exp: step through execlp call
FAIL: gdb.base/foll-exec.exp: step after execlp call
FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp)
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execlp)
FAIL: gdb.base/foll-exec.exp: print follow-exec/local_k (after execlp)
FAIL: gdb.base/foll-exec.exp: step through execl call
FAIL: gdb.base/foll-exec.exp: step after execl call
FAIL: gdb.base/foll-exec.exp: print execd-program/local_j (after execl)

gdb/testsuite/ChangeLog:
        * gdb.base/foll-exec.exp: Additional next commands added for
        clang-12 and above.
---
 gdb/testsuite/ChangeLog              |  5 +++++
 gdb/testsuite/gdb.base/foll-exec.exp | 15 +++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 52b0752276..ad289c135d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-16  Bhuvanendra Kumar  <Bhuvanendra.KumarN@amd.com>
+
+        * gdb.base/foll-exec.exp: Additional next commands added for
+	clang-12 and above.
+
 2021-02-12  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.fortran/allocated.exp: New file.
diff --git a/gdb/testsuite/gdb.base/foll-exec.exp b/gdb/testsuite/gdb.base/foll-exec.exp
index 6d5e21f54d..8b1fa42db9 100644
--- a/gdb/testsuite/gdb.base/foll-exec.exp
+++ b/gdb/testsuite/gdb.base/foll-exec.exp
@@ -118,7 +118,14 @@ proc do_exec_tests {} {
    # We should stop in execd-program, at its first statement.
    #
    set execd_line [gdb_get_line_number "after-exec" $srcfile2]
-   send_gdb "next\n"
+   # Clang-12 and above will emit extra .debug_line entries when
+   # parameters span across multiple source lines, hence additional
+   # next commands were added.
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execlp call"}
@@ -269,7 +276,11 @@ proc do_exec_tests {} {
    # the newly-exec'd program, not after the remaining step-count
    # reaches zero.
    #
-   send_gdb "next 2\n"
+   if {[test_compiler_info {clang-1[2-9]-*}]} {
+       send_gdb "next 3\n"
+   } else {
+       send_gdb "next 2\n"
+   }
    gdb_expect {
      -re ".*xecuting new program: .*${testfile2}.*${srcfile2}:${execd_line}.*int  local_j = argc;.*$gdb_prompt $"\
                      {pass "step through execl call"}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
  2021-06-01 11:10   ` Kumar N, Bhuvanendra
@ 2021-06-01 13:50     ` Simon Marchi
  2021-06-01 15:40       ` Kumar N, Bhuvanendra
  0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2021-06-01 13:50 UTC (permalink / raw)
  To: Kumar N, Bhuvanendra, gdb-patches
  Cc: George, Jini Susan, Achra, Nitika, Sharma, Alok Kumar, E,
	Nagajyothi, Tomar, Sourabh Singh

> I used my email client (which is Microsoft outlook) for submitting the patch for review. I am reattaching the same old patch(sent on 2021-04-15) as an attachment this time, in case if it helps for you

Thanks, that's better, at least I can apply the patch.  However, please
consider using git-send-email, that makes it much easier to send review
comments.

>> Does it work if we just put the whole execlp call on a single line instead?
> 
> Yes, it works fine. if we are putting everything under one line as shred below, then it works fine with existing foll-exec.exp itself and no further change is required to foll-exec.exp.
> 
> 42   execlp (prog, /* tbreak-execlp */ prog, "execlp arg1 from foll-exec", (char *) 0);
> . . .
> 46   execl (prog,  /* tbreak-execl */ prog, "execl arg1 from foll-exec", "execl arg2 from foll-exec", (char *) 0);

So, let's do that then, there's less chance of it breaking again.

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp
  2021-06-01 13:50     ` Simon Marchi
@ 2021-06-01 15:40       ` Kumar N, Bhuvanendra
  0 siblings, 0 replies; 7+ messages in thread
From: Kumar N, Bhuvanendra @ 2021-06-01 15:40 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches
  Cc: George, Jini Susan, Achra, Nitika, Sharma, Alok Kumar, E,
	Nagajyothi, Tomar, Sourabh Singh

[AMD Official Use Only - Internal Distribution Only]

> So, let's do that then, there's less chance of it breaking again.

Thanks a lot, will send separate patch for this change(in foll-exec.c) then. I will make this change applicable for all compilers or default, not just for clang, which does not going to harm others, while it definitely helps clang

> 42   execlp (prog, /* tbreak-execlp */ prog, "execlp arg1 from foll-exec", (char *) 0);
> . . .
> 46   execl (prog,  /* tbreak-execl */ prog, "execl arg1 from foll-exec", "execl arg2 from foll-exec", (char *) 0);

regards,
bhuvan

-----Original Message-----
From: Simon Marchi <simon.marchi@polymtl.ca> 
Sent: Tuesday, June 1, 2021 7:21 PM
To: Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>; gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>
Subject: Re: [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp

[CAUTION: External Email]

> I used my email client (which is Microsoft outlook) for submitting the 
> patch for review. I am reattaching the same old patch(sent on 
> 2021-04-15) as an attachment this time, in case if it helps for you

Thanks, that's better, at least I can apply the patch.  However, please consider using git-send-email, that makes it much easier to send review comments.

>> Does it work if we just put the whole execlp call on a single line instead?
>
> Yes, it works fine. if we are putting everything under one line as shred below, then it works fine with existing foll-exec.exp itself and no further change is required to foll-exec.exp.
>
> 42   execlp (prog, /* tbreak-execlp */ prog, "execlp arg1 from foll-exec", (char *) 0);
> . . .
> 46   execl (prog,  /* tbreak-execl */ prog, "execl arg1 from foll-exec", "execl arg2 from foll-exec", (char *) 0);

So, let's do that then, there's less chance of it breaking again.

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-01 15:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 19:37 [PATCH] [gdb.base] Add additional next commands for clang-12 and above in foll-exec.exp Kumar N, Bhuvanendra
2021-04-30  8:59 ` Kumar N, Bhuvanendra
2021-05-26  8:00   ` Kumar N, Bhuvanendra
2021-05-29  2:20 ` Simon Marchi
2021-06-01 11:10   ` Kumar N, Bhuvanendra
2021-06-01 13:50     ` Simon Marchi
2021-06-01 15:40       ` Kumar N, Bhuvanendra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).