Re: __dead functions

[Alexander Shishkin <alexander.shishkin%teleca.com@localhost>]

On Fri, Jul 18, 2008 at 06:06:39PM +0400, Valeriy E. Ushakov wrote:
> On Fri, Jul 18, 2008 at 16:50:11 +0300, Alexander Shishkin wrote:
> 
> > --- a/sys/kern/kern_lwp.c
> > +++ b/sys/kern/kern_lwp.c
> > @@ -837,6 +837,8 @@ lwp_exit(struct lwp *l)
> >  #endif
> >             lwp_exit_switchaway(l);
> >     }
> > +
> > +   __NOTREACHED;
> >  }
> 
> Is that really needed/correct?  lwp_exit_switchaway is declared dead
> in this patch, so the body of the if should be covered by that.  What
> is intended to happen is "current" is false?
You're right, this particular function does seem to return under certain
conditions and its users prove this. Turns out it was declared as __dead
by mistake.

> So, from a quick look either this hunk is not necessary at all or it
> should be replaced with an assertion.
Or maybe an assertion can be placed right before __NOTREACHED?

> This should be protected with GCC_PREREQ I guess.
Ok.

> I also think that explicit 
> 
> #define __NOTREACHED for (;;) continue
> 
> is better so that compiler barfs on something like an accidental
> __NOTREACHED { /* ... */ }
Ok.

Updated patch [1] is also appended, please consider applying.

[1]: http://koowaldah.org/people/ash/netbsd/noreturn-fix-try2.patch

Regards,
--
Alex

diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c
index 544f8e1..3e3007a 100644
--- a/sys/kern/kern_synch.c
+++ b/sys/kern/kern_synch.c
@@ -886,7 +886,7 @@ lwp_exit_switchaway(lwp_t *l)
        /* Switch to the new LWP.. */
        (void)cpu_switchto(NULL, newl, false);
 
-       /* NOTREACHED */
+       __NOTREACHED;
 }
 
 /*
diff --git a/sys/sys/cdefs.h b/sys/sys/cdefs.h
index 3e61274..b93a46c 100644
--- a/sys/sys/cdefs.h
+++ b/sys/sys/cdefs.h
@@ -164,6 +164,16 @@
 #endif
 
 /*
+ * Explicitly specify that a certain point in the codepath should not
+ * be reached and indicate so to gcc.
+ */
+#if __GNUC_PREREQ__(4, 3)
+#define __NOTREACHED for (;;) continue
+#else
+#define __NOTREACHED
+#endif
+
+/*
  * GCC1 and some versions of GCC2 declare dead (non-returning) and
  * pure (no side effects) functions using "volatile" and "const";
  * unfortunately, these then cause warnings under "-ansi -pedantic".
diff --git a/sys/sys/lwp.h b/sys/sys/lwp.h
index aee970c..ce5bc7a 100644
--- a/sys/sys/lwp.h
+++ b/sys/sys/lwp.h
@@ -279,8 +279,8 @@ void        lwp_continue(lwp_t *);
 void   cpu_setfunc(lwp_t *, void (*)(void *), void *);
 void   startlwp(void *);
 void   upcallret(lwp_t *);
-void   lwp_exit(lwp_t *) __dead;
-void   lwp_exit_switchaway(lwp_t *);
+void   lwp_exit(lwp_t *);
+void   lwp_exit_switchaway(lwp_t *) __dead;
 int    lwp_suspend(lwp_t *, lwp_t *);
 int    lwp_create1(lwp_t *, const void *, size_t, u_long, lwpid_t *);
 void   lwp_update_creds(lwp_t *);
-- 
1.5.5.4