You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Dengke Du <de...@windriver.com> on 2016/11/02 09:21:49 UTC
[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 <de...@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,
svn_checksum_to_cstring(l2p_checksum,
scratch_pool),
- p2l_offset,
+ (APR_OFF_T_FMT=="lld") ? (apr_int64_t)p2l_offset : p2l_offset,
svn_checksum_to_cstring(p2l_checksum,
scratch_pool));
}
--
2.7.4
Re: [PATCH] fix "svnadmin create" fail on x86
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Bert Huijben wrote on Wed, Nov 02, 2016 at 11:57:29 +0100:
>
>
> > -----Original Message-----
> > From: Dengke Du [mailto:dengke.du@windriver.com]
> > Sent: woensdag 2 november 2016 10:22
> > To: dev@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 <de...@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
RE: [PATCH] fix "svnadmin create" fail on x86
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Dengke Du [mailto:dengke.du@windriver.com]
> Sent: woensdag 2 november 2016 10:22
> To: dev@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 <de...@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...
The result of the '?:' operator is always the same type, following a set of
rules set by the C standard.
Besides that the result of the '==' operator on static strings is not
defined and commonly different between release and debug builds.
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.
Bert