FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
debug/elf: do not skip first symbol in the symbol table

Do not skip the first symbol in the symbol table. Any other indexes
into the symbol table (for example, indexes in relocation entries)
will now refer to the symbol following the one that was intended.

Add an object that contains debug relocations, which debug/dwarf
failed to decode correctly. Extend the relocation tests to cover
this case.

Note that the existing tests passed since the symbol following the
symbol that required relocation is also of type STT_SECTION.

Fixes issue 4107.

Please review this at http://codereview.appspot.com/6848044/

Affected files:
M src/pkg/debug/elf/file.go
M src/pkg/debug/elf/file_test.go
A src/pkg/debug/elf/testdata/gcc-amd64-openbsd-debug-with-rela.obj


Index: src/pkg/debug/elf/file.go
===================================================================
--- a/src/pkg/debug/elf/file.go
+++ b/src/pkg/debug/elf/file.go
@@ -417,10 +417,6 @@
return nil, nil, errors.New("cannot load string table section")
}

- // The first entry is all zeros.
- var skip [Sym32Size]byte
- symtab.Read(skip[0:])
-
symbols := make([]Symbol, symtab.Len()/Sym32Size)

i := 0
@@ -460,10 +456,6 @@
return nil, nil, errors.New("cannot load string table section")
}

- // The first entry is all zeros.
- var skip [Sym64Size]byte
- symtab.Read(skip[0:])
-
symbols := make([]Symbol, symtab.Len()/Sym64Size)

i := 0
Index: src/pkg/debug/elf/file_test.go
===================================================================
--- a/src/pkg/debug/elf/file_test.go
+++ b/src/pkg/debug/elf/file_test.go
@@ -175,23 +175,41 @@
}
}

+type relocationTestEntry struct {
+ entryNumber int
+ entry *dwarf.Entry
+}
+
type relocationTest struct {
- file string
- firstEntry *dwarf.Entry
+ file string
+ entries []relocationTestEntry
}

var relocationTests = []relocationTest{
{
"testdata/go-relocation-test-gcc441-x86-64.obj",
- &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit, Children: true,
Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C 4.4.1"}, {Attr:
dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName,
Val: "go-relocation-test.c"}, {Attr: dwarf.AttrCompDir, Val: "/tmp"},
{Attr: dwarf.AttrLowpc, Val: uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val:
uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val: int64(0)}}},
+ []relocationTestEntry{
+ {0, &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit, Children:
true, Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C 4.4.1"},
{Attr: dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName,
Val: "go-relocation-test.c"}, {Attr: dwarf.AttrCompDir, Val: "/tmp"},
{Attr: dwarf.AttrLowpc, Val: uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val:
uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val: int64(0)}}}},
+ },
},
{
"testdata/go-relocation-test-gcc441-x86.obj",
- &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit, Children: true,
Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C 4.4.1"}, {Attr:
dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName, Val: "t.c"},
{Attr: dwarf.AttrCompDir, Val: "/tmp"}, {Attr: dwarf.AttrLowpc, Val:
uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val: uint64(0x5)}, {Attr:
dwarf.AttrStmtList, Val: int64(0)}}},
+ []relocationTestEntry{
+ {0, &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit, Children:
true, Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C 4.4.1"},
{Attr: dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName,
Val: "t.c"}, {Attr: dwarf.AttrCompDir, Val: "/tmp"}, {Attr:
dwarf.AttrLowpc, Val: uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val:
uint64(0x5)}, {Attr: dwarf.AttrStmtList, Val: int64(0)}}}},
+ },
},
{
"testdata/go-relocation-test-gcc424-x86-64.obj",
- &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit, Children: true,
Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C 4.2.4 (Ubuntu
4.2.4-1ubuntu4)"}, {Attr: dwarf.AttrLanguage, Val: int64(1)}, {Attr:
dwarf.AttrName, Val: "go-relocation-test-gcc424.c"}, {Attr:
dwarf.AttrCompDir, Val: "/tmp"}, {Attr: dwarf.AttrLowpc, Val: uint64(0x0)},
{Attr: dwarf.AttrHighpc, Val: uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val:
int64(0)}}},
+ []relocationTestEntry{
+ {0, &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit, Children:
true, Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C 4.2.4
(Ubuntu 4.2.4-1ubuntu4)"}, {Attr: dwarf.AttrLanguage, Val: int64(1)},
{Attr: dwarf.AttrName, Val: "go-relocation-test-gcc424.c"}, {Attr:
dwarf.AttrCompDir, Val: "/tmp"}, {Attr: dwarf.AttrLowpc, Val: uint64(0x0)},
{Attr: dwarf.AttrHighpc, Val: uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val:
int64(0)}}}},
+ },
+ },
+ {
+ "testdata/gcc-amd64-openbsd-debug-with-rela.obj",
+ []relocationTestEntry{
+ {203, &dwarf.Entry{Offset: 0xc62, Tag: dwarf.TagMember, Children:
false, Field: []dwarf.Field{{Attr: dwarf.AttrName, Val: "it_interval"},
{Attr: dwarf.AttrDeclFile, Val: int64(7)}, {Attr: dwarf.AttrDeclLine, Val:
int64(236)}, {Attr: dwarf.AttrType, Val: dwarf.Offset(0xb7f)}, {Attr:
dwarf.AttrDataMemberLoc, Val: []byte{0x23, 0x0}}}}},
+ {204, &dwarf.Entry{Offset: 0xc70, Tag: dwarf.TagMember, Children:
false, Field: []dwarf.Field{{Attr: dwarf.AttrName, Val: "it_value"}, {Attr:
dwarf.AttrDeclFile, Val: int64(7)}, {Attr: dwarf.AttrDeclLine, Val:
int64(237)}, {Attr: dwarf.AttrType, Val: dwarf.Offset(0xb7f)}, {Attr:
dwarf.AttrDataMemberLoc, Val: []byte{0x23, 0x10}}}}},
+ },
},
}

