|
From: Florian K. <fl...@ei...> - 2014-10-22 12:56:00
|
Greetings.
I've just enabled the -Wcast-qual option for compilation of the
valgrind code base. Testcases are not compiled with -Wcast-qual as
that is not very interesting.
I've tested this:
- on s390x with GCC 3.4.6 and 4.8.3
- on ppc64be with GCC 4.7.2
- on amd64 with GCC 4.8.2, clang 3.0 and clang 3.5.0
I would appreciate if somebody with access to Darwin, ARM and MIPS
platforms could give it a spin.
For the curious, there are some spots in the code where a cast that
drops a type qualifier is unavoidable. For instance:
HChar* VG_(strchr) ( const HChar* s, HChar c )
{
while (True) {
if (*s == c) return (HChar *)s;
if (*s == 0) return NULL;
s++;
}
}
To get this compiled with -Wcast-qual and have no warnings there are
basically two options.
One is to enclose such offensive code with
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wcast-qual"
HChar* VG_(strchr) .......
#pragma GCC diagnostic pop
The other option is to replace the cast with some type punning trickery:
#define CONST_CAST(T,x) \
({ \
union { \
const T in; \
T out; \
} var = { .in = x }; var.out; \
})
and use it like so:
if (*s == c) return CONST_CAST(HChar *,s);
I opted for the latter as it is less intrusive. The construct is
guaranteed to work even with -fstrict-aliasing (which we're not using).
Also, C99 says (6.5.2.3, footnote 82):
If the member used to access the contents of a union object is not the
same as the member last used to store a value in the object, the
appropriate part of the object representation of the value is
reinterpreted as an object representation in the new type.
Which is exactly what is being done in that macro.
Florian
|
|
From: Bart V. A. <bva...@ac...> - 2014-10-22 15:04:20
|
On 10/22/14 14:55, Florian Krohm wrote:
> The other option is to replace the cast with some type punning trickery:
>
> #define CONST_CAST(T,x) \
> ({ \
> union { \
> const T in; \
> T out; \
> } var = { .in = x }; var.out; \
> })
>
> and use it like so:
>
> if (*s == c) return CONST_CAST(HChar *,s);
>
> I opted for the latter as it is less intrusive. The construct is
> guaranteed to work even with -fstrict-aliasing (which we're not using).
Hello Florian,
Does the above construct cause the compiler to generate additional code
compared to a regular cast ? Some time ago I introduced the following
construct in the Net-SNMP code base:
/**
* @def NETSNMP_REMOVE_CONST(t, e)
*
* Cast away constness without that gcc -Wcast-qual prints a compiler
* warning, similar to const_cast<> in C++.
*
* @param[in] t A pointer type.
* @param[in] e An expression of a type that can be assigned to the
* type (const t).
*/
#if defined(__GNUC__)
#define NETSNMP_REMOVE_CONST(t, e) \
(__extension__ ({ const t tmp = (e); (t)(uintptr_t)tmp; }))
#else
#define NETSNMP_REMOVE_CONST(t, e) ((t)(uintptr_t)(e))
#endif
Bart.
|
|
From: Florian K. <fl...@ei...> - 2014-10-22 15:28:45
|
On 22.10.2014 17:04, Bart Van Assche wrote:
> On 10/22/14 14:55, Florian Krohm wrote:
>> The other option is to replace the cast with some type punning trickery:
>>
>> #define CONST_CAST(T,x) \
>> ({ \
>> union { \
>> const T in; \
>> T out; \
>> } var = { .in = x }; var.out; \
>> })
>>
>> and use it like so:
>>
>> if (*s == c) return CONST_CAST(HChar *,s);
>>
>> I opted for the latter as it is less intrusive. The construct is
>> guaranteed to work even with -fstrict-aliasing (which we're not using).
>
> Hello Florian,
>
> Does the above construct cause the compiler to generate additional code
> compared to a regular cast ? Some time ago I introduced the following
> construct in the Net-SNMP code base:
Hi Bart,
I just made an experiment, compiling this:
const char *from;
char *to;
void foo()
{
to = MAGIC_CAST(char *,from);
}
with -O2 -g (as we do) and replacing MAGIC_CAST with the casts we came
up with. In both cases the generated code for foo is:
movq from(%rip), %rax
movq %rax, to(%rip)
ret
Which is what I would have expected.
Florian
|
|
From: Rhys K. <rhy...@gm...> - 2014-10-23 11:02:10
Attachments:
fix-r14656-for-mac.patch
|
Hi Florian and list,
A short patch to SVN trunk after r14656 is required to compile cleanly on
Darwin. See attached and below for the patch to
coregrind/m_debuginfo/readmacho.c
Regards,
Rhys
Index: coregrind/m_debuginfo/readmacho.c
===================================================================
--- coregrind/m_debuginfo/readmacho.c (revision 14656)
+++ coregrind/m_debuginfo/readmacho.c (working copy)
@@ -703,8 +703,8 @@
Bool have_uuid = False;
UChar uuid[16];
Word i;
- struct _DebugInfoMapping* rx_map = NULL;
- struct _DebugInfoMapping* rw_map = NULL;
+ const DebugInfoMapping* rx_map = NULL;
+ const DebugInfoMapping* rw_map = NULL;
/* mmap the object file to look for di->soname and di->text_bias
and uuid and nlist */
@@ -715,7 +715,7 @@
vg_assert(di->fsm.have_rw_map);
for (i = 0; i < VG_(sizeXA)(di->fsm.maps); i++) {
- struct _DebugInfoMapping* map = VG_(indexXA)(di->fsm.maps, i);
+ const DebugInfoMapping* map = VG_(indexXA)(di->fsm.maps, i);
if (map->rx && !rx_map)
rx_map = map;
if (map->rw && !rw_map)
On 22 October 2014 23:55, Florian Krohm <fl...@ei...> wrote:
> Greetings.
>
> I've just enabled the -Wcast-qual option for compilation of the
> valgrind code base. Testcases are not compiled with -Wcast-qual as
> that is not very interesting.
> I've tested this:
> - on s390x with GCC 3.4.6 and 4.8.3
> - on ppc64be with GCC 4.7.2
> - on amd64 with GCC 4.8.2, clang 3.0 and clang 3.5.0
>
> I would appreciate if somebody with access to Darwin, ARM and MIPS
> platforms could give it a spin.
>
> For the curious, there are some spots in the code where a cast that
> drops a type qualifier is unavoidable. For instance:
>
> HChar* VG_(strchr) ( const HChar* s, HChar c )
> {
> while (True) {
> if (*s == c) return (HChar *)s;
> if (*s == 0) return NULL;
> s++;
> }
> }
>
> To get this compiled with -Wcast-qual and have no warnings there are
> basically two options.
> One is to enclose such offensive code with
>
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wcast-qual"
>
> HChar* VG_(strchr) .......
>
> #pragma GCC diagnostic pop
>
> The other option is to replace the cast with some type punning trickery:
>
> #define CONST_CAST(T,x) \
> ({ \
> union { \
> const T in; \
> T out; \
> } var = { .in = x }; var.out; \
> })
>
> and use it like so:
>
> if (*s == c) return CONST_CAST(HChar *,s);
>
> I opted for the latter as it is less intrusive. The construct is
> guaranteed to work even with -fstrict-aliasing (which we're not using).
>
> Also, C99 says (6.5.2.3, footnote 82):
> If the member used to access the contents of a union object is not the
> same as the member last used to store a value in the object, the
> appropriate part of the object representation of the value is
> reinterpreted as an object representation in the new type.
>
> Which is exactly what is being done in that macro.
>
> Florian
>
>
> ------------------------------------------------------------------------------
> Comprehensive Server Monitoring with Site24x7.
> Monitor 10 servers for $9/Month.
> Get alerted through email, SMS, voice calls or mobile push notifications.
> Take corrective actions from your mobile device.
> http://p.sf.net/sfu/Zoho
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
>
|
|
From: Florian K. <fl...@ei...> - 2014-10-23 11:39:43
|
On 23.10.2014 13:02, Rhys Kidd wrote:
> Hi Florian and list,
>
> A short patch to SVN trunk after r14656 is required to compile cleanly on
> Darwin. See attached and below for the patch to
> coregrind/m_debuginfo/readmacho.c
>
Thanks! Applied as r14657
Florian
|