[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 Sent: Saturday, May 29, 2021 7:51 AM To: Kumar N, Bhuvanendra ; gdb-patches@sourceware.org Cc: George, Jini Susan ; Achra, Nitika ; Sharma, Alok Kumar ; E, Nagajyothi ; Tomar, Sourabh Singh 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&data=04%7C01%7CBhuvanendra.KumarN%40amd.com%7Cbdc30f93800b4f6c032d08d92248612a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637578516557363689%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=FAPo1TaCGE%2FImUyN3Fhex96GdMVqercM6HnzS46bNQ0%3D&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&data=04%7C01%7CBhuvanendra.KumarN%40amd.com%7Cbdc30f93800b4f6c032d08d92248612a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637578516557373684%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TW7QjCio73C%2B8cqsa56Gd4nh8qxVe04u2gjXPHDDmxg%3D&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 > 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&data=04%7C01%7CBhuvanendra.KumarN%40am > d.com%7Cbdc30f93800b4f6c032d08d92248612a%7C3dd8961fe4884e608e11a82d994 > e183d%7C0%7C0%7C637578516557373684%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 > wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda > ta=TW7QjCio73C%2B8cqsa56Gd4nh8qxVe04u2gjXPHDDmxg%3D&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 > + > + * 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 Does it work if we just put the whole execlp call on a single line instead? Simon