[PATCH] Fix skip.exp test failure observed with gcc-9.2.0

[PATCH] Fix skip.exp test failure observed with gcc-9.2.0

[Bernd Edlinger]

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

Hi,

this is the split out patch on skip.exp which fixes a pre-existing
compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
trunk of a few weeks ago at least, likely other versions too).


Is it OK for trunk?


Thanks
Bernd.



[-- Attachment #2: changelog.txt --]
[-- Type: text/plain, Size: 136 bytes --]

gdb/testsuite:
2019-12-15  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.base/skip.exp: Fix test failure observed with gcc-9.2.0.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Fix-skip.exp-test-failure-observed-with-gcc-9.2.0.patch --]
[-- Type: text/x-patch; name="0001-Fix-skip.exp-test-failure-observed-with-gcc-9.2.0.patch", Size: 2595 bytes --]

From b15964b769373f25f276430914c5efa84d411032 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Sun, 15 Dec 2019 11:05:47 +0100
Subject: [PATCH] Fix skip.exp test failure observed with gcc-9.2.0

Need to step a second time because with this gcc version
the first step jumps back to main before entering foo.
---
 gdb/testsuite/gdb.base/skip.exp | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index d763194..15dec42 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -21,8 +21,8 @@ load_lib completion-support.exp
 standard_testfile
 
 if { [prepare_for_testing "failed to prepare" "skip" \
-                          {skip.c skip1.c } \
-                          {debug nowarnings}] } {
+			  {skip.c skip1.c } \
+			  {debug nowarnings}] } {
     return -1
 }
 
@@ -142,7 +142,9 @@ with_test_prefix "step after disabling 3" {
 
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # With gcc 9.2.0 we jump once back to main before entering foo here.
+    # If that happens try to step a second time.
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at .*" "step"
     gdb_test "step" ".*" "step 4"; # Return from bar()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -261,7 +263,9 @@ with_test_prefix "step using -fu for baz" {
     gdb_test_no_output "skip enable 7"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from bar()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # With gcc 9.2.0 we jump once back to main before entering foo here.
+    # If that happens try to step a second time.
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -276,7 +280,9 @@ with_test_prefix "step using -rfu for baz" {
     gdb_test_no_output "skip enable 8"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from bar()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # With gcc 9.2.0 we jump once back to main before entering foo here.
+    # If that happens try to step a second time.
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
-- 
1.9.1

Re: [PATCH] Fix skip.exp test failure observed with gcc-9.2.0

[Simon Marchi]

On 2019-12-15 6:30 a.m., Bernd Edlinger wrote:
> Hi,
> 
> this is the split out patch on skip.exp which fixes a pre-existing
> compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
> trunk of a few weeks ago at least, likely other versions too).
> 
> 
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 

Hi Bernd,

Just wondering, were you able to figure out which change in debug info lead
to this behavior change?  The behavior with gcc 9.2.0 seems better to be me,
I think your patch is ok.

I would just remove the unrelated whitespace fix before merging.

Simon

Re: [PATCH] Fix skip.exp test failure observed with gcc-9.2.0

[Bernd Edlinger]

On 12/15/19 2:05 PM, Simon Marchi wrote:
> On 2019-12-15 6:30 a.m., Bernd Edlinger wrote:
>> Hi,
>>
>> this is the split out patch on skip.exp which fixes a pre-existing
>> compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
>> trunk of a few weeks ago at least, likely other versions too).
>>
>>
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
> 
> Hi Bernd,
> 
> Just wondering, were you able to figure out which change in debug info lead
> to this behavior change?  The behavior with gcc 9.2.0 seems better to be me,
> I think your patch is ok.
> 

Yes indeed.  The change started with gcc-8.1.0 when -gcolumn-info was enabled
by default.  -gcolumn-info was first implemented in gcc-7.1.0 but default-disabled,
so you can get the altered behavior already with gcc-7 if you manually enable
-gcolumn-info.

