This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patchv2] PR gdb/17968 - [ppc64] SEGV in ppc64_elf_get_synthetic_symtab reading a separate debug file
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Martin Sebor <msebor at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 25 Feb 2015 22:04:04 +0100
- Subject: [patchv2] PR gdb/17968 - [ppc64] SEGV in ppc64_elf_get_synthetic_symtab reading a separate debug file
- Authentication-results: sourceware.org; auth=none
- References: <54E39F4C dot 6050607 at redhat dot com> <54E3D152 dot 6020804 at redhat dot com>
Hi Martin,
[ppc64] SEGV in ppc64_elf_get_synthetic_symtab reading a separate debug file
https://sourceware.org/bugzilla/show_bug.cgi?id=17968
Martin Sebor 2015-02-16 00:42:58 CET
# The problem appears to be due to the change to gdb/elfread.c introduced in the commit below:
# # commit 63524580f8372e38a6a62fd875a4252068c31150
# # Author: Jan Kratochvil <jan.kratochvil@redhat.com>
# # Date: Sun Apr 17 18:38:46 2011 +0000
# # gdb/
# # Fix convert_code_addr_to_desc_addr for ppc64 files after eu-strip.
# # * elfread.c (elf_symfile_read): New variable synth_abfd, pass it to
# # bfd_get_synthetic_symtab.
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=63524580f8372e38a6a62fd875a4252068c31150
I find the patch of mine above right even for object files and your patch only
avoided hitting the stale memory reference by disabling proper .opd parsing.
Valgrind on F-21 x86_64 host showed me more clear what is the problem:
Reading symbols from /home/jkratoch/t/cordic.ko...Reading symbols from /home/jkratoch/t/cordic.ko.debug...=================================================================
==22763==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000461c8 at pc 0x150cdbd bp 0x7fffffffc7e0 sp 0x7fffffffc7d0
READ of size 8 at 0x6120000461c8 thread T0
#0 0x150cdbc in ppc64_elf_get_synthetic_symtab /home/jkratoch/redhat/gdb-test-asan/bfd/elf64-ppc.c:3282
#1 0x8c5274 in elf_read_minimal_symbols /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1205
#2 0x8c55e7 in elf_symfile_read /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1268
[...]
0x6120000461c8 is located 264 bytes inside of 288-byte region [0x6120000460c0,0x6120000461e0)
freed by thread T0 here:
#0 0x7ffff715454f in __interceptor_free (/lib64/libasan.so.1+0x5754f)
#1 0xde9cde in xfree common/common-utils.c:98
#2 0x9a04f7 in do_my_cleanups common/cleanups.c:155
#3 0x9a05d3 in do_cleanups common/cleanups.c:177
#4 0x8c538a in elf_read_minimal_symbols /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1229
#5 0x8c55e7 in elf_symfile_read /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1268
[...]
previously allocated by thread T0 here:
#0 0x7ffff71547c7 in malloc (/lib64/libasan.so.1+0x577c7)
#1 0xde9b95 in xmalloc common/common-utils.c:41
#2 0x8c4da2 in elf_read_minimal_symbols /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1147
#3 0x8c55e7 in elf_symfile_read /home/jkratoch/redhat/gdb-test-asan/gdb/elfread.c:1268
[...]
SUMMARY: AddressSanitizer: heap-use-after-free /home/jkratoch/redhat/gdb-test-asan/bfd/elf64-ppc.c:3282 ppc64_elf_get_synthetic_symtab
[...]
==22763==ABORTING
A similar case a few lines later I have fixed in 2010 by:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3f1eff0a2c7f0e7078f011f55b8e7f710aae0cc2
Your testcase may be fine although it does not reproduce the crash on x86_64
host. Therefore attaching the binaries themselves to make the crash more
reproducible.
Although... my testcase does not always reproduce it but at least a bit:
* GDB without ppc64 target (even as a secondary one) is reported as "untested"
* ASAN-built GDB with ppc64 target always crashes (and PASSes with this fix)
* unpatched non-ASAN-built GDB with ppc64 target crashes from commandline
* unpatched non-ASAN-built GDB with ppc64 target PASSes from runtest (?)
Jan
On Wed, 18 Feb 2015 00:40:02 +0100, Martin Sebor wrote:
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 0bd0792..288ec7d 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2015-02-16 Martin Sebor <msebor@redhat.com>
> +
> + PR gdb/17968
> + * elread.c (elf_symfile_read): Use synth_abfd only for shared and
elfread
> + executable objects (but not relocatable files).
> +
> 2015-02-13 Doug Evans <dje@google.com>
>
> * cp-namespace.c (cp_basic_lookup_symbol): Rename parameter
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 65c63f0..b1074e0 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1183,19 +1183,22 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
> elf_rel_plt_read (objfile, dyn_symbol_table);
> }
>
> - /* Contrary to binutils --strip-debug/--only-keep-debug the strip command from
> - elfutils (eu-strip) moves even the .symtab section into the .debug file.
> + /* Both the Binutils strip command and the Elfutils eu-strip command
> + remove the .symtab section from executables or dynamic object files
> + (but not from reloacatable object files like Linux kernel modules)
relocatable
> + and move it into the .debug file.
>
> bfd_get_synthetic_symtab on ppc64 for each function descriptor ELF symbol
> 'name' creates a new BSF_SYNTHETIC ELF symbol '.name' with its code
> - address. But with eu-strip files bfd_get_synthetic_symtab would fail to
> - read the code address from .opd while it reads the .symtab section from
> - a separate debug info file as the .opd section is SHT_NOBITS there.
> + address. But with stripped files without the .symtab section
> + bfd_get_synthetic_symtab would fail to read the code address from .opd
> + while it reads the .symtab section from a separate debug info file as
> + the .opd section is SHT_NOBITS there.
>
> With SYNTH_ABFD the .opd section will be read from the original
> backlinked binary where it is valid. */
>
> - if (objfile->separate_debug_objfile_backlink)
> +if (abfd->flags & (EXEC_P | DYNAMIC) && objfile->separate_debug_objfile_backlink)
missing/removed indentation
> synth_abfd = objfile->separate_debug_objfile_backlink->obfd;
> else
> synth_abfd = abfd;
[...]
> +if { [# Copy debug info from the relocatable file into a separate .dbg file.
> + verify "objcopy --only-keep-debug $binfile.bu $binfile.bu.dbg"] &&
> + [# Strip debug (and other unneeded) sections from the relocatable
> + # file.
> + verify "strip --strip-debug --strip-unneeded $binfile.bu"] &&
> + [# Insert the .gnu_debuglink section into the relocatable file
> + # pointing at the separate .dbg file.
> + verify "objcopy --add-gnu-debuglink $binfile.bu.dbg $binfile.bu"]} {
I haven't tried that but lib/gdb.exp contains gdb_gnu_strip_debug(), would not
it be applicable here?
Jan
gdb/ChangeLog
2015-02-25 Jan Kratochvil <jan.kratochvil@redhat.com>
* elfread.c (elf_read_minimal_symbols): Use bfd_alloc for
bfd_canonicalize_symtab.
gdb/testsuite/ChangeLog
2015-02-25 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.arch/cordic.ko.bz2: New file.
* gdb.arch/cordic.ko.debug.bz2: New file.
* gdb.arch/ppc64-symtab-cordic.exp: New file.
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 65c63f0..4a6576f 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1144,8 +1144,10 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
if (storage_needed > 0)
{
- symbol_table = (asymbol **) xmalloc (storage_needed);
- make_cleanup (xfree, symbol_table);
+ /* Memory gets permanently referenced from ABFD after
+ bfd_canonicalize_symtab so it must not get freed before ABFD gets. */
+
+ symbol_table = bfd_alloc (abfd, storage_needed);
symcount = bfd_canonicalize_symtab (objfile->obfd, symbol_table);
if (symcount < 0)
diff --git a/gdb/testsuite/gdb.arch/cordic.ko.bz2 b/gdb/testsuite/gdb.arch/cordic.ko.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..8cb5d664ac0d21ed57b0f76920f6d87f95bd7840
GIT binary patch
literal 2208
zcmV;R2w(R?T4*^jL0KkKS*&srrvL?||NsC0|NsC0|Nj5~|NsC0|Ns5}`~TPf{r>NL
z|M&O*{=3iy7`zRzT{*RJ$_5=MY9NATJv~w6%6O-gZA_Xn4XE@$$Yj%MWY7S}2Gr4r
z0BC61gG~&9h-CDEpfu5-$)Toz3{OzeqehwxLrfskO$eh!PftW@dTFVeo{9R9(dag)
z!fB6CpQ$}SWHj=cdYWQs=`wm`g8+>*!T>g)0QCS5Q${09i~=$kh9gWsXvEV^05m;8
zq6C;GO#skSDeW|-Cz5HWsCrLR5CGF75$bw|O)>!WJwP%vGf|-R0MG%D44O348k<PX
zKy6I`8UO~38UQrW0000027mwn27t%_00000007VcG-v<-000000BC3p001&*1c@Oj
z=%<=Ym{VmvO$LB%Mrv(C#XTpe(9qGL>KYmWrqpN|8V^Xw01s0kplASS0MG%T0B8UJ
z0009cSM{HpRWnAvc)7>A1fvBIIfjc$5&b$19k-0qsJ2^U0?y{?l^LzbHro4n)RqK#
zO8}Ym4m|_Ju%LnNp$2Anq*le45RkO^TD-8UUgd8}JFTBI%S&BLb6<o?;%%+W3F>La
zqjBtYtMS{oLX*##<3f~=ZaB#>%lb9Sd0js4KJ%hMyq~1^7*=Q!9Hb^EJYfI?Mp6;V
zyS!_6ngGsfkbt<_>dt>fIFL!UB&fdDoDNs4%rk>_S6kL!FISGGkgTfcY-XKaRHSOZ
zS3!y2E4_>YtV-(Y0|x#J{WJk_L?BCoWRjE!h=_;=!wH}zD2RX%+kOkGln5`NfenCM
zu^7f;nYVD7d%3#iy+*RlV6(C^Ty(Q-Z!dmnBO2A=ub5L!*|dNySR?GTh-g==5EZOQ
z)3htLkx8s}E)uLe7)u{#Yki{9Wo1T>M%y}u;Spb%NzQj!u5HL)PYDQ6TMRhWE#)#S
zWKZK)yBQAGIUyul+j8vX4<K+-J;B0|8UZy;OO7mIP9WBD@O1TdAJwlevXJW|)MRVS
z?CgqS!m1sc79%4p35~-jh~+1P4TTb9d=w=Cq_9GSkWRpD6gXiTim4Ka6xQS=8AvNL
zv4tv9q9c%Go_U?S$yY;RN`v>8_}2i@X6cOtFMZ?u>kZ!_Mt=}0i&tuqv9U^15;G@q
zWalxrG=vsi<O;!vB=sa_CB;jL?Pe}D9+p;RXzQ8D*uu`U0#lDauTCZW_jLR@j*qR=
zOL4=2AH(}vTA>3#5d88x2%?bl!m5_^Yn~Y0=2cgwC0`>8ia-E*3cZ|cCQ<rvCvOva
zL%JXFgz?8Dff+31MTqAeu3l8neBcOU<0E6qhnAu=PZ{%15`WjQT$4EA)+d{L<h`<2
zW?)N~pk7Cso8hv#2Pw!NInR-9vvR3`M!3PfDBGKvPv1Qnk!@siiiu|3sO_!!+|H#Q
zBMhZVNmE7v76j08BA6D@7zhkJwrQ|&!aZLDWKU>o;9Wzo`=ZgXU59zuwwH{@TIZc=
zR-#iLy~c9$I9#p`{-z9_I`@$|T21umhNGOZYGwe#uiiELiJSTH0Lw0ZOK5|_N8sbR
zEF4p!;cx6O@ddWx-bgSe^RU|UM=36>l?lYbHq4^<%}b92CX}$&v&|aR=75$<(Gu=^
znY%ZQ0M8-f+IA7B1WxA^k<8#C6ECo<`p-@7vG%7v9erdjOI4P16FLcFfIMvQ)POH)
zA$jZ7C`y6-OF^_LH3mL0h=@#_2ByocNsM%awaOX??P^kv4nJmqlj<ZzZ9<=%jl}MX
zA(lO*X5!(Y&H*36Am{!O>1K!VEo?VL*REv+8Qd>FJ#%T~Y?fDoX;TwsfsLY;XbMVw
z4rX$6rWB={B@||*%S{*iHkZG+x4pf+EC1e$EPdD|29)kJ2!R~w)fU#tWM_4`JXyxu
zTF-A;V~j=PMy*osSsteq1O|D5Qmrb~s!UgMSd>l;o~%R^;SG;zkmBHf<%(=l$`qV3
z&dlM2h%~TZu-wTyQrbmItXMS)0`%Grz=9IVu(56^yn+l+O1~EA;UaKN0Qo?_La7z|
zDSK&9J3*yP!=zSCS6@MEPdQjq8&nge@(_P}7v=2Hf39p>3@Qsy7aIC@SY`=nBCAAX
zOp>ogqt8yn!8|TewqE*gfcL-5LxyA6F$@TpPs}A4mjeldhFD=7-?F_r&skiA8D$Ku
zoFLJZ6oxTmLQZb1cU<?MB6-gepQ4Bdt|VrXMg3oJ9$V!woyLTb3kpI47YJxX$cvU~
zDFni1KFm>je5{&=+NOsR5TN!XMne|!sIX;;gDeTxv<b?_b5MjEh@AtWP`DeQ7Qr|N
zO?~o<kDV;1kbN>_2DbNmeKd+Q0ubwXTHUqZVt&;eAMVOeS`ioz<kL);@AaOKRal=t
zC3MO2PCRJKAZU9fs~6DQbZ9EDpzC1hku=gFE|2b!5^5zELL)@hxn(~+%l$@L6K^>`
z$vq0jZp!^<p<aI%l>WiAwd$HnG~${w-n;J;h``*#t(>y+x&@e~e*RixEWQ?;?b{hY
z3tq$3XXUnG^4<69+i1gnSUTw0kp)Kf=%XoTFJ=C7tKG5b^nI0Mi}VxPQ>-<Q%oZ-t
zFft%OJEMz8&XMOgY?hMkz%^7BCWg6;Yy?W3%`@|Azw3?l4m&WdlHU*P;^6XNIN$&{
z;O9G%1(AT{sAy7|;%H}Xy8LWZ`%ASAj>d(O6LU+68YCf+9Re+&Dw_815H&s&9))ag
z#@<TzO14EUyHL7|HwSTi`*wE}xfTL~@x)TUYcQiSB`q}&i!3JUlf*H;)fuXoGsyxP
z24k!s#=i(03J_DeL0QbHVI64`s>s68XBlV!Wo<ji8g^8tLqLwi#fA3&BI@}>Jk7(N
igT(b%hF-emOD7!&h)y8@P((08{x0N-aG@bs<RwmNjR|1@
literal 0
HcmV?d00001
diff --git a/gdb/testsuite/gdb.arch/cordic.ko.debug.bz2 b/gdb/testsuite/gdb.arch/cordic.ko.debug.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..8685f82fbce62d24fac020bbab43d1b930f27218
GIT binary patch
literal 910
zcmV;919AL9T4*^jL0KkKS+>*FwEzR>fB*mg|L0w6e^yWJenh|T|6r(7V8A3uV44O{
z3PAw~Ad<iX0mTDKMj9eDPf6-}O$|Lj(WWCn01W^GKxoj=0ig7N8fY|V4F-rr@=r+g
znMvr4pe7L0Lrn}u03$R4U?3V834lyA&_RFz00000000EQ0000+00LkDNRcL*38dPd
zr>W?snHWtx(rDVD>R<r%Ov)MzBV>Sj8i9!P7=XQf_tu%wQ66ijj|>8(N-SHcU`gj?
zA?pNyWnX!os)mt37+O-nD`DLvmYG%^2d>a+K_HCG#1uk}^RA}vP{T(SZFOygwjMJK
zLm^fMDTKp*U^cT1s-c#xVLPg~xVdOMr+`#M680nhi_eOl+yxXv12QTB091wuE@UI2
z00A@E_6}Pd`!ZYj#q@4;85%NIx(FL=Kxbah82{<x(W8Yk6pMca-nO1Hxu1p?+ocOh
zqN*VzlqC=m4Z>|Cq99Qa2pepz5D4T!3ABdB&}=Z{G?Egm#i`1L{S~7#rN<L~<A!EC
z4T2Jnb~8;fm`w$O>M*AiyZ1$)V4y=GAtUE>U}KUqGkkkHTYH|qP79y5chjMBS*bGV
z^<^SbXMK@5l4xtaWhDs-P{6#yH|veP2u;*A57@v73Y4=Tl2U>MBPEc`bA&k$t7d2n
zhoP$i3Rz1<PEDmKfzwyaaL<gyan3<rrt81ZHO=`uUxkvYHG^C<bZzX@ib{t$LegR!
z-bhc2Xs|MSG~NwzDes5QZx2HVG8tlkdj#+8@31w&YMB(8g>(EC*SrfsuPvX^F-dEM
z=q?abVBmxkfMhQ?6u~pi+SeI<0Zj{|h_`NeB579>XTrwO<KU%@QNNinNKx?-6mTUc
zj{XzOhLPv6D7E*PO0C=xhb+@(sth>-t}Ln7tkQS{9T6=|0>7b?!(kHTg@6+Ylf@96
zxK+#+39rc>&`}mGUARXS-w!w#R#z`nZ82`R_$84&<y9%jTL-UqBPE6OT^0*x0nvKR
z{J9fbfq0o=*|Mo5NR{Jq?UtAhd{!8OH2^0Dweln=p^+d4u!o5ctwu>CvQ+wn#VMg^
zQyfw*r4D){23+f$10yqu2tl$W#vlrxK`=@IhJh2LC}3&NO$i_Z(Jup~Nfg-i9~kn3
krH211sj3TA8HyMfOUhbY@Ke(~!GDXnBAh5lTWRXrfWMrBLI3~&
literal 0
HcmV?d00001
diff --git a/gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp b/gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp
new file mode 100644
index 0000000..99184b6
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/ppc64-symtab-cordic.exp
@@ -0,0 +1,49 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+set kobz2file ${srcdir}/${subdir}/cordic.ko.bz2
+set kofile ${objdir}/${subdir}/cordic.ko
+set kodebugbz2file ${srcdir}/${subdir}/cordic.ko.debug.bz2
+set kodebugfile ${objdir}/${subdir}/cordic.ko.debug
+
+if {[catch "system \"bzip2 -dc ${kobz2file} >${kofile}\""] != 0} {
+ untested "failed bzip2 for ${kobz2file}"
+ return -1
+}
+if {[catch "system \"bzip2 -dc ${kodebugbz2file} >${kodebugfile}\""] != 0} {
+ untested "failed bzip2 for ${kodebugbz2file}"
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# This test won't work properly if system debuginfo is installed.
+gdb_test_no_output "set debug-file-directory" ""
+
+gdb_load ${kofile}
+
+set test "show architecture"
+gdb_test_multiple $test $test {
+ -re "\r\nThe target architecture is set automatically \\(currently powerpc:common64\\)\r\n$gdb_prompt $" {
+ pass $test
+ }
+ -re "\r\nThe target architecture is set automatically \\(currently .*\\)\r\n$gdb_prompt $" {
+ untested "powerpc:common64 is not supported"
+ }
+}