From 57dad9342a9443e298d43fcc653475408402131e Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 12 Dec 2023 20:43:41 +0100 Subject: [PATCH] proc: make some type casts less counterintuitive * proc: make some type casts less counterintuitive The interaction of type casts with load configuration is sometimes counterintuitive. This commit changes the way it is performed so that when converting slices to strings and vice versa the maximum size corresponding to the target type is used (i.e. MaxStringLen when converting slices to strings and MaxArrayValues when converting slices to strings). This doesn't fully solve the problem (conversions to []rune are problematic and multiple chained type casts will still be confusing) but removes the problem for the majority of practical uses. Fixes #3595, #3539 --- _fixtures/testvariables2.go | 3 +- pkg/proc/eval.go | 53 ++++++++++++++++++++++++++++-------- pkg/proc/variables_test.go | 22 ++++++++------- pkg/terminal/command_test.go | 21 ++++++++++++++ 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/_fixtures/testvariables2.go b/_fixtures/testvariables2.go index 9d5f18f2..311cd8c0 100644 --- a/_fixtures/testvariables2.go +++ b/_fixtures/testvariables2.go @@ -344,6 +344,7 @@ func main() { runearray := [4]rune{116, 232, 115, 116} longstr := "very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789" + longbyteslice := []byte(longstr) m5 := map[C]int{{longstr}: 1} m6 := map[string]int{longstr: 123} m7 := map[C]C{{longstr}: {"hello"}} @@ -403,5 +404,5 @@ func main() { longslice := make([]int, 100, 100) runtime.Breakpoint() - fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, m5, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, rettm, errtypednil, emptyslice, emptymap, byteslice, bytestypeslice, runeslice, bytearray, bytetypearray, runearray, longstr, nilstruct, as2, as2.NonPointerReceiverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice, val, m6, m7, cl, tim1, tim2, typedstringvar, namedA1, namedA2, astructName1(namedA2), badslice, tim3, int3chan) + fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, m5, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, rettm, errtypednil, emptyslice, emptymap, byteslice, bytestypeslice, runeslice, bytearray, bytetypearray, runearray, longstr, nilstruct, as2, as2.NonPointerReceiverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice, val, m6, m7, cl, tim1, tim2, typedstringvar, namedA1, namedA2, astructName1(namedA2), badslice, tim3, int3chan, longbyteslice) } diff --git a/pkg/proc/eval.go b/pkg/proc/eval.go index 851493b0..7e5ca8f7 100644 --- a/pkg/proc/eval.go +++ b/pkg/proc/eval.go @@ -1316,36 +1316,44 @@ func (scope *EvalScope) evalTypeCast(op *evalop.TypeCast, stack *evalStack) { if scope.loadCfg != nil { cfg = *scope.loadCfg } - argv.loadValue(cfg) - if argv.Unreadable != nil { - stack.err = argv.Unreadable - return - } switch ttyp := typ.(type) { case *godwarf.SliceType: switch ttyp.ElemType.Common().ReflectKind { case reflect.Uint8: + // string -> []uint8 if argv.Kind != reflect.String { stack.err = converr return } + cfg.MaxStringLen = cfg.MaxArrayValues + argv.loadValue(cfg) + if argv.Unreadable != nil { + stack.err = argv.Unreadable + return + } for i, ch := range []byte(constant.StringVal(argv.Value)) { e := newVariable("", argv.Addr+uint64(i), typ.(*godwarf.SliceType).ElemType, scope.BinInfo, argv.mem) e.loaded = true e.Value = constant.MakeInt64(int64(ch)) v.Children = append(v.Children, *e) } - v.Len = int64(len(v.Children)) + v.Len = argv.Len v.Cap = v.Len stack.push(v) return case reflect.Int32: + // string -> []rune if argv.Kind != reflect.String { stack.err = converr return } + argv.loadValue(cfg) + if argv.Unreadable != nil { + stack.err = argv.Unreadable + return + } for i, ch := range constant.StringVal(argv.Value) { e := newVariable("", argv.Addr+uint64(i), typ.(*godwarf.SliceType).ElemType, scope.BinInfo, argv.mem) e.loaded = true @@ -1361,12 +1369,18 @@ func (scope *EvalScope) evalTypeCast(op *evalop.TypeCast, stack *evalStack) { case *godwarf.StringType: switch argv.Kind { case reflect.String: - s := constant.StringVal(argv.Value) - v.Value = constant.MakeString(s) - v.Len = int64(len(s)) - stack.push(v) + // string -> string + argv.DwarfType = v.DwarfType + argv.RealType = v.RealType + stack.push(argv) return case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uint, reflect.Uintptr: + // integer -> string + argv.loadValue(cfg) + if argv.Unreadable != nil { + stack.err = argv.Unreadable + return + } b, _ := constant.Int64Val(argv.Value) s := string(rune(b)) v.Value = constant.MakeString(s) @@ -1382,34 +1396,51 @@ func (scope *EvalScope) evalTypeCast(op *evalop.TypeCast, stack *evalStack) { } switch elemType := elem.(type) { case *godwarf.UintType: + // []uint8 -> string if elemType.Name != "uint8" && elemType.Name != "byte" { stack.err = converr return } + cfg.MaxArrayValues = cfg.MaxStringLen + argv.loadValue(cfg) + if argv.Unreadable != nil { + stack.err = argv.Unreadable + return + } bytes := make([]byte, len(argv.Children)) for i := range argv.Children { n, _ := constant.Int64Val(argv.Children[i].Value) bytes[i] = byte(n) } v.Value = constant.MakeString(string(bytes)) + v.Len = argv.Len case *godwarf.IntType: + // []rune -> string if elemType.Name != "int32" && elemType.Name != "rune" { stack.err = converr return } + cfg.MaxArrayValues = cfg.MaxStringLen + argv.loadValue(cfg) + if argv.Unreadable != nil { + stack.err = argv.Unreadable + return + } runes := make([]rune, len(argv.Children)) for i := range argv.Children { n, _ := constant.Int64Val(argv.Children[i].Value) runes[i] = rune(n) } v.Value = constant.MakeString(string(runes)) + // The following line is wrong but the only way to get the correct value + // would be to decode the entire slice. + v.Len = int64(len(constant.StringVal(v.Value))) default: stack.err = converr return } - v.Len = int64(len(constant.StringVal(v.Value))) stack.push(v) return } diff --git a/pkg/proc/variables_test.go b/pkg/proc/variables_test.go index b90abaf1..58b5cc4f 100644 --- a/pkg/proc/variables_test.go +++ b/pkg/proc/variables_test.go @@ -770,21 +770,23 @@ func getEvalExpressionTestCases() []varTest { {"ptrinf2", false, `**(main.pptr)(…`, `(main.pptr)(…`, "main.pptr", nil}, - // conversions between string/[]byte/[]rune (issue #548) + // conversions between string/[]byte/[]rune (issue #548, #3595, #3539) {"runeslice", true, `[]int32 len: 4, cap: 4, [116,232,115,116]`, `[]int32 len: 4, cap: 4, [...]`, "[]int32", nil}, {"byteslice", true, `[]uint8 len: 5, cap: 5, [116,195,168,115,116]`, `[]uint8 len: 5, cap: 5, [...]`, "[]uint8", nil}, - {"[]byte(str1)", false, `[]uint8 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, `[]uint8 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, "[]uint8", nil}, - {"[]uint8(str1)", false, `[]uint8 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, `[]uint8 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, "[]uint8", nil}, + {"[]byte(str1)", false, `[]uint8 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, `[]uint8 len: 11, cap: 11, nil`, "[]uint8", nil}, + {"[]uint8(str1)", false, `[]uint8 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, `[]uint8 len: 11, cap: 11, nil`, "[]uint8", nil}, {"[]rune(str1)", false, `[]int32 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, `[]int32 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, "[]int32", nil}, {"[]int32(str1)", false, `[]int32 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, `[]int32 len: 11, cap: 11, [48,49,50,51,52,53,54,55,56,57,48]`, "[]int32", nil}, - {"string(byteslice)", false, `"tèst"`, `""`, "string", nil}, - {"[]int32(string(byteslice))", false, `[]int32 len: 4, cap: 4, [116,232,115,116]`, `[]int32 len: 0, cap: 0, nil`, "[]int32", nil}, - {"string(runeslice)", false, `"tèst"`, `""`, "string", nil}, - {"[]byte(string(runeslice))", false, `[]uint8 len: 5, cap: 5, [116,195,168,115,116]`, `[]uint8 len: 0, cap: 0, nil`, "[]uint8", nil}, + {"string(byteslice)", false, `"tèst"`, `"tèst"`, "string", nil}, + {"[]int32(string(byteslice))", false, `[]int32 len: 4, cap: 4, [116,232,115,116]`, `[]int32 len: 4, cap: 4, [116,232,115,116]`, "[]int32", nil}, + {"string(runeslice)", false, `"tèst"`, `"tèst"`, "string", nil}, + {"[]byte(string(runeslice))", false, `[]uint8 len: 5, cap: 5, [116,195,168,115,116]`, `[]uint8 len: 5, cap: 5, [116,195,168,115,116]`, "[]uint8", nil}, {"*(*[5]byte)(uintptr(&byteslice[0]))", false, `[5]uint8 [116,195,168,115,116]`, `[5]uint8 [...]`, "[5]uint8", nil}, - {"string(bytearray)", false, `"tèst"`, `""`, "string", nil}, - {"string(runearray)", false, `"tèst"`, `""`, "string", nil}, + {"string(bytearray)", false, `"tèst"`, `"tèst"`, "string", nil}, + {"string(runearray)", false, `"tèst"`, `"tèst"`, "string", nil}, {"string(str1)", false, `"01234567890"`, `"01234567890"`, "string", nil}, + {"len([]byte(longstr))", false, `137`, `137`, "", nil}, + {"len(string(longbyteslice))", false, `137`, `137`, "", nil}, // access to channel field members {"ch1.qcount", false, "4", "4", "uint", nil}, @@ -831,7 +833,7 @@ func getEvalExpressionTestCases() []varTest { {`m6["very long string 0123456789a0123456789b0123456789c0123456789d0123456789e0123456789f0123456789g012345678h90123456789i0123456789j0123456789"]`, false, `123`, `123`, "int", nil}, // issue #1423 - special typecasts everywhere - {`string(byteslice) == "tèst"`, false, `true`, `false`, "", nil}, + {`string(byteslice) == "tèst"`, false, `true`, `true`, "", nil}, // issue #3138 - typecast to *interface{} breaking {`*(*interface {})(uintptr(&iface1))`, false, `interface {}(*main.astruct) *{A: 1, B: 2}`, `interface {}(*main.astruct)…`, "interface {}", nil}, diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index 14505cde..d97d0bcb 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -1514,3 +1514,24 @@ func TestSubstitutePathAndList(t *testing.T) { } }) } + +func TestDisplay(t *testing.T) { + // Tests that display command works. See issue #3595. + type testCase struct{ in, tgt string } + withTestTerminal("testvariables2", t, func(term *FakeTerminal) { + term.MustExec("continue") + + for _, tc := range []testCase{ + {"string(byteslice)", `0: string(byteslice) = "tèst"`}, + {"string(byteslice[1:])", `0: string(byteslice[1:]) = "èst"`}, + {"%s string(byteslice)", `0: string(byteslice) = tèst`}, + } { + out := term.MustExec("display -a " + tc.in) + t.Logf("%q -> %q", tc.in, out) + if !strings.Contains(out, tc.tgt) { + t.Errorf("wrong output for 'display -a %s':\n\tgot: %q\n\texpected: %q", tc.in, out, tc.tgt) + } + term.MustExec("display -d 0") + } + }) +}