[patch] Fix ASAN crash for gdb.compile/compile.exp

[patch] Fix ASAN crash for gdb.compile/compile.exp

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

Hi,

(gdb) PASS: gdb.compile/compile.exp: set unwindonsignal on
compile code *(volatile int *) 0 = 0;
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fba426 in _gdb_expr (__regs=0x7ffff7fb8000) at gdb command line:1
1       gdb command line: No such file or directory.
=================================================================
==10462==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000cf7a3d at pc 0x0000004e46b9 bp 0x7ffdeb0f7a40 sp 0x7ffdeb0f71b8
READ of size 10 at 0x621000cf7a3d thread T0
    #0 0x4e46b8 in printf_common(void*, char const*, __va_list_tag*) [clone .isra.6] (/home/jkratoch/redhat/gdb-clean-asan/gdb/gdb+0x4e46b8) 
    #1 0x4f645e in vasprintf (/home/jkratoch/redhat/gdb-clean-asan/gdb/gdb+0x4f645e)
    #2 0xe5cf00 in xstrvprintf common/common-utils.c:120
    #3 0xe74192 in throw_it common/common-exceptions.c:332
    #4 0xe742f6 in throw_verror common/common-exceptions.c:361
    #5 0xddc89e in verror /home/jkratoch/redhat/gdb-clean-asan/gdb/utils.c:541
    #6 0xe734bd in error common/errors.c:43
    #7 0xafa1d6 in call_function_by_hand_dummy /home/jkratoch/redhat/gdb-clean-asan/gdb/infcall.c:1031
    #8 0xe81858 in compile_object_run compile/compile-object-run.c:119
    #9 0xe7733c in eval_compile_command compile/compile.c:577
    #10 0xe7541e in compile_code_command compile/compile.c:153


It is obvious why that happens, dummy_frame_pop() will call compile objfile
cleanup which will free that objfile and NAME then becomes a stale pointer.

Whether the objfile really should be already freed during
call_function_by_hand_dummy() I am not sure.

No regressions on {x86_64,x86_64-m32,i686}-fedora23pre-linux-gnu.


Jan

