This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch v7 3/5] x86* unwinder: libdwfl/


On Fri, 25 Oct 2013 11:44:57 +0200, Mark Wielaard wrote:
> On Wed, 2013-10-23 at 15:57 +0200, Jan Kratochvil wrote:
> > diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
> > @@ -256,34 +226,35 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
> > [...]
> > +  Dwfl_Thread thread;
> > +  thread.process = process;
> > +  thread.unwound = NULL;
> 
> And  thread.callbacks_args = NULL;
> 
> >    for (;;)
> >      {

It should be rather here.  thread.callbacks_args is specific for each thread
and it would be a mistake to rely on the same value across different threads.


> > -      nthread->tid = process->callbacks->next_thread (dwfl, nthread,
> > -						      process->callbacks_arg,
> > -						      &nthread->callbacks_arg);
> > -      if (nthread->tid < 0)
> > +      assert (thread.unwound == NULL);
> > +      thread.tid = process->callbacks->next_thread (dwfl,
> > +						    process->callbacks_arg,
> > +						    &thread.callbacks_arg);
> 
> So also the first time the next_thread callback is called
> thread.callbacks_arg has a defined (NULL) value.

I was looking that it was uninitialized already in the first post (different
API) and it was intentional.  Documentation does not guarantee NULL and if the
application does not use thread.callbacks_arg at all it does not have to be
touched.

But I see now it should be initialized due to false warnings by Valgrind.


Thanks,
Jan

diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
index 0806678..ee20afd 100644
--- a/libdwfl/dwfl_frame.c
+++ b/libdwfl/dwfl_frame.c
@@ -231,12 +231,15 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
       __libdwfl_seterrno (DWFL_E_NO_ATTACH_STATE);
       return -1;
     }
-  Dwfl_Thread thread;
-  thread.process = process;
-  thread.unwound = NULL;
   for (;;)
     {
-      assert (thread.unwound == NULL);
+      Dwfl_Thread thread;
+      thread.process = process;
+      thread.unwound = NULL;
+      /* Nobody guarantees NULL but tools like valgrind could warn on accessing
+	 uninitialized data if the application does not set it (as the
+	 application does not use it).  */
+      thread.callbacks_arg = NULL;
       thread.tid = process->callbacks->next_thread (dwfl,
 						    process->callbacks_arg,
 						    &thread.callbacks_arg);
@@ -259,6 +262,7 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
 	  thread_free_all_states (&thread);
 	  return err;
 	}
+      assert (thread.unwound == NULL);
     }
   /* NOTREACHED */
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]