Invalid memory address of string object

[Bernd Fix]

Hi folks,

I am using go1.19.1 for development and see a strange panic ("invalid
memory address") in an application; the panic is definitely not
triggered by a nil dereference (the object in question is a string):

key = peer.Key()
entry, ok := tbl.recs[key]

The panic happens in the second line of the code. After checking that
'tbl' and 'tbl.recs' are valid (not nil), I inserted a log line in between:

key = peer.Key()
log.Printf("Key=%v", key)
entry, ok := tbl.recs[key]

Now the panic happens in the log line; the object 'key' seems to have an
invalid address (but I don't know how I can check if I can't access the
object and printf, where the new panic happens, obviously doesn't know
either).

The 'Key()' function returns a string (there is no way to return nil
from a function with a string return value that I know of), so obviously
something strange is going on.

It even got a bit stranger when I removed the log line and tried a recover:

defer func() {
if r := recover(); r != nil {
log.Printf("key=%s", peer.Key())
log.Printf("key=%v", key)
}
}()

The first log line (now) works (it delivers a valid string result), but
the second line still panics within recovery.

Maybe I should mention that the application heavily uses channels and go
routines; I have the subjective impression that the more go routines are
running the more likely the panic occurs after some time. But maybe that
is irrelevant.

I compiled the same source with go1.18 (the earliest version that could
compile the code), and the panic still occurs.

Is there anthing I can do to hunt down the problem? I am happy to share
the code (it is free software anyway) and can explain how to reproduce
the panic on your own machine (I guess that running the application
might requires some explanation).

Cheers, Bernd.

[Jason Phillips]

Have your tried building and running your application with the race detector enabled[1]? You may have a data race.

1 - https://go.dev/doc/articles/race_detector

[Kurtis Rader]

Insufficient information. Show us the panic. Are you using CGO? Can you show the definition of peer.Key()? Is it (or any function it calls) using "unsafe"?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3fb65401-086f-9450-1e0d-39eee1e529da%40hoi-polloi.org.

--

Kurtis Rader

Caretaker of the exceptional canines Junior and Hank

[Axel Wagner]

Another possibility is someone trying (and failing) to use unsafe to speed things up in a []byte/string conversion and accidentally converts a nil-slice.

It's also possible that the value is not nil, but a pointer to another piece of invalid memory - an mmaped region which got unmapped or a string pointing into the C stack, or something.

FWIW you can check the actual data pointer of the string using unsafe, for debugging. That should at least tell you whether it's nil, as a first step.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/1c12eec5-470a-491c-b18f-741f34cecba6n%40googlegroups.com.

[Bernd Fix]

On 9/25/22 18:35, Jason Phillips wrote:
> Have your tried building and running your application with the race
> detector enabled[1]? You may have a data race.

Thanks for the advice; that is possible although I have checked that
access to all unsafe types like maps and arrays is controlled by
mutexes, but something might have escaped my attention. Will check as
soon as I can. >Y<

[Bernd Fix]

On 9/25/22 19:06, 'Axel Wagner' via golang-nuts wrote:
> Another possibility is someone trying (and failing) to use unsafe to speed
> things up in a []byte/string conversion and accidentally converts a
> nil-slice.

No unsafe in my own code, but I use a third-party library ("sdlcanvas")
for rendering that I think is using cgo and might use unsafe too, but
that does not relate to the part of the code that panics.

> It's also possible that the value is not nil, but a pointer to another
> piece of invalid memory - an mmaped region which got unmapped or a string
> pointing into the C stack, or something.
>
> FWIW you can check the actual data pointer of the string using unsafe

> <https://go.dev/play/p/AGMEBkD3JbA>, for debugging. That should at least

> tell you whether it's nil, as a first step.

I will try that; thanks for the advice. >Y<

[Bernd Fix]

On 9/25/22 19:05, Kurtis Rader wrote:
> Insufficient information. Show us the panic

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x49388d]

goroutine 100037 [running]:
leatea/core.(*ForwardTable).Learn(0xc0000b6d80, 0xc000c22660)
/prj/net/routing/leatea/core/forward_table.go:455 +0x25a
leatea/core.(*Node).Receive(0xc0000b6d80, {0x7275b0?, 0xc000c22660?})
/prj/net/routing/leatea/core/node.go:164 +0x2f5
created by leatea/core.(*Node).Start
/prj/net/routing/leatea/core/node.go:98 +0x2ea

> Are you using CGO?

A third party library ("sdlcanvas") does, but I don't think it is
related to the problem as rendering happens later (after the panic).

> Can you show the definition of peer.Key()?

Sure:

