* [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
@ 2014-01-04 16:23 Hui Zhu
2014-01-05 15:06 ` Sergio Durigan Junior
0 siblings, 1 reply; 12+ messages in thread
From: Hui Zhu @ 2014-01-04 16:23 UTC (permalink / raw)
To: gdb-patches ml
Got double free or corruption with new GDB:
(gdb) r
Starting program: /home/teawater/tmp/a.out
*** glibc detected *** /home/teawater/gdb/git/bgdbno/gdb/gdb: double free or corruption (out): 0x00000000011ed4d0 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7ffff65f5b96]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x7c4c95]
/home/teawater/gdb/git/bgdbno/gdb/gdb(bfd_close+0xb3)[0x7c5683]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x651d0b]
/home/teawater/gdb/git/bgdbno/gdb/gdb(gdb_bfd_unref+0x146)[0x651fb3]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x5f7e2d]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x5f80a0]
/home/teawater/gdb/git/bgdbno/gdb/gdb(find_separate_debug_file_by_debuglink+0x18e)[0x5f85cd]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x563ee9]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x5f7206]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x5f76f5]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x5f773d]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x5f792d]
/home/teawater/gdb/git/bgdbno/gdb/gdb(symbol_file_add_from_bfd+0x43)[0x5f7b23]
/home/teawater/gdb/git/bgdbno/gdb/gdb(solib_read_symbols+0x173)[0x753fdb]
/home/teawater/gdb/git/bgdbno/gdb/gdb(solib_add+0x15c)[0x75468a]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x48f824]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x490bf2]
/home/teawater/gdb/git/bgdbno/gdb/gdb(solib_create_inferior_hook+0x2b)[0x754cd6]
/home/teawater/gdb/git/bgdbno/gdb/gdb(post_create_inferior+0xbf)[0x60b8ed]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x60bc92]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x60bcec]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x51caab]
/home/teawater/gdb/git/bgdbno/gdb/gdb(cmd_func+0x3f)[0x51fd40]
/home/teawater/gdb/git/bgdbno/gdb/gdb(execute_command+0x293)[0x736569]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x636ad6]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x63713f]
/home/teawater/gdb/git/bgdbno/gdb/gdb(rl_callback_read_char+0x341)[0x792534]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x636569]
/home/teawater/gdb/git/bgdbno/gdb/gdb(stdin_event_handler+0x78)[0x6369ed]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x6354f5]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x6349bb]
/home/teawater/gdb/git/bgdbno/gdb/gdb(gdb_do_one_event+0xb9)[0x634a82]
/home/teawater/gdb/git/bgdbno/gdb/gdb(start_event_loop+0x3f)[0x634ad3]
/home/teawater/gdb/git/bgdbno/gdb/gdb(cli_command_loop+0x1b)[0x63659b]
/home/teawater/gdb/git/bgdbno/gdb/gdb(current_interp_command_loop+0x5b)[0x62cdc2]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x62d823]
/home/teawater/gdb/git/bgdbno/gdb/gdb(catch_errors+0x5f)[0x62b16d]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x62ec36]
/home/teawater/gdb/git/bgdbno/gdb/gdb(catch_errors+0x5f)[0x62b16d]
/home/teawater/gdb/git/bgdbno/gdb/gdb(gdb_main+0x34)[0x62ec6c]
/home/teawater/gdb/git/bgdbno/gdb/gdb(main+0x5b)[0x45d63f]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7ffff659876d]
/home/teawater/gdb/git/bgdbno/gdb/gdb[0x45d529]
======= Memory map: ========
00400000-00bf1000 r-xp 00000000 08:07 10618880 /home/teawater/gdb/git/bgdbno/gdb/gdb
00df0000-00df1000 r--p 007f0000 08:07 10618880 /home/teawater/gdb/git/bgdbno/gdb/gdb
00df1000-00e10000 rw-p 007f1000 08:07 10618880 /home/teawater/gdb/git/bgdbno/gdb/gdb
00e10000-011fc000 rw-p 00000000 00:00 0 [heap]
7ffff54b0000-7ffff54b3000 r-xp 00000000 08:05 2504246 /usr/lib/python2.7/lib-dynload/_heapq.so
7ffff54b3000-7ffff56b2000 ---p 00003000 08:05 2504246 /usr/lib/python2.7/lib-dynload/_heapq.so
7ffff56b2000-7ffff56b3000 r--p 00002000 08:05 2504246 /usr/lib/python2.7/lib-dynload/_heapq.so
7ffff56b3000-7ffff56b5000 rw-p 00003000 08:05 2504246 /usr/lib/python2.7/lib-dynload/_heapq.so
7ffff56b5000-7ffff5b25000 r--p 00000000 08:05 2500039 /usr/lib/locale/locale-archive
7ffff5b25000-7ffff5b3a000 r-xp 00000000 08:05 268475 /lib/x86_64-linux-gnu/libgcc_s.so.1
7ffff5b3a000-7ffff5d39000 ---p 00015000 08:05 268475 /lib/x86_64-linux-gnu/libgcc_s.so.1
7ffff5d39000-7ffff5d3a000 r--p 00014000 08:05 268475 /lib/x86_64-linux-gnu/libgcc_s.so.1
7ffff5d3a000-7ffff5d3b000 rw-p 00015000 08:05 268475 /lib/x86_64-linux-gnu/libgcc_s.so.1
7ffff5d3b000-7ffff5d3d000 r-xp 00000000 08:05 263571 /lib/x86_64-linux-gnu/libutil-2.15.so
7ffff5d3d000-7ffff5f3c000 ---p 00002000 08:05 263571 /lib/x86_64-linux-gnu/libutil-2.15.so
7ffff5f3c000-7ffff5f3d000 r--p 00001000 08:05 263571 /lib/x86_64-linux-gnu/libutil-2.15.so
7ffff5f3d000-7ffff5f3e000 rw-p 00002000 08:05 263571 /lib/x86_64-linux-gnu/libutil-2.15.so
7ffff5f3e000-7ffff60ef000 r-xp 00000000 08:05 274377 /lib/x86_64-linux-gnu/libcrypto.so.1.0.0
7ffff60ef000-7ffff62ef000 ---p 001b1000 08:05 274377 /lib/x86_64-linux-gnu/libcrypto.so.1.0.0
7ffff62ef000-7ffff630a000 r--p 001b1000 08:05 274377 /lib/x86_64-linux-gnu/libcrypto.so.1.0.0
7ffff630a000-7ffff6315000 rw-p 001cc000 08:05 274377 /lib/x86_64-linux-gnu/libcrypto.so.1.0.0
7ffff6315000-7ffff6319000 rw-p 00000000 00:00 0
7ffff6319000-7ffff636d000 r-xp 00000000 08:05 274381 /lib/x86_64-linux-gnu/libssl.so.1.0.0
7ffff636d000-7ffff656d000 ---p 00054000 08:05 274381 /lib/x86_64-linux-gnu/libssl.so.1.0.0
7ffff656d000-7ffff6570000 r--p 00054000 08:05 274381 /lib/x86_64-linux-gnu/libssl.so.1.0.0
7ffff6570000-7ffff6576000 rw-p 00057000 08:05 274381 /lib/x86_64-linux-gnu/libssl.so.1.0.0
7ffff6576000-7ffff6577000 rw-p 00000000 00:00 0
7ffff6577000-7ffff672c000 r-xp 00000000 08:05 263572 /lib/x86_64-linux-gnu/libc-2.15.so
7ffff672c000-7ffff692c000 ---p 001b5000 08:05 263572 /lib/x86_64-linux-gnu/libc-2.15.so
7ffff692c000-7ffff6930000 r--p 001b5000 08:05 263572 /lib/x86_64-linux-gnu/libc-2.15.so
7ffff6930000-7ffff6932000 rw-p 001b9000 08:05 263572 /lib/x86_64-linux-gnu/libc-2.15.so
7ffff6932000-7ffff6937000 rw-p 00000000 00:00 0
7ffff6937000-7ffff695e000 r-xp 00000000 08:05 262183 /lib/x86_64-linux-gnu/libexpat.so.1.5.2
7ffff695e000-7ffff6b5e000 ---p 00027000 08:05 262183 /lib/x86_64-linux-gnu/libexpat.so.1.5.2
7ffff6b5e000-7ffff6b60000 r--p 00027000 08:05 262183 /lib/x86_64-linux-gnu/libexpat.so.1.5.2
7ffff6b60000-7ffff6b61000 rw-p 00029000 08:05 262183 /lib/x86_64-linux-gnu/libexpat.so.1.5.2
7ffff6b61000-7ffff6de2000 r-xp 00000000 08:05 2491217 /usr/lib/libpython2.7.so.1.0
7ffff6de2000-7ffff6fe1000 ---p 00281000 08:05 2491217 /usr/lib/libpython2.7.so.1.0
7ffff6fe1000-7ffff6fe3000 r--p 00280000 08:05 2491217 /usr/lib/libpython2.7.so.1.0
7ffff6fe3000-7ffff704c000 rw-p 00282000 08:05 2491217 /usr/lib/libpython2.7.so.1.0
7ffff704c000-7ffff705e000 rw-p 00000000 00:00 0
7ffff705e000-7ffff7076000 r-xp 00000000 08:05 269928 /lib/x86_64-linux-gnu/libpthread-2.15.so
7ffff7076000-7ffff7275000 ---p 00018000 08:05 269928 /lib/x86_64-linux-gnu/libpthread-2.15.so
7ffff7275000-7ffff7276000 r--p 00017000 08:05 269928 /lib/x86_64-linux-gnu/libpthread-2.15.so
7ffff7276000-7ffff7277000 rw-p 00018000 08:05 269928 /lib/x86_64-linux-gnu/libpthread-2.15.so
7ffff7277000-7ffff727b000 rw-p 00000000 00:00 0
7ffff727b000-7ffff7376000 r-xp 00000000 08:05 269930 /lib/x86_64-linux-gnu/libm-2.15.so
7ffff7376000-7ffff7575000 ---p 000fb000 08:05 269930 /lib/x86_64-linux-gnu/libm-2.15.so
7ffff7575000-7ffff7576000 r--p 000fa000 08:05 269930 /lib/x86_64-linux-gnu/libm-2.15.so
7ffff7576000-7ffff7577000 rw-p 000fb000 08:05 269930 /lib/x86_64-linux-gnu/libm-2.15.so
7ffff7577000-7ffff758d000 r-xp 00000000 08:05 268782 /lib/x86_64-linux-gnu/libz.so.1.2.3.4
7ffff758d000-7ffff778c000 ---p 00016000 08:05 268782 /lib/x86_64-linux-gnu/libz.so.1.2.3.4
7ffff778c000-7ffff778d000 r--p 00015000 08:05 268782 /lib/x86_64-linux-gnu/libz.so.1.2.3.4
7ffff778d000-7ffff778e000 rw-p 00016000 08:05 268782 /lib/x86_64-linux-gnu/libz.so.1.2.3.4
7ffff778e000-7ffff77b0000 r-xp 00000000 08:05 262294 /lib/x86_64-linux-gnu/libtinfo.so.5.9
7ffff77b0000-7ffff79b0000 ---p 00022000 08:05 262294 /lib/x86_64-linux-gnu/libtinfo.so.5.9
7ffff79b0000-7ffff79b4000 r--p 00022000 08:05 262294 /lib/x86_64-linux-gnu/libtinfo.so.5.9
7ffff79b4000-7ffff79b5000 rw-p 00026000 08:05 262294 /lib/x86_64-linux-gnu/libtinfo.so.5.9
7ffff79b5000-7ffff79d4000 r-xp 00000000 08:05 263652 /lib/x86_64-linux-gnu/libncurses.so.5.9
7ffff79d4000-7ffff7bd4000 ---p 0001f000 08:05 263652 /lib/x86_64-linux-gnu/libncurses.so.5.9
7ffff7bd4000-7ffff7bd5000 r--p 0001f000 08:05 263652 /lib/x86_64-linux-gnu/libncurses.so.5.9
7ffff7bd5000-7ffff7bd6000 rw-p 00020000 08:05 263652 /lib/x86_64-linux-gnu/libncurses.so.5.9
7ffff7bd6000-7ffff7bd8000 r-xp 00000000 08:05 275939 /lib/x86_64-linux-gnu/libdl-2.15.so
7ffff7bd8000-7ffff7dd8000 ---p 00002000 08:05 275939 /lib/x86_64-linux-gnu/libdl-2.15.so
7ffff7dd8000-7ffff7dd9000 r--p 00002000 08:05 275939 /lib/x86_64-linux-gnu/libdl-2.15.so
7ffff7dd9000-7ffff7dda000 rw-p 00003000 08:05 275939 /lib/x86_64-linux-gnu/libdl-2.15.so
7ffff7dda000-7ffff7dfc000 r-xp 00000000 08:05 269931 /lib/x86_64-linux-gnu/ld-2.15.so
7ffff7f10000-7ffff7f41000 rw-p 00000000 00:00 0
7ffff7f42000-7ffff7fcc000 rw-p 00000000 00:00 0
7ffff7fd2000-7ffff7fd8000 rw-p 00000000 00:00 0
7ffff7fd8000-7ffff7fed000 r--p 00000000 08:05 2915761 /usr/share/locale-langpack/zh_CN/LC_MESSAGES/libc.mo
7ffff7fed000-7ffff7ff4000 r--s 00000000 08:05 2516792 /usr/lib/x86_64-linux-gnu/gconv/gconv-modules.cache
7ffff7ff4000-7ffff7ff9000 r--p 00000000 08:05 2910407 /usr/share/locale-langpack/zh_CN/LC_MESSAGES/gdb.mo
7ffff7ff9000-7ffff7ffb000 rw-p 00000000 00:00 0
7ffff7ffb000-7ffff7ffc000 r-xp 00000000 00:00 0 [vdso]
7ffff7ffc000-7ffff7ffd000 r--p 00022000 08:05 269931 /lib/x86_64-linux-gnu/ld-2.15.so
7ffff7ffd000-7ffff7fff000 rw-p 00023000 08:05 269931 /lib/x86_64-linux-gnu/ld-2.15.so
7ffffffde000-7ffffffff000 rw-p 00000000 00:00 0 [stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
Program received signal SIGABRT, Aborted.
0x00007ffff65ad425 in raise () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
#0 0x00007ffff65ad425 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x00007ffff65b0b8b in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2 0x00007ffff65eb39e in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#3 0x00007ffff65f5b96 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#4 0x00000000007c4c95 in _bfd_delete_bfd (abfd=0x11ebef0) at ../../binutils-gdb/bfd/opncls.c:127
#5 0x00000000007c5683 in bfd_close (abfd=0x11ebef0) at ../../binutils-gdb/bfd/opncls.c:743
#6 0x0000000000651d0b in gdb_bfd_close_or_warn (abfd=0x11ebef0) at ../../binutils-gdb/gdb/gdb_bfd.c:246
#7 0x0000000000651fb3 in gdb_bfd_unref (abfd=0x11ebef0) at ../../binutils-gdb/gdb/gdb_bfd.c:331
#8 0x00000000005f7e2d in separate_debug_file_exists (name=0x11ebe80 "/lib/x86_64-linux-gnu/ld-2.15.so", crc=2945490929,
parent_objfile=0x11e19c0) at ../../binutils-gdb/gdb/symfile.c:1329
#9 0x00000000005f80a0 in find_separate_debug_file (dir=0x11ec070 "/lib/x86_64-linux-gnu/",
canon_dir=0x11ec070 "/lib/x86_64-linux-gnu/", debuglink=0x11ec370 "ld-2.15.so", crc32=2945490929, objfile=0x11e19c0)
at ../../binutils-gdb/gdb/symfile.c:1420
#10 0x00000000005f85cd in find_separate_debug_file_by_debuglink (objfile=0x11e19c0)
at ../../binutils-gdb/gdb/symfile.c:1549
#11 0x0000000000563ee9 in elf_symfile_read (objfile=0x11e19c0, symfile_flags=8) at ../../binutils-gdb/gdb/elfread.c:1324
#12 0x00000000005f7206 in read_symbols (objfile=0x11e19c0, add_flags=8) at ../../binutils-gdb/gdb/symfile.c:839
#13 0x00000000005f76f5 in syms_from_objfile_1 (objfile=0x11e19c0, addrs=0x11c3970, add_flags=8)
at ../../binutils-gdb/gdb/symfile.c:1014
#14 0x00000000005f773d in syms_from_objfile (objfile=0x11e19c0, addrs=0x11c3970, add_flags=8)
at ../../binutils-gdb/gdb/symfile.c:1030
#15 0x00000000005f792d in symbol_file_add_with_addrs (abfd=0x1171e60, name=0x11e0db0 "/lib64/ld-linux-x86-64.so.2",
add_flags=8, addrs=0x11c3970, flags=2, parent=0x0) at ../../binutils-gdb/gdb/symfile.c:1127
#16 0x00000000005f7b23 in symbol_file_add_from_bfd (abfd=0x1171e60, name=0x11e0db0 "/lib64/ld-linux-x86-64.so.2",
add_flags=8, addrs=0x11c3970, flags=2, parent=0x0) at ../../binutils-gdb/gdb/symfile.c:1216
#17 0x0000000000753fdb in solib_read_symbols (so=0x11e0ba0, flags=8) at ../../binutils-gdb/gdb/solib.c:640
#18 0x000000000075468a in solib_add (pattern=0x0, from_tty=0, target=0xe2f360 <current_target>, readsyms=1)
at ../../binutils-gdb/gdb/solib.c:951
#19 0x000000000048f824 in enable_break (info=0x1154ec0, from_tty=0) at ../../binutils-gdb/gdb/solib-svr4.c:2282
---Type <return> to continue, or q <return> to quit---q
The reason is when GDB open bfd, it will call gdb_bfd_stash_filename to the memory of abfd.
But in binutils/11983, it add "(_bfd_delete_bfd): Free filename." So when gdb try to close the bfd, it will crash.
I make a patch to remove all gdb_bfd_stash_filename to fix this issue.
Thanks,
Hui
2014-01-05 Hui Zhu <hui@codesourcery.com>
* gdb_bfd.c (gdb_bfd_stash_filename): Removed.
(gdb_bfd_open): Removed gdb_bfd_stash_filename.
(gdb_bfd_fopen): Ditto.
(gdb_bfd_openr): Ditto.
(gdb_bfd_openw): Ditto.
(gdb_bfd_openr_iovec): Ditto.
(gdb_bfd_fdopenr): Ditto.
* gdb_bfd.h (gdb_bfd_stash_filename): Removed.
* symfile-mem.c (symbol_file_add_from_memory): Removed
gdb_bfd_stash_filename.
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -57,21 +57,6 @@ struct gdb_bfd_section_data
static htab_t all_bfds;
-/* See gdb_bfd.h. */
-
-void
-gdb_bfd_stash_filename (struct bfd *abfd)
-{
- char *name = bfd_get_filename (abfd);
- char *data;
-
- data = bfd_alloc (abfd, strlen (name) + 1);
- strcpy (data, name);
-
- /* Unwarranted chumminess with BFD. */
- abfd->filename = data;
-}
-
/* An object of this type is stored in each BFD's user data. */
struct gdb_bfd_data
@@ -204,7 +189,6 @@ gdb_bfd_open (const char *name, const ch
gdb_assert (!*slot);
*slot = abfd;
- gdb_bfd_stash_filename (abfd);
gdb_bfd_ref (abfd);
return abfd;
}
@@ -490,10 +474,7 @@ gdb_bfd_fopen (const char *filename, con
bfd *result = bfd_fopen (filename, target, mode, fd);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -506,10 +487,7 @@ gdb_bfd_openr (const char *filename, con
bfd *result = bfd_openr (filename, target);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -522,10 +500,7 @@ gdb_bfd_openw (const char *filename, con
bfd *result = bfd_openw (filename, target);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -553,10 +528,7 @@ gdb_bfd_openr_iovec (const char *filenam
pread_func, close_func, stat_func);
if (result)
- {
- gdb_bfd_ref (result);
- gdb_bfd_stash_filename (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -603,10 +575,7 @@ gdb_bfd_fdopenr (const char *filename, c
bfd *result = bfd_fdopenr (filename, target, fd);
if (result)
- {
- gdb_bfd_ref (result);
- gdb_bfd_stash_filename (result);
- }
+ gdb_bfd_ref (result);
return result;
}
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -24,12 +24,6 @@
DECLARE_REGISTRY (bfd);
-/* Make a copy ABFD's filename using bfd_alloc, and reassign it to the
- BFD. This ensures that the BFD's filename has the same lifetime as
- the BFD itself. */
-
-void gdb_bfd_stash_filename (struct bfd *abfd);
-
/* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
Returns NULL on error. On success, returns a new reference to the
BFD, which must be freed with gdb_bfd_unref. BFDs returned by this
@@ -79,22 +73,22 @@ int gdb_bfd_crc (struct bfd *abfd, unsig
\f
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_fopen (const char *, const char *, const char *, int);
/* A wrapper for bfd_openr that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_openr (const char *, const char *);
/* A wrapper for bfd_openw that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_openw (const char *, const char *);
/* A wrapper for bfd_openr_iovec that initializes the gdb-specific
- reference count and calls gdb_bfd_stash_filename. */
+ reference count. */
bfd *gdb_bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
@@ -112,12 +106,12 @@ bfd *gdb_bfd_openr_iovec (const char *fi
struct stat *sb));
/* A wrapper for bfd_openr_next_archived_file that initializes the
- gdb-specific reference count and calls gdb_bfd_stash_filename. */
+ gdb-specific reference count. */
bfd *gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous);
/* A wrapper for bfd_fdopenr that initializes the gdb-specific
- reference count and calls gdb_bfd_stash_filename. */
+ reference count. */
bfd *gdb_bfd_fdopenr (const char *filename, const char *target, int fd);
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
if (name == NULL)
nbfd->filename = "shared object read from target memory";
else
- {
- nbfd->filename = name;
- gdb_bfd_stash_filename (nbfd);
- xfree (name);
- }
+ nbfd->filename = name;
cleanup = make_cleanup_bfd_unref (nbfd);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-04 16:23 [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983 Hui Zhu
@ 2014-01-05 15:06 ` Sergio Durigan Junior
2014-01-05 15:48 ` Hui Zhu
0 siblings, 1 reply; 12+ messages in thread
From: Sergio Durigan Junior @ 2014-01-05 15:06 UTC (permalink / raw)
To: Hui Zhu
Cc: gdb-patches ml, Edjunior Barbosa Machado, Nick Clifton, Tom Tromey
On Saturday, January 04 2014, Hui Zhu wrote:
> Got double free or corruption with new GDB:
> (gdb) r
> Starting program: /home/teawater/tmp/a.out
> *** glibc detected *** /home/teawater/gdb/git/bgdbno/gdb/gdb: double free or corruption (out): 0x00000000011ed4d0 ***
> ======= Backtrace: =========
> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7ffff65f5b96]
[...]
> The reason is when GDB open bfd, it will call gdb_bfd_stash_filename to the memory of abfd.
> But in binutils/11983, it add "(_bfd_delete_bfd): Free filename." So when gdb try to close the bfd, it will crash.
>
> I make a patch to remove all gdb_bfd_stash_filename to fix this issue.
Hm, given what Tom said on
<https://sourceware.org/bugzilla/show_bug.cgi?id=11983#c12>, I believe
this is the right way to solve the problem.
Edjunior posted a patch a few days ago that duplicated the name. I'm
adding him on the Cc. I'm also adding Nick and Tom.
BTW, Hui, your patch seems to have been mangled by your MUA.
Thanks,
> 2014-01-05 Hui Zhu <hui@codesourcery.com>
>
> * gdb_bfd.c (gdb_bfd_stash_filename): Removed.
> (gdb_bfd_open): Removed gdb_bfd_stash_filename.
> (gdb_bfd_fopen): Ditto.
> (gdb_bfd_openr): Ditto.
> (gdb_bfd_openw): Ditto.
> (gdb_bfd_openr_iovec): Ditto.
> (gdb_bfd_fdopenr): Ditto.
> * gdb_bfd.h (gdb_bfd_stash_filename): Removed.
> * symfile-mem.c (symbol_file_add_from_memory): Removed
> gdb_bfd_stash_filename.
>
> --- a/gdb/gdb_bfd.c
> +++ b/gdb/gdb_bfd.c
> @@ -57,21 +57,6 @@ struct gdb_bfd_section_data
> static htab_t all_bfds;
> -/* See gdb_bfd.h. */
> -
> -void
> -gdb_bfd_stash_filename (struct bfd *abfd)
> -{
> - char *name = bfd_get_filename (abfd);
> - char *data;
> -
> - data = bfd_alloc (abfd, strlen (name) + 1);
> - strcpy (data, name);
> -
> - /* Unwarranted chumminess with BFD. */
> - abfd->filename = data;
> -}
> -
> /* An object of this type is stored in each BFD's user data. */
> struct gdb_bfd_data
> @@ -204,7 +189,6 @@ gdb_bfd_open (const char *name, const ch
> gdb_assert (!*slot);
> *slot = abfd;
> - gdb_bfd_stash_filename (abfd);
> gdb_bfd_ref (abfd);
> return abfd;
> }
> @@ -490,10 +474,7 @@ gdb_bfd_fopen (const char *filename, con
> bfd *result = bfd_fopen (filename, target, mode, fd);
> if (result)
> - {
> - gdb_bfd_stash_filename (result);
> - gdb_bfd_ref (result);
> - }
> + gdb_bfd_ref (result);
> return result;
> }
> @@ -506,10 +487,7 @@ gdb_bfd_openr (const char *filename, con
> bfd *result = bfd_openr (filename, target);
> if (result)
> - {
> - gdb_bfd_stash_filename (result);
> - gdb_bfd_ref (result);
> - }
> + gdb_bfd_ref (result);
> return result;
> }
> @@ -522,10 +500,7 @@ gdb_bfd_openw (const char *filename, con
> bfd *result = bfd_openw (filename, target);
> if (result)
> - {
> - gdb_bfd_stash_filename (result);
> - gdb_bfd_ref (result);
> - }
> + gdb_bfd_ref (result);
> return result;
> }
> @@ -553,10 +528,7 @@ gdb_bfd_openr_iovec (const char *filenam
> pread_func, close_func, stat_func);
> if (result)
> - {
> - gdb_bfd_ref (result);
> - gdb_bfd_stash_filename (result);
> - }
> + gdb_bfd_ref (result);
> return result;
> }
> @@ -603,10 +575,7 @@ gdb_bfd_fdopenr (const char *filename, c
> bfd *result = bfd_fdopenr (filename, target, fd);
> if (result)
> - {
> - gdb_bfd_ref (result);
> - gdb_bfd_stash_filename (result);
> - }
> + gdb_bfd_ref (result);
> return result;
> }
> --- a/gdb/gdb_bfd.h
> +++ b/gdb/gdb_bfd.h
> @@ -24,12 +24,6 @@
> DECLARE_REGISTRY (bfd);
> -/* Make a copy ABFD's filename using bfd_alloc, and reassign it to
> the
> - BFD. This ensures that the BFD's filename has the same lifetime as
> - the BFD itself. */
> -
> -void gdb_bfd_stash_filename (struct bfd *abfd);
> -
> /* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
> Returns NULL on error. On success, returns a new reference to the
> BFD, which must be freed with gdb_bfd_unref. BFDs returned by this
> @@ -79,22 +73,22 @@ int gdb_bfd_crc (struct bfd *abfd, unsig
> \f
> /* A wrapper for bfd_fopen that initializes the gdb-specific
> reference
> - count and calls gdb_bfd_stash_filename. */
> + count. */
> bfd *gdb_bfd_fopen (const char *, const char *, const char *, int);
> /* A wrapper for bfd_openr that initializes the gdb-specific
> reference
> - count and calls gdb_bfd_stash_filename. */
> + count. */
> bfd *gdb_bfd_openr (const char *, const char *);
> /* A wrapper for bfd_openw that initializes the gdb-specific
> reference
> - count and calls gdb_bfd_stash_filename. */
> + count. */
> bfd *gdb_bfd_openw (const char *, const char *);
> /* A wrapper for bfd_openr_iovec that initializes the gdb-specific
> - reference count and calls gdb_bfd_stash_filename. */
> + reference count. */
> bfd *gdb_bfd_openr_iovec (const char *filename, const char *target,
> void *(*open_func) (struct bfd *nbfd,
> @@ -112,12 +106,12 @@ bfd *gdb_bfd_openr_iovec (const char *fi
> struct stat *sb));
> /* A wrapper for bfd_openr_next_archived_file that initializes the
> - gdb-specific reference count and calls gdb_bfd_stash_filename. */
> + gdb-specific reference count. */
> bfd *gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous);
> /* A wrapper for bfd_fdopenr that initializes the gdb-specific
> - reference count and calls gdb_bfd_stash_filename. */
> + reference count. */
> bfd *gdb_bfd_fdopenr (const char *filename, const char *target, int
> fd);
> --- a/gdb/symfile-mem.c
> +++ b/gdb/symfile-mem.c
> @@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
> if (name == NULL)
> nbfd->filename = "shared object read from target memory";
> else
> - {
> - nbfd->filename = name;
> - gdb_bfd_stash_filename (nbfd);
> - xfree (name);
> - }
> + nbfd->filename = name;
> cleanup = make_cleanup_bfd_unref (nbfd);
--
Sergio
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-05 15:06 ` Sergio Durigan Junior
@ 2014-01-05 15:48 ` Hui Zhu
2014-01-06 8:25 ` Tom Tromey
0 siblings, 1 reply; 12+ messages in thread
From: Hui Zhu @ 2014-01-05 15:48 UTC (permalink / raw)
To: Sergio Durigan Junior
Cc: gdb-patches ml, Edjunior Barbosa Machado, Nick Clifton, Tom Tromey
On 01/05/14 23:05, Sergio Durigan Junior wrote:
> On Saturday, January 04 2014, Hui Zhu wrote:
>
>> Got double free or corruption with new GDB:
>> (gdb) r
>> Starting program: /home/teawater/tmp/a.out
>> *** glibc detected *** /home/teawater/gdb/git/bgdbno/gdb/gdb: double free or corruption (out): 0x00000000011ed4d0 ***
>> ======= Backtrace: =========
>> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7ffff65f5b96]
>
> [...]
>
>> The reason is when GDB open bfd, it will call gdb_bfd_stash_filename to the memory of abfd.
>> But in binutils/11983, it add "(_bfd_delete_bfd): Free filename." So when gdb try to close the bfd, it will crash.
>>
>> I make a patch to remove all gdb_bfd_stash_filename to fix this issue.
>
> Hm, given what Tom said on
> <https://sourceware.org/bugzilla/show_bug.cgi?id=11983#c12>, I believe
> this is the right way to solve the problem.
>
> Edjunior posted a patch a few days ago that duplicated the name. I'm
> adding him on the Cc. I'm also adding Nick and Tom.
>
> BTW, Hui, your patch seems to have been mangled by your MUA.
Thanks. Post a new version.
Best,
Hui
2014-01-05 Hui Zhu <hui@codesourcery.com>
* gdb_bfd.c (gdb_bfd_stash_filename): Removed.
(gdb_bfd_open): Removed gdb_bfd_stash_filename.
(gdb_bfd_fopen): Ditto.
(gdb_bfd_openr): Ditto.
(gdb_bfd_openw): Ditto.
(gdb_bfd_openr_iovec): Ditto.
(gdb_bfd_fdopenr): Ditto.
* gdb_bfd.h (gdb_bfd_stash_filename): Removed.
* symfile-mem.c (symbol_file_add_from_memory): Removed
gdb_bfd_stash_filename.
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -57,21 +57,6 @@ struct gdb_bfd_section_data
static htab_t all_bfds;
-/* See gdb_bfd.h. */
-
-void
-gdb_bfd_stash_filename (struct bfd *abfd)
-{
- char *name = bfd_get_filename (abfd);
- char *data;
-
- data = bfd_alloc (abfd, strlen (name) + 1);
- strcpy (data, name);
-
- /* Unwarranted chumminess with BFD. */
- abfd->filename = data;
-}
-
/* An object of this type is stored in each BFD's user data. */
struct gdb_bfd_data
@@ -204,7 +189,6 @@ gdb_bfd_open (const char *name, const ch
gdb_assert (!*slot);
*slot = abfd;
- gdb_bfd_stash_filename (abfd);
gdb_bfd_ref (abfd);
return abfd;
}
@@ -490,10 +474,7 @@ gdb_bfd_fopen (const char *filename, con
bfd *result = bfd_fopen (filename, target, mode, fd);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -506,10 +487,7 @@ gdb_bfd_openr (const char *filename, con
bfd *result = bfd_openr (filename, target);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -522,10 +500,7 @@ gdb_bfd_openw (const char *filename, con
bfd *result = bfd_openw (filename, target);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -553,10 +528,7 @@ gdb_bfd_openr_iovec (const char *filenam
pread_func, close_func, stat_func);
if (result)
- {
- gdb_bfd_ref (result);
- gdb_bfd_stash_filename (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -603,10 +575,7 @@ gdb_bfd_fdopenr (const char *filename, c
bfd *result = bfd_fdopenr (filename, target, fd);
if (result)
- {
- gdb_bfd_ref (result);
- gdb_bfd_stash_filename (result);
- }
+ gdb_bfd_ref (result);
return result;
}
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -24,12 +24,6 @@
DECLARE_REGISTRY (bfd);
-/* Make a copy ABFD's filename using bfd_alloc, and reassign it to the
- BFD. This ensures that the BFD's filename has the same lifetime as
- the BFD itself. */
-
-void gdb_bfd_stash_filename (struct bfd *abfd);
-
/* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
Returns NULL on error. On success, returns a new reference to the
BFD, which must be freed with gdb_bfd_unref. BFDs returned by this
@@ -79,22 +73,22 @@ int gdb_bfd_crc (struct bfd *abfd, unsig
\f
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_fopen (const char *, const char *, const char *, int);
/* A wrapper for bfd_openr that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_openr (const char *, const char *);
/* A wrapper for bfd_openw that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_openw (const char *, const char *);
/* A wrapper for bfd_openr_iovec that initializes the gdb-specific
- reference count and calls gdb_bfd_stash_filename. */
+ reference count. */
bfd *gdb_bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
@@ -112,12 +106,12 @@ bfd *gdb_bfd_openr_iovec (const char *fi
struct stat *sb));
/* A wrapper for bfd_openr_next_archived_file that initializes the
- gdb-specific reference count and calls gdb_bfd_stash_filename. */
+ gdb-specific reference count. */
bfd *gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous);
/* A wrapper for bfd_fdopenr that initializes the gdb-specific
- reference count and calls gdb_bfd_stash_filename. */
+ reference count. */
bfd *gdb_bfd_fdopenr (const char *filename, const char *target, int fd);
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
if (name == NULL)
nbfd->filename = "shared object read from target memory";
else
- {
- nbfd->filename = name;
- gdb_bfd_stash_filename (nbfd);
- xfree (name);
- }
+ nbfd->filename = name;
cleanup = make_cleanup_bfd_unref (nbfd);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-05 15:48 ` Hui Zhu
@ 2014-01-06 8:25 ` Tom Tromey
2014-01-06 10:50 ` Hui Zhu
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 8:25 UTC (permalink / raw)
To: Hui Zhu
Cc: Sergio Durigan Junior, gdb-patches ml, Edjunior Barbosa Machado,
Nick Clifton
>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
Hui> Thanks. Post a new version.
Thanks Hui. This is definitely the direction I think the code should
go.
Hui> --- a/gdb/symfile-mem.c
Hui> +++ b/gdb/symfile-mem.c
Hui> @@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
Hui> if (name == NULL)
Hui> nbfd-> filename = "shared object read from target memory";
Hui> else
Hui> - {
Hui> - nbfd->filename = name;
Hui> - gdb_bfd_stash_filename (nbfd);
Hui> - xfree (name);
Hui> - }
Hui> + nbfd->filename = name;
Hui> cleanup = make_cleanup_bfd_unref (nbfd);
In this hunk there are two things to note.
First, there is an earlier assignment to filename (in the context above)
that should use xstrdup.
Second, the new assignment really ought to free the old nbfd->filename
first.
There are also some assignments that must be fixed that do not use
gdb_bfd_stash_filename:
solib-aix.c:solib_aix_bfd_open
solib-darwin.c:darwin_bfd_open
solib-spu.c:spu_bfd_open
spu-linux-nat.c:spu_bfd_open
If you don't get to this soon, no worries, I will add these to your
patch tomorrow morning and check it in.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-06 8:25 ` Tom Tromey
@ 2014-01-06 10:50 ` Hui Zhu
2014-01-06 16:14 ` Tom Tromey
2014-01-06 17:12 ` Doug Evans
0 siblings, 2 replies; 12+ messages in thread
From: Hui Zhu @ 2014-01-06 10:50 UTC (permalink / raw)
To: Tom Tromey
Cc: Sergio Durigan Junior, gdb-patches ml, Edjunior Barbosa Machado,
Nick Clifton
On 01/06/14 16:25, Tom Tromey wrote:
>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>
> Hui> Thanks. Post a new version.
>
> Thanks Hui. This is definitely the direction I think the code should
> go.
>
> Hui> --- a/gdb/symfile-mem.c
> Hui> +++ b/gdb/symfile-mem.c
> Hui> @@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
> Hui> if (name == NULL)
> Hui> nbfd-> filename = "shared object read from target memory";
> Hui> else
> Hui> - {
> Hui> - nbfd->filename = name;
> Hui> - gdb_bfd_stash_filename (nbfd);
> Hui> - xfree (name);
> Hui> - }
> Hui> + nbfd->filename = name;
> Hui> cleanup = make_cleanup_bfd_unref (nbfd);
>
> In this hunk there are two things to note.
>
> First, there is an earlier assignment to filename (in the context above)
> that should use xstrdup.
>
> Second, the new assignment really ought to free the old nbfd->filename
> first.
I changed this part to:
xfree (bfd_get_filename (nbfd));
if (name == NULL)
nbfd->filename = xstrdup ("shared object read from target memory");
else
nbfd->filename = name;
>
> There are also some assignments that must be fixed that do not use
> gdb_bfd_stash_filename:
>
> solib-aix.c:solib_aix_bfd_open
> solib-darwin.c:darwin_bfd_open
I fixed them.
> solib-spu.c:spu_bfd_open
> spu-linux-nat.c:spu_bfd_open
These two functions have used xstrdup, for example:
if (sect_size > 20)
{
char *buf = alloca (sect_size - 20 + strlen (original_name) + 1);
bfd_get_section_contents (abfd, spu_name, buf, 20, sect_size - 20);
buf[sect_size - 20] = '\0';
strcat (buf, original_name);
xfree ((char *)abfd->filename);
abfd->filename = xstrdup (buf);
}
So I didn't update them.
>
> If you don't get to this soon, no worries, I will add these to your
> patch tomorrow morning and check it in.
>
> Tom
>
Thunderbird and maybe some others will see some empty lines because this patch include page identifier.
Thanks,
Hui
2014-01-06 Hui Zhu <hui@codesourcery.com>
* gdb_bfd.c (gdb_bfd_stash_filename): Removed.
(gdb_bfd_open): Removed gdb_bfd_stash_filename.
(gdb_bfd_fopen): Ditto.
(gdb_bfd_openr): Ditto.
(gdb_bfd_openw): Ditto.
(gdb_bfd_openr_iovec): Ditto.
(gdb_bfd_fdopenr): Ditto.
* gdb_bfd.h (gdb_bfd_stash_filename): Removed.
* solib-aix.c (solib_aix_bfd_open): Alloc object_bfd->filename
with xstrdup.
* solib-darwin.c (darwin_bfd_open): Alloc res->filename
with xstrdup.
* symfile-mem.c (symbol_file_add_from_memory): Removed
gdb_bfd_stash_filename.
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -57,21 +57,6 @@ struct gdb_bfd_section_data
static htab_t all_bfds;
-/* See gdb_bfd.h. */
-
-void
-gdb_bfd_stash_filename (struct bfd *abfd)
-{
- char *name = bfd_get_filename (abfd);
- char *data;
-
- data = bfd_alloc (abfd, strlen (name) + 1);
- strcpy (data, name);
-
- /* Unwarranted chumminess with BFD. */
- abfd->filename = data;
-}
-
/* An object of this type is stored in each BFD's user data. */
struct gdb_bfd_data
@@ -204,7 +189,6 @@ gdb_bfd_open (const char *name, const ch
gdb_assert (!*slot);
*slot = abfd;
- gdb_bfd_stash_filename (abfd);
gdb_bfd_ref (abfd);
return abfd;
}
@@ -490,10 +474,7 @@ gdb_bfd_fopen (const char *filename, con
bfd *result = bfd_fopen (filename, target, mode, fd);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -506,10 +487,7 @@ gdb_bfd_openr (const char *filename, con
bfd *result = bfd_openr (filename, target);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -522,10 +500,7 @@ gdb_bfd_openw (const char *filename, con
bfd *result = bfd_openw (filename, target);
if (result)
- {
- gdb_bfd_stash_filename (result);
- gdb_bfd_ref (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -553,10 +528,7 @@ gdb_bfd_openr_iovec (const char *filenam
pread_func, close_func, stat_func);
if (result)
- {
- gdb_bfd_ref (result);
- gdb_bfd_stash_filename (result);
- }
+ gdb_bfd_ref (result);
return result;
}
@@ -603,10 +575,7 @@ gdb_bfd_fdopenr (const char *filename, c
bfd *result = bfd_fdopenr (filename, target, fd);
if (result)
- {
- gdb_bfd_ref (result);
- gdb_bfd_stash_filename (result);
- }
+ gdb_bfd_ref (result);
return result;
}
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -24,12 +24,6 @@
DECLARE_REGISTRY (bfd);
-/* Make a copy ABFD's filename using bfd_alloc, and reassign it to the
- BFD. This ensures that the BFD's filename has the same lifetime as
- the BFD itself. */
-
-void gdb_bfd_stash_filename (struct bfd *abfd);
-
/* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
Returns NULL on error. On success, returns a new reference to the
BFD, which must be freed with gdb_bfd_unref. BFDs returned by this
@@ -79,22 +73,22 @@ int gdb_bfd_crc (struct bfd *abfd, unsig
\f
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_fopen (const char *, const char *, const char *, int);
/* A wrapper for bfd_openr that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_openr (const char *, const char *);
/* A wrapper for bfd_openw that initializes the gdb-specific reference
- count and calls gdb_bfd_stash_filename. */
+ count. */
bfd *gdb_bfd_openw (const char *, const char *);
/* A wrapper for bfd_openr_iovec that initializes the gdb-specific
- reference count and calls gdb_bfd_stash_filename. */
+ reference count. */
bfd *gdb_bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
@@ -112,12 +106,12 @@ bfd *gdb_bfd_openr_iovec (const char *fi
struct stat *sb));
/* A wrapper for bfd_openr_next_archived_file that initializes the
- gdb-specific reference count and calls gdb_bfd_stash_filename. */
+ gdb-specific reference count. */
bfd *gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous);
/* A wrapper for bfd_fdopenr that initializes the gdb-specific
- reference count and calls gdb_bfd_stash_filename. */
+ reference count. */
bfd *gdb_bfd_fdopenr (const char *filename, const char *target, int fd);
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -728,12 +728,8 @@ solib_aix_bfd_open (char *pathname)
to allow commands listing all shared libraries to display that
synthetic name. Otherwise, we would only be displaying the name
of the archive member object. */
- {
- char *data = bfd_alloc (object_bfd, path_len + 1);
-
- strcpy (data, pathname);
- object_bfd->filename = data;
- }
+ xfree (bfd_get_filename (object_bfd));
+ object_bfd->filename = xstrdup (pathname);
gdb_bfd_unref (archive_bfd);
do_cleanups (cleanup);
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -621,12 +621,8 @@ darwin_bfd_open (char *pathname)
/* The current filename for fat-binary BFDs is a name generated
by BFD, usually a string containing the name of the architecture.
Reset its value to the actual filename. */
- {
- char *data = bfd_alloc (res, strlen (pathname) + 1);
-
- strcpy (data, pathname);
- res->filename = data;
- }
+ xfree (bfd_get_filename (res));
+ res->filename = xstrdup (pathname);
gdb_bfd_unref (abfd);
return res;
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -101,14 +101,11 @@ symbol_file_add_from_memory (struct bfd
error (_("Failed to read a valid object file image from memory."));
gdb_bfd_ref (nbfd);
+ xfree (bfd_get_filename (nbfd));
if (name == NULL)
- nbfd->filename = "shared object read from target memory";
+ nbfd->filename = xstrdup ("shared object read from target memory");
else
- {
- nbfd->filename = name;
- gdb_bfd_stash_filename (nbfd);
- xfree (name);
- }
+ nbfd->filename = name;
cleanup = make_cleanup_bfd_unref (nbfd);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-06 10:50 ` Hui Zhu
@ 2014-01-06 16:14 ` Tom Tromey
2014-01-06 17:12 ` Doug Evans
1 sibling, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 16:14 UTC (permalink / raw)
To: Hui Zhu
Cc: Sergio Durigan Junior, gdb-patches ml, Edjunior Barbosa Machado,
Nick Clifton
>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>> solib-spu.c:spu_bfd_open
>> spu-linux-nat.c:spu_bfd_open
Hui> These two functions have used xstrdup, for example:
Oops, yeah. Thanks.
Hui> 2014-01-06 Hui Zhu <hui@codesourcery.com>
Hui> * gdb_bfd.c (gdb_bfd_stash_filename): Removed.
Hui> (gdb_bfd_open): Removed gdb_bfd_stash_filename.
Hui> (gdb_bfd_fopen): Ditto.
Hui> (gdb_bfd_openr): Ditto.
Hui> (gdb_bfd_openw): Ditto.
Hui> (gdb_bfd_openr_iovec): Ditto.
Hui> (gdb_bfd_fdopenr): Ditto.
Hui> * gdb_bfd.h (gdb_bfd_stash_filename): Removed.
Hui> * solib-aix.c (solib_aix_bfd_open): Alloc object_bfd->filename
Hui> with xstrdup.
Hui> * solib-darwin.c (darwin_bfd_open): Alloc res->filename
Hui> with xstrdup.
Hui> * symfile-mem.c (symbol_file_add_from_memory): Removed
Hui> gdb_bfd_stash_filename.
This is ok.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-06 10:50 ` Hui Zhu
2014-01-06 16:14 ` Tom Tromey
@ 2014-01-06 17:12 ` Doug Evans
2014-01-06 21:07 ` Tom Tromey
1 sibling, 1 reply; 12+ messages in thread
From: Doug Evans @ 2014-01-06 17:12 UTC (permalink / raw)
To: Hui Zhu
Cc: Tom Tromey, Sergio Durigan Junior, gdb-patches ml,
Edjunior Barbosa Machado, Nick Clifton
On Mon, Jan 6, 2014 at 2:50 AM, Hui Zhu <hui_zhu@mentor.com> wrote:
> On 01/06/14 16:25, Tom Tromey wrote:
>>>>>>>
>>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>>
>>
>> Hui> Thanks. Post a new version.
>>
>> Thanks Hui. This is definitely the direction I think the code should
>> go.
>>
>> Hui> --- a/gdb/symfile-mem.c
>> Hui> +++ b/gdb/symfile-mem.c
>> Hui> @@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
>> Hui> if (name == NULL)
>> Hui> nbfd-> filename = "shared object read from target memory";
>> Hui> else
>> Hui> - {
>> Hui> - nbfd->filename = name;
>> Hui> - gdb_bfd_stash_filename (nbfd);
>> Hui> - xfree (name);
>> Hui> - }
>> Hui> + nbfd->filename = name;
>> Hui> cleanup = make_cleanup_bfd_unref (nbfd);
>>
>> In this hunk there are two things to note.
>>
>> First, there is an earlier assignment to filename (in the context above)
>> that should use xstrdup.
>>
>> Second, the new assignment really ought to free the old nbfd->filename
>> first.
>
>
> I changed this part to:
> xfree (bfd_get_filename (nbfd));
> if (name == NULL)
> nbfd->filename = xstrdup ("shared object read from target memory");
> else
> nbfd->filename = name;
I would prefer a new bfd routine to set the file name.
Then *it* is responsible for freeing the old name.
Any reason to not go that route?
> --- a/gdb/symfile-mem.c
> +++ b/gdb/symfile-mem.c
> @@ -101,14 +101,11 @@ symbol_file_add_from_memory (struct bfd
> error (_("Failed to read a valid object file image from memory."));
> gdb_bfd_ref (nbfd);
> + xfree (bfd_get_filename (nbfd));
This line still screams of excessive chumminess with bfd.
> if (name == NULL)
> - nbfd->filename = "shared object read from target memory";
> + nbfd->filename = xstrdup ("shared object read from target memory");
> else
> - {
>
> - nbfd->filename = name;
> - gdb_bfd_stash_filename (nbfd);
> - xfree (name);
> - }
> + nbfd->filename = name;
> cleanup = make_cleanup_bfd_unref (nbfd);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-06 17:12 ` Doug Evans
@ 2014-01-06 21:07 ` Tom Tromey
2014-01-07 12:35 ` Pedro Alves
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 21:07 UTC (permalink / raw)
To: Doug Evans
Cc: Hui Zhu, Sergio Durigan Junior, gdb-patches ml,
Edjunior Barbosa Machado, Nick Clifton
>>>>> "Doug" == Doug Evans <dje@google.com> writes:
Doug> I would prefer a new bfd routine to set the file name.
Doug> Then *it* is responsible for freeing the old name.
Doug> Any reason to not go that route?
It seems like a reasonable cleanup to me.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-06 21:07 ` Tom Tromey
@ 2014-01-07 12:35 ` Pedro Alves
2014-01-07 13:55 ` Pedro Alves
2014-01-07 17:41 ` Tom Tromey
0 siblings, 2 replies; 12+ messages in thread
From: Pedro Alves @ 2014-01-07 12:35 UTC (permalink / raw)
To: Tom Tromey
Cc: Doug Evans, Hui Zhu, Sergio Durigan Junior, gdb-patches ml,
Edjunior Barbosa Machado, Nick Clifton, Binutils Development
On 01/06/2014 09:06 PM, Tom Tromey wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> I would prefer a new bfd routine to set the file name.
> Doug> Then *it* is responsible for freeing the old name.
>
> Doug> Any reason to not go that route?
>
> It seems like a reasonable cleanup to me.
Hmm. It actually seems odd and wrong to me for bfd (a library)
to use xstrdup (a routine that aborts on error). Actually,
the only uses of xstrdup in bfd are exactly in the
filename handling. There are only 5 uses of xmalloc in bfd,
and I'll guess they're a mistake where either bfd_malloc or
bfd_alloc should be used instead.
gdb has been confused and went in circles, with bfd's filename
ownership. In some places, it ended up xmalloc/xstrdup'ing the
filename instead of allocating it in the bfd's memory.
That resulted in the invention of gdb_bfd_stash_filename
https://sourceware.org/ml/gdb-patches/2012-07/msg00291.html
as a workaround.
I think it'd be better to allocate the filename
in the bfd's memory, like it used to be. In bfd, most places
already have the filename in some structure that is
itself already allocated in the bfd (with bfd_alloc),
it can just point the filename to that memory. The problematic
case of binutils/11983 that led to xstrdups was the bfd_open*
routines, which were just copying the caller's pointer, which
obviously couldn't have been in the bfd's memory, because, well,
the caller doesn't have a bfd yet, it's asking bfd to create one.
So instead of xstrdup in those bfd_open* routines, we should
IMO use bfd_alloc in that spot too instead, and handle memory
errors gracefully in addition. Actually as that's done more than
a few times, I added a bfd_strdup function. Then we make gdb do
what it should have always done -- allocate the filename in
the bfd's memory throughout.
WDYT?
Tested on x86_64 Fedora 17, gdb and binutils.
Subject: [PATCH] bfd/ 2014-01-07 Pedro Alves <palves@redhat.com>
PR binutils/11983
* archive.c (_bfd_get_elt_at_filepos): Don't xstrdup the filename.
On error, set the bfd's filename to a const string.
* bfd-in2.h: Regenerate.
* elfcode.h: Don't include libiberty.h.
(bfd_from_remote_memory): Don't xstrdup the bfd's filename.
* ieee.c: Don't include libiberty.h.
(ieee_object_p): Don't xstrdup the bfd's filename.
* mach-o.c (bfd_mach_o_fat_member_init): Don't xstrdup the
architecture's printable name, it's const. If allocating a
filename, allocate it on the bfd's memory.
* oasys.c: Don't include libiberty.h.
(oasys_openr_next_archived_file): Don't xstrdup the bfd's
filename.
* opncls.c (_bfd_delete_bfd): Don't free the bfd's filename.
(bfd_fopen, bfd_openstreamr, bfd_openr_iovec, bfd_openw)
(bfd_create): Use bfd_strdup to copy the filename and handle
memory error.
(bfd_strdup): New function.
* syms.c (_bfd_stab_section_find_nearest_line): Delete coment.
* vms-lib.c: Don't include libiberty.h.
(_bfd_vms_lib_get_module): Don't xstrdup the bfd's filename.
gdb/
2014-01-07 Pedro Alves <palves@redhat.com>
PR binutils/11983
* gdb_bfd.c (gdb_bfd_strdup): New function.
* gdb_bfd.h (gdb_bfd_strdup): Declare.
* solib-aix.c (solib_aix_bfd_open): Don't free the bfd's previous
filename. Use gdb_bfd_strdup instead of xstrdup.
* solib-spu.c (spu_bfd_open): Likewise.
* spu-linux-nat.c (spu_bfd_open): Likewise.
* symfile-mem.c (symbol_file_add_from_memory): Don't free the
bfd's previous filename. If passed a filename, dup it into bfd's
memory. If not, set the bfd's filename to a const read only
string.
(add_vsyscall_page): Free filename.
* solib-darwin.c (darwin_bfd_open): Don't free the bfd's previous
filename. Use gdb_bfd_strdup instead of xstrdup.
---
bfd/archive.c | 3 ++-
bfd/bfd-in2.h | 4 +++-
bfd/elfcode.h | 3 +--
bfd/ieee.c | 3 +--
bfd/mach-o.c | 4 ++--
bfd/oasys.c | 3 +--
bfd/opncls.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++------
bfd/syms.c | 3 ---
bfd/vms-lib.c | 3 +--
gdb/gdb_bfd.c | 14 ++++++++++++
gdb/gdb_bfd.h | 4 ++++
gdb/solib-aix.c | 3 +--
gdb/solib-darwin.c | 3 +--
gdb/solib-spu.c | 3 +--
gdb/spu-linux-nat.c | 3 +--
gdb/symfile-mem.c | 6 ++---
16 files changed, 92 insertions(+), 33 deletions(-)
diff --git a/bfd/archive.c b/bfd/archive.c
index dc39751..d76b782 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -705,7 +705,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
else
{
n_nfd->origin = n_nfd->proxy_origin;
- n_nfd->filename = xstrdup (filename);
+ n_nfd->filename = filename;
}
n_nfd->arelt_data = new_areldata;
@@ -718,6 +718,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
free (new_areldata);
n_nfd->arelt_data = NULL;
+ n_nfd->filename = "<no memory>";
return NULL;
}
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 71996db..46eb7d1 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1029,7 +1029,7 @@ bfd *bfd_openr (const char *filename, const char *target);
bfd *bfd_fdopenr (const char *filename, const char *target, int fd);
-bfd *bfd_openstreamr (const char *, const char *, void *);
+bfd *bfd_openstreamr (const char * filename, const char * target, void * stream);
bfd *bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
@@ -1060,6 +1060,8 @@ bfd_boolean bfd_make_readable (bfd *abfd);
void *bfd_alloc (bfd *abfd, bfd_size_type wanted);
+char *bfd_strdup (bfd *abfd, const char *str);
+
void *bfd_zalloc (bfd *abfd, bfd_size_type wanted);
unsigned long bfd_calc_gnu_debuglink_crc32
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 0328748..c3135f5 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -71,7 +71,6 @@
#include "bfdlink.h"
#include "libbfd.h"
#include "elf-bfd.h"
-#include "libiberty.h"
/* Renaming structures, typedefs, macros and functions to be size-specific. */
#define Elf_External_Ehdr NAME(Elf,External_Ehdr)
@@ -1803,7 +1802,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
bfd_set_error (bfd_error_no_memory);
return NULL;
}
- nbfd->filename = xstrdup ("<in-memory>");
+ nbfd->filename = "<in-memory>";
nbfd->xvec = templ->xvec;
bim->size = contents_size;
bim->buffer = contents;
diff --git a/bfd/ieee.c b/bfd/ieee.c
index e1734ec..237736c 100644
--- a/bfd/ieee.c
+++ b/bfd/ieee.c
@@ -33,7 +33,6 @@
#include "ieee.h"
#include "libieee.h"
#include "safe-ctype.h"
-#include "libiberty.h"
struct output_buffer_struct
{
@@ -1823,7 +1822,7 @@ ieee_object_p (bfd *abfd)
goto got_wrong_format;
ieee->mb.module_name = read_id (&(ieee->h));
if (abfd->filename == (const char *) NULL)
- abfd->filename = xstrdup (ieee->mb.module_name);
+ abfd->filename = ieee->mb.module_name;
/* Determine the architecture and machine type of the object file. */
{
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 6640a6a..ffe7332 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -4353,13 +4353,13 @@ bfd_mach_o_fat_member_init (bfd *abfd,
if (ap)
{
/* Use the architecture name if known. */
- abfd->filename = xstrdup (ap->printable_name);
+ abfd->filename = ap->printable_name;
}
else
{
/* Forge a uniq id. */
const size_t namelen = 2 + 8 + 1 + 2 + 8 + 1;
- char *name = xmalloc (namelen);
+ char *name = bfd_alloc (abfd, namelen);
snprintf (name, namelen, "0x%lx-0x%lx",
entry->cputype, entry->cpusubtype);
abfd->filename = name;
diff --git a/bfd/oasys.c b/bfd/oasys.c
index b8e457e..671d4c7 100644
--- a/bfd/oasys.c
+++ b/bfd/oasys.c
@@ -26,7 +26,6 @@
#include "libbfd.h"
#include "oasys.h"
#include "liboasys.h"
-#include "libiberty.h"
/* Read in all the section data and relocation stuff too. */
@@ -1117,7 +1116,7 @@ oasys_openr_next_archived_file (bfd *arch, bfd *prev)
{
p->abfd = _bfd_create_empty_archive_element_shell (arch);
p->abfd->origin = p->pos;
- p->abfd->filename = xstrdup (p->name);
+ p->abfd->filename = p->name;
/* Fixup a pointer to this element for the member. */
p->abfd->arelt_data = (void *) p;
diff --git a/bfd/opncls.c b/bfd/opncls.c
index 54744ce..4ef474e 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -123,8 +123,6 @@ _bfd_delete_bfd (bfd *abfd)
objalloc_free ((struct objalloc *) abfd->memory);
}
- if (abfd->filename)
- free ((char *) abfd->filename);
free (abfd->arelt_data);
free (abfd);
}
@@ -228,7 +226,12 @@ bfd_fopen (const char *filename, const char *target, const char *mode, int fd)
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
/* Figure out whether the user is opening the file for reading,
writing, or both, by looking at the MODE argument. */
@@ -395,7 +398,13 @@ bfd_openstreamr (const char *filename, const char *target, void *streamarg)
nbfd->iostream = stream;
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
+
nbfd->direction = read_direction;
if (! bfd_cache_init (nbfd))
@@ -589,7 +598,12 @@ bfd_openr_iovec (const char *filename, const char *target,
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
nbfd->direction = read_direction;
/* `open_p (...)' would get expanded by an the open(2) syscall macro. */
@@ -656,7 +670,12 @@ bfd_openw (const char *filename, const char *target)
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
nbfd->direction = write_direction;
if (bfd_open_file (nbfd) == NULL)
@@ -808,7 +827,12 @@ bfd_create (const char *filename, bfd *templ)
return NULL;
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
if (templ)
nbfd->xvec = templ->xvec;
nbfd->direction = no_direction;
@@ -991,6 +1015,31 @@ bfd_alloc2 (bfd *abfd, bfd_size_type nmemb, bfd_size_type size)
/*
FUNCTION
+ bfd_strdup
+
+SYNOPSIS
+ char *bfd_strdup (bfd *abfd, const char *str);
+
+DESCRIPTION
+ Copy a string into memory attached to <<abfd>> and return a
+ pointer to it.
+*/
+
+char *
+bfd_strdup (bfd *abfd, const char *str)
+{
+ bfd_size_type size;
+ char *p;
+
+ size = strlen (str) + 1;
+ p = bfd_alloc (abfd, size);
+ if (p != NULL)
+ memcpy (p, str, size);
+ return p;
+}
+
+/*
+FUNCTION
bfd_zalloc
SYNOPSIS
diff --git a/bfd/syms.c b/bfd/syms.c
index 27b40eb..aa4718f 100644
--- a/bfd/syms.c
+++ b/bfd/syms.c
@@ -1383,9 +1383,6 @@ _bfd_stab_section_find_nearest_line (bfd *abfd,
{
size_t len;
- /* Don't free info->filename here. objdump and other
- apps keep a copy of a previously returned file name
- pointer. */
len = strlen (file_name) + 1;
info->filename = (char *) bfd_alloc (abfd, dirlen + len);
if (info->filename == NULL)
diff --git a/bfd/vms-lib.c b/bfd/vms-lib.c
index 407c186..afbb54a 100644
--- a/bfd/vms-lib.c
+++ b/bfd/vms-lib.c
@@ -25,7 +25,6 @@
#include "libbfd.h"
#include "safe-ctype.h"
#include "bfdver.h"
-#include "libiberty.h"
#include "vms.h"
#include "vms/lbr.h"
#include "vms/dcx.h"
@@ -1377,7 +1376,7 @@ _bfd_vms_lib_get_module (bfd *abfd, unsigned int modidx)
default:
break;
}
- res->filename = xstrdup (name);
+ res->filename = name;
tdata->cache[modidx] = res;
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 5230d21..9d84815 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -634,6 +634,20 @@ gdb_bfd_requires_relocations (bfd *abfd)
return gdata->needs_relocations;
}
+/* See gdb_bfd.h. */
+
+char *
+gdb_bfd_strdup (bfd *abfd, const char *str)
+{
+ char *p;
+
+ p = bfd_strdup (abfd, str);
+ if (p == NULL)
+ malloc_failure (0);
+
+ return p;
+}
+
\f
/* A callback for htab_traverse that prints a single BFD. */
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index d415005..502c131 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -70,6 +70,10 @@ const gdb_byte *gdb_bfd_map_section (asection *section, bfd_size_type *size);
int gdb_bfd_crc (struct bfd *abfd, unsigned long *crc_out);
+/* A wrapper for bfd_strdup that never returns NULL. */
+
+char *gdb_bfd_strdup (bfd *abfd, const char *str);
+
\f
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index fefa51f..9191667 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -728,8 +728,7 @@ solib_aix_bfd_open (char *pathname)
to allow commands listing all shared libraries to display that
synthetic name. Otherwise, we would only be displaying the name
of the archive member object. */
- xfree (bfd_get_filename (object_bfd));
- object_bfd->filename = xstrdup (pathname);
+ object_bfd->filename = gdb_bfd_strdup (object_bfd, pathname);
gdb_bfd_unref (archive_bfd);
do_cleanups (cleanup);
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index ba807a2..e5b43a1 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -621,8 +621,7 @@ darwin_bfd_open (char *pathname)
/* The current filename for fat-binary BFDs is a name generated
by BFD, usually a string containing the name of the architecture.
Reset its value to the actual filename. */
- xfree (bfd_get_filename (res));
- res->filename = xstrdup (pathname);
+ res->filename = gdb_bfd_strdup (pathname);
gdb_bfd_unref (abfd);
return res;
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index abb8c15..2dedd5a 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -382,8 +382,7 @@ spu_bfd_open (char *pathname)
strcat (buf, original_name);
- xfree ((char *)abfd->filename);
- abfd->filename = xstrdup (buf);
+ abfd->filename = gdb_bfd_strdup (abfd, buf);
}
}
diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c
index e9b155b..1c8c5c7 100644
--- a/gdb/spu-linux-nat.c
+++ b/gdb/spu-linux-nat.c
@@ -341,8 +341,7 @@ spu_bfd_open (ULONGEST addr)
bfd_get_section_contents (nbfd, spu_name, buf, 20, sect_size - 20);
buf[sect_size - 20] = '\0';
- xfree ((char *)nbfd->filename);
- nbfd->filename = xstrdup (buf);
+ nbfd->filename = gdb_bfd_strdup (nbfd, buf);
}
}
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..652c032 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -101,11 +101,10 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
error (_("Failed to read a valid object file image from memory."));
gdb_bfd_ref (nbfd);
- xfree (bfd_get_filename (nbfd));
if (name == NULL)
- nbfd->filename = xstrdup ("shared object read from target memory");
+ nbfd->filename = "shared object read from target memory";
else
- nbfd->filename = name;
+ nbfd->filename = gdb_bfd_strdup (nbfd, name);
cleanup = make_cleanup_bfd_unref (nbfd);
@@ -225,6 +224,7 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
args.from_tty = 0;
catch_exceptions (current_uiout, symbol_file_add_from_memory_wrapper,
&args, RETURN_MASK_ALL);
+ xfree (args.name);
}
}
--
1.7.11.7
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-07 12:35 ` Pedro Alves
@ 2014-01-07 13:55 ` Pedro Alves
2014-01-07 17:41 ` Tom Tromey
1 sibling, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2014-01-07 13:55 UTC (permalink / raw)
To: Binutils Development
Cc: Tom Tromey, Doug Evans, Hui Zhu, Sergio Durigan Junior,
gdb-patches ml, Edjunior Barbosa Machado, Nick Clifton
On 01/07/2014 12:35 PM, Pedro Alves wrote:
> On 01/06/2014 09:06 PM, Tom Tromey wrote:
>>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>>
>> Doug> I would prefer a new bfd routine to set the file name.
>> Doug> Then *it* is responsible for freeing the old name.
>>
>> Doug> Any reason to not go that route?
>>
>> It seems like a reasonable cleanup to me.
>
> Hmm. It actually seems odd and wrong to me for bfd (a library)
> to use xstrdup (a routine that aborts on error). Actually,
> the only uses of xstrdup in bfd are exactly in the
> filename handling. There are only 5 uses of xmalloc in bfd,
> and I'll guess they're a mistake where either bfd_malloc or
> bfd_alloc should be used instead.
>
> gdb has been confused and went in circles, with bfd's filename
> ownership. In some places, it ended up xmalloc/xstrdup'ing the
> filename instead of allocating it in the bfd's memory.
> That resulted in the invention of gdb_bfd_stash_filename
> https://sourceware.org/ml/gdb-patches/2012-07/msg00291.html
> as a workaround.
>
> I think it'd be better to allocate the filename
> in the bfd's memory, like it used to be. In bfd, most places
> already have the filename in some structure that is
> itself already allocated in the bfd (with bfd_alloc),
> it can just point the filename to that memory. The problematic
> case of binutils/11983 that led to xstrdups was the bfd_open*
> routines, which were just copying the caller's pointer, which
> obviously couldn't have been in the bfd's memory, because, well,
> the caller doesn't have a bfd yet, it's asking bfd to create one.
>
> So instead of xstrdup in those bfd_open* routines, we should
> IMO use bfd_alloc in that spot too instead, and handle memory
> errors gracefully in addition. Actually as that's done more than
> a few times, I added a bfd_strdup function. Then we make gdb do
> what it should have always done -- allocate the filename in
> the bfd's memory throughout.
>
> WDYT?
>
> Tested on x86_64 Fedora 17, gdb and binutils.
>
> Subject: [PATCH] bfd/ 2014-01-07 Pedro Alves <palves@redhat.com>
Bah, I noticed now that I sent the wrong patch. That one didn't
even build...
Here's the right one.
-----
Subject: [PATCH] Switch back to allocating the bfd's filename in the bfd's
memory.
bfd/
2014-01-07 Pedro Alves <palves@redhat.com>
PR binutils/11983
* archive.c (_bfd_get_elt_at_filepos): Don't xstrdup the filename.
On error, set the bfd's filename to a const string.
* bfd-in2.h: Regenerate.
* elfcode.h: Don't include libiberty.h.
(bfd_from_remote_memory): Don't xstrdup the bfd's filename.
* ieee.c: Don't include libiberty.h.
(ieee_object_p): Don't xstrdup the bfd's filename.
* mach-o.c (bfd_mach_o_fat_member_init): Don't xstrdup the
architecture's printable name, it's const. If allocating a
filename, allocate it on the bfd's memory.
* oasys.c: Don't include libiberty.h.
(oasys_openr_next_archived_file): Don't xstrdup the bfd's
filename.
* opncls.c (_bfd_delete_bfd): Don't free the bfd's filename.
(bfd_fopen, bfd_openstreamr, bfd_openr_iovec, bfd_openw)
(bfd_create): Use bfd_strdup to copy the filename and handle
memory error.
(bfd_strdup): New function.
* syms.c (_bfd_stab_section_find_nearest_line): Delete coment.
* vms-lib.c: Don't include libiberty.h.
(_bfd_vms_lib_get_module): Don't xstrdup the bfd's filename.
gdb/
2014-01-07 Pedro Alves <palves@redhat.com>
PR binutils/11983
* gdb_bfd.c (gdb_bfd_strdup): New function.
* gdb_bfd.h (gdb_bfd_strdup): Declare.
* solib-aix.c (solib_aix_bfd_open): Don't free the bfd's previous
filename. Use gdb_bfd_strdup instead of xstrdup.
* solib-spu.c (spu_bfd_open): Likewise.
* spu-linux-nat.c (spu_bfd_open): Likewise.
* symfile-mem.c (symbol_file_add_from_memory): Don't free the
bfd's previous filename. If passed a filename, dup it into bfd's
memory. If not, set the bfd's filename to a const read only
string.
(add_vsyscall_page): Free filename.
* solib-darwin.c (darwin_bfd_open): Don't free the bfd's previous
filename. Use gdb_bfd_strdup instead of xstrdup.
---
bfd/archive.c | 3 ++-
bfd/bfd-in2.h | 4 +++-
bfd/elfcode.h | 3 +--
bfd/ieee.c | 3 +--
bfd/mach-o.c | 4 ++--
bfd/oasys.c | 3 +--
bfd/opncls.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++------
bfd/syms.c | 3 ---
bfd/vms-lib.c | 3 +--
gdb/gdb_bfd.c | 14 ++++++++++++
gdb/gdb_bfd.h | 4 ++++
gdb/solib-aix.c | 3 +--
gdb/solib-darwin.c | 3 +--
gdb/solib-spu.c | 3 +--
gdb/spu-linux-nat.c | 3 +--
gdb/symfile-mem.c | 6 ++---
16 files changed, 92 insertions(+), 33 deletions(-)
diff --git a/bfd/archive.c b/bfd/archive.c
index dc39751..8f59395 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -705,7 +705,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
else
{
n_nfd->origin = n_nfd->proxy_origin;
- n_nfd->filename = xstrdup (filename);
+ n_nfd->filename = filename;
}
n_nfd->arelt_data = new_areldata;
@@ -718,6 +718,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
free (new_areldata);
n_nfd->arelt_data = NULL;
+ n_nfd->filename = "<out of memory>";
return NULL;
}
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 71996db..46eb7d1 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1029,7 +1029,7 @@ bfd *bfd_openr (const char *filename, const char *target);
bfd *bfd_fdopenr (const char *filename, const char *target, int fd);
-bfd *bfd_openstreamr (const char *, const char *, void *);
+bfd *bfd_openstreamr (const char * filename, const char * target, void * stream);
bfd *bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
@@ -1060,6 +1060,8 @@ bfd_boolean bfd_make_readable (bfd *abfd);
void *bfd_alloc (bfd *abfd, bfd_size_type wanted);
+char *bfd_strdup (bfd *abfd, const char *str);
+
void *bfd_zalloc (bfd *abfd, bfd_size_type wanted);
unsigned long bfd_calc_gnu_debuglink_crc32
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 0328748..c3135f5 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -71,7 +71,6 @@
#include "bfdlink.h"
#include "libbfd.h"
#include "elf-bfd.h"
-#include "libiberty.h"
/* Renaming structures, typedefs, macros and functions to be size-specific. */
#define Elf_External_Ehdr NAME(Elf,External_Ehdr)
@@ -1803,7 +1802,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
bfd_set_error (bfd_error_no_memory);
return NULL;
}
- nbfd->filename = xstrdup ("<in-memory>");
+ nbfd->filename = "<in-memory>";
nbfd->xvec = templ->xvec;
bim->size = contents_size;
bim->buffer = contents;
diff --git a/bfd/ieee.c b/bfd/ieee.c
index e1734ec..237736c 100644
--- a/bfd/ieee.c
+++ b/bfd/ieee.c
@@ -33,7 +33,6 @@
#include "ieee.h"
#include "libieee.h"
#include "safe-ctype.h"
-#include "libiberty.h"
struct output_buffer_struct
{
@@ -1823,7 +1822,7 @@ ieee_object_p (bfd *abfd)
goto got_wrong_format;
ieee->mb.module_name = read_id (&(ieee->h));
if (abfd->filename == (const char *) NULL)
- abfd->filename = xstrdup (ieee->mb.module_name);
+ abfd->filename = ieee->mb.module_name;
/* Determine the architecture and machine type of the object file. */
{
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 6640a6a..ffe7332 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -4353,13 +4353,13 @@ bfd_mach_o_fat_member_init (bfd *abfd,
if (ap)
{
/* Use the architecture name if known. */
- abfd->filename = xstrdup (ap->printable_name);
+ abfd->filename = ap->printable_name;
}
else
{
/* Forge a uniq id. */
const size_t namelen = 2 + 8 + 1 + 2 + 8 + 1;
- char *name = xmalloc (namelen);
+ char *name = bfd_alloc (abfd, namelen);
snprintf (name, namelen, "0x%lx-0x%lx",
entry->cputype, entry->cpusubtype);
abfd->filename = name;
diff --git a/bfd/oasys.c b/bfd/oasys.c
index b8e457e..671d4c7 100644
--- a/bfd/oasys.c
+++ b/bfd/oasys.c
@@ -26,7 +26,6 @@
#include "libbfd.h"
#include "oasys.h"
#include "liboasys.h"
-#include "libiberty.h"
/* Read in all the section data and relocation stuff too. */
@@ -1117,7 +1116,7 @@ oasys_openr_next_archived_file (bfd *arch, bfd *prev)
{
p->abfd = _bfd_create_empty_archive_element_shell (arch);
p->abfd->origin = p->pos;
- p->abfd->filename = xstrdup (p->name);
+ p->abfd->filename = p->name;
/* Fixup a pointer to this element for the member. */
p->abfd->arelt_data = (void *) p;
diff --git a/bfd/opncls.c b/bfd/opncls.c
index 54744ce..4ef474e 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -123,8 +123,6 @@ _bfd_delete_bfd (bfd *abfd)
objalloc_free ((struct objalloc *) abfd->memory);
}
- if (abfd->filename)
- free ((char *) abfd->filename);
free (abfd->arelt_data);
free (abfd);
}
@@ -228,7 +226,12 @@ bfd_fopen (const char *filename, const char *target, const char *mode, int fd)
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
/* Figure out whether the user is opening the file for reading,
writing, or both, by looking at the MODE argument. */
@@ -395,7 +398,13 @@ bfd_openstreamr (const char *filename, const char *target, void *streamarg)
nbfd->iostream = stream;
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
+
nbfd->direction = read_direction;
if (! bfd_cache_init (nbfd))
@@ -589,7 +598,12 @@ bfd_openr_iovec (const char *filename, const char *target,
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
nbfd->direction = read_direction;
/* `open_p (...)' would get expanded by an the open(2) syscall macro. */
@@ -656,7 +670,12 @@ bfd_openw (const char *filename, const char *target)
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
nbfd->direction = write_direction;
if (bfd_open_file (nbfd) == NULL)
@@ -808,7 +827,12 @@ bfd_create (const char *filename, bfd *templ)
return NULL;
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ nbfd->filename = bfd_strdup (nbfd, filename);
+ if (nbfd->filename == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
if (templ)
nbfd->xvec = templ->xvec;
nbfd->direction = no_direction;
@@ -991,6 +1015,31 @@ bfd_alloc2 (bfd *abfd, bfd_size_type nmemb, bfd_size_type size)
/*
FUNCTION
+ bfd_strdup
+
+SYNOPSIS
+ char *bfd_strdup (bfd *abfd, const char *str);
+
+DESCRIPTION
+ Copy a string into memory attached to <<abfd>> and return a
+ pointer to it.
+*/
+
+char *
+bfd_strdup (bfd *abfd, const char *str)
+{
+ bfd_size_type size;
+ char *p;
+
+ size = strlen (str) + 1;
+ p = bfd_alloc (abfd, size);
+ if (p != NULL)
+ memcpy (p, str, size);
+ return p;
+}
+
+/*
+FUNCTION
bfd_zalloc
SYNOPSIS
diff --git a/bfd/syms.c b/bfd/syms.c
index 27b40eb..aa4718f 100644
--- a/bfd/syms.c
+++ b/bfd/syms.c
@@ -1383,9 +1383,6 @@ _bfd_stab_section_find_nearest_line (bfd *abfd,
{
size_t len;
- /* Don't free info->filename here. objdump and other
- apps keep a copy of a previously returned file name
- pointer. */
len = strlen (file_name) + 1;
info->filename = (char *) bfd_alloc (abfd, dirlen + len);
if (info->filename == NULL)
diff --git a/bfd/vms-lib.c b/bfd/vms-lib.c
index 407c186..afbb54a 100644
--- a/bfd/vms-lib.c
+++ b/bfd/vms-lib.c
@@ -25,7 +25,6 @@
#include "libbfd.h"
#include "safe-ctype.h"
#include "bfdver.h"
-#include "libiberty.h"
#include "vms.h"
#include "vms/lbr.h"
#include "vms/dcx.h"
@@ -1377,7 +1376,7 @@ _bfd_vms_lib_get_module (bfd *abfd, unsigned int modidx)
default:
break;
}
- res->filename = xstrdup (name);
+ res->filename = name;
tdata->cache[modidx] = res;
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 5230d21..9d84815 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -634,6 +634,20 @@ gdb_bfd_requires_relocations (bfd *abfd)
return gdata->needs_relocations;
}
+/* See gdb_bfd.h. */
+
+char *
+gdb_bfd_strdup (bfd *abfd, const char *str)
+{
+ char *p;
+
+ p = bfd_strdup (abfd, str);
+ if (p == NULL)
+ malloc_failure (0);
+
+ return p;
+}
+
\f
/* A callback for htab_traverse that prints a single BFD. */
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index d415005..502c131 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -70,6 +70,10 @@ const gdb_byte *gdb_bfd_map_section (asection *section, bfd_size_type *size);
int gdb_bfd_crc (struct bfd *abfd, unsigned long *crc_out);
+/* A wrapper for bfd_strdup that never returns NULL. */
+
+char *gdb_bfd_strdup (bfd *abfd, const char *str);
+
\f
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index fefa51f..9191667 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -728,8 +728,7 @@ solib_aix_bfd_open (char *pathname)
to allow commands listing all shared libraries to display that
synthetic name. Otherwise, we would only be displaying the name
of the archive member object. */
- xfree (bfd_get_filename (object_bfd));
- object_bfd->filename = xstrdup (pathname);
+ object_bfd->filename = gdb_bfd_strdup (object_bfd, pathname);
gdb_bfd_unref (archive_bfd);
do_cleanups (cleanup);
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index ba807a2..af8275b 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -621,8 +621,7 @@ darwin_bfd_open (char *pathname)
/* The current filename for fat-binary BFDs is a name generated
by BFD, usually a string containing the name of the architecture.
Reset its value to the actual filename. */
- xfree (bfd_get_filename (res));
- res->filename = xstrdup (pathname);
+ res->filename = gdb_bfd_strdup (res, pathname);
gdb_bfd_unref (abfd);
return res;
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index abb8c15..2dedd5a 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -382,8 +382,7 @@ spu_bfd_open (char *pathname)
strcat (buf, original_name);
- xfree ((char *)abfd->filename);
- abfd->filename = xstrdup (buf);
+ abfd->filename = gdb_bfd_strdup (abfd, buf);
}
}
diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c
index e9b155b..1c8c5c7 100644
--- a/gdb/spu-linux-nat.c
+++ b/gdb/spu-linux-nat.c
@@ -341,8 +341,7 @@ spu_bfd_open (ULONGEST addr)
bfd_get_section_contents (nbfd, spu_name, buf, 20, sect_size - 20);
buf[sect_size - 20] = '\0';
- xfree ((char *)nbfd->filename);
- nbfd->filename = xstrdup (buf);
+ nbfd->filename = gdb_bfd_strdup (nbfd, buf);
}
}
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..652c032 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -101,11 +101,10 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
error (_("Failed to read a valid object file image from memory."));
gdb_bfd_ref (nbfd);
- xfree (bfd_get_filename (nbfd));
if (name == NULL)
- nbfd->filename = xstrdup ("shared object read from target memory");
+ nbfd->filename = "shared object read from target memory";
else
- nbfd->filename = name;
+ nbfd->filename = gdb_bfd_strdup (nbfd, name);
cleanup = make_cleanup_bfd_unref (nbfd);
@@ -225,6 +224,7 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
args.from_tty = 0;
catch_exceptions (current_uiout, symbol_file_add_from_memory_wrapper,
&args, RETURN_MASK_ALL);
+ xfree (args.name);
}
}
--
1.7.11.7
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-07 12:35 ` Pedro Alves
2014-01-07 13:55 ` Pedro Alves
@ 2014-01-07 17:41 ` Tom Tromey
2014-01-07 19:54 ` Pedro Alves
1 sibling, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2014-01-07 17:41 UTC (permalink / raw)
To: Pedro Alves
Cc: Doug Evans, Hui Zhu, Sergio Durigan Junior, gdb-patches ml,
Edjunior Barbosa Machado, Nick Clifton, Binutils Development
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> gdb has been confused and went in circles, with bfd's filename
Pedro> ownership. In some places, it ended up xmalloc/xstrdup'ing the
Pedro> filename instead of allocating it in the bfd's memory.
Pedro> That resulted in the invention of gdb_bfd_stash_filename
Pedro> https://sourceware.org/ml/gdb-patches/2012-07/msg00291.html
Pedro> as a workaround.
Yeah, in retrospect I should have tried to fix up BFD at that time.
Pedro> I think it'd be better to allocate the filename
Pedro> in the bfd's memory, like it used to be.
I agree. I think your patch is better due to keeping the same
error-handling approach as the rest of BFD.
Pedro> WDYT?
If you don't mind I think it would be good -- and easy -- to also
implement Doug's suggestion, say a "bfd_set_filename" macro.
Pedro> +/* A wrapper for bfd_strdup that never returns NULL. */
Pedro> +
Pedro> +char *gdb_bfd_strdup (bfd *abfd, const char *str);
This can be marked ATTRIBUTE_RETURNS_NONNULL.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
2014-01-07 17:41 ` Tom Tromey
@ 2014-01-07 19:54 ` Pedro Alves
0 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2014-01-07 19:54 UTC (permalink / raw)
To: Tom Tromey
Cc: Doug Evans, Hui Zhu, Sergio Durigan Junior, gdb-patches ml,
Edjunior Barbosa Machado, Nick Clifton, Binutils Development
On 01/07/2014 05:40 PM, Tom Tromey wrote:
> If you don't mind I think it would be good -- and easy -- to also
> implement Doug's suggestion, say a "bfd_set_filename" macro.
Sure thing, though in Doug's suggestion that would be used
to free the previous name IIRC, which now won't be necessary.
We can still use it to add some documentation though.
Done.
> Pedro> +/* A wrapper for bfd_strdup that never returns NULL. */
> Pedro> +
> Pedro> +char *gdb_bfd_strdup (bfd *abfd, const char *str);
>
> This can be marked ATTRIBUTE_RETURNS_NONNULL.
Indeed. Below's the updated patch.
---------
Subject: [PATCH] Switch back to allocating the bfd's filename in the bfd's
memory.
Don't use xstrdup in bfd, as it aborts on error.
bfd/
2014-01-07 Pedro Alves <palves@redhat.com>
PR binutils/11983
* bfd-in.h (bfd_set_filename): New macro.
* bfd-in2.h: Regenerate.
* archive.c (_bfd_get_elt_at_filepos): Don't xstrdup the filename.
Use bfd_set_filename. On error, set the bfd's filename to a const
string.
* elfcode.h: Don't include libiberty.h.
(bfd_from_remote_memory): Don't xstrdup the bfd's filename. Use
bfd_set_filename.
* ieee.c: Don't include libiberty.h.
(ieee_object_p): Don't xstrdup the bfd's filename. Use
bfd_set_filename.
* mach-o.c (bfd_mach_o_fat_member_init): Don't xstrdup the
architecture's printable name, it's const. If allocating a
filename, allocate it on the bfd's memory. Use bfd_set_filename.
* oasys.c: Don't include libiberty.h.
(oasys_openr_next_archived_file): Don't xstrdup the bfd's
filename. Use bfd_set_filename.
* opncls.c (_bfd_delete_bfd): Don't free the bfd's filename.
(bfd_fopen, bfd_openstreamr, bfd_openr_iovec, bfd_openw)
(bfd_create): Use bfd_strdup to copy the filename and handle
memory error. Use bfd_set_filename.
(bfd_strdup): New function.
* syms.c (_bfd_stab_section_find_nearest_line): Delete comment.
* vms-lib.c: Don't include libiberty.h.
(_bfd_vms_lib_get_module): Don't xstrdup the bfd's filename. Use
bfd_set_filename.
gdb/
2014-01-07 Pedro Alves <palves@redhat.com>
PR binutils/11983
* gdb_bfd.c (gdb_bfd_strdup): New function.
* gdb_bfd.h (gdb_bfd_strdup): Declare.
* solib-aix.c (solib_aix_bfd_open): Don't free the bfd's previous
filename. Use gdb_bfd_strdup instead of xstrdup. Use
bfd_set_filename.
* solib-spu.c (spu_bfd_open): Likewise.
* spu-linux-nat.c (spu_bfd_open): Likewise.
* symfile-mem.c (symbol_file_add_from_memory): Don't free the
bfd's previous filename. If passed a filename, dup it into bfd's
memory. If not, set the bfd's filename to a const read only
string. Use bfd_set_filename.
(add_vsyscall_page): Free filename.
* solib-darwin.c (darwin_bfd_open): Don't free the bfd's previous
filename. Use gdb_bfd_strdup instead of xstrdup. Use
bfd_set_filename.
---
bfd/archive.c | 3 ++-
bfd/bfd-in.h | 6 ++++++
bfd/bfd-in2.h | 9 ++++++++-
bfd/elfcode.h | 3 +--
bfd/ieee.c | 3 +--
bfd/mach-o.c | 6 +++---
bfd/oasys.c | 3 +--
bfd/opncls.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-------
bfd/syms.c | 3 ---
bfd/vms-lib.c | 3 +--
gdb/gdb_bfd.c | 14 +++++++++++++
gdb/gdb_bfd.h | 5 +++++
gdb/solib-aix.c | 3 +--
gdb/solib-darwin.c | 3 +--
gdb/solib-spu.c | 3 +--
gdb/spu-linux-nat.c | 3 +--
gdb/symfile-mem.c | 6 +++---
17 files changed, 100 insertions(+), 34 deletions(-)
diff --git a/bfd/archive.c b/bfd/archive.c
index dc39751..039d8cb 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -705,7 +705,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
else
{
n_nfd->origin = n_nfd->proxy_origin;
- n_nfd->filename = xstrdup (filename);
+ bfd_set_filename (n_nfd, filename);
}
n_nfd->arelt_data = new_areldata;
@@ -718,6 +718,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
free (new_areldata);
n_nfd->arelt_data = NULL;
+ bfd_set_filename (n_nfd, "<out of memory>");
return NULL;
}
diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index 3afd71b..ca372f6 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -519,6 +519,12 @@ extern void warn_deprecated (const char *, const char *, int, const char *);
#define bfd_set_cacheable(abfd,bool) (((abfd)->cacheable = bool), TRUE)
+/* Set the BFD's filename to point to NAME. NAME should have at least
+ the same lifetime as the BFD itself. Most frequently the caller
+ allocates it in the BFD's memory or passes in a constant string.
+ Returns NAME. */
+#define bfd_set_filename(abfd, name) ((abfd)->filename = (name))
+
extern bfd_boolean bfd_cache_close
(bfd *abfd);
/* NB: This declaration should match the autogenerated one in libbfd.h. */
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 71996db..c11e29b 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -526,6 +526,11 @@ extern void warn_deprecated (const char *, const char *, int, const char *);
#define bfd_set_cacheable(abfd,bool) (((abfd)->cacheable = bool), TRUE)
+/* Set the BFD's filename to NAME. NAME should have at least the same
+ lifetime as the BFD itself. Usually the caller allocates it in the
+ BFD's memory. Returns NAME. */
+#define bfd_set_filename(abfd, name) ((abfd)->filename = (name))
+
extern bfd_boolean bfd_cache_close
(bfd *abfd);
/* NB: This declaration should match the autogenerated one in libbfd.h. */
@@ -1029,7 +1034,7 @@ bfd *bfd_openr (const char *filename, const char *target);
bfd *bfd_fdopenr (const char *filename, const char *target, int fd);
-bfd *bfd_openstreamr (const char *, const char *, void *);
+bfd *bfd_openstreamr (const char * filename, const char * target, void * stream);
bfd *bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
@@ -1060,6 +1065,8 @@ bfd_boolean bfd_make_readable (bfd *abfd);
void *bfd_alloc (bfd *abfd, bfd_size_type wanted);
+char *bfd_strdup (bfd *abfd, const char *str);
+
void *bfd_zalloc (bfd *abfd, bfd_size_type wanted);
unsigned long bfd_calc_gnu_debuglink_crc32
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 0328748..728faf5 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -71,7 +71,6 @@
#include "bfdlink.h"
#include "libbfd.h"
#include "elf-bfd.h"
-#include "libiberty.h"
/* Renaming structures, typedefs, macros and functions to be size-specific. */
#define Elf_External_Ehdr NAME(Elf,External_Ehdr)
@@ -1803,7 +1802,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
bfd_set_error (bfd_error_no_memory);
return NULL;
}
- nbfd->filename = xstrdup ("<in-memory>");
+ bfd_set_filename (nbfd, "<in-memory>");
nbfd->xvec = templ->xvec;
bim->size = contents_size;
bim->buffer = contents;
diff --git a/bfd/ieee.c b/bfd/ieee.c
index e1734ec..132b287 100644
--- a/bfd/ieee.c
+++ b/bfd/ieee.c
@@ -33,7 +33,6 @@
#include "ieee.h"
#include "libieee.h"
#include "safe-ctype.h"
-#include "libiberty.h"
struct output_buffer_struct
{
@@ -1823,7 +1822,7 @@ ieee_object_p (bfd *abfd)
goto got_wrong_format;
ieee->mb.module_name = read_id (&(ieee->h));
if (abfd->filename == (const char *) NULL)
- abfd->filename = xstrdup (ieee->mb.module_name);
+ bfd_set_filename (abfd, ieee->mb.module_name);
/* Determine the architecture and machine type of the object file. */
{
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 6640a6a..0364a40 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -4353,16 +4353,16 @@ bfd_mach_o_fat_member_init (bfd *abfd,
if (ap)
{
/* Use the architecture name if known. */
- abfd->filename = xstrdup (ap->printable_name);
+ bfd_set_filename (abfd, ap->printable_name);
}
else
{
/* Forge a uniq id. */
const size_t namelen = 2 + 8 + 1 + 2 + 8 + 1;
- char *name = xmalloc (namelen);
+ char *name = bfd_alloc (abfd, namelen);
snprintf (name, namelen, "0x%lx-0x%lx",
entry->cputype, entry->cpusubtype);
- abfd->filename = name;
+ bfd_set_filename (abfd, name);
}
areltdata = bfd_zmalloc (sizeof (struct areltdata));
diff --git a/bfd/oasys.c b/bfd/oasys.c
index b8e457e..b96187f 100644
--- a/bfd/oasys.c
+++ b/bfd/oasys.c
@@ -26,7 +26,6 @@
#include "libbfd.h"
#include "oasys.h"
#include "liboasys.h"
-#include "libiberty.h"
/* Read in all the section data and relocation stuff too. */
@@ -1117,7 +1116,7 @@ oasys_openr_next_archived_file (bfd *arch, bfd *prev)
{
p->abfd = _bfd_create_empty_archive_element_shell (arch);
p->abfd->origin = p->pos;
- p->abfd->filename = xstrdup (p->name);
+ bfd_set_filename (p->abfd, p->name);
/* Fixup a pointer to this element for the member. */
p->abfd->arelt_data = (void *) p;
diff --git a/bfd/opncls.c b/bfd/opncls.c
index 54744ce..7466921 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -123,8 +123,6 @@ _bfd_delete_bfd (bfd *abfd)
objalloc_free ((struct objalloc *) abfd->memory);
}
- if (abfd->filename)
- free ((char *) abfd->filename);
free (abfd->arelt_data);
free (abfd);
}
@@ -228,7 +226,11 @@ bfd_fopen (const char *filename, const char *target, const char *mode, int fd)
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
/* Figure out whether the user is opening the file for reading,
writing, or both, by looking at the MODE argument. */
@@ -395,7 +397,12 @@ bfd_openstreamr (const char *filename, const char *target, void *streamarg)
nbfd->iostream = stream;
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
+
nbfd->direction = read_direction;
if (! bfd_cache_init (nbfd))
@@ -589,7 +596,11 @@ bfd_openr_iovec (const char *filename, const char *target,
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
nbfd->direction = read_direction;
/* `open_p (...)' would get expanded by an the open(2) syscall macro. */
@@ -656,7 +667,11 @@ bfd_openw (const char *filename, const char *target)
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
nbfd->direction = write_direction;
if (bfd_open_file (nbfd) == NULL)
@@ -808,7 +823,11 @@ bfd_create (const char *filename, bfd *templ)
return NULL;
/* PR 11983: Do not cache the original filename, but
rather make a copy - the original might go away. */
- nbfd->filename = xstrdup (filename);
+ if (bfd_set_filename (nbfd, bfd_strdup (nbfd, filename)) == NULL)
+ {
+ _bfd_delete_bfd (nbfd);
+ return NULL;
+ }
if (templ)
nbfd->xvec = templ->xvec;
nbfd->direction = no_direction;
@@ -991,6 +1010,31 @@ bfd_alloc2 (bfd *abfd, bfd_size_type nmemb, bfd_size_type size)
/*
FUNCTION
+ bfd_strdup
+
+SYNOPSIS
+ char *bfd_strdup (bfd *abfd, const char *str);
+
+DESCRIPTION
+ Copy a string into memory attached to <<abfd>> and return a
+ pointer to it.
+*/
+
+char *
+bfd_strdup (bfd *abfd, const char *str)
+{
+ bfd_size_type size;
+ char *p;
+
+ size = strlen (str) + 1;
+ p = bfd_alloc (abfd, size);
+ if (p != NULL)
+ memcpy (p, str, size);
+ return p;
+}
+
+/*
+FUNCTION
bfd_zalloc
SYNOPSIS
diff --git a/bfd/syms.c b/bfd/syms.c
index 27b40eb..aa4718f 100644
--- a/bfd/syms.c
+++ b/bfd/syms.c
@@ -1383,9 +1383,6 @@ _bfd_stab_section_find_nearest_line (bfd *abfd,
{
size_t len;
- /* Don't free info->filename here. objdump and other
- apps keep a copy of a previously returned file name
- pointer. */
len = strlen (file_name) + 1;
info->filename = (char *) bfd_alloc (abfd, dirlen + len);
if (info->filename == NULL)
diff --git a/bfd/vms-lib.c b/bfd/vms-lib.c
index 407c186..709c457 100644
--- a/bfd/vms-lib.c
+++ b/bfd/vms-lib.c
@@ -25,7 +25,6 @@
#include "libbfd.h"
#include "safe-ctype.h"
#include "bfdver.h"
-#include "libiberty.h"
#include "vms.h"
#include "vms/lbr.h"
#include "vms/dcx.h"
@@ -1377,7 +1376,7 @@ _bfd_vms_lib_get_module (bfd *abfd, unsigned int modidx)
default:
break;
}
- res->filename = xstrdup (name);
+ bfd_set_filename (res, name);
tdata->cache[modidx] = res;
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 5230d21..9d84815 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -634,6 +634,20 @@ gdb_bfd_requires_relocations (bfd *abfd)
return gdata->needs_relocations;
}
+/* See gdb_bfd.h. */
+
+char *
+gdb_bfd_strdup (bfd *abfd, const char *str)
+{
+ char *p;
+
+ p = bfd_strdup (abfd, str);
+ if (p == NULL)
+ malloc_failure (0);
+
+ return p;
+}
+
\f
/* A callback for htab_traverse that prints a single BFD. */
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index d415005..8bcdf9a 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -70,6 +70,11 @@ const gdb_byte *gdb_bfd_map_section (asection *section, bfd_size_type *size);
int gdb_bfd_crc (struct bfd *abfd, unsigned long *crc_out);
+/* A wrapper for bfd_strdup that never returns NULL. */
+
+char *gdb_bfd_strdup (bfd *abfd, const char *str)
+ ATTRIBUTE_RETURNS_NONNULL;
+
\f
/* A wrapper for bfd_fopen that initializes the gdb-specific reference
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index fefa51f..f01c307 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -728,8 +728,7 @@ solib_aix_bfd_open (char *pathname)
to allow commands listing all shared libraries to display that
synthetic name. Otherwise, we would only be displaying the name
of the archive member object. */
- xfree (bfd_get_filename (object_bfd));
- object_bfd->filename = xstrdup (pathname);
+ bfd_set_filename (object_bfd, gdb_bfd_strdup (object_bfd, pathname));
gdb_bfd_unref (archive_bfd);
do_cleanups (cleanup);
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index ba807a2..d8e64dc 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -621,8 +621,7 @@ darwin_bfd_open (char *pathname)
/* The current filename for fat-binary BFDs is a name generated
by BFD, usually a string containing the name of the architecture.
Reset its value to the actual filename. */
- xfree (bfd_get_filename (res));
- res->filename = xstrdup (pathname);
+ bfd_set_filename (res, gdb_bfd_strdup (res, pathname));
gdb_bfd_unref (abfd);
return res;
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index abb8c15..adcc72e 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -382,8 +382,7 @@ spu_bfd_open (char *pathname)
strcat (buf, original_name);
- xfree ((char *)abfd->filename);
- abfd->filename = xstrdup (buf);
+ bfd_set_filename (abfd, gdb_bfd_strdup (abfd, buf));
}
}
diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c
index e9b155b..e07f854 100644
--- a/gdb/spu-linux-nat.c
+++ b/gdb/spu-linux-nat.c
@@ -341,8 +341,7 @@ spu_bfd_open (ULONGEST addr)
bfd_get_section_contents (nbfd, spu_name, buf, 20, sect_size - 20);
buf[sect_size - 20] = '\0';
- xfree ((char *)nbfd->filename);
- nbfd->filename = xstrdup (buf);
+ bfd_set_filename (nbfd, gdb_bfd_strdup (nbfd, buf));
}
}
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..9bf6318 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -101,11 +101,10 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
error (_("Failed to read a valid object file image from memory."));
gdb_bfd_ref (nbfd);
- xfree (bfd_get_filename (nbfd));
if (name == NULL)
- nbfd->filename = xstrdup ("shared object read from target memory");
+ bfd_set_filename (nbfd, "shared object read from target memory");
else
- nbfd->filename = name;
+ bfd_set_filename (nbfd, gdb_bfd_strdup (nbfd, name));
cleanup = make_cleanup_bfd_unref (nbfd);
@@ -225,6 +224,7 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
args.from_tty = 0;
catch_exceptions (current_uiout, symbol_file_add_from_memory_wrapper,
&args, RETURN_MASK_ALL);
+ xfree (args.name);
}
}
--
1.7.11.7
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-01-07 19:54 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-04 16:23 [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983 Hui Zhu
2014-01-05 15:06 ` Sergio Durigan Junior
2014-01-05 15:48 ` Hui Zhu
2014-01-06 8:25 ` Tom Tromey
2014-01-06 10:50 ` Hui Zhu
2014-01-06 16:14 ` Tom Tromey
2014-01-06 17:12 ` Doug Evans
2014-01-06 21:07 ` Tom Tromey
2014-01-07 12:35 ` Pedro Alves
2014-01-07 13:55 ` Pedro Alves
2014-01-07 17:41 ` Tom Tromey
2014-01-07 19:54 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).