## Description
When sending a value to a channel that is a type alias defined in a binary package, yaegi panics with a nil pointer dereference.
This PR was created with AI assistance (Claude) as I hit this issue in a downstream project using yaegi.
The fix and tests have been reviewed and verified for correctness and functionality by myself.
## Root Cause
In the `send` function (`interp/run.go`), the code directly accesses `c0.typ.val` to get the channel element type:
```go
value1 := genDestValue(c0.typ.val, c1)
```
For source-defined channels (category `chanT`), `.val` contains the element type. But for binary channel type aliases (category `valueT`), `.val` is nil - the element type is only available via reflection in `.rtype`.
## Fix
Use the existing `elem()` method which correctly handles both cases:
```go
value1 := genDestValue(c0.typ.elem(), c1)
```
The `elem()` method (in `type.go`) already handles this properly:
```go
func (t *itype) elem() *itype {
if t.cat == valueT {
return valueTOf(t.rtype.Elem()) // Binary types: create from reflect
}
return t.val // Source types: use existing itype with all metadata
}
```
This is a one-line fix that fixes binary channel type aliases by using reflection to get the element type
## Test Results
| Test | Without Fix | With Fix |
|------|-------------|----------|
| `TestSendToSourceDefinedChannel` | PASS | PASS |
| `TestSendToSourceDefinedChannelTypeAlias` | PASS | PASS |
| `TestSendToBinaryChannelTypeAlias` | FAIL | PASS |
## Minimal Reproduction
### mypkg/mypkg.go
```go
package mypkg
// IntChan is a channel type alias
type IntChan chan int
// NewIntChan creates a new IntChan
func NewIntChan() IntChan {
return make(IntChan, 1)
}
```
### symbols.go
```go
package main
import (
"reflect"
"yaegi-repro/mypkg"
)
var Symbols = map[string]map[string]reflect.Value{}
func init() {
Symbols["yaegi-repro/mypkg/mypkg"] = map[string]reflect.Value{
"IntChan": reflect.ValueOf((*mypkg.IntChan)(nil)),
"NewIntChan": reflect.ValueOf(mypkg.NewIntChan),
}
}
```
### main.go
```go
package main
import (
"fmt"
"log"
"github.com/traefik/yaegi/interp"
)
func main() {
i := interp.New(interp.Options{})
if err := i.Use(Symbols); err != nil {
log.Fatal(err)
}
code := `
package main
import "yaegi-repro/mypkg"
func main() {
ch := mypkg.NewIntChan()
ch <- 42 // This line panics without the fix
val := <-ch
println("Received:", val)
}
`
_, err := i.Eval(code)
if err != nil {
log.Fatal(err)
}
fmt.Println("Success!")
}
```
## Results
**Without fix:**
```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x128 pc=0x...]
goroutine 1 [running]:
github.com/traefik/yaegi/interp.(*itype).refType(0x0?, ...)
.../interp/type.go:2065 +0x7a
github.com/traefik/yaegi/interp.(*itype).TypeOf(...)
.../interp/type.go:2223
github.com/traefik/yaegi/interp.genDestValue(0x0, ...)
.../interp/value.go:157 +0x25
github.com/traefik/yaegi/interp.send(...)
.../interp/run.go:3766 +0x...
```
**With fix:**
```
Received: 42
Success!
```
The golangci-lint configuration is migrated from 1.x to 2.x version. Linters are now enabled using a white list instead of a black list, as the list of linters keeps growing and few are relevant for us.
<!-- AIKIDO_SECURITY_PR_SUMMARY_START -->
## Summary by Aikido
| Security Issues: 0 | Quality Issues: 0 | Resolved Issues: 0 |
| :--- | :--- | :--- |
**⚡ Enhancements**
* Migrated golangci-lint configuration to v2 and enabled whitelist linters
* Updated GitHub Actions to install golangci-lint v2.4.0 binary
**🔧 Refactors**
* Normalized test error message text from 'No tests found' to lowercase
<sup>[More info](https://app.aikido.dev/featurebranch/scan/77159580?groupId=48744)</sup>
<!-- AIKIDO_SECURITY_PR_SUMMARY_END -->
Currently, yaegi uses the `filepath` package to join paths.
However, this has issues: YAEGI uses an `fs.FS` internally for source code access, and `fs` specifies that `/` separated paths must be used. On Unix systems, this obviously makes no difference; but on Windows, with a non-default FS, this causes errors (since YAEGI creates `\` separated paths, but the FS expects `/` separated paths).
Furthermore, generally speaking, Golang import paths and the like are always `/` separated anyway, regardless of OS.
To clear this up, this PR changes path handling to `/` separated paths. This has the advantage of being accepted by all OS (even Windows will accept `/` separated paths). Paths passed by the user to `CompilePath` or similar are changed to `/` separators, if necessary.
* chore: bump golangci-lint and fix config
This fixes the CI lint step. No change in code except a fix
in test output.
* bump golangci-lint from 1.56.2 to 1.63.4
* test: fix test check
When parsing binary operator expressions, make sure that implicit type conversions for untyped expressions are performed. It involves walking the sub-expression subtree at post-processing of binary expressions.
Fixes#1653.
This builds on #1644 and adds automatic per-loop variables that are consistent with go 1.22 behavior. See #1643 for discussion.
This is still a draft because the for7 version ends up capturing the per-loop var values when they are +1 relative to what they should be. Maybe somehow the incrementing and conditional code is somehow capturing the within loop variables and incrementing them? not sure how that would work. anyway, need to investigate further before this is ready to go, but pushing it here in case there are other issues or someone might figure out this bug before I do..
I found these basic functions and Stringer methods essential for tracking down issues fixed recently -- probably be of use to others and I couldn't find something else in the code that would provide similar utility, though I never tried the debugger.
This fixes issue #1634
includes special case for defer function.
I could remove or significantly reduce the comment description, and just have that here for future reference:
// per #1634, if v is already a func, then don't re-wrap! critically, the original wrapping
// clones the frame, whereas the one here (below) does _not_ clone the frame, so it doesn't
// generate the proper closure capture effects!
// this path is the same as genValueAsFunctionWrapper which is the path taken above if
// the value has an associated node, which happens when you do f := func() ..
Follow by the [Spec](https://go.dev/ref/spec#Assignment_statements):
The number of operands on the left hand side must match the number of values. For instance, if f is a function returning two values `x, y = f()` assigns the first value to x and the second to y.
In the second form, the number of operands on the left must equal the number of expressions on the right, each of which must be single-valued, and the nth expression on the right is assigned to the nth operand on the left.
Fixes#1606
A side effect of #1281 is that it added unnecessary additional newlines in generated interface wrappers from the `extract` tool. This PR removes those newlines from the extract tool template and updates the generated code with that change.
* feat: support go1.22
* Temporary fix for consistency tests due to language change in for loops
* review: clean old files
---------
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>
* feat: generate go1.21 files
* chore: update CI
* feat: add support for generic symbols in standard library packages
This is necessary to fully support go1.21 and beyond, which now
provide some generic packages such as `cmp`, `maps` or `slices`
in the standard library.
The principle is to embed the generic symbols in source form (as
strings) so they can be instantiated as required during interpretation.
Extract() has been modified to skip the generic types, functions and
constraint interfaces which can't be represented as reflect.Values.
A new stdlib/generic package has been added to provide the corresponding
source files as embedded strings.
The `Use()` function has been changed to pre-parse generic symbols as
doing lazy parsing was causing cyclic dependencies issues at compiling.
This is something we may improve in the future.
A unit test using `cmp` has been added.
For now, there are still some issues with generic stdlib packages
inter-dependencies, for example `slices` importing `cmp`, or when
generic types or function signatures depends on pre-compiled types
in the same package, which we will support shortly.
* fixup
* fixup
* fixup
* fixup
* fixup
* fixup
* fixes for go1.20
* fix previous
* update unsafe2 for go1.21, skip faky tests
In go1.21, the reflect rtype definition has been move to internal/abi.
We follow this change for maintainability, even if there is no layout
change (the go1.20 unsafe2 is compatible with go1.21).
We have isolated a few problematic tests which are failing sometimes
in go1.21, but work in go1.20, and also in go1.22. Those tests are
skipped if in go1.21. A preliminary investigation can not confirm that
something is wrong in yaegi, and the problem disappears with go1.22.
* add new wrapper for go1.21 package testing/slogtest
* add missing wrapper for go/doc/comment
* add support for slices generic package
---------
Co-authored-by: Marc Vertes <mvertes@free.fr>
Adds `wasip1` to known OS list, introduced in golang/go#58141.
Without this change, `yaegi extract` may fail on Go 1.21 with the following message:
```
type-checking package "time" failed (<GOROOT>/src/time/zoneinfo_wasip1.go:8:5: platformZoneSources redeclared in this block)
```
Currently, yaegi only records source positions when writing panic trace to stderr.
This change adds function names to the panic output.
Unfortunately, yaegi walks the trace in reverse order, comparing to Go runtime. This can be improved in the future.
Avoid a spurious optimisation which forces a variable to be reused instead of redefined for assignment operation. This ensures that a variable defined in a loop is re-allocated, preserving the previous instance when used by a closure for example.
Fix#1594
Embedding files using `//go:embed` and `embed` packages can not be supported in yaegi, so it is better to not generate the wrapper to embed and have a graceful error in case of usage of `embed.FS` in the interpreter.
Also update README about unsuported directives.
Fixes#1557.
I implemented fs.FS. When the file does not exist, `xerrors.Wrap(fs.ErrNotExist, "")` will be returned. However, `os.IsNotExist` cannot handle this situation. I found the following comment:
```
// IsNotExist returns a boolean indicating whether the error is known to
// report that a file or directory does not exist. It is satisfied by
// ErrNotExist as well as some syscall errors.
//
// This function predates errors.Is. It only supports errors returned by
// the os package. New code should use errors.Is(err, fs.ErrNotExist).
```
Therefore, we should use `errors.Is(err, fs.ErrNotExist)` instead.
Unsafe functions such as `unsafe.Alignof`, `unsafe.Offsetof` and `unsafe.Sizeof` can be used for type declarations early on during compile, and as such, must be treated as builtins returning constants at compile time. It is still necessary to explicitely enable unsafe support in yaegi.
The support of `unsafe.Add` has also been added.
Fixes#1544.
Only structures with one embedded field can be marked anonymous, due to golang/go#15924. Also check only that the method is defined, do not verify its complete signature, as the receiver may or not be defined in the arguments of the method.
Fixes#1537.
The result of the expression giving the size of an array may be an `int` instead of `constant.Value` in case of an out of order declaration. Handle both cases.
Fixes#1536.
consider packages from git.sr.ht, the user namespace is prefixed with `~`
so running something like `yaegi extract git.sr.ht/~emersion/scfg`
was producing syntax errors with `~` in identifiers. and also `~` in filenames which worked but probably best not to have it there either
thanks!
In the case of a Go short definition (i.e. `a, b := f()`), the new defined variables must be (re-)created in order to preserve the previous value (if in a loop) which can be still in use in the context of a closure. This must not apply to redeclared variables which simply see their value reassigned.
The problem was both occuring in callBin() for runtime functions and assignFromCall() for functions created in the interpreter.
Fixes#1497.
hi!
this issue is sorta blocking for me so i thought i would try to fix it.
im still learning the codebase and understanding how yaegi works, but I thought I would attempt to add a test in the style of other tests as a start.
please let me know if there is anything i need to change / run, or if anyone knows perhaps a good place to start for tackling this issue
Fixes#1496
The logic of patching reflect struct representation has been fixed in presence of function fields and to better handle indirect recursivity (struct recursive through nested struct).
Fixes#1519.
This pull request replaces `os.MkdirTemp` with `t.TempDir`. We can use the `t.TempDir` function from the `testing` package to create temporary directory. The directory created by `t.TempDir` is automatically removed when the test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.TempDir
```go
func TestFoo(t *testing.T) {
// before
tmpDir, err := os.MkdirTemp("", "")
if err != nil {
t.Fatalf("failed to create tmp directory: %v", err)
}
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
// now
tmpDir := t.TempDir()
}
```
This should allow to build the package on AlpineLinux. Also document the constraint of having to install the source under $GOPATH/src/github.com/traefik/yaegi until Go modules are supported.
Fixes#1523.
closes#1514
hi!
I had the same problem as #1514 and I wanted to fix it, I found
When asserting *crypto/rsa.PublicKey, using the typ attribute of node to get an nil rtype, resulting in the assertion result being nok
This code contains the same problem
```go
package main
import (
"log"
"crypto/rsa"
)
func main() {
var pKey interface{} = &rsa.PublicKey{}
if _, ok := pKey.(*rsa.PublicKey); ok {
log.Println("ok")
} else {
log.Println("nok")
}
}
```
So I submitted this Pull Request, hope it will be merged
With certain yaegi configuration on Windows, the loop in `previousRoot` can be infinite, because it fails to recognize driver letters as root.
This change adds a few more safeguards: checks path prefix earlier and checks if `filepath.Dir` produces an empty path.
For methods defined on interfaces (vs concrete methods), the resolution of the method is necessarily delayed at the run time and can not be completed at compile time.
The selectorExpr processing has been changed to correctly identify calls on interface methods which were confused as fields rather than methods (due to the fact that in a interface definition, methods are fields of the interface).
Then at runtime, method lookup has been fixed to correctly recurse in nested valueInterface wrappers and to find embedded interface fields in case of struct or pointer to struct.
Finally, remove receiver processing in `call()`.The receiver is already processed at method resolution and in genFunctionWrapper. Removing redundant processing in call fixes handling of variadic method, simplifies the code and makes it faster.
With those fixes, it is now possible to load and run `go.uber.org/zap` in yaegi. In turn, it makes possible for yaegi to run plugins dependent on zap, such as coraza-waf.
Fixes#1515,
Fixes#1172,
Fixes#1275,
Fixes#1485.
It allows to use interpreter functions as parameters in the runtime, even for defered callbacks, or when passed as empty interfaces, as for runtime.SetFinalizer.
Fixes#1503
When generating a new type, the parameter type was not correctly duplicated in the new AST. This is fixed by making copyNode recursive if needed. The out of order processing of generic types has also been fixed.
Fixes#1488
pk910
Marc Vertes
Rémi BUISSON
alingse
secDre4mer
Randall O'Reilly
Kevin Chen
Kai O'Reilly
Larry Clapp
Ludovic Fernandez
Denys Smirnov
bysir
Hiro
laushunyu
Senan Kelly
a
Eng Zer Jun
sasaba
mpl
ttoad