So previously there was just one point where line line 30 (of skip.c) started:

  [0x00000032]  Advance Line by 27 to 28
  [0x00000034]  Copy
  [0x00000035]  Special opcode 63: advance Address by 4 to 0x4004cb and Line by 2 to 30
  [0x00000036]  Advance PC by constant 17 to 0x4004dc
  [0x00000037]  Special opcode 7: advance Address by 0 to 0x4004dc and Line by 2 to 32

with column-info we have line 30 three times with different column:

  [0x00000034]  Advance Line by 27 to 28
  [0x00000036]  Copy
  [0x00000037]  Set column to 9
  [0x00000039]  Special opcode 63: advance Address by 4 to 0x4004c6 and Line by 2 to 30
  [0x0000003a]  Set column to 17
  [0x0000003c]  Special opcode 75: advance Address by 5 to 0x4004cb and Line by 0 to 30
  [0x0000003d]  Set column to 3
  [0x0000003f]  Special opcode 75: advance Address by 5 to 0x4004d0 and Line by 0 to 30
  [0x00000040]  Special opcode 105: advance Address by 7 to 0x4004d7 and Line by 2 to 32


That could probably be filtered in dwarf2read.c to keep the old behavior, but I agree
that the new behavior makes still sense, even if we cannot really use the column info
in the line number info.

> I would just remove the unrelated whitespace fix before merging.
> 

Okay, I just replicated you advice regarding 8-space tab columns on
skip-inline.exp in the other patch.  Exactly those 2 lines were copied
where the tabs were not used correctly.


Thanks
Bernd.

> Simon
> 

Re: [PATCH] Fix skip.exp test failure observed with gcc-9.2.0

[Simon Marchi]

On 2019-12-15 1:12 p.m., Bernd Edlinger wrote:
> On 12/15/19 2:05 PM, Simon Marchi wrote:
>> On 2019-12-15 6:30 a.m., Bernd Edlinger wrote:
>>> Hi,
>>>
>>> this is the split out patch on skip.exp which fixes a pre-existing
>>> compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
>>> trunk of a few weeks ago at least, likely other versions too).
>>>
>>>
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
>>
>> Hi Bernd,
>>
>> Just wondering, were you able to figure out which change in debug info lead
>> to this behavior change?  The behavior with gcc 9.2.0 seems better to be me,
>> I think your patch is ok.
>>
> 
> Yes indeed.  The change started with gcc-8.1.0 when -gcolumn-info was enabled
> by default.  -gcolumn-info was first implemented in gcc-7.1.0 but default-disabled,
> so you can get the altered behavior already with gcc-7 if you manually enable
> -gcolumn-info.
> 
> So previously there was just one point where line line 30 (of skip.c) started:
> 
>   [0x00000032]  Advance Line by 27 to 28
>   [0x00000034]  Copy
>   [0x00000035]  Special opcode 63: advance Address by 4 to 0x4004cb and Line by 2 to 30
>   [0x00000036]  Advance PC by constant 17 to 0x4004dc
>   [0x00000037]  Special opcode 7: advance Address by 0 to 0x4004dc and Line by 2 to 32
> 
> with column-info we have line 30 three times with different column:
> 
>   [0x00000034]  Advance Line by 27 to 28
>   [0x00000036]  Copy
>   [0x00000037]  Set column to 9
>   [0x00000039]  Special opcode 63: advance Address by 4 to 0x4004c6 and Line by 2 to 30
>   [0x0000003a]  Set column to 17
>   [0x0000003c]  Special opcode 75: advance Address by 5 to 0x4004cb and Line by 0 to 30
>   [0x0000003d]  Set column to 3
>   [0x0000003f]  Special opcode 75: advance Address by 5 to 0x4004d0 and Line by 0 to 30
>   [0x00000040]  Special opcode 105: advance Address by 7 to 0x4004d7 and Line by 2 to 32
> 
> 
> That could probably be filtered in dwarf2read.c to keep the old behavior, but I agree
> that the new behavior makes still sense, even if we cannot really use the column info
> in the line number info.