// Key returns a string used for map operations
func (p *PeerID) Key() string {
if p == nil {
return "(nil)"
}
if len(p.str64) == 0 {
p.str64 = base64.StdEncoding.EncodeToString(p.Data)
}
return p.str64

}

> Is it (or any function it calls) using "unsafe"?

As said before, "sdlcanvas" as a cgo library might use that but it is
not logically involved in the process that panics. >Y<

[Andrew Harris]

could p.Data could be nil here?

[Bernd Fix]

No, p.Data is not nil - and the panic does not happen in the Key()
method, but in the code from my first email. >Y<

--
"Es sind nicht die besten Massen, die für Brot und Spiele den Verlust
der Freiheit verschmerzen." (Kautsky, 1919, "Diktatur des Proletariats")

[Bernd Fix]

On 9/25/22 18:35, Jason Phillips wrote:

> Have your tried building and running your application with the race
> detector enabled[1]? You may have a data race.

This did help to figure out what happens (shame on me I didn't check
first myself before bothering others).

There was a race condition on 'p.str64' in the Key() method:

// Key returns a string used for map operations
func (p *PeerID) Key() string {
if p == nil {
return "(nil)"
}
if len(p.str64) == 0 {
p.str64 = base64.StdEncoding.EncodeToString(p.Data)
}
return p.str64
}

I just didn't realize that string is not thread-safe; guess I have to
replace my "create-on-first-use" approach (or make it work).

Thanks again for all efforts to help,
Bernd.

[Axel Wagner]

On Mon, Sep 26, 2022 at 9:03 AM Bernd Fix <b...@hoi-polloi.org> wrote:

Sure:

// Key returns a string used for map operations
func (p *PeerID) Key() string {
     if p == nil {
         return "(nil)"
     }
     if len(p.str64) == 0 {
         p.str64 = base64.StdEncoding.EncodeToString(p.Data)
     }
     return p.str64
}

Looking at this code, the only cay I can imagine this returning such an invalid string is a data race.

Note that calling PeerID.Key is not concurrency safe, as it both reads an writes p.str64, without any sort of mutex or atomicity - but it might very well *seem* that way, as it might *seem* it should be a read-only operation.

So I would guess something is calling this method concurrently (or calling it concurrently with a similar method), causing a data-race, returning a string which has the length of the base64.EncodeToString return value, but not the data pointer.

 

> Is it (or any function it calls) using "unsafe"?

As said before, "sdlcanvas" as a cgo library might use that but it is
not logically involved in the process that panics.    >Y<

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3a7e6721-1a4e-c789-97aa-132f1a76466b%40hoi-polloi.org.

[Axel Wagner]

Also, this is consistent with your observation that calling peer.Key again returns a valid string. After the first call (returning the invalid string) the concurrent modification of p.str64 has finished and subsequent calls return the full value.

Which is to say: Run your code under the race detector. It should catch this.

You mentioned above that you checked that all accesses to "unsafe structures like maps or arrays" are protected by mutexes. But that's a fallacy. *Any* concurrent access must be made safe. There is no such thing as a benign data race.

[Bernd Fix]

(Looks like the mail didn't make it to the mailing list, so I resend it
here:)

On 9/25/22 18:35, Jason Phillips wrote:

> Have your tried building and running your application with the race
> detector enabled[1]? You may have a data race.

This did help to figure out what happens (shame on me I didn't check
first myself before bothering others).

There was a race condition on 'p.str64' in the Key() method:

// Key returns a string used for map operations
func (p *PeerID) Key() string {
if p == nil {
return "(nil)"
}
if len(p.str64) == 0 {
p.str64 = base64.StdEncoding.EncodeToString(p.Data)
}
return p.str64
}

[Kurtis Rader]

On Mon, Sep 26, 2022 at 12:03 AM Bernd Fix <b...@hoi-polloi.org> wrote:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x49388d]

Notice the "addr=0x0"? That's a nil pointer dereference.

 

> Can you show the definition of peer.Key()?

Sure:

// Key returns a string used for map operations
func (p *PeerID) Key() string {
     if p == nil {
         return "(nil)"
     }
     if len(p.str64) == 0 {
         p.str64 = base64.StdEncoding.EncodeToString(p.Data)
     }
     return p.str64
}

I agree with Axel's hypothesis when he writes: "So I would guess something is calling this method concurrently (or calling it concurrently with a similar method), causing a data-race, returning a string which has the length of the base64.EncodeToString return value, but not the data pointer."  Try doing everything inside that function while holding a mutex. I'll bet the problem disappears. Which is not to say that is the correct fix but would confirm or refute the hypothesis.