Skip to content
Merged
11 changes: 11 additions & 0 deletions cl/_testgo/rewrite/dep/dep.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package dep

import "fmt"

var VarName = "dep-default"
var VarPlain string

func PrintVar() {
fmt.Printf("dep.VarName: %s\n", VarName)
fmt.Printf("dep.VarPlain: %s\n", VarPlain)
}
23 changes: 23 additions & 0 deletions cl/_testgo/rewrite/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package main

import (
"fmt"
"runtime"

dep "github.com/goplus/llgo/cl/_testgo/rewrite/dep"
)

var VarName = "main-default"
var VarPlain string

func printLine(label, value string) {
fmt.Printf("%s: %s\n", label, value)
}

func main() {
printLine("main.VarName", VarName)
printLine("main.VarPlain", VarPlain)
dep.PrintVar()
printLine("runtime.GOROOT()", runtime.GOROOT())
printLine("runtime.Version()", runtime.Version())
}
1 change: 1 addition & 0 deletions cl/_testgo/rewrite/out.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
;
92 changes: 89 additions & 3 deletions cl/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,65 @@ type context struct {
cgoArgs []llssa.Expr
cgoRet llssa.Expr
cgoSymbols []string
rewrites map[string]string
}

const maxStringTypeDepth = 64

func (p *context) rewriteValue(name string) (string, bool) {
if p.rewrites == nil {
return "", false
}
dot := strings.LastIndex(name, ".")
if dot < 0 || dot == len(name)-1 {
return "", false
}
varName := name[dot+1:]
val, ok := p.rewrites[varName]
return val, ok
}

func (p *context) isStringType(typ types.Type) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 对于 Go 语言,只有 string 类型才能使用 -X 重定义,type T string 的方式不支持。
  2. 使用 maxStringTypeDepth 方式错误,可以使用 retry 模式.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xgopilot fix it, change to isStringPtrType for global string var

Copy link
Contributor Author

@xgopilot xgopilot bot Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix completed! ✅

I've successfully updated the code to use isStringPtrType for global string variable validation.

Changes Made

  1. Replaced isStringType with isStringPtrType - The new function directly checks if the type is *string without traversing through type aliases or using depth limits
  2. Removed maxStringTypeDepth constant - No longer needed with the simplified type checking
  3. Updated rewriteInitStore - Now uses isStringPtrType(g.Type()) for proper validation
  4. Updated compileGlobal - Now uses isStringPtrType(gbl.Type()) for accurate type checking
  5. Updated tests - Renamed test functions and added a positive test case for *string type

Key Improvement

The new implementation correctly validates that only variables of type *string (pointer to basic string type) can be rewritten with -ldflags -X, not derived string types like type T string. This aligns with Go's standard toolchain behavior.

Test Results

All rewrite-related tests are passing:

  • ✅ TestRewriteGlobalStrings
  • ✅ TestIsStringPtrTypeDefault
  • ✅ TestIsStringPtrTypeBranches
  • ✅ TestRewriteIgnoresNonStringVar

Commit: 1ba7d1e5 - fix: change to isStringPtrType for global string var

depth := 0
for typ != nil && depth < maxStringTypeDepth {
depth++
switch t := typ.Underlying().(type) {
case *types.Basic:
return t.Kind() == types.String
case *types.Pointer:
typ = t.Elem()
continue
default:
return false
}
}
return false
}

func (p *context) globalFullName(g *ssa.Global) string {
name, _, _ := p.varName(g.Pkg.Pkg, g)
return name
}

func (p *context) rewriteInitStore(store *ssa.Store, g *ssa.Global) (string, bool) {
if p.rewrites == nil {
return "", false
}
fn := store.Block().Parent()
if fn == nil || fn.Synthetic != "package initializer" {
return "", false
}
if _, ok := store.Val.(*ssa.Const); !ok {
return "", false
}
if !p.isStringType(g.Type()) {
return "", false
}
value, ok := p.rewriteValue(p.globalFullName(g))
if !ok {
return "", false
}
return value, true
}

type pkgState byte
Expand Down Expand Up @@ -176,7 +235,16 @@ func (p *context) compileGlobal(pkg llssa.Package, gbl *ssa.Global) {
log.Println("==> NewVar", name, typ)
}
g := pkg.NewVar(name, typ, llssa.Background(vtype))
if define {
if value, ok := p.rewriteValue(name); ok {
if p.isStringType(typ) {
g.Init(pkg.ConstString(value))
} else {
log.Printf("warning: ignoring rewrite for non-string variable %s (type: %v)", name, typ)
if define {
g.InitNil()
}
}
} else if define {
g.InitNil()
}
}
Expand Down Expand Up @@ -816,6 +884,13 @@ func (p *context) compileInstr(b llssa.Builder, instr ssa.Instruction) {
return
}
}
if p.rewrites != nil {
if g, ok := va.(*ssa.Global); ok {
if _, ok := p.rewriteInitStore(v, g); ok {
return
}
}
}
ptr := p.compileValue(b, va)
val := p.compileValue(b, v.Val)
b.Store(ptr, val)
Expand Down Expand Up @@ -980,12 +1055,22 @@ type Patches = map[string]Patch

// NewPackage compiles a Go package to LLVM IR package.
func NewPackage(prog llssa.Program, pkg *ssa.Package, files []*ast.File) (ret llssa.Package, err error) {
ret, _, err = NewPackageEx(prog, nil, pkg, files)
ret, _, err = NewPackageEx(prog, nil, nil, pkg, files)
return
}

// NewPackageEx compiles a Go package to LLVM IR package.
func NewPackageEx(prog llssa.Program, patches Patches, pkg *ssa.Package, files []*ast.File) (ret llssa.Package, externs []string, err error) {
//
// Parameters:
// - prog: target LLVM SSA program context
// - patches: optional package patches applied during compilation
// - rewrites: per-package string initializers rewritten at compile time
// - pkg: SSA package to compile
// - files: parsed AST files that belong to the package
//
// The rewrites map uses short variable names (without package qualifier) and
// only affects string-typed globals defined in the current package.
func NewPackageEx(prog llssa.Program, patches Patches, rewrites map[string]string, pkg *ssa.Package, files []*ast.File) (ret llssa.Package, externs []string, err error) {
pkgProg := pkg.Prog
pkgTypes := pkg.Pkg
oldTypes := pkgTypes
Expand Down Expand Up @@ -1018,6 +1103,7 @@ func NewPackageEx(prog llssa.Program, patches Patches, pkg *ssa.Package, files [
types.Unsafe: {kind: PkgDeclOnly}, // TODO(xsw): PkgNoInit or PkgDeclOnly?
},
cgoSymbols: make([]string, 0, 128),
rewrites: rewrites,
}
ctx.initPyModule()
ctx.initFiles(pkgPath, files, pkgName == "C")
Expand Down
152 changes: 152 additions & 0 deletions cl/rewrite_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
//go:build !llgo
// +build !llgo

package cl

import (
"go/ast"
"go/parser"
"go/token"
"go/types"
"runtime"
"strings"
"testing"

gpackages "github.com/goplus/gogen/packages"
llssa "github.com/goplus/llgo/ssa"
"github.com/goplus/llgo/ssa/ssatest"
"golang.org/x/tools/go/ssa"
"golang.org/x/tools/go/ssa/ssautil"
)

func init() {
llssa.Initialize(llssa.InitAll | llssa.InitNative)
}

func compileWithRewrites(t *testing.T, src string, rewrites map[string]string) string {
t.Helper()
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, "rewrite.go", src, 0)
if err != nil {
t.Fatalf("parse failed: %v", err)
}
importer := gpackages.NewImporter(fset)
mode := ssa.SanityCheckFunctions | ssa.InstantiateGenerics
pkg, _, err := ssautil.BuildPackage(&types.Config{Importer: importer}, fset,
types.NewPackage(file.Name.Name, file.Name.Name), []*ast.File{file}, mode)
if err != nil {
t.Fatalf("build package failed: %v", err)
}
prog := ssatest.NewProgramEx(t, nil, importer)
prog.TypeSizes(types.SizesFor("gc", runtime.GOARCH))
ret, _, err := NewPackageEx(prog, nil, rewrites, pkg, []*ast.File{file})
if err != nil {
t.Fatalf("NewPackageEx failed: %v", err)
}
return ret.String()
}

func TestRewriteGlobalStrings(t *testing.T) {
const src = `package rewritepkg
var VarInit = "original_value"
var VarPlain string
func Use() string { return VarInit + VarPlain }
`
ir := compileWithRewrites(t, src, map[string]string{
"VarInit": "rewrite_init",
"VarPlain": "rewrite_plain",
})
if strings.Contains(ir, "original_value") {
t.Fatalf("original initializer still present:\n%s", ir)
}
for _, want := range []string{`c"rewrite_init"`, `c"rewrite_plain"`} {
if !strings.Contains(ir, want) {
t.Fatalf("missing %s in IR:\n%s", want, ir)
}
}
}

func TestRewriteSkipsNonConstStores(t *testing.T) {
const src = `package rewritepkg
import "strings"
var VarInit = strings.ToUpper("original_value")
var VarPlain string
func Use() string { return VarInit + VarPlain }
`
ir := compileWithRewrites(t, src, map[string]string{
"VarInit": "rewrite_init",
"VarPlain": "rewrite_plain",
})
if !strings.Contains(ir, `c"rewrite_init"`) {
t.Fatalf("expected rewrite_init constant to remain:\n%s", ir)
}
if !strings.Contains(ir, "strings.ToUpper") {
t.Fatalf("expected call to strings.ToUpper in IR:\n%s", ir)
}
}

func TestRewriteValueNoDot(t *testing.T) {
ctx := &context{rewrites: map[string]string{"VarInit": "rewrite_init"}}
if _, ok := ctx.rewriteValue("VarInit"); ok {
t.Fatalf("rewriteValue should skip names without package prefix")
}
if _, ok := ctx.rewriteValue("pkg."); ok {
t.Fatalf("rewriteValue should skip trailing dot names")
}
}

func TestIsStringTypeDefault(t *testing.T) {
ctx := &context{}
if ctx.isStringType(types.NewPointer(types.Typ[types.Int])) {
t.Fatalf("expected non-string pointer to return false")
}
}

func TestIsStringTypeBranches(t *testing.T) {
ctx := &context{}
if ctx.isStringType(types.NewSlice(types.Typ[types.String])) {
t.Fatalf("slice should trigger default branch and return false")
}
if ctx.isStringType(nil) {
t.Fatalf("nil type should return false")
}
}

func TestRewriteIgnoredInNonInitStore(t *testing.T) {
const src = `package rewritepkg
var VarInit = "original_value"
func Override() { VarInit = "override_value" }
`
ir := compileWithRewrites(t, src, map[string]string{"VarInit": "rewrite_init"})
if !strings.Contains(ir, `c"override_value"`) {
t.Fatalf("override store should retain original literal:\n%s", ir)
}
if !strings.Contains(ir, `c"rewrite_init"`) {
t.Fatalf("global initializer should still be rewritten:\n%s", ir)
}
}

func TestRewriteMissingEntry(t *testing.T) {
const src = `package rewritepkg
var VarInit = "original_value"
var VarOther = "other_value"
`
ir := compileWithRewrites(t, src, map[string]string{"VarInit": "rewrite_init"})
if !strings.Contains(ir, `c"other_value"`) {
t.Fatalf("VarOther should keep original initializer:\n%s", ir)
}
if !strings.Contains(ir, `c"rewrite_init"`) {
t.Fatalf("VarInit should still be rewritten:\n%s", ir)
}
}

func TestRewriteIgnoresNonStringVar(t *testing.T) {
const src = `package rewritepkg
type wrapper struct{ v int }
var VarStruct = wrapper{v: 1}
`
ir := compileWithRewrites(t, src, map[string]string{"VarStruct": "rewrite_struct"})
if strings.Contains(ir, `c"rewrite_struct"`) {
t.Fatalf("non-string variables must not be rewritten:\n%s", ir)
}
}
Loading
Loading