[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Crashes in 1.8.0 test suite on Solaris Sparc (wrong alignment in cache_lookup())

From: Sergey Raevskiy <sergey.raevskiy_at_visualsvn.com>
Date: Thu, 20 Jun 2013 19:37:22 +0400

> Rainer, can you check if the patch below works for you?
> If so, we can merge both patches and put the new code into an #else block.
>
...
> + for (j = 0; j < 4; j++)
> + val |= (path[i + j] << j);
...

It seems that this sode should be:
+ for (j = 0; j < 4; j++)
+ val |= (path[i + j] << (j * 8));

On Thu, Jun 20, 2013 at 7:07 PM, Stefan Sperling <stsp_at_elego.de> wrote:

> On Thu, Jun 20, 2013 at 06:54:15PM +0400, Ivan Zhakov wrote:
> > On Thu, Jun 20, 2013 at 6:43 PM, Rainer Jung <rainer.jung_at_kippdata.de>
> wrote:
> > > Hi there,
> > >
> > > I built and tested svn 1.8.0 today on Solaris 0 Sparc and got lots of
> > > test failures due to core dumps.
> > >
> > > The first few dumps I inspected all showed a bus error in
> > >
> > > #0 0xfe660760 in cache_lookup (path=0x10fce06 "/A/D/H/pi3",
> revision=3,
> > > cache=0x17c1820) at subversion/libsvn_fs_fs/tree.c:357
> > >
> > > The code is:
> > >
> > > for (i = 0; i + 4 <= path_len; i += 4)
> > > hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path +
> i);
> > >
> > It seems the code is missing #ifdef SVN_UNALIGNED_ACCESS_IS_OK . The
> > attached patch should fix problem, but I'm not sure that this is right
> > solution for the problem.
>
> This needs an #else implementation that works on other platforms.
> We need to calculate some hash value, after all.
>
> Rainer, can you check if the patch below works for you?
> If so, we can merge both patches and put the new code into an #else block.
>
> Index: subversion/libsvn_fs_fs/tree.c
> ===================================================================
> --- subversion/libsvn_fs_fs/tree.c (revision 1494951)
> +++ subversion/libsvn_fs_fs/tree.c (working copy)
> @@ -354,8 +354,16 @@ cache_lookup( fs_fs_dag_cache_t *cache
> /* need to do a full lookup. Calculate the hash value
> (HASH_VALUE has been initialized to REVISION). */
> for (i = 0; i + 4 <= path_len; i += 4)
> - hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path +
> i);
> + {
> + apr_uint32_t val = 0;
> + int j;
>
> + for (j = 0; j < 4; j++)
> + val |= (path[i + j] << j);
> +
> + hash_value = hash_value * 0xd1f3da69 + val;
> + }
> +
> for (; i < path_len; ++i)
> hash_value = hash_value * 33 + path[i];
>
>
> > Index: subversion/libsvn_fs_fs/tree.c
> > ===================================================================
> > --- subversion/libsvn_fs_fs/tree.c (revision 1494562)
> > +++ subversion/libsvn_fs_fs/tree.c (working copy)
> > @@ -353,8 +353,11 @@
> >
> > /* need to do a full lookup. Calculate the hash value
> > (HASH_VALUE has been initialized to REVISION). */
> > +
> > +#ifdef SVN_UNALIGNED_ACCESS_IS_OK
> > for (i = 0; i + 4 <= path_len; i += 4)
> > hash_value = hash_value * 0xd1f3da69 + *(const apr_uint32_t*)(path
> + i);
> > +#endif
> >
> > for (; i < path_len; ++i)
> > hash_value = hash_value * 33 + path[i];
>
>
Received on 2013-06-20 17:37:52 CEST

This is an archived mail posted to the Subversion Dev mailing list.