That is actually some very good info for anyone wondering why this change was introduced!

Could you please put it in the commit message for posterity?

>> I would just remove the unrelated whitespace fix before merging.
>>
> 
> Okay, I just replicated you advice regarding 8-space tab columns on
> skip-inline.exp in the other patch.  Exactly those 2 lines were copied
> where the tabs were not used correctly.

Great thanks.  Note that we can also fix skip.exp, just in another, obvious patch.

Simon

[PATCHv2] Fix skip.exp test failure observed with gcc-9.2.0

[Bernd Edlinger]

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

On 12/17/19 3:44 AM, Simon Marchi wrote:
> On 2019-12-15 1:12 p.m., Bernd Edlinger wrote:
>> On 12/15/19 2:05 PM, Simon Marchi wrote:
>>> On 2019-12-15 6:30 a.m., Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> this is the split out patch on skip.exp which fixes a pre-existing
>>>> compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
>>>> trunk of a few weeks ago at least, likely other versions too).
>>>>
>>>>
>>>> Is it OK for trunk?
>>>>
>>>>
>>>> Thanks
>>>> Bernd.
>>>>
>>>>
>>>
>>> Hi Bernd,
>>>
>>> Just wondering, were you able to figure out which change in debug info lead
>>> to this behavior change?  The behavior with gcc 9.2.0 seems better to be me,
>>> I think your patch is ok.
>>>
>>
>> Yes indeed.  The change started with gcc-8.1.0 when -gcolumn-info was enabled
>> by default.  -gcolumn-info was first implemented in gcc-7.1.0 but default-disabled,
>> so you can get the altered behavior already with gcc-7 if you manually enable
>> -gcolumn-info.
>>
>> So previously there was just one point where line line 30 (of skip.c) started:
>>
>>   [0x00000032]  Advance Line by 27 to 28
>>   [0x00000034]  Copy
>>   [0x00000035]  Special opcode 63: advance Address by 4 to 0x4004cb and Line by 2 to 30
>>   [0x00000036]  Advance PC by constant 17 to 0x4004dc
>>   [0x00000037]  Special opcode 7: advance Address by 0 to 0x4004dc and Line by 2 to 32
>>
>> with column-info we have line 30 three times with different column:
>>
>>   [0x00000034]  Advance Line by 27 to 28
>>   [0x00000036]  Copy
>>   [0x00000037]  Set column to 9
>>   [0x00000039]  Special opcode 63: advance Address by 4 to 0x4004c6 and Line by 2 to 30
>>   [0x0000003a]  Set column to 17
>>   [0x0000003c]  Special opcode 75: advance Address by 5 to 0x4004cb and Line by 0 to 30
>>   [0x0000003d]  Set column to 3
>>   [0x0000003f]  Special opcode 75: advance Address by 5 to 0x4004d0 and Line by 0 to 30
>>   [0x00000040]  Special opcode 105: advance Address by 7 to 0x4004d7 and Line by 2 to 32
>>
>>
>> That could probably be filtered in dwarf2read.c to keep the old behavior, but I agree
>> that the new behavior makes still sense, even if we cannot really use the column info
>> in the line number info.
> 
> That is actually some very good info for anyone wondering why this change was introduced!
> 
> Could you please put it in the commit message for posterity?
> 

Sure, good point.

Attached is the new, reworded version of the patch.
When I looked at it again, I stumbled over two wrong comments nearby,
telling "# Return from foo()" when we actually return from bar(),
and "# Return from bar()" when we actually return from foo()... :)

>>> I would just remove the unrelated whitespace fix before merging.
>>>
>>
>> Okay, I just replicated you advice regarding 8-space tab columns on
>> skip-inline.exp in the other patch.  Exactly those 2 lines were copied
>> where the tabs were not used correctly.
> 
> Great thanks.  Note that we can also fix skip.exp, just in another, obvious patch.
> 

Okay, will send a separate patch for the whitespace.


Thanks
Bernd.

[-- Attachment #2: changelog.txt --]
[-- Type: text/plain, Size: 136 bytes --]

gdb/testsuite:
2019-12-15  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.base/skip.exp: Fix test failure observed with gcc-9.2.0.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Fix-skip.exp-test-failure-observed-with-gcc-9.2.0.patch --]
[-- Type: text/x-patch; name="0001-Fix-skip.exp-test-failure-observed-with-gcc-9.2.0.patch", Size: 3926 bytes --]

From 5884713ee87992c4011ae9b2d45fbe1bd29b9140 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Sun, 15 Dec 2019 11:05:47 +0100
Subject: [PATCH] Fix skip.exp test failure observed with gcc-9.2.0

We need to step a second time with this gcc version.
The first step jumps back to main before entering foo.
Previously the control flow was from bar directly to foo.

Further ananlysis suggests, that this change in behavior started
with gcc-8.1.0 when -gcolumn-info was enabled by default.
The option -gcolumn-info was first implemented in gcc-7.1.0 but
default-disabled, so you can get the altered behavior already with
gcc-7 if you manually enable -gcolumn-info.

Previously there was just one point where line 30 (of skip.c) started:

  [0x00000032]  Advance Line by 27 to 28
  [0x00000034]  Copy
  [0x00000035]  Special opcode 63: advance Address by 4 to 0x4004cb and Line by 2 to 30
  [0x00000036]  Advance PC by constant 17 to 0x4004dc
  [0x00000037]  Special opcode 7: advance Address by 0 to 0x4004dc and Line by 2 to 32

But with -gcolumn-info enabled, we have line 30 three times with different column:

  [0x00000034]  Advance Line by 27 to 28
  [0x00000036]  Copy
  [0x00000037]  Set column to 9
  [0x00000039]  Special opcode 63: advance Address by 4 to 0x4004c6 and Line by 2 to 30
  [0x0000003a]  Set column to 17
  [0x0000003c]  Special opcode 75: advance Address by 5 to 0x4004cb and Line by 0 to 30
  [0x0000003d]  Set column to 3
  [0x0000003f]  Special opcode 75: advance Address by 5 to 0x4004d0 and Line by 0 to 30
  [0x00000040]  Special opcode 105: advance Address by 7 to 0x4004d7 and Line by 2 to 32

That could probably be filtered in dwarf2read.c to keep the old behavior, but
the new behavior makes still sense, even if we cannot really make use of the
column in the line number info for now.
---
 gdb/testsuite/gdb.base/skip.exp | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index d763194..cf27d5b 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -141,9 +141,11 @@ with_test_prefix "step after disabling 3" {
     }
 
     gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
-    gdb_test "step" ".*" "step 4"; # Return from bar()
+    gdb_test "step" ".*" "step 2"; # Return from bar()
+    # With gcc 9.2.0 we jump once back to main before entering foo here.
+    # If that happens try to step a second time.
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at .*" "step"
+    gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
 
