Hello,
On Sun, Feb 23, 2020 at 02:08:07PM +0100, Lucas Nussbaum wrote:
> Source: golang-github-mendersoftware-scopestack
> Version: 0.0~git20180403.c2f5599-2
[...]
> > === RUN TestPushPopNotInSameFunction
> > --- FAIL: TestPushPopNotInSameFunction (0.00s)
> > scope_stack_test.go:57: Should never get here
> > scope_stack_test.go:52: Pop() should have panicked when used in a
> > different function than Push()
> > === RUN TestDifferentScopeDistance
> > --- FAIL: TestDifferentScopeDistance (0.00s)
> > scope_stack_test.go:95: Should never get here
> > scope_stack_test.go:89: Should have panicked because scope stack
> > distance should point to this function
[...]
I assume the problem is caused by newer golang. It appears that atleast
nowadays (with go1.13.8) an inlined (anonymous) function will have
the same Func(tion) Entry(point) as the parent function it was inlined
into, eg. as used in the TestPushPopNotInSameFunction testcase.
The two Func(tions) however will have different Name()s, so simply
adding those to the Entry() comparisons will make the test-suite pass
again. See attached patch.
I'm not quite sure if this is the most robust fix, so would be great to
have some feedback from upstream (hopefully via Lluis in CC).
Some semi-relevant discussion is available at:
https://github.com/golang/go/issues/29582
... where it's discussed that rather than runtime.Caller and FuncForPC
it's better to use runtime.Callers and runtime.CallersFrames which is
supposed to deal properly with inlining, whatever that means.
Regards,
Andreas Henriksson
>From fd8f4d2fcd0ee6e541d7e7dd3e66a94b570a93ad Mon Sep 17 00:00:00 2001
From: Andreas Henriksson <andr...@fatal.se>
Date: Mon, 2 Mar 2020 02:21:35 +0100
Subject: [PATCH] Also compare function names to catch inlined funcs
Inlined Func will have the same Entry value as the parent
Func it was inlined into. It will however have a different
Name(), so compare that as well.
---
scope_stack.go | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scope_stack.go b/scope_stack.go
index bf68c81..f900752 100644
--- a/scope_stack.go
+++ b/scope_stack.go
@@ -17,6 +17,7 @@ package scopestack
import "fmt"
import "container/list"
import "runtime"
+import "strings"
// A stack type that tries to verify that each push and pop happens in the same
// function.
@@ -82,7 +83,7 @@ func (self *ScopeStack) Pop() interface{} {
oldFunc := runtime.FuncForPC(*oldFrame)
newFunc := runtime.FuncForPC(newFrame)
- if oldFunc.Entry() != newFunc.Entry() {
+ if oldFunc.Entry() != newFunc.Entry() || strings.Compare(oldFunc.Name(), newFunc.Name()) != 0 {
oldFile, oldLine := oldFunc.FileLine(*oldFrame)
newFile, newLine := newFunc.FileLine(newFrame)
msg := fmt.Sprintf("Unbalanced ScopeStack.Pop(). "+
--
2.20.1