[-- Attachment #2: compileasan.patch --]
[-- Type: text/plain, Size: 821 bytes --]

gdb/ChangeLog
2015-04-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix ASAN crash for gdb.compile/compile.exp.
	* infcall.c (call_function_by_hand_dummy): Use xstrdup for NAME.

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 26c64f6..bbe163f 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1005,8 +1005,11 @@ When the function is done executing, GDB will silently stop."),
 
   if (stopped_by_random_signal || stop_stack_dummy != STOP_STACK_DUMMY)
     {
-      const char *name = get_function_name (funaddr,
-					    name_buf, sizeof (name_buf));
+      /* Make a copy as NAME may be in an objfile freed by dummy_frame_pop.  */
+      char *name = xstrdup (get_function_name (funaddr,
+					       name_buf, sizeof (name_buf)));
+      make_cleanup (xfree, name);
+
 
       if (stopped_by_random_signal)
 	{

ping: [patch] Fix ASAN crash for gdb.compile/compile.exp

[Jan Kratochvil]

Sat, 18 Apr 2015 19:01:38 +0200, Jan Kratochvil wrote:
Hi,

(gdb) PASS: gdb.compile/compile.exp: set unwindonsignal on
compile code *(volatile int *) 0 = 0;
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fba426 in _gdb_expr (__regs=0x7ffff7fb8000) at gdb command line:1
1       gdb command line: No such file or directory.
=================================================================
==10462==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000cf7a3d at pc 0x0000004e46b9 bp 0x7ffdeb0f7a40 sp 0x7ffdeb0f71b8
READ of size 10 at 0x621000cf7a3d thread T0
    #0 0x4e46b8 in printf_common(void*, char const*, __va_list_tag*) [clone .isra.6] (/home/jkratoch/redhat/gdb-clean-asan/gdb/gdb+0x4e46b8) 
    #1 0x4f645e in vasprintf (/home/jkratoch/redhat/gdb-clean-asan/gdb/gdb+0x4f645e)
    #2 0xe5cf00 in xstrvprintf common/common-utils.c:120
    #3 0xe74192 in throw_it common/common-exceptions.c:332
    #4 0xe742f6 in throw_verror common/common-exceptions.c:361
    #5 0xddc89e in verror /home/jkratoch/redhat/gdb-clean-asan/gdb/utils.c:541
    #6 0xe734bd in error common/errors.c:43
    #7 0xafa1d6 in call_function_by_hand_dummy /home/jkratoch/redhat/gdb-clean-asan/gdb/infcall.c:1031
    #8 0xe81858 in compile_object_run compile/compile-object-run.c:119
    #9 0xe7733c in eval_compile_command compile/compile.c:577
    #10 0xe7541e in compile_code_command compile/compile.c:153


It is obvious why that happens, dummy_frame_pop() will call compile objfile
cleanup which will free that objfile and NAME then becomes a stale pointer.

Whether the objfile really should be already freed during
call_function_by_hand_dummy() I am not sure.

No regressions on {x86_64,x86_64-m32,i686}-fedora23pre-linux-gnu.


Jan

gdb/ChangeLog
2015-04-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix ASAN crash for gdb.compile/compile.exp.
	* infcall.c (call_function_by_hand_dummy): Use xstrdup for NAME.

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 26c64f6..bbe163f 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1005,8 +1005,11 @@ When the function is done executing, GDB will silently stop."),
 
   if (stopped_by_random_signal || stop_stack_dummy != STOP_STACK_DUMMY)
     {
-      const char *name = get_function_name (funaddr,
-					    name_buf, sizeof (name_buf));
+      /* Make a copy as NAME may be in an objfile freed by dummy_frame_pop.  */
+      char *name = xstrdup (get_function_name (funaddr,
+					       name_buf, sizeof (name_buf)));
+      make_cleanup (xfree, name);
+
 
       if (stopped_by_random_signal)
 	{

Re: [patch] Fix ASAN crash for gdb.compile/compile.exp

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Hi Jan,
I didn't follow the compile work in GDB very much, so I may ask
something wrong or discussed before.

> Whether the objfile really should be already freed during
> call_function_by_hand_dummy() I am not sure.

Is there any reason we release OBJFILE in the dummy frame dtor?  Why
don't we register a cleanup to release in OBJFILE in compile_object_run?
together with releasing compile_module?  'struct compile_module' has a
field objfile, which should be released together with
'struct compile_module' instead of dummy_frame.

-- 
Yao (齐尧)

Re: [patch] Fix ASAN crash for gdb.compile/compile.exp

[Jan Kratochvil]

On Tue, 19 May 2015 13:46:53 +0200, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> > Whether the objfile really should be already freed during
> > call_function_by_hand_dummy() I am not sure.
> 
> Is there any reason we release OBJFILE in the dummy frame dtor?  Why
> don't we register a cleanup to release in OBJFILE in compile_object_run?
> together with releasing compile_module?  'struct compile_module' has a
> field objfile, which should be released together with
> 'struct compile_module' instead of dummy_frame.

(gdb) break puts
Breakpoint 2 at 0x3830c6fd30: file ioputs.c, line 34.
(gdb) compile code puts("hello")
Breakpoint 2, _IO_puts (str=0x7ffff7ff8000 "hello") at ioputs.c:34
34	{
The program being debugged stopped while in a function called from GDB.
Evaluation of the expression containing the function
(_gdb_expr) will be abandoned.
When the function is done executing, GDB will silently stop.
(gdb) bt
#0  _IO_puts (str=0x7ffff7ff8000 "hello") at ioputs.c:34
#1  0x00007ffff7ff9183 in _gdb_expr (__regs=0x7ffff7ff7000) at gdb command line:1
#2  <function called from gdb>
#3  main (argc=1, argv=0x7fffffffd7a8) at gdb.c:25
(gdb) _

Now compile_object_run() called from line
	(gdb) compile code puts("hello")
has finished for a long time.  But we still need to have that injected code
OBJFILE valid when GDB is executing it.  Therefore OBJFILE is freed only from
destructor of the frame #1.

At the patched line of call_function_by_hand_dummy() the dummy frame
destructor has not yet been run but it will be run before the fetched NAME
will get used.

In fact I do not see now how to fix it differently than what the patch does.

Obviously all these ugly hacks are needed only because GDB does not have
convenient memory management like C++/Java/others.


Jan

Re: [patch] Fix ASAN crash for gdb.compile/compile.exp

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> Now compile_object_run() called from line
> 	(gdb) compile code puts("hello")
> has finished for a long time.  But we still need to have that injected code
> OBJFILE valid when GDB is executing it.  Therefore OBJFILE is freed only from
> destructor of the frame #1.

Hmm, you are right.

>
> At the patched line of call_function_by_hand_dummy() the dummy frame
> destructor has not yet been run but it will be run before the fetched NAME
> will get used.
>
> In fact I do not see now how to fix it differently than what the patch does.

Your patch is fine by me.

-- 
Yao (齐尧)

[commit] [patch] Fix ASAN crash for gdb.compile/compile.exp

[Jan Kratochvil]

On Tue, 19 May 2015 15:42:53 +0200, Yao Qi wrote:
> Your patch is fine by me.

Checked in:
	5fe75eec33c0f55536f09b2f3d692fb688a2c423


Jan