Bug 17018 - XFAIL: gdb.go/hello.exp: Starting string check
Summary: XFAIL: gdb.go/hello.exp: Starting string check
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: go (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-04 13:18 UTC by Pedro Alves
Modified: 2020-02-20 16:57 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Alves 2014-06-04 13:18:00 UTC
On Fedora 20, I noticed this:

print st
$1 = 0xc200016210 "\000\000\000\000\000\000\000\000\200c\000\000\302", '\000' <repeats 11 times>, "d\000\000\000\000\000\000\000P$\322\367\377\177\000\000\000\000\000\000\0
00\000\000\000pb\001\000\302", '\000' <repeats 43 times>, "\240b\001\000\302", '\000' <repeats 43 times>, "\320b\001\000\302", '\000' <repeats 44 times>, "c\001\000\302\000
\000\000"...
(gdb) XFAIL: gdb.go/hello.exp: Starting string check

The comment in the test says this is XFAIL for a different reason:

# This used to print "", i.e., the local "st" initialized as "".
setup_xfail "*-*-*"

gdb_test "print st" \
    ".* = $hex \"\"" \
    "Starting string check"

So I wonder if this is really expected.
Comment 1 Tom de Vries 2020-02-20 11:14:59 UTC
The test-case source is:
...
package main

import "fmt"

var myst = "Shall we?"

func main () {
  fmt.Println ("Before assignment")
  st := "Hello, world!" // this intentionally shadows the global "st"
  fmt.Println (st) // set breakpoint 1 here
  fmt.Println (myst)
}
...

The comment about shadowing is somewhat confusing given that there is no global variable st.

I've made a variant renaming myst to st, which does contain shadowing:
...
package main

import "fmt"

var st = "Shall we?"

func main () {
  fmt.Println (st)
  fmt.Println ("Before assignment")
  st := "Hello, world!"
  fmt.Println (st)
}
...

Using this test-case, I ran into a gcc bug, filed as PR gcc/go/93844 ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93844 ).
[ Filed as go problem, but as richi mentioned, also happens for c++, and c99. ]
[ Same problem with go1.11.13 ].

The problem is as follows: the scope of local variable st start at the ':=' line, but the debug info fails to wrap the local variable in an appropriate DW_TAG_lexical_block (or to emit an appropriate DW_AT_start_scope), and consequently when accessing the value of st at the start main, we incorrectly get the uninitialized local value, instead of the global value.

If you're lucky, the uninitialized local value is 0, and will be printed as:
...
(gdb) p st
$3 = 0x0 ""
...

And using a compiler without this bug, it should print:
...
(gdb) p st
$3 = 0x12345678 "Shall we?"
...
[ Just that I don't know if there's such a compiler. ]

This compiler bug matches the xfail comment:
...
# This used to print "", i.e., the local "st" initialized as "".                         
setup_xfail "*-*-*"
...
in the sense that printing "" indicates presence of the bug.

But then again it says "used to print", implying that there's a compiler where this bug is fixed, and that doesn't match.
Comment 2 Tom de Vries 2020-02-20 11:28:22 UTC
(In reply to Tom de Vries from comment #1)
> The test-case source is:
> ...
> package main
> 
> import "fmt"
> 
> var myst = "Shall we?"
> 
> func main () {
>   fmt.Println ("Before assignment")
>   st := "Hello, world!" // this intentionally shadows the global "st"
>   fmt.Println (st) // set breakpoint 1 here
>   fmt.Println (myst)
> }
> ...

Anyway, looking at the actual test-case, the same gcc PR is triggered. It's just that the valid behaviour for printing st at the start of main should be 'No symbol "st" in current context' or some such.
Comment 3 Tom de Vries 2020-02-20 11:29:17 UTC
(In reply to Pedro Alves from comment #0)
> On Fedora 20, I noticed this:
> 
> print st
> $1 = 0xc200016210 "\000\000\000\000\000\000\000\000\200c\000\000\302",
> '\000' <repeats 11 times>,
> "d\000\000\000\000\000\000\000P$\322\367\377\177\000\000\000\000\000\000\0
> 00\000\000\000pb\001\000\302", '\000' <repeats 43 times>,
> "\240b\001\000\302", '\000' <repeats 43 times>, "\320b\001\000\302", '\000'
> <repeats 44 times>, "c\001\000\302\000
> \000\000"...
> (gdb) XFAIL: gdb.go/hello.exp: Starting string check
> 

I can reproduce this with the google go compiler (go1.11.13).
Comment 4 Tom de Vries 2020-02-20 11:44:52 UTC
(In reply to Pedro Alves from comment #0)
> The comment in the test says this is XFAIL for a different reason:
> 
> # This used to print "", i.e., the local "st" initialized as "".
> setup_xfail "*-*-*"
> 
> gdb_test "print st" \
>     ".* = $hex \"\"" \
>     "Starting string check"
> 
> So I wonder if this is really expected.

I'd say a more accurate formulation of the xfail would be:
...
diff --git a/gdb/testsuite/gdb.go/hello.exp b/gdb/testsuite/gdb.go/hello.exp
index 1096f6475b..d857a69dee 100644
--- a/gdb/testsuite/gdb.go/hello.exp
+++ b/gdb/testsuite/gdb.go/hello.exp
@@ -34,12 +34,17 @@ if { [go_runto_main] < 0 } {
     return -1
 }
 
-# This used to print "", i.e., the local "st" initialized as "".
-setup_xfail "*-*-*"
-
-gdb_test "print st" \
-    ".* = $hex \"\"" \
-    "starting string check"
+gdb_test_multiple "print st" "starting string check" {
+    -re -wrap "No symbol \"st\" in current context" {
+       pass $gdb_test_name
+    }
+    -re -wrap ".*" {
+       # GCC PR 93844 (Same problem with google go compiler go1.11.13).
+       # Due to the PR, gdb prints an uninitialized value, which can manifest
+       # as '$3 = 0x0 ""', but also as printing a wild pointer.
+       xfail $gdb_test_name
+    }
+}
 
 if { [gdb_breakpoint ${srcfile}:${bp_location1}] } {
     pass "setting breakpoint 1"
...

I wonder though if that's all a bit too complicated for what should be "basic tests". We could also just remove the print of the out-of-scope variable.
Comment 5 Tom de Vries 2020-02-20 12:41:43 UTC
(In reply to Tom de Vries from comment #4)
> I wonder though if that's all a bit too complicated for what should be
> "basic tests". We could also just remove the print of the out-of-scope
> variable.

After some more consideration, I went for a two-testcase solution. Submitted here: https://sourceware.org/ml/gdb-patches/2020-02/msg00818.html .
Comment 6 Sourceware Commits 2020-02-20 16:47:11 UTC
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a9c798035de33ccc3bc3e494449bbe931e900372

commit a9c798035de33ccc3bc3e494449bbe931e900372
Author: Tom de Vries <tdevries@suse.de>
Date:   Thu Feb 20 17:46:17 2020 +0100

    [gdb/testsuite] Fix hello.go xpass
    
    With gdb.go/hello.go, we run into an xpass:
    ...
    Thread 1 "hello" hit Breakpoint 1, main.main () at hello.go:7^M
    7       func main () {^M
    (gdb) print st^M
    $1 = 0x0 ""^M
    (gdb) XPASS: gdb.go/hello.exp: starting string check
    ...
    
    The xfail is setup as follows:
    ...
    \# This used to print "", i.e., the local "st" initialized as "".
    setup_xfail "*-*-*"
    
    gdb_test "print st" \
        ".* = $hex \"\"" \
        "starting string check"
    ...
    
    It's not clear what gccgo/gc PR this xfail refers to.
    
    It's also not clear why the empty string is both:
    - listed as reason for xfail, and
    - used in the pass pattern.
    
    Furthermore, there's a comment in the hello.go testcase:
    ...
      st := "Hello, world!" // this intentionally shadows the global "st"
    ...
    while there's no global st variable present, only a variable myst:
    ...
    var myst = "Shall we?"
    ...
    
    Fix this by splitting up the test-case in two test-cases, hello.{go,exp} and
    global-local-var-shadow.{go,exp}.
    
    In hello.exp we no longer attempt to print st before its declaration.  In
    hello.go we remove the myst variable as well the comment related to shadowing.
    
    In global-local-var-shadow.go, we rename myst to st, such that the comment
    related to shadowing is correct.  In global-local-var-shadow.exp we attempt to
    print the value of st before the local definition, which should print the
    value of the global definition, and xfail this with reference to GCC PR93844.
    
    Tested on x86_64-linux, with gccgo 10.
    
    gdb/testsuite/ChangeLog:
    
    2020-02-20  Tom de Vries  <tdevries@suse.de>
    
    	PR go/17018
    	* gdb.go/hello.exp: Copy ...
    	* gdb.go/global-local-var-shadow.exp: ... here.  New file.  Expect
    	print of st to print value of global definition. Add xfail for GCC
    	PR93844.
    	* gdb.go/hello.exp: Remove printing of st before definition.
    	* gdb.go/hello.go: Copy ...
    	* gdb.go/global-local-var-shadow.go: ... here. New test.  Rename myst
    	to st.
    	* gdb.go/hello.go: Remove myst.  Remove comment about shadowing.
Comment 7 Tom de Vries 2020-02-20 16:57:10 UTC
Test-case updated, marking resolved-fixed.