@@ -261,7 +263,9 @@ with_test_prefix "step using -fu for baz" {
     gdb_test_no_output "skip enable 7"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from bar()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # With gcc 9.2.0 we jump once back to main before entering foo here.
+    # If that happens try to step a second time.
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -276,7 +280,9 @@ with_test_prefix "step using -rfu for baz" {
     gdb_test_no_output "skip enable 8"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
     gdb_test "step" ".*" "step 2"; # Return from bar()
-    gdb_test "step" "foo \\(\\) at.*" "step 3"
+    # With gcc 9.2.0 we jump once back to main before entering foo here.
+    # If that happens try to step a second time.
+    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
-- 
1.9.1

Re: [PATCHv2] Fix skip.exp test failure observed with gcc-9.2.0

[Simon Marchi]

On 2019-12-17 9:56 a.m., Bernd Edlinger wrote:
> On 12/17/19 3:44 AM, Simon Marchi wrote:
>> On 2019-12-15 1:12 p.m., Bernd Edlinger wrote:
>>> On 12/15/19 2:05 PM, Simon Marchi wrote:
>>>> On 2019-12-15 6:30 a.m., Bernd Edlinger wrote:
>>>>> Hi,
>>>>>
>>>>> this is the split out patch on skip.exp which fixes a pre-existing
>>>>> compatibilty issue with that test case and gcc-9.2.0 (and gcc-10 from
>>>>> trunk of a few weeks ago at least, likely other versions too).
>>>>>
>>>>>
>>>>> Is it OK for trunk?
>>>>>
>>>>>
>>>>> Thanks
>>>>> Bernd.
>>>>>
>>>>>
>>>>
>>>> Hi Bernd,
>>>>
>>>> Just wondering, were you able to figure out which change in debug info lead
>>>> to this behavior change?  The behavior with gcc 9.2.0 seems better to be me,
>>>> I think your patch is ok.
>>>>
>>>
>>> Yes indeed.  The change started with gcc-8.1.0 when -gcolumn-info was enabled
>>> by default.  -gcolumn-info was first implemented in gcc-7.1.0 but default-disabled,
>>> so you can get the altered behavior already with gcc-7 if you manually enable
>>> -gcolumn-info.
>>>
>>> So previously there was just one point where line line 30 (of skip.c) started:
>>>
>>>   [0x00000032]  Advance Line by 27 to 28
>>>   [0x00000034]  Copy
>>>   [0x00000035]  Special opcode 63: advance Address by 4 to 0x4004cb and Line by 2 to 30
>>>   [0x00000036]  Advance PC by constant 17 to 0x4004dc
>>>   [0x00000037]  Special opcode 7: advance Address by 0 to 0x4004dc and Line by 2 to 32
>>>
>>> with column-info we have line 30 three times with different column:
>>>
>>>   [0x00000034]  Advance Line by 27 to 28
>>>   [0x00000036]  Copy
>>>   [0x00000037]  Set column to 9
>>>   [0x00000039]  Special opcode 63: advance Address by 4 to 0x4004c6 and Line by 2 to 30
>>>   [0x0000003a]  Set column to 17
>>>   [0x0000003c]  Special opcode 75: advance Address by 5 to 0x4004cb and Line by 0 to 30
>>>   [0x0000003d]  Set column to 3
>>>   [0x0000003f]  Special opcode 75: advance Address by 5 to 0x4004d0 and Line by 0 to 30
>>>   [0x00000040]  Special opcode 105: advance Address by 7 to 0x4004d7 and Line by 2 to 32
>>>
>>>
>>> That could probably be filtered in dwarf2read.c to keep the old behavior, but I agree
>>> that the new behavior makes still sense, even if we cannot really use the column info
>>> in the line number info.
>>
>> That is actually some very good info for anyone wondering why this change was introduced!
>>
>> Could you please put it in the commit message for posterity?
>>
> 
> Sure, good point.
> 
> Attached is the new, reworded version of the patch.
> When I looked at it again, I stumbled over two wrong comments nearby,
> telling "# Return from foo()" when we actually return from bar(),
> and "# Return from bar()" when we actually return from foo()... :)
> 
>>>> I would just remove the unrelated whitespace fix before merging.
>>>>
>>>
>>> Okay, I just replicated you advice regarding 8-space tab columns on
>>> skip-inline.exp in the other patch.  Exactly those 2 lines were copied
>>> where the tabs were not used correctly.
>>
>> Great thanks.  Note that we can also fix skip.exp, just in another, obvious patch.
>>
> 
> Okay, will send a separate patch for the whitespace.
> 
> 
> Thanks
> Bernd.
> 

Thanks, I pushed it.

Simon