Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH]13217 thread apply all detach throws a SEGFAULT
       [not found] <97B73E257CC18646B0B5D3DD77DCBDD158E8FD@EU-MBX-02.mgc.mentorg.com>
@ 2013-01-14 13:38 ` Bilal, Muhammad
  2013-01-14 15:25   ` Yao Qi
                     ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Bilal, Muhammad @ 2013-01-14 13:38 UTC (permalink / raw)
  To: Tom Tromey, eliz, brobecker, dje, jan.kratochvil, gdb-patches

Hi,



I worked on bug#13217 "thread apply all detach throws a  SEGFAULT " of gdb

and fixed this problem by making a Patch.

I am upstreaming of it 



Actually when command "thread apply all detach" is applied then the function thread_apply_command (char *tidlist, int from_tty) in thread.c:1179 called a function init_thread_list (void)in thread.c:140

which makes the 'thread_list' struck to null so when for (tp = thread_list; tp; tp = tp->next) loop in thread_apply_command (char *tidlist, int from_tty) in thread.c:1179 iterates 2nd time it throws a segmentation fault when try to access the bogus value of
 thread_list

so i have fixed this problem

I have written the test case for it also

I have worked on gdb-7.5.50.20121127 snapshot



1) 

gdb/thread.c



diff -u  thread.c new_thread.c > change_thread.c

 ------------PATCH---------------------------------------

--- thread.c    2012-07-27 05:52:36.000000000 +0500

+++ new_thread.c    2013-01-02 12:06:51.876346782 +0500

@@ -1203,6 +1203,8 @@

     execute_command (cmd, from_tty);

     strcpy (cmd, saved_cmd);    /* Restore exact command used

                        previously.  */

+       if(thread_list == NULL)

+         break;

       }

 

   do_cleanups (old_chain);





2)

gdb/testsuit/gdb.thread

-----------------------------------test------------------------------------



--- threadapply.exp    2013-01-14 16:30:25.000000000 +0500

+++ new_threadapply.exp    2013-01-14 16:39:07.000000000 +0500

@@ -63,3 +63,4 @@

 gdb_test "up" ".*in main.*" "go up in the stack frame" 

 gdb_test "thread apply all print 1"  "Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1" "run a simple print command on all threads"

 gdb_test "down" "#0.*thread_function.*" "go down and check selected frame"

+gdb_test "thread apply all detach" "Thread.*\nDetaching from.*" "detaching from all threads"



------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------



please review it and add me sourceware.org



thanks

Bilal











































 

 






^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH]13217 thread apply all detach throws a SEGFAULT
  2013-01-14 13:38 ` [PATCH]13217 thread apply all detach throws a SEGFAULT Bilal, Muhammad
@ 2013-01-14 15:25   ` Yao Qi
  2013-01-14 15:41   ` Jan Kratochvil
  2013-01-14 15:46   ` Pedro Alves
  2 siblings, 0 replies; 4+ messages in thread
From: Yao Qi @ 2013-01-14 15:25 UTC (permalink / raw)
  To: Bilal, Muhammad; +Cc: gdb-patches

[Don't need to copy so many maintainers :)]

On 01/14/2013 09:38 PM, Bilal, Muhammad wrote:

> Actually when command "thread apply all detach" is applied then the function thread_apply_command (char *tidlist, int from_tty) in thread.c:1179 called a function init_thread_list (void)in thread.c:140
>
> which makes the 'thread_list' struck to null so when for (tp = thread_list; tp; tp = tp->next) loop in thread_apply_command (char *tidlist, int from_tty) in thread.c:1179 iterates 2nd time it throws a segmentation fault when try to access the bogus value of
>   thread_list
>
> so i have fixed this problem
>
> I have written the test case for it also
>

Bilal,
thanks for trying to fix the bug, and adding a test case for it.

> I have worked on gdb-7.5.50.20121127 snapshot

We work on GDB CVS trunk in the development.  You should re-generate 
your patch on the basis of GDB trunk.

The patch looks ill-formated, which prohibits maintainers giving a 
review.  Please read gdb/CONTRIBUTE to see how to submit a patch.

-- 
Yao (齐尧)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH]13217 thread apply all detach throws a SEGFAULT
  2013-01-14 13:38 ` [PATCH]13217 thread apply all detach throws a SEGFAULT Bilal, Muhammad
  2013-01-14 15:25   ` Yao Qi
@ 2013-01-14 15:41   ` Jan Kratochvil
  2013-01-14 15:46   ` Pedro Alves
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Kratochvil @ 2013-01-14 15:41 UTC (permalink / raw)
  To: Bilal, Muhammad; +Cc: Tom Tromey, eliz, brobecker, dje, gdb-patches

On Mon, 14 Jan 2013 14:38:22 +0100, Bilal, Muhammad wrote:
> I worked on bug#13217 "thread apply all detach throws a  SEGFAULT " of gdb

patch on this PR has been already posted in a more complete state, it got
a review but it has not yet been finished by its
submitter Ali <ali_anwar@codesourcery.com>.

	Re: Updated patch for Bug 13217 - thread apply all detach throws a SEGFAULT
	http://sourceware.org/ml/gdb-patches/2012-09/msg00580.html
	http://sourceware.org/ml/gdb-patches/2012-12/msg00268.html
	Message-ID: <5062EA9B.1060505@codesourcery.com>



Thanks,
Jan


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH]13217 thread apply all detach throws a SEGFAULT
  2013-01-14 13:38 ` [PATCH]13217 thread apply all detach throws a SEGFAULT Bilal, Muhammad
  2013-01-14 15:25   ` Yao Qi
  2013-01-14 15:41   ` Jan Kratochvil
@ 2013-01-14 15:46   ` Pedro Alves
  2 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2013-01-14 15:46 UTC (permalink / raw)
  To: Bilal, Muhammad; +Cc: gdb-patches

Hi Bilal,

The problem with this fix is that it handles one
specific crash, but is not a complete fix, as it
still leaves undefined behavior in place, which may
well manifest in similar crashes, or worse, random corruption.
The issue is that the command applies to each thread may
not remove _all_ threads (as detected by your patch), but cause
the currently iterated thread to exit, and thus, (at least)
this "tp->next" reference:

  for (tp = thread_list; tp; tp = tp->next)
                             ^^^^^^^^^^^^^
    if (thread_alive (tp))

Please coordinate with Ali Anwar @ Mentor.  He was
working on this issue a couple months back, and posted a
more complete patch, though review showed some more
work was necessary.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-01-14 15:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <97B73E257CC18646B0B5D3DD77DCBDD158E8FD@EU-MBX-02.mgc.mentorg.com>
2013-01-14 13:38 ` [PATCH]13217 thread apply all detach throws a SEGFAULT Bilal, Muhammad
2013-01-14 15:25   ` Yao Qi
2013-01-14 15:41   ` Jan Kratochvil
2013-01-14 15:46   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox