Bert Huijben wrote on Wed, Nov 02, 2016 at 11:57:29 +0100:
>
>
> > -----Original Message-----
> > From: Dengke Du [mailto:dengke.du_at_windriver.com]
> > Sent: woensdag 2 november 2016 10:22
> > To: dev_at_subversion.apache.org
> > Subject: [PATCH] fix "svnadmin create" fail on x86
> >
> > When run the following command on x86:
> >
> > svnadmin create /var/test_repo
> >
> > It cause segmentation fault error like the following:
> >
> > [16499.751837] svnadmin[21117]: segfault at 83 ip 00000000f74bf7f6
> sp
> > 00000000ffdd9b34 error 4 in libc-2.24.so[f7441000+1af000]
> > Segmentation fault (core dumped)
> >
> > This is because in source code ./subversion/libsvn_fs_fs/low_level.c,
> > function svn_fs_fs__unparse_footer, when:
> >
> > target arch: x86
> > apr_off_t: 4 bytes
> >
> > if the "APR_OFF_T_FMT" is "lld", it still use type "apr_off_t" to pass
> > data to apr, but in apr source code file apr_snprintf.c the function
> > apr_vformatter meet "lld", it would use the:
> >
> > i_quad = va_arg(ap, apr_int64_t);
> >
> > It uses the apr_int64_t to deal data, it read 8 bytes, so the follow-up
> > data may be error.
> >
> > Signed-off-by: Dengke Du <dengke.du_at_windriver.com>
> > ---
> > subversion/libsvn_fs_fs/low_level.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/subversion/libsvn_fs_fs/low_level.c
> > b/subversion/libsvn_fs_fs/low_level.c
> > index a27bbcc..6ddbe28 100644
> > --- a/subversion/libsvn_fs_fs/low_level.c
> > +++ b/subversion/libsvn_fs_fs/low_level.c
> > @@ -250,10 +250,10 @@ svn_fs_fs__unparse_footer(apr_off_t l2p_offset,
> > {
> > return svn_stringbuf_createf(result_pool,
> > "%" APR_OFF_T_FMT " %s %" APR_OFF_T_FMT " %s",
> > - l2p_offset,
> > + (APR_OFF_T_FMT=="lld") ? (apr_int64_t)l2p_offset : l2p_offset,
>
> This doesn't change the size of the argument...
Actually, it does. On Dengke's system apr_off_t is a 32-bit type, so
the type of the ternary expression is 32-bit before the patch and 64-bit
after it, for the reasons you gave (snipped).
> And if there is a bug in formatting apr_off_t with that macro it should
> really be fixed in APR... These macros are designed to abstract away
> platform specifics, we can't just fix all similar cases in our code.
I agree, it looks like APR on Dengke's system set APR_OFF_T_FMT to
a 64-bit type and apr_off_t to a 32-bit type. This should be fixed in
APR (in fact, it may have already been fixed; I doubt Dengke was using
APR trunk). Anyway, svn_fs_fs__unparse_footer() is definitely correct
as it stands in trunk today.
Dengke: try upgrading APR and recompiling svn against that. (And check
the apr_off_t/APR_OFF_T_FMT definitions)
Cheers,
Daniel
Received on 2016-11-02 17:27:32 CET