|
From: David W. <dw...@in...> - 2011-01-12 07:54:00
|
On Tue, 2011-01-11 at 15:12 +0000, Tony Finch wrote: > Exim doesn't compile with Sun or HP CC since Valgrind support was > added. Although valgrind.h protects against usage on unsupported > platforms, memcheck.h uses the __extension__() macro without checking. > Remove all uses since VALGRIND_DO_CLIENT_REQUEST() has all the > necessary portability support. > --- > > Chris and Steen, > > Could you please try this patch and see if it fixes the > compilation problems with 4.73? Thanks. This wants to go to Valgrind upstream too. > Tony. > -- > <fa...@ex...> <do...@do...> http://dotat.at/ ${sg{\N${sg{\ > N\}{([^N]*)(.)(.)(.*)}{\$1\$3\$2\$1\$3\n\$2\$3\$4\$3\n\$3\$2\$4}}\ > \N}{([^N]*)(.)(.)(.*)}{\$1\$3\$2\$1\$3\n\$2\$3\$4\$3\n\$3\$2\$4}} > > > src/src/memcheck.h | 48 ++++++++++++++++++++++++------------------------ > 1 files changed, 24 insertions(+), 24 deletions(-) > > > diff --git a/src/src/memcheck.h b/src/src/memcheck.h > index fc50dab..d159c22 100644 > --- a/src/src/memcheck.h > +++ b/src/src/memcheck.h > @@ -107,66 +107,66 @@ typedef > > /* Mark memory at _qzz_addr as unaddressable for _qzz_len bytes. */ > #define VALGRIND_MAKE_MEM_NOACCESS(_qzz_addr,_qzz_len) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* default return */, \ > VG_USERREQ__MAKE_MEM_NOACCESS, \ > _qzz_addr, _qzz_len, 0, 0, 0); \ > _qzz_res; \ > - })) > - > + } > + > /* Similarly, mark memory at _qzz_addr as addressable but undefined > for _qzz_len bytes. */ > #define VALGRIND_MAKE_MEM_UNDEFINED(_qzz_addr,_qzz_len) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* default return */, \ > VG_USERREQ__MAKE_MEM_UNDEFINED, \ > _qzz_addr, _qzz_len, 0, 0, 0); \ > _qzz_res; \ > - })) > + } > > /* Similarly, mark memory at _qzz_addr as addressable and defined > for _qzz_len bytes. */ > #define VALGRIND_MAKE_MEM_DEFINED(_qzz_addr,_qzz_len) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* default return */, \ > VG_USERREQ__MAKE_MEM_DEFINED, \ > _qzz_addr, _qzz_len, 0, 0, 0); \ > _qzz_res; \ > - })) > + } > > /* Similar to VALGRIND_MAKE_MEM_DEFINED except that addressability is > not altered: bytes which are addressable are marked as defined, > but those which are not addressable are left unchanged. */ > #define VALGRIND_MAKE_MEM_DEFINED_IF_ADDRESSABLE(_qzz_addr,_qzz_len) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* default return */, \ > VG_USERREQ__MAKE_MEM_DEFINED_IF_ADDRESSABLE, \ > _qzz_addr, _qzz_len, 0, 0, 0); \ > _qzz_res; \ > - })) > + } > > /* Create a block-description handle. The description is an ascii > string which is included in any messages pertaining to addresses > within the specified memory range. Has no other effect on the > properties of the memory range. */ > -#define VALGRIND_CREATE_BLOCK(_qzz_addr,_qzz_len, _qzz_desc) \ > - (__extension__({unsigned long _qzz_res; \ > +#define VALGRIND_CREATE_BLOCK(_qzz_addr,_qzz_len, _qzz_desc) \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* default return */, \ > VG_USERREQ__CREATE_BLOCK, \ > _qzz_addr, _qzz_len, _qzz_desc, \ > 0, 0); \ > - _qzz_res; \ > - })) > + _qzz_res; \ > + } > > /* Discard a block-description-handle. Returns 1 for an > invalid handle, 0 for a valid handle. */ > #define VALGRIND_DISCARD(_qzz_blkindex) \ > - (__extension__ ({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* default return */, \ > VG_USERREQ__DISCARD, \ > 0, _qzz_blkindex, 0, 0, 0); \ > _qzz_res; \ > - })) > + } > > > /* Client-code macros to check the state of memory. */ > @@ -176,24 +176,24 @@ typedef > error message and returns the address of the first offending byte. > Otherwise it returns zero. */ > #define VALGRIND_CHECK_MEM_IS_ADDRESSABLE(_qzz_addr,_qzz_len) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \ > VG_USERREQ__CHECK_MEM_IS_ADDRESSABLE,\ > _qzz_addr, _qzz_len, 0, 0, 0); \ > _qzz_res; \ > - })) > + } > > /* Check that memory at _qzz_addr is addressable and defined for > _qzz_len bytes. If suitable addressibility and definedness are not > established, Valgrind prints an error message and returns the > address of the first offending byte. Otherwise it returns zero. */ > #define VALGRIND_CHECK_MEM_IS_DEFINED(_qzz_addr,_qzz_len) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \ > VG_USERREQ__CHECK_MEM_IS_DEFINED, \ > _qzz_addr, _qzz_len, 0, 0, 0); \ > _qzz_res; \ > - })) > + } > > /* Use this macro to force the definedness and addressibility of an > lvalue to be checked. If suitable addressibility and definedness > @@ -215,7 +215,7 @@ typedef > } > > /* Do a summary memory leak check (like --leak-check=summary) mid-execution. */ > -#define VALGRIND_DO_QUICK_LEAK_CHECK \ > +#define VALGRIND_DO_QUICK_LEAK_CHECK \ > {unsigned long _qzz_res; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \ > VG_USERREQ__DO_LEAK_CHECK, \ > @@ -277,14 +277,14 @@ typedef > impossible to segfault your system by using this call. > */ > #define VALGRIND_GET_VBITS(zza,zzvbits,zznbytes) \ > - (__extension__({unsigned long _qzz_res; \ > + {unsigned long _qzz_res; \ > char* czza = (char*)zza; \ > char* czzvbits = (char*)zzvbits; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \ > VG_USERREQ__GET_VBITS, \ > czza, czzvbits, zznbytes, 0, 0 ); \ > _qzz_res; \ > - })) > + } > > /* Set the validity data for addresses [zza..zza+zznbytes-1], copying it > from the provided zzvbits array. Return values: > @@ -296,14 +296,14 @@ typedef > impossible to segfault your system by using this call. > */ > #define VALGRIND_SET_VBITS(zza,zzvbits,zznbytes) \ > - (__extension__({unsigned int _qzz_res; \ > + {unsigned int _qzz_res; \ > char* czza = (char*)zza; \ > char* czzvbits = (char*)zzvbits; \ > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \ > VG_USERREQ__SET_VBITS, \ > czza, czzvbits, zznbytes, 0, 0 ); \ > _qzz_res; \ > - })) > + } > > #endif > > -- > 1.7.3.GIT > > -- dwmw2 |
|
From: Bart V. A. <bva...@ac...> - 2011-01-12 11:17:24
|
On Wed, Jan 12, 2011 at 8:53 AM, David Woodhouse <dw...@in...> wrote:
>
> On Tue, 2011-01-11 at 15:12 +0000, Tony Finch wrote:
> > Exim doesn't compile with Sun or HP CC since Valgrind support was
> > added. Although valgrind.h protects against usage on unsupported
> > platforms, memcheck.h uses the __extension__() macro without checking.
> > Remove all uses since VALGRIND_DO_CLIENT_REQUEST() has all the
> > necessary portability support.
> > ---
> >
> > Chris and Steen,
> >
> > Could you please try this patch and see if it fixes the
> > compilation problems with 4.73?
>
> Thanks.
>
> This wants to go to Valgrind upstream too.
> [ ... ]
> > /* Client-code macros to check the state of memory. */
> > @@ -176,24 +176,24 @@ typedef
> > error message and returns the address of the first offending byte.
> > Otherwise it returns zero. */
> > #define VALGRIND_CHECK_MEM_IS_ADDRESSABLE(_qzz_addr,_qzz_len) \
> > - (__extension__({unsigned long _qzz_res; \
> > + {unsigned long _qzz_res; \
> > VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \
> > VG_USERREQ__CHECK_MEM_IS_ADDRESSABLE,\
> > _qzz_addr, _qzz_len, 0, 0, 0); \
> > _qzz_res; \
> > - }))
> > + }
> > [ ... ]
Hello Tony,
Did you realize that with that patch you have broken
VALGRIND_CHECK_MEM_IS_ADDRESSABLE() and several other macros ? The
above patch converts e.g. the following code into a syntax error:
void* addr;
int len;
int addressable;
addressable = VALGRIND_CHECK_MEM_IS_ADDRESSABLE(addr, len);
Bart.
|
|
From: Julian S. <js...@ac...> - 2011-01-12 13:48:26
|
On Wednesday, January 12, 2011, Bart Van Assche wrote: > Did you realize that with that patch you have broken > VALGRIND_CHECK_MEM_IS_ADDRESSABLE() and several other macros ? Yeah .. the __extension is there for a reason. Nuking it isn't a good solution. Can you fix it by using an #if defined(__GNUC__) (or whatever?) J |
|
From: David W. <dw...@in...> - 2011-01-12 14:20:40
|
On Wed, 2011-01-12 at 13:52 +0000, Tony Finch wrote:
>
> which is clearly bogus because on non-gcc it leads to syntax errors in
> many macros like the following and the ones I broke in memcheck.h
>
> #define RUNNING_ON_VALGRIND __extension__ \
> ({unsigned int _qzz_res; \
> VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0 /* if not */, \
> VG_USERREQ__RUNNING_ON_VALGRIND, \
> 0, 0, 0, 0, 0); \
> _qzz_res; \
> })
>
> I'm not immediately sure how to fix it properly.
Can we turn the ({ ... }) extension into a static inline function? Or is
that not sufficiently portable either?
--
David Woodhouse Open Source Technology Centre
Dav...@in... Intel Corporation
|
|
From: David W. <dw...@in...> - 2011-01-12 14:32:49
|
On Wed, 2011-01-12 at 14:28 +0000, Tony Finch wrote:
> On Wed, 12 Jan 2011, David Woodhouse wrote:
> >
> > Can we turn the ({ ... }) extension into a static inline function? Or is
> > that not sufficiently portable either?
>
> No, nested functions are not allowed in standard C.
Nested? I meant
static inline VALGRIND_MAKE_MEM_NOACCESS(void *addr, int *len)
{
unsigned long res;
VALGRIND_DO_CLIENT_REQUEST(_res, 0 /* default return */,
VG_USERREQ__MAKE_MEM_NOACCESS,
addr, len, 0, 0, 0);
return res;
}
Where's the nesting?
--
David Woodhouse Open Source Technology Centre
Dav...@in... Intel Corporation
|
|
From: Bart V. A. <bva...@ac...> - 2011-01-12 17:31:55
|
On Wed, Jan 12, 2011 at 3:28 PM, Tony Finch <do...@do...> wrote:
> On Wed, 12 Jan 2011, David Woodhouse wrote:
>>
>> Can we turn the ({ ... }) extension into a static inline function? Or is
>> that not sufficiently portable either?
>
> No, nested functions are not allowed in standard C.
>
> It looks to me like the macros were originally written so that the first
> argument was supposed to be an lvalue to receive the result, and they were
> not supposed to be used in an expression context. To make the
> block-expression wrappers work, I think the basic
> VALGRIND_DO_CLIENT_REQUEST() macro will have to be changed to return a
> result rather than take an lvalue.
Please have a look at vg_VALGRIND_DO_CLIENT_REQUEST_EXPR() in
<valgrind/valgrind.h> (as included in version 3.6.0).
Bart.
|