Re: Undefined behaviour in subr_hibernate.c

2016-08-31 Thread Mike Larkin
On Wed, Aug 31, 2016 at 10:47:50AM +0200, Michal Mazurek wrote:
> Section "J.2 Undefined behavior" of n1570 includes:
> * Pointers that do not point to the same aggregate or union (nor just
>   beyond the same array object) are compared using relational
>   operators (6.5.8).
> 
> I think that's what's happening when using RB trees in subr_hibernate.c.
> subr_pool.c fixes this issue by casting the address to vaddr_t. Diff
> to do the same in subr_hibernate.c below.
> 

If this passes a couple ZZZ/resume cycles, go for it.

> Index: sys/kern/subr_hibernate.c
> ===
> RCS file: /cvs/src/sys/kern/subr_hibernate.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 subr_hibernate.c
> --- sys/kern/subr_hibernate.c 4 May 2015 02:18:05 -   1.116
> +++ sys/kern/subr_hibernate.c 31 Aug 2016 08:35:53 -
> @@ -156,7 +156,10 @@ hibernate_sort_ranges(union hibernate_in
>  static __inline int
>  hibe_cmp(struct hiballoc_entry *l, struct hiballoc_entry *r)
>  {
> - return l < r ? -1 : (l > r);
> + vaddr_t vl = (vaddr_t)l;
> + vaddr_t vr = (vaddr_t)r;
> +
> + return vl < vr ? -1 : (vl > vr);
>  }
>  
>  RB_PROTOTYPE(hiballoc_addr, hiballoc_entry, hibe_entry, hibe_cmp)
> 
> -- 
> Michal Mazurek
> 



Undefined behaviour in subr_hibernate.c

2016-08-31 Thread Michal Mazurek
Section "J.2 Undefined behavior" of n1570 includes:
* Pointers that do not point to the same aggregate or union (nor just
  beyond the same array object) are compared using relational
  operators (6.5.8).

I think that's what's happening when using RB trees in subr_hibernate.c.
subr_pool.c fixes this issue by casting the address to vaddr_t. Diff
to do the same in subr_hibernate.c below.

Index: sys/kern/subr_hibernate.c
===
RCS file: /cvs/src/sys/kern/subr_hibernate.c,v
retrieving revision 1.116
diff -u -p -r1.116 subr_hibernate.c
--- sys/kern/subr_hibernate.c   4 May 2015 02:18:05 -   1.116
+++ sys/kern/subr_hibernate.c   31 Aug 2016 08:35:53 -
@@ -156,7 +156,10 @@ hibernate_sort_ranges(union hibernate_in
 static __inline int
 hibe_cmp(struct hiballoc_entry *l, struct hiballoc_entry *r)
 {
-   return l < r ? -1 : (l > r);
+   vaddr_t vl = (vaddr_t)l;
+   vaddr_t vr = (vaddr_t)r;
+
+   return vl < vr ? -1 : (vl > vr);
 }
 
 RB_PROTOTYPE(hiballoc_addr, hiballoc_entry, hibe_entry, hibe_cmp)

-- 
Michal Mazurek