@@ -207,20 +225,24 @@
t.Error(err)
continue
}
- reader := dwarf.Reader()
- // Checking only the first entry is sufficient since it has
- // many different strings. If the relocation had failed, all
- // the string offsets would be zero and all the strings would
- // end up being the same.
- firstEntry, err := reader.Next()
- if err != nil {
- t.Error(err)
- continue
- }
-
- if !reflect.DeepEqual(test.firstEntry, firstEntry) {
- t.Errorf("#%d: mismatch: got:%#v want:%#v", i, firstEntry,
test.firstEntry)
- continue
+ for _, testEntry := range test.entries {
+ reader := dwarf.Reader()
+ for j := 0; j < testEntry.entryNumber; j++ {
+ entry, err := reader.Next()
+ if entry == nil || err != nil {
+ t.Errorf("Failed to skip to entry %d: %v", testEntry.entryNumber, err)
+ continue
+ }
+ }
+ entry, err := reader.Next()
+ if err != nil {
+ t.Error(err)
+ continue
+ }
+ if !reflect.DeepEqual(testEntry.entry, entry) {
+ t.Errorf("#%d/%d: mismatch: got:%#v want:%#v", i,
testEntry.entryNumber, entry, testEntry.entry)
+ continue
+ }
}
}
}
Index: src/pkg/debug/elf/testdata/gcc-amd64-openbsd-debug-with-rela.obj
===================================================================
new file mode 100644
index
0000000000000000000000000000000000000000..f62b1ea1cada83530e45320cc58394933753dddb
GIT binary patch
literal 6544
zc$}44YmD4h6}}$ZV~=Mav)NDzjl`CKT4=M6Bu%#2ZIY&sJeo#*kfuVFEYFN*##xU&
zu|2cd^pUhlq0OTYAeHz50xDIAKdn%t2!2or_)+?!LPA2I{-~(<)fTCRQcw#G=iGCz
z$Lq`xu(IQOzVp22o_p?{$?-e(-&s%;CLe`uXML*w?3wJ3cZ$z3R=!M5>M9e;m(9uY
zHRgn}MyV7TD@-co^0x9qSpiGxC{s-&_D;@QjA`Sn3`55BFTr4YSv46eUX_Y4W+=+u
zNmMkiWqT)Qz-@^n8M7u8)$Cg?qbbHlKAZ72I+`&x2E<t3^?0hTqi98l>cpMcoNDeE
zDFDFsRbVQ7hNiOnz`?28rZ!J+oSvK7I6J#_cI({DQ%sp@g>84{aOnCoaa5aWhWu4)
z*Jo<Ya6oFoU_~kP;H`ET4GQa-8o>AzADc`wAAyms6l*OfV!Eb#LBfjm?(qI3@`9!f
zq6f*2`ns^AX6&fKj#^8{BV2vgO5(<#t$q){qHTlOc{^c+g?Gta>@@?oKE=HPyDRyN
zWd-?WH^@nVz}yy;xMc%M6!f?iM#&U~%k_!9eLrU##7@l!Ie2Z_h+;cVQC!^=+4&?Y
zYQ_MSEt^qb*Gnj1>V6nBr}#(4If>l<8!lq>MFv>2eb<R;w!7c#N0EW|e4HkYP@3A_
zbxKZ}@O|5q)#5avyQ+C{!uQ8Haf<!!^LcT?4_xevH)MqOW^JeFeer5h3h(~BEWuyP
z2~%(f^TGr_kQ1iR4xP^l6Z~L5;!YAF(sb1R(q&dnuv;h8{;~~Nv9@y*#g;Pe1mp`>
zk=yj*#O2mPbOu<@sn<CWogknifmi^&*gNJTkVpv$bOliJ!`c!8M@7AIz^fy)BA|NM
zt@>Q+*d7h%p%H?-h95c!vL3D>OVc=joUqYA{E?Sv0+5NiN6@1pei_SSB9>SJ$Av)R
zwK;u41cEf5-{S)4IkpdJVTF@_0`JC$@bLlsnfL*X)-<eC#!j3jyIN&U9hywDgW<()
z!c>O45}QrqGE7s$o*Ti0mTf0$!k_CfGJp*UR>Y2-p1T^au(An(En`=~NKcDMm$8C?
zJ}c;`8}O-}ll)o|IYErjGiZsRl@1qrRv@)bm#>U=oyb_-?IC%+sOg4xw1Zx>uZU`H
zlK59;adYa1wA5`;FWQZLr~_RMp$+Mt50RV}5=-t1k~3n3+F>0d)Mf=c@Vx-bNt+Wu
zhkR}n5k|h(MtYMd>)3&9)%JXDbhGG6vD=J~A#jt>ilHQZH$Z5Mup2K|eeP&$x}UkD
zdBN|-ZY>Pzm{9Fzsgnh_$(+StsKysZTev8(hUW`x5wVdI+OC!X$1%*f8YfQT&NJ<{
z)yRIP-H!X08F=6wHAER6=V>Alw0Fk>aEe`<j>B0zjez2Z3Df8H6N#Nc-}odE*ff~F
zi7$CKAibMNPpEAnHLkUlv`A^5E6K9ax4nvWlxZdU!p~7aoQ?V(ep<+*Qooz8hdlW7
z`_@S>L?7Z6B9Axyp{isP?Zkr3d#2tvFWE#dT}-FSorT<4H$8^+&bCy53?7M7fJ{W(
z#F4JUC)XthwxoN+6(z1Ot0lOtnZL*TnQWZ;!*Ag9W!uyrc?(I|H1$W{=UhoXrf8T<
zDf|T9Qi^pXDZZ0D0d>hgbv`92+$Ra(TUP(ZZ*eK)hSk6M7Lv4G^{3uPg2qo@;$yxE
z_0yZ+Hd>@DsGr$|gxG-k*?W<a+fP5|AW54~e<nggY(4$i$B~kIOn>erPKxcMKmU4>
zFB4b%)|;H@?;8Ev?{MiX^qo%;%4{S3{G`E=bQ9?>TxalE$cCf;V6;epXgB%~r?}xv
zJJ5ghJcrV~ufKQ^0eNESKUt$mlEQs?JpyvME<D32N&f8nNQx6h|HaFa;^F-2RV3-e
z(qHp6>H>AU?Ziv0xb+|ujRhng4kwBW4&2h!2F$+dB{3^5A`K@9Gd95BNLg(BFF*=N
zrFaX!?W4Dz<FiWf>#qPkhDVgK5gREUKS_`GEM{yX#3Y5d16ajacstctRLiMGz%_1J
zrEx3_TxJ^kzBmF=Cdcfy7j)sOyzIg`Y20;6T&;moj}w$<#(n}dW>sNtAyhHW+=J@e
z{8{O+5jjAfJ824U9?u_6Y07x__=pVfz2hWL)L)3IFpE|yXoPI7@wd0peCrsT*=a;x
zZHXCw7qM{CsUK#>NAep0wougghxi(xcA~I>z&{D7$cT8(rpz1HjEa!V4X<QEGN)gW
zAth}#Uuyk%6f@PfakeEdji-&NW2M7sL~Xkjb|Y3=k`bXu-C1EJUxtK82uf`c5&|nO
zbnH?<$XY~}xl%~T)m(-n9YWS2;>EB&taOB+DTJEs1WJ^ZB3ke35#y^}is>8EEGt$!
z4eFT8n#Qu_KvzOD7qF5Mg4{JDr_CzX_FEw(Y6t7s<<VrK1pYD_FR
z8IJ>I5~$Xo`!MU17fsSRoG|NOXSjiadbq)?e-ojqOU(N8f=N}Tndp%l!zBe{cmA4(
z5W_9E)??Q0G=|u&M`4FqdkE1;3od1`2U*sv2QG2N9&FUZVZ^LM-(vxB&a~D
zau1r;<D17tby!baFjwzE>!gg8E4AtoTWdZ2!Z`YB*$G75hM-Pg8WYZ1cFpPV^qnC^
z6S2CJI2=4n;0S_Xi(}^~XB9S%9_oI=@39q2Q+@|-D%FNh>!B+m83dt_Rpc`C<r=vY
za5HR%wmr+rl@EcVlvMfkTr;uOrNA63Pkg456Ttmx;3fTris)Pgp-^|L-6o#~E3dg`
zwH|vPD~`Q`zn3VVj9Ku>e~M9Nx^Xn)1vS50cW2s8t>r<1t^(tgcondk8$iRHo}Ph!
zouwwq)*RnwY%xyiUa%cK+juRGr(5`g&0@9d`N@Vnr@&Zd7`wh(O9TuBKqI?96eE}%
zECviTp>BsAH>k$->1N2_7-ox1{(^D<vhhbqb(;B)`@|g2WIluVcnNMqYMD@LSZV+P
z)pZ%0j#t`=Q-yaD(Yqx_iF*{1!p%%q;~0kAu2Zc>ZjVhzuI~tlw^uRtG4okwr&|pi
zI8vklm>>9?&j0yb!|1oOVh-OqL|)6$*9v_#hfDpdb9g03Uk88Y0R0F5NB=i6Ht=^t
zssD!@o|oM*M1NL*2XX=DWw#B837_l5#P1ObaANW*GzDMg<*p(AADf`}hdDei`_K^m
zXbru0UP1p58yx@9&#jIV<2Tvm@%uyk|5W&2cLo1A&HUQq^M=r0zJfl|g#r4nT}AJC
zp#Zl8hQFic<1Y>IF9p_L!T*1IH1@%zLAW}^iaKy6h!2bq&_OGQxIP3ohT!H9+`0_I
z$R{#!$~;c+zrS9RmgM{ML-5O5Y}$;|M$8}CmoSp-tZvQ3lg--RA^5!+JJ~EeSMK9j
z3nK#m<q*6!vKp7qksL1n+v1}k^4jQN`L*MN{k}KepI7#OKSnEkd?bV4k-@X8n9XOl
G=l=(aF1s24

Search Discussions

  • Mikioh Mikioh at Nov 13, 2012 at 2:31 pm
    LGTM

    Thanks! it also passes on freebsd.

    https://codereview.appspot.com/6848044/
  • Ian Lance Taylor at Nov 13, 2012 at 2:33 pm
    This is going to change the behaviour of the Symbols function in
    debug/elf and potentially break any current users. I don't know if
    that is a good idea. Certainly any change would need to be documented
    in doc/go1.1.html.

    Ian
    On Tue, Nov 13, 2012 at 5:49 AM, wrote:
    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com,

    I'd like you to review this change to
    https://go.googlecode.com/hg/


    Description:
    debug/elf: do not skip first symbol in the symbol table

    Do not skip the first symbol in the symbol table. Any other indexes
    into the symbol table (for example, indexes in relocation entries)
    will now refer to the symbol following the one that was intended.

    Add an object that contains debug relocations, which debug/dwarf
    failed to decode correctly. Extend the relocation tests to cover
    this case.

    Note that the existing tests passed since the symbol following the
    symbol that required relocation is also of type STT_SECTION.

    Fixes issue 4107.

    Please review this at http://codereview.appspot.com/6848044/

    Affected files:
    M src/pkg/debug/elf/file.go
    M src/pkg/debug/elf/file_test.go
    A src/pkg/debug/elf/testdata/gcc-amd64-openbsd-debug-with-rela.obj


    Index: src/pkg/debug/elf/file.go
    ===================================================================
    --- a/src/pkg/debug/elf/file.go
    +++ b/src/pkg/debug/elf/file.go
    @@ -417,10 +417,6 @@
    return nil, nil, errors.New("cannot load string table
    section")
    }

    - // The first entry is all zeros.
    - var skip [Sym32Size]byte
    - symtab.Read(skip[0:])
    -
    symbols := make([]Symbol, symtab.Len()/Sym32Size)

    i := 0
    @@ -460,10 +456,6 @@
    return nil, nil, errors.New("cannot load string table
    section")
    }

    - // The first entry is all zeros.
    - var skip [Sym64Size]byte
    - symtab.Read(skip[0:])
    -
    symbols := make([]Symbol, symtab.Len()/Sym64Size)

    i := 0
    Index: src/pkg/debug/elf/file_test.go
    ===================================================================
    --- a/src/pkg/debug/elf/file_test.go
    +++ b/src/pkg/debug/elf/file_test.go
    @@ -175,23 +175,41 @@
    }
    }

    +type relocationTestEntry struct {
    + entryNumber int
    + entry *dwarf.Entry
    +}
    +
    type relocationTest struct {
    - file string
    - firstEntry *dwarf.Entry
    + file string
    + entries []relocationTestEntry
    }

    var relocationTests = []relocationTest{
    {
    "testdata/go-relocation-test-gcc441-x86-64.obj",
    - &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit,
    Children: true, Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C
    4.4.1"}, {Attr: dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName,
    Val: "go-relocation-test.c"}, {Attr: dwarf.AttrCompDir, Val: "/tmp"}, {Attr:
    dwarf.AttrLowpc, Val: uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val:
    uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val: int64(0)}}},
    + []relocationTestEntry{
    + {0, &dwarf.Entry{Offset: 0xb, Tag:
    dwarf.TagCompileUnit, Children: true, Field: []dwarf.Field{{Attr:
    dwarf.AttrProducer, Val: "GNU C 4.4.1"}, {Attr: dwarf.AttrLanguage, Val:
    int64(1)}, {Attr: dwarf.AttrName, Val: "go-relocation-test.c"}, {Attr:
    dwarf.AttrCompDir, Val: "/tmp"}, {Attr: dwarf.AttrLowpc, Val: uint64(0x0)},
    {Attr: dwarf.AttrHighpc, Val: uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val:
    int64(0)}}}},
    + },
    },
    {
    "testdata/go-relocation-test-gcc441-x86.obj",
    - &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit,
    Children: true, Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C
    4.4.1"}, {Attr: dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName,
    Val: "t.c"}, {Attr: dwarf.AttrCompDir, Val: "/tmp"}, {Attr: dwarf.AttrLowpc,
    Val: uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val: uint64(0x5)}, {Attr:
    dwarf.AttrStmtList, Val: int64(0)}}},
    + []relocationTestEntry{
    + {0, &dwarf.Entry{Offset: 0xb, Tag:
    dwarf.TagCompileUnit, Children: true, Field: []dwarf.Field{{Attr:
    dwarf.AttrProducer, Val: "GNU C 4.4.1"}, {Attr: dwarf.AttrLanguage, Val:
    int64(1)}, {Attr: dwarf.AttrName, Val: "t.c"}, {Attr: dwarf.AttrCompDir,
    Val: "/tmp"}, {Attr: dwarf.AttrLowpc, Val: uint64(0x0)}, {Attr:
    dwarf.AttrHighpc, Val: uint64(0x5)}, {Attr: dwarf.AttrStmtList, Val:
    int64(0)}}}},
    + },
    },
    {
    "testdata/go-relocation-test-gcc424-x86-64.obj",
    - &dwarf.Entry{Offset: 0xb, Tag: dwarf.TagCompileUnit,
    Children: true, Field: []dwarf.Field{{Attr: dwarf.AttrProducer, Val: "GNU C
    4.2.4 (Ubuntu 4.2.4-1ubuntu4)"}, {Attr: dwarf.AttrLanguage, Val: int64(1)},
    {Attr: dwarf.AttrName, Val: "go-relocation-test-gcc424.c"}, {Attr:
    dwarf.AttrCompDir, Val: "/tmp"}, {Attr: dwarf.AttrLowpc, Val: uint64(0x0)},
    {Attr: dwarf.AttrHighpc, Val: uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val:
    int64(0)}}},
    + []relocationTestEntry{
    + {0, &dwarf.Entry{Offset: 0xb, Tag:
    dwarf.TagCompileUnit, Children: true, Field: []dwarf.Field{{Attr:
    dwarf.AttrProducer, Val: "GNU C 4.2.4 (Ubuntu 4.2.4-1ubuntu4)"}, {Attr:
    dwarf.AttrLanguage, Val: int64(1)}, {Attr: dwarf.AttrName, Val:
    "go-relocation-test-gcc424.c"}, {Attr: dwarf.AttrCompDir, Val: "/tmp"},
    {Attr: dwarf.AttrLowpc, Val: uint64(0x0)}, {Attr: dwarf.AttrHighpc, Val:
    uint64(0x6)}, {Attr: dwarf.AttrStmtList, Val: int64(0)}}}},
    + },
    + },
    + {
    + "testdata/gcc-amd64-openbsd-debug-with-rela.obj",
    + []relocationTestEntry{
    + {203, &dwarf.Entry{Offset: 0xc62, Tag:
    dwarf.TagMember, Children: false, Field: []dwarf.Field{{Attr:
    dwarf.AttrName, Val: "it_interval"}, {Attr: dwarf.AttrDeclFile, Val:
    int64(7)}, {Attr: dwarf.AttrDeclLine, Val: int64(236)}, {Attr:
    dwarf.AttrType, Val: dwarf.Offset(0xb7f)}, {Attr: dwarf.AttrDataMemberLoc,
    Val: []byte{0x23, 0x0}}}}},
    + {204, &dwarf.Entry{Offset: 0xc70, Tag:
    dwarf.TagMember, Children: false, Field: []dwarf.Field{{Attr:
    dwarf.AttrName, Val: "it_value"}, {Attr: dwarf.AttrDeclFile, Val: int64(7)},
    {Attr: dwarf.AttrDeclLine, Val: int64(237)}, {Attr: dwarf.AttrType, Val:
    dwarf.Offset(0xb7f)}, {Attr: dwarf.AttrDataMemberLoc, Val: []byte{0x23,
    0x10}}}}},
    + },
    },
    }

    @@ -207,20 +225,24 @@
    t.Error(err)
    continue
    }
    - reader := dwarf.Reader()
    - // Checking only the first entry is sufficient since it has
    - // many different strings. If the relocation had failed, all
    - // the string offsets would be zero and all the strings
    would
    - // end up being the same.
    - firstEntry, err := reader.Next()
    - if err != nil {
    - t.Error(err)
    - continue
    - }
    -
    - if !reflect.DeepEqual(test.firstEntry, firstEntry) {
    - t.Errorf("#%d: mismatch: got:%#v want:%#v", i,
    firstEntry, test.firstEntry)
    - continue
    + for _, testEntry := range test.entries {
    + reader := dwarf.Reader()
    + for j := 0; j < testEntry.entryNumber; j++ {
    + entry, err := reader.Next()
    + if entry == nil || err != nil {
    + t.Errorf("Failed to skip to entry
    %d: %v", testEntry.entryNumber, err)
    + continue
    + }
    + }
    + entry, err := reader.Next()
    + if err != nil {
    + t.Error(err)
    + continue
    + }
    + if !reflect.DeepEqual(testEntry.entry, entry) {
    + t.Errorf("#%d/%d: mismatch: got:%#v
    want:%#v", i, testEntry.entryNumber, entry, testEntry.entry)
    + continue
    + }
    }
    }
    }
    Index: src/pkg/debug/elf/testdata/gcc-amd64-openbsd-debug-with-rela.obj
    ===================================================================
    new file mode 100644
    index
    0000000000000000000000000000000000000000..f62b1ea1cada83530e45320cc58394933753dddb
    GIT binary patch
    literal 6544
    zc$}44YmD4h6}}$ZV~=Mav)NDzjl`CKT4=M6Bu%#2ZIY&sJeo#*kfuVFEYFN*##xU&
    zu|2cd^pUhlq0OTYAeHz50xDIAKdn%t2!2or_)+?!LPA2I{-~(<)fTCRQcw#G=iGCz
    z$Lq`xu(IQOzVp22o_p?{$?-e(-&s%;CLe`uXML*w?3wJ3cZ$z3R=!M5>M9e;m(9uY
    zHRgn}MyV7TD@-co^0x9qSpiGxC{s-&_D;@QjA`Sn3`55BFTr4YSv46eUX_Y4W+=+u
    zNmMkiWqT)Qz-@^n8M7u8)$Cg?qbbHlKAZ72I+`&x2E<t3^?0hTqi98l>cpMcoNDeE
    zDFDFsRbVQ7hNiOnz`?28rZ!J+oSvK7I6J#_cI({DQ%sp@g>84{aOnCoaa5aWhWu4)
    z*Jo<Ya6oFoU_~kP;H`ET4GQa-8o>AzADc`wAAyms6l*OfV!Eb#LBfjm?(qI3@`9!f
    zq6f*2`ns^AX6&fKj#^8{BV2vgO5(<#t$q){qHTlOc{^c+g?Gta>@@?oKE=HPyDRyN
    zWd-?WH^@nVz}yy;xMc%M6!f?iM#&U~%k_!9eLrU##7@l!Ie2Z_h+;cVQC!^=+4&?Y
    zYQ_MSEt^qb*Gnj1>V6nBr}#(4If>l<8!lq>MFv>2eb<R;w!7c#N0EW|e4HkYP@3A_
    zbxKZ}@O|5q)#5avyQ+C{!uQ8Haf<!!^LcT?4_xevH)MqOW^JeFeer5h3h(~BEWuyP
    z2~%(f^TGr_kQ1iR4xP^l6Z~L5;!YAF(sb1R(q&dnuv;h8{;~~Nv9@y*#g;Pe1mp`>
    zk=yj*#O2mPbOu<@sn<CWogknifmi^&*gNJTkVpv$bOliJ!`c!8M@7AIz^fy)BA|NM
    zt@>Q+*d7h%p%H?-h95c!vL3D>OVc=joUqYA{E?Sv0+5NiN6@1pei_SSB9>SJ$Av)R
    zwK;u41cEf5-{S)4IkpdJVTF@_0`JC$@bLlsnfL*X)-<eC#!j3jyIN&U9hywDgW<()
    z!c>O45}QrqGE7s$o*Ti0mTf0$!k_CfGJp*UR>Y2-p1T^au(An(En`=~NKcDMm$8C?
    zJ}c;`8}O-}ll)o|IYErjGiZsRl@1qrRv@)bm#>U=oyb_-?IC%+sOg4xw1Zx>uZU`H
    zlK59;adYa1wA5`;FWQZLr~_RMp$+Mt50RV}5=-t1k~3n3+F>0d)Mf=c@Vx-bNt+Wu
    zhkR}n5k|h(MtYMd>)3&9)%JXDbhGG6vD=J~A#jt>ilHQZH$Z5Mup2K|eeP&$x}UkD
    zdBN|-ZY>Pzm{9Fzsgnh_$(+StsKysZTev8(hUW`x5wVdI+OC!X$1%*f8YfQT&NJ<{
    z)yRIP-H!X08F=6wHAER6=V>Alw0Fk>aEe`<j>B0zjez2Z3Df8H6N#Nc-}odE*ff~F
    zi7$CKAibMNPpEAnHLkUlv`A^5E6K9ax4nvWlxZdU!p~7aoQ?V(ep<+*Qooz8hdlW7
    z`_@S>L?7Z6B9Axyp{isP?Zkr3d#2tvFWE#dT}-FSorT<4H$8^+&bCy53?7M7fJ{W(
    z#F4JUC)XthwxoN+6(z1Ot0lOtnZL*TnQWZ;!*Ag9W!uyrc?(I|H1$W{=UhoXrf8T<
    zDf|T9Qi^pXDZZ0D0d>hgbv`92+$Ra(TUP(ZZ*eK)hSk6M7Lv4G^{3uPg2qo@;$yxE
    z_0yZ+Hd>@DsGr$|gxG-k*?W<a+fP5|AW54~e<nggY(4$i$B~kIOn>erPKxcMKmU4>
    zFB4b%)|;H@?;8Ev?{MiX^qo%;%4{S3{G`E=bQ9?>TxalE$cCf;V6;epXgB%~r?}xv
    zJJ5ghJcrV~ufKQ^0eNESKUt$mlEQs?JpyvME<D32N&f8nNQx6h|HaFa;^F-2RV3-e
    z(qHp6>H>AU?Ziv0xb+|ujRhng4kwBW4&2h!2F$+dB{3^5A`K@9Gd95BNLg(BFF*=N
    zrFaX!?W4Dz<FiWf>#qPkhDVgK5gREUKS_`GEM{yX#3Y5d16ajacstctRLiMGz%_1J
    zrEx3_TxJ^kzBmF=Cdcfy7j)sOyzIg`Y20;6T&;moj}w$<#(n}dW>sNtAyhHW+=J@e
    z{8{O+5jjAfJ824U9?u_6Y07x__=pVfz2hWL)L)3IFpE|yXoPI7@wd0peCrsT*=a;x
    zZHXCw7qM{CsUK#>NAep0wougghxi(xcA~I>z&{D7$cT8(rpz1HjEa!V4X<QEGN)gW
    zAth}#Uuyk%6f@PfakeEdji-&NW2M7sL~Xkjb|Y3=k`bXu-C1EJUxtK82uf`c5&|nO
    zbnH?<$XY~}xl%~T)m(-n9YWS2;>EB&taOB+DTJEs1WJ^ZB3ke35#y^}is>8EEGt$!
    z4eFT8n#Qu_KvzOD7qF5Mg4{JDr_CzX_FEw(Y6t7s<<VrK1pYD_FR
    z8IJ>I5~$Xo`!MU17fsSRoG|NOXSjiadbq)?e-ojqOU(N8f=N}Tndp%l!zBe{cmA4(
    z5W_9E)??Q0G=|u&M`4FqdkE1;3od1`2U*sv2QG2N9&FUZVZ^LM-(vxB&a~D
    zau1r;<D17tby!baFjwzE>!gg8E4AtoTWdZ2!Z`YB*$G75hM-Pg8WYZ1cFpPV^qnC^
    z6S2CJI2=4n;0S_Xi(}^~XB9S%9_oI=@39q2Q+@|-D%FNh>!B+m83dt_Rpc`C<r=vY
    za5HR%wmr+rl@EcVlvMfkTr;uOrNA63Pkg456Ttmx;3fTris)Pgp-^|L-6o#~E3dg`
    zwH|vPD~`Q`zn3VVj9Ku>e~M9Nx^Xn)1vS50cW2s8t>r<1t^(tgcondk8$iRHo}Ph!
    zouwwq)*RnwY%xyiUa%cK+juRGr(5`g&0@9d`N@Vnr@&Zd7`wh(O9TuBKqI?96eE}%
    zECviTp>BsAH>k$->1N2_7-ox1{(^D<vhhbqb(;B)`@|g2WIluVcnNMqYMD@LSZV+P
    z)pZ%0j#t`=Q-yaD(Yqx_iF*{1!p%%q;~0kAu2Zc>ZjVhzuI~tlw^uRtG4okwr&|pi
    zI8vklm>>9?&j0yb!|1oOVh-OqL|)6$*9v_#hfDpdb9g03Uk88Y0R0F5NB=i6Ht=^t
    zssD!@o|oM*M1NL*2XX=DWw#B837_l5#P1ObaANW*GzDMg<*p(AADf`}hdDei`_K^m
    zXbru0UP1p58yx@9&#jIV<2Tvm@%uyk|5W&2cLo1A&HUQq^M=r0zJfl|g#r4nT}AJC
    zp#Zl8hQFic<1Y>IF9p_L!T*1IH1@%zLAW}^iaKy6h!2bq&_OGQxIP3ohT!H9+`0_I
    z$R{#!$~;c+zrS9RmgM{ML-5O5Y}$;|M$8}CmoSp-tZvQ3lg--RA^5!+JJ~EeSMK9j
    z3nK#m<q*6!vKp7qksL1n+v1}k^4jQN`L*MN{k}KepI7#OKSnEkd?bV4k-@X8n9XOl
    G=l=(aF1s24

  • Joel Sing at Nov 13, 2012 at 3:15 pm

    On 14 November 2012 01:33, Ian Lance Taylor wrote:
    This is going to change the behaviour of the Symbols function in
    debug/elf and potentially break any current users. I don't know if
    that is a good idea. Certainly any change would need to be documented
    in doc/go1.1.html.
    Indeed. As I've noted in the issue, there are two obvious fixes - stop
    skipping the first symbol, or adjust the symbol index in the
    relocation code. While the second of these avoids a change in existing
    behaviour, I think the first option is more "correct" since we expose
    the ELF and DWARF data. Anyone using these packages is currently
    expected to know that an index into the symbol table needs to be
    adjusted to account for the fact that debug/elf is discarding the
    first symbol.
  • Ian Lance Taylor at Nov 13, 2012 at 4:46 pm

    On Tue, Nov 13, 2012 at 7:15 AM, Joel Sing wrote:
    On 14 November 2012 01:33, Ian Lance Taylor wrote:
    This is going to change the behaviour of the Symbols function in
    debug/elf and potentially break any current users. I don't know if
    that is a good idea. Certainly any change would need to be documented
    in doc/go1.1.html.
    Indeed. As I've noted in the issue, there are two obvious fixes - stop
    skipping the first symbol, or adjust the symbol index in the
    relocation code. While the second of these avoids a change in existing
    behaviour, I think the first option is more "correct" since we expose
    the ELF and DWARF data. Anyone using these packages is currently
    expected to know that an index into the symbol table needs to be
    adjusted to account for the fact that debug/elf is discarding the
    first symbol.
    Makes sense to me. Can you please add a change to go1.1.html to this
    CL? Thanks.

    Ian
  • Jsing at Nov 14, 2012 at 10:35 am

    On 2012/11/13 16:46:32, iant2 wrote:
    On Tue, Nov 13, 2012 at 7:15 AM, Joel Sing
    wrote:
    On 14 November 2012 01:33, Ian Lance Taylor
    wrote:
    This is going to change the behaviour of the Symbols function in
    debug/elf and potentially break any current users. I don't know if
    that is a good idea. Certainly any change would need to be
    documented
    in doc/go1.1.html.
    Indeed. As I've noted in the issue, there are two obvious fixes -
    stop
    skipping the first symbol, or adjust the symbol index in the
    relocation code. While the second of these avoids a change in
    existing
    behaviour, I think the first option is more "correct" since we
    expose
    the ELF and DWARF data. Anyone using these packages is currently
    expected to know that an index into the symbol table needs to be
    adjusted to account for the fact that debug/elf is discarding the
    first symbol.
    Makes sense to me. Can you please add a change to go1.1.html to this
    CL? Thanks.
    Done. PTAL.

    https://codereview.appspot.com/6848044/
  • Iant at Nov 14, 2012 at 2:19 pm
    LGTM with one change.


    https://codereview.appspot.com/6848044/diff/10001/doc/go1.1.html
    File doc/go1.1.html (right):

    https://codereview.appspot.com/6848044/diff/10001/doc/go1.1.html#newcode71
    doc/go1.1.html:71: processes the symbol table returned from debug/elf
    may need to be adjusted to
    s|processes the symbol table returned from debug/elf|calls the debug/elf
    functions Symbols or ImportedSymbols|

    https://codereview.appspot.com/6848044/
  • Jsing at Nov 14, 2012 at 3:22 pm

    On 2012/11/14 14:19:36, iant wrote:
    LGTM with one change.
    https://codereview.appspot.com/6848044/diff/10001/doc/go1.1.html
    File doc/go1.1.html (right):

    https://codereview.appspot.com/6848044/diff/10001/doc/go1.1.html#newcode71
    doc/go1.1.html:71: processes the symbol table returned from debug/elf may need
    to be adjusted to
    s|processes the symbol table returned from debug/elf|calls the debug/elf
    functions Symbols or ImportedSymbols|
    Done.

    https://codereview.appspot.com/6848044/
  • Jsing at Nov 14, 2012 at 3:27 pm
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=9ea9e7e6e0c8 ***

    debug/elf: do not skip first symbol in the symbol table

    Do not skip the first symbol in the symbol table. Any other indexes
    into the symbol table (for example, indexes in relocation entries)
    will now refer to the symbol following the one that was intended.

    Add an object that contains debug relocations, which debug/dwarf
    failed to decode correctly. Extend the relocation tests to cover
    this case.

    Note that the existing tests passed since the symbol following the
    symbol that required relocation is also of type STT_SECTION.

    Fixes issue 4107.

    R=golang-dev, mikioh.mikioh, iant, iant
    CC=golang-dev
    http://codereview.appspot.com/6848044


    http://codereview.appspot.com/6848044/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 13, '12 at 1:49p
activeNov 14, '12 at 3:27p
posts9
users4
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase