From e256e640604bff7916ef09451da7f4a6423152a6 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 7 Mar 2025 14:16:28 -0500 Subject: [PATCH] debug/dwarf: fix problem with DWARF 5 and Seek method When clients use debug/dwarf to examine DWARF 5 binaries, we can run into problems when the Seek() method is used to skip ahead from a DIE in one compilation unit to a DIE in another unit. The problem here is that it is common for DWARF 5 comp units to have attributes (ex: DW_AT_addr_base) whose value must be applied as an offset when reading certain forms (ex: DW_FORM_addrx) within that unit. The existing implementation didn't have a good way to recover these attrs following the Seek call, and had to essentially punt in this case, resulting in incorrect attr values. This patch adds new support for reading and caching the key comp unit DIE attributes (DW_AT_addr_base, DW_AT_loclists_base, etc) prior to visiting any of the DIE entries in a unit, storing the cache values of these attrs the main table of units. This base attribute reading/caching behavior also happens (where needed) after Seek calls. Should resolve delve issue 3861. Supercedes Go pull request 70400. Updates #26379. Fixes #57046. Change-Id: I536a57e2ba4fc55132d91c7f36f67a91ac408dc3 Reviewed-on: https://go-review.googlesource.com/c/go/+/655976 Reviewed-by: Alessandro Arzilli Reviewed-by: Cherry Mui Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI --- src/debug/dwarf/entry.go | 88 ++++-------------- src/debug/dwarf/entry_test.go | 60 ++++++++++++ .../dwarf/testdata/issue57046-clang.elf5 | Bin 0 -> 19360 bytes src/debug/dwarf/testdata/issue57046_part1.c | 42 +++++++++ src/debug/dwarf/testdata/issue57046_part2.c | 10 ++ src/debug/dwarf/unit.go | 65 ++++++++++++- 6 files changed, 192 insertions(+), 73 deletions(-) create mode 100755 src/debug/dwarf/testdata/issue57046-clang.elf5 create mode 100644 src/debug/dwarf/testdata/issue57046_part1.c create mode 100644 src/debug/dwarf/testdata/issue57046_part2.c diff --git a/src/debug/dwarf/entry.go b/src/debug/dwarf/entry.go index 3e54a1a13a..ed0f213a3c 100644 --- a/src/debug/dwarf/entry.go +++ b/src/debug/dwarf/entry.go @@ -426,16 +426,6 @@ func (b *buf) entry(cu *Entry, u *unit) *Entry { Field: make([]Field, len(a.field)), } - // If we are currently parsing the compilation unit, - // we can't evaluate Addrx or Strx until we've seen the - // relevant base entry. - type delayed struct { - idx int - off uint64 - fmt format - } - var delay []delayed - resolveStrx := func(strBase, off uint64) string { off += strBase if uint64(int(off)) != off { @@ -532,18 +522,7 @@ func (b *buf) entry(cu *Entry, u *unit) *Entry { return nil } - // We have to adjust by the offset of the - // compilation unit. This won't work if the - // program uses Reader.Seek to skip over the - // unit. Not much we can do about that. - var addrBase int64 - if cu != nil { - addrBase, _ = cu.Val(AttrAddrBase).(int64) - } else if a.tag == TagCompileUnit { - delay = append(delay, delayed{i, off, formAddrx}) - break - } - + addrBase := int64(u.addrBase()) var err error val, err = b.dwarf.debugAddr(b.format, uint64(addrBase), off) if err != nil { @@ -683,18 +662,7 @@ func (b *buf) entry(cu *Entry, u *unit) *Entry { off *= 4 } - // We have to adjust by the offset of the - // compilation unit. This won't work if the - // program uses Reader.Seek to skip over the - // unit. Not much we can do about that. - var strBase int64 - if cu != nil { - strBase, _ = cu.Val(AttrStrOffsetsBase).(int64) - } else if a.tag == TagCompileUnit { - delay = append(delay, delayed{i, off, formStrx}) - break - } - + strBase := int64(u.strOffsetsBase()) val = resolveStrx(uint64(strBase), off) case formStrpSup: @@ -743,18 +711,7 @@ func (b *buf) entry(cu *Entry, u *unit) *Entry { case formRnglistx: off := b.uint() - // We have to adjust by the rnglists_base of - // the compilation unit. This won't work if - // the program uses Reader.Seek to skip over - // the unit. Not much we can do about that. - var rnglistsBase int64 - if cu != nil { - rnglistsBase, _ = cu.Val(AttrRnglistsBase).(int64) - } else if a.tag == TagCompileUnit { - delay = append(delay, delayed{i, off, formRnglistx}) - break - } - + rnglistsBase := int64(u.rngListsBase()) val = resolveRnglistx(uint64(rnglistsBase), off) } @@ -763,32 +720,6 @@ func (b *buf) entry(cu *Entry, u *unit) *Entry { if b.err != nil { return nil } - - for _, del := range delay { - switch del.fmt { - case formAddrx: - addrBase, _ := e.Val(AttrAddrBase).(int64) - val, err := b.dwarf.debugAddr(b.format, uint64(addrBase), del.off) - if err != nil { - b.err = err - return nil - } - e.Field[del.idx].Val = val - case formStrx: - strBase, _ := e.Val(AttrStrOffsetsBase).(int64) - e.Field[del.idx].Val = resolveStrx(uint64(strBase), del.off) - if b.err != nil { - return nil - } - case formRnglistx: - rnglistsBase, _ := e.Val(AttrRnglistsBase).(int64) - e.Field[del.idx].Val = resolveRnglistx(uint64(rnglistsBase), del.off) - if b.err != nil { - return nil - } - } - } - return e } @@ -840,6 +771,7 @@ func (r *Reader) Seek(off Offset) { u := &d.unit[0] r.unit = 0 r.b = makeBuf(r.d, u, "info", u.off, u.data) + r.collectDwarf5BaseOffsets(u) r.cu = nil return } @@ -855,6 +787,7 @@ func (r *Reader) Seek(off Offset) { u := &d.unit[i] r.unit = i r.b = makeBuf(r.d, u, "info", off, u.data[off-u.off:]) + r.collectDwarf5BaseOffsets(u) } // maybeNextUnit advances to the next unit if this one is finished. @@ -870,6 +803,17 @@ func (r *Reader) nextUnit() { u := &r.d.unit[r.unit] r.b = makeBuf(r.d, u, "info", u.off, u.data) r.cu = nil + r.collectDwarf5BaseOffsets(u) +} + +func (r *Reader) collectDwarf5BaseOffsets(u *unit) { + if u.vers < 5 || u.unit5 != nil { + return + } + u.unit5 = new(unit5) + if err := r.d.collectDwarf5BaseOffsets(u); err != nil { + r.err = err + } } // Next reads the next entry from the encoded entry stream. diff --git a/src/debug/dwarf/entry_test.go b/src/debug/dwarf/entry_test.go index 1ce1c98f60..ee0c80a503 100644 --- a/src/debug/dwarf/entry_test.go +++ b/src/debug/dwarf/entry_test.go @@ -6,6 +6,7 @@ package dwarf_test import ( . "debug/dwarf" + "debug/elf" "encoding/binary" "path/filepath" "reflect" @@ -457,3 +458,62 @@ func TestIssue52045(t *testing.T) { t.Errorf("got non-nil entry0, wanted nil") } } + +func TestIssue57046(t *testing.T) { + f, err := elf.Open("testdata/issue57046-clang.elf5") + if err != nil { + t.Fatalf("elf.Open returns err: %v", err) + } + d, err := f.DWARF() + if err != nil { + t.Fatalf("f.DWARF returns err: %v", err) + } + // Write down all the subprogram DIEs. + spdies := []Offset{} + lopcs := []uint64{} + r := d.Reader() + for { + e, err := r.Next() + if err != nil { + t.Fatalf("r.Next() returns err: %v", err) + } + if e == nil { + break + } + if e.Tag != TagSubprogram { + continue + } + var name string + var lopc uint64 + if n, ok := e.Val(AttrName).(string); ok { + name = n + } + if lo, ok := e.Val(AttrLowpc).(uint64); ok { + lopc = lo + } + if name == "" || lopc == 0 { + continue + } + spdies = append(spdies, e.Offset) + lopcs = append(lopcs, lopc) + } + + // Seek to the second entry in spdies (corresponding to mom() in + // issue57046_part2.c) and take a look at it. + r2 := d.Reader() + r2.Seek(spdies[1]) + e, err := r2.Next() + if err != nil { + t.Fatalf("r2.Next() returns err: %v", err) + } + if e == nil { + t.Fatalf("r2.Next() returned nil") + } + + // Verify that the lopc we see matches what we saw before. + got := e.Val(AttrLowpc).(uint64) + if got != lopcs[1] { + t.Errorf("bad lopc for fn2 following seek: want %x got %x\n", + lopcs[1], got) + } +} diff --git a/src/debug/dwarf/testdata/issue57046-clang.elf5 b/src/debug/dwarf/testdata/issue57046-clang.elf5 new file mode 100755 index 0000000000000000000000000000000000000000..009af83135a100b61f38496b973b84171c829757 GIT binary patch literal 19360 zcmeHPdvH|Oc|Z5=Lo2Pct0$7cF>5UGNXzPh5MT?lD%$8P@}=vkT}kSmNASojngz!rqmtRZi%QJ$L=JJoM~#Nv1@4jkn7kXsZ-m@nCjT9&CO#bN}%{kvWzw`ah`ObIG-h1xj=z;dmb*iEW4i<5npwe)$f%s&_S-;Fc z_{2(4gmkf3B4vAYJiC0CL8RQKHELgIIB#Ad$I7S8`uoQ9} z#IsR_ESu$3X(vmr&-CXZGJ;1>Gx8WIAh68lktq{P^=dJ(db;e9{q2^Lc~m6c70J6I zc^N7DyCNm~lYByPrSzv+dQeBfqo+ccc|DS6mTgj>CG|mJ%Uh|?|E9k#$*c0{%?Pt> zF<`-x`}+cT^3Y0+-!AwtZAscCOPia# z$+)*s@0jeOK4?y@-?UYz2gNvH%%^-gBDt07&%9c>;YBTQHFW&#XYctz^y0rnFFe6* z5QlUqkUmcdDpQ`073mn`KZM-W@p~ow&qF#eOR@xb=?rz$=1h1GaL)|-)c;KS^|Rn9 z;2uPCI1IpJ22q}ixL^icBvXmOa7<_#4oIJtOa&7uZ73Keq;FqP>yO5QgV6(F5ecWl zvAtqA5sjt#1u4_PY8LA|J6hLjjoxKVnQWtXxzIYgH)^49A{>b(Q{hDS#_@JnoDZZw}*opP*vUf|zj zSBX(^S^n-BFP6B+XVf#!^PIDAHqaGK{FihV|oaQ%GD6*-ynK+Xd>59B>9iQ2o1C(g?NxNM&03y*r_o zpQSP_LEjzM%Lh>IKS=}%4exi1y|ODX_8)=6Z(i%_Zf_WGcsX!l^#~eBSJXlE!+x)O z{t;c5`a|6zpn(%MYA3Mj<5UIAbx|JX=BKZ?=N}~H=cR=H7j*sQ8%Qj<9vHh8xb(rA zz@?8Z0p-QOE7w!yXyA3P(SRd;rQbcjZHoV2?i+t~aCH&5V&spu1`e-&7Mdb3_ExGe zaD4R#C`>NFuqH!DUbMZ1va%C>XZk1pyQ8R-_H6ZU^WW*;>hIpt1=GIh0{`!>r_&u{ zF9ycGJ9!b=z?ikh@4o15cVCPtN8e9vLSdKMd1Cc2l-nELeD8+-{wbeyq+jS9`)Kkg zh@Gph?7eC1^5IXz?zY$6ZIf#cwk+%$`S;GTblce54davdKtl$D!Tc@V?pJ@4%qRAAL&pPVT+AZR{EhZgaQ&(y+L$Z{)Skv3J_WK0x2!hSq!E z4;)7iC(3)2|AYx)G#WUbswqbGSyVTSV;EK0!^oO0IF6VoEucKF&+BP38RN=@oCk6q z$ax^=ft&|&9>{qh=YgCDavsQe;Qz4)=zUd}zdIlTQBQ5sQyVG}L-8Tp)y}irO7CFl z-H_v2I(-Z>0C^s=2a?_s27aDSzXUl6S&SR_XMO=(z67FopA?h>n}xEkSeaLt=Qye4 z6%$VH^gsStI=z4tWSCiBh#J(B{yymYNY_=o&Q-O+?HqL+6l><*`uSxwdNahw?VmUa zz3lf>gzZ4w2b)hr3cstk)m7E%^0c}Zx4P>6u9be5?{MD3_7k>8tdCm$L{*OvRIgV< z397|@mxJ2*8sY@nxioV-ZLY3q?L0M8JM>=gr-(i9S+`9kn=#!133@mJdpE1 z&I36QkAB6oRQKgWtEf_QnKK07$@cT zi2SW%Kq~NkB!9bDDRH0t$dU1b(lEArTLD7r5d@j018 zAF3q?9}-Sel%iUm=TOV?td`X%h;sFuuy!D0+eiuY?TX#%$g|u{wI21HGF598mU{?Y zte#V+&;p^_Y}PyrJqA#=E!2DSQmQmls@{e)e;s8ln~)X=hjkmIa9%-TbzVnmE2Pw3 zq-<61hmj?y&MQ*7)q4d&51`IvR~}J+hK&0|B%+usF8%~b3FTaQ>Q|H^u#8&Ok>_yb zTSCfxsB_;0yi}ACue?B%5mw<8R;YVtw;VbWbZ-;t;L!lC;jf65mHx-S%mQ}=~%svs8V zE^+EEDb!t3B*(A2#6>Q-Pj|_^(o1eY)>Flg;rPxi6(uyLV)ByMhDh^a8BEdV!!Q7v z393FvPNJkPMI!QX{4J}Hv96{>*yQog)Af{XHsR<(g@Zn$&nG>hlqj{eWj0G~qO!Ph zUTw*|QhTy8Rr#=jbAnnUEVZ{eojBcCnnXw0EzVn=Dt_};Md>JmMK=Lkfl52+Ii0s5 z3;$3PzN!P&4N!4333UNw@|y&S0;jU|W=xiVc%bY)oHYvrijgVm5Ng%^XxrsgEVby! zwy3(JtfhJlPPJ|u`hhFlx+qzwi&8rGl&#S7<&xvI7ECEue@!6CM035FrVMjtb{V{t?IMfeA}hPdHs#7`-V*FB zwt!4>7$eO$Mk;o~d>lA&fDV_U{MuDS0w8@$j?8>aHvG5|Ey%lVH(K?%P`Gt%(^rqY zvGeu$WeYDHwU+sypxnOxwL{;ioBz&%ia##+%bO!r^m|iWSLZsS3)emq?EWOOed42E zZ6@&PZ{#1{s;&R_GmoV2denV)0+*(Ib+`P$-cqXA$_u__bG~J_?0L{uVz{(%xdLc+h5Pu|&MKLc3+I*Om{+ZQldgW6QJG*)6+YqMDk1 zw6eiwae8gS?$~InDtOgy=}Fnj?UwSM?0$NFr+!*wKl^{9pA`CO!SABcQD=90RX03% zC&o(-rY#9GI+%RT&QO!NzM1Nf>oJ@s+si5c^2qv%# z!@qg6f4jD&(D$Qi; z$^m7IbzqOuW!-Z~QML$c-7-rF^xAeSJFLBcb_nZP2^wmu<|ZbrI+ zysW&TDw?qVt8zhgk2tGO==qF#0VQF5L-~oK zcmAPrQdM?ZzeM;>fh&OS3OaAmTBewW*s#pnc1RJ{tKe8aM5iL8t06&Hg4lvcZz+O^ zBZ&JE4ZXS42hTenbb6+>1!<^ZgLx$BjJG~Q=t!Dk!q z_x1M6#F3l0e7cob*%_B)#oNHiPP6_mfP0vnCF#D2`^tjFBH*Pe&b+4Ik=W8l*V;HP zqru*&mI_7$fMjX}-xBwUsf~|XYDnv&t&d4)hT>XeFy0#+)IzCvBB=#O_KCju(C{Gk zR))OwGi%Tm%%~PjB!c_3a4ePBFZvU~p|BPj85-ITN>)jOW@?(M>1slk9uq`YTTNTH z*}t(}Yv0s{MS#|}eUpD<$6CPHHLJA;WNn~rv(VOe-qGss)b3cfZcBT&*6nZYY}e$H z!N*%c_5Gjb{iK;%Y!v;in?u9;5=9@i+?<-BQp3gu)M*eeN8n(rZj61UFn;;$+$luzrlqQ_6 zJ`hX}2ybYA3|$y9l`v}LV}fY~4Rwj|V2}thH$0dUUYa<x;iMC7+iWrt5w- zpVu8MJ*+6DnZFJAf8!Y(+t1G>S#tmK@!)UR9|Rw_%9-)=b4!+&Wxr%2`Hsg=_wW>= zh&1F(|I)y-Ql25DtYG&4J*cB+jLhffiY)mW%l5M#OFX6$^vsgW{M?ddz3kg;Kbv#{ z88Vgm2jy~-<+xPj{>|h6isZLSJ$??$GLXe5pPBjRfRRtQ{mgYvUhgU;pM`WQ7xk5M zHs2$8EPWYGBg2)&EG96ShBs`43}A+H}QGhz*65S#bikCmmyl!XZO$RES9t$ z&kpAC|2-;+&&N-Aq_&iw<-Zr8p3Se4JeDo8ITq&re`wGwl{ zd3+q{ewS33&;O@kypFW-;MEYBl=&<_MNKxJ*Zq@{U!P$b8RoIHgPhIhb##>^vYp%v z^SE3HGGS~#uU8jKzFD98$`161oG`PU%nxKMBhP%6b0y!TCWd8$domN-&|n_x(<5#( zMs+`KG&G(xRho*TX%_zdO$Prt6TwszKFK$ae-&LA5$JtFc3?is+a*6+hw>{{7|2;e zWK!laKu_%nGxz_h literal 0 HcmV?d00001 diff --git a/src/debug/dwarf/testdata/issue57046_part1.c b/src/debug/dwarf/testdata/issue57046_part1.c new file mode 100644 index 0000000000..8866ca66e1 --- /dev/null +++ b/src/debug/dwarf/testdata/issue57046_part1.c @@ -0,0 +1,42 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Part 1 of the sources for issue 57046 test case. + +// Build instructions: +// +// clang-16 -O -g -gdwarf-5 -c issue57046_part1.c +// clang-16 -O -g -gdwarf-5 -c issue57046_part2.c +// clang-16 -o issue57046-clang.elf5 issue57046_part1.o issue57046_part2.o + +#include +#include +#include + +extern const char *mom(); + +int gadgety() { + const char *ev = getenv("PATH"); + int n = strlen(ev); + int s1 = (int)ev[0]; + int s2 = (int)ev[1]; + int s3 = (int)ev[2]; + for (int i = 0; i < strlen(ev); i++) { + if (s1 == 101) { + int t = s1; + s1 = s3; + s3 = t; + } + if (ev[i] == 99) { + printf("%d\n", i); + } + } + s2 *= 2; + return n + s1 + s2; +} + +int main(int argc, char **argv) { + printf("Hi %s %d\n", mom(), gadgety()); + return 0; +} diff --git a/src/debug/dwarf/testdata/issue57046_part2.c b/src/debug/dwarf/testdata/issue57046_part2.c new file mode 100644 index 0000000000..2f4e9f0d24 --- /dev/null +++ b/src/debug/dwarf/testdata/issue57046_part2.c @@ -0,0 +1,10 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Part 2 of the sources for issue 57046 test case. +// See part 1 for build instructions. + +const char *mom() { + return "mom"; +} diff --git a/src/debug/dwarf/unit.go b/src/debug/dwarf/unit.go index 8b810d0a00..7a384f32c3 100644 --- a/src/debug/dwarf/unit.go +++ b/src/debug/dwarf/unit.go @@ -17,10 +17,18 @@ type unit struct { off Offset // byte offset of data within the aggregate info data []byte atable abbrevTable + *unit5 // info specific to DWARF 5 units asize int vers int - utype uint8 // DWARF 5 unit type is64 bool // True for 64-bit DWARF format + utype uint8 // DWARF 5 unit type +} + +type unit5 struct { + addrBase uint64 + strOffsetsBase uint64 + rngListsBase uint64 + locListsBase uint64 } // Implement the dataFormat interface. @@ -37,6 +45,34 @@ func (u *unit) addrsize() int { return u.asize } +func (u *unit) addrBase() uint64 { + if u.unit5 != nil { + return u.unit5.addrBase + } + return 0 +} + +func (u *unit) strOffsetsBase() uint64 { + if u.unit5 != nil { + return u.unit5.strOffsetsBase + } + return 0 +} + +func (u *unit) rngListsBase() uint64 { + if u.unit5 != nil { + return u.unit5.rngListsBase + } + return 0 +} + +func (u *unit) locListsBase() uint64 { + if u.unit5 != nil { + return u.unit5.locListsBase + } + return 0 +} + func (d *Data) parseUnits() ([]unit, error) { // Count units. nunit := 0 @@ -135,3 +171,30 @@ func (d *Data) offsetToUnit(off Offset) int { } return -1 } + +func (d *Data) collectDwarf5BaseOffsets(u *unit) error { + if u.unit5 == nil { + panic("expected unit5 to be set up already") + } + b := makeBuf(d, u, "info", u.off, u.data) + cu := b.entry(nil, u) + if cu == nil { + // Unknown abbreviation table entry or some other fatal + // problem; bail early on the assumption that this will be + // detected at some later point. + return b.err + } + if iAddrBase, ok := cu.Val(AttrAddrBase).(int64); ok { + u.unit5.addrBase = uint64(iAddrBase) + } + if iStrOffsetsBase, ok := cu.Val(AttrStrOffsetsBase).(int64); ok { + u.unit5.strOffsetsBase = uint64(iStrOffsetsBase) + } + if iRngListsBase, ok := cu.Val(AttrRnglistsBase).(int64); ok { + u.unit5.rngListsBase = uint64(iRngListsBase) + } + if iLocListsBase, ok := cu.Val(AttrLoclistsBase).(int64); ok { + u.unit5.locListsBase = uint64(iLocListsBase) + } + return nil +} -- 2.51.0