|
From: Matthias S. <zz...@ge...> - 2016-01-12 22:14:38
|
When compiling code that includes valgrind.h on visual studio I get compiler errors about VALGRIND_PRINTF: 1>include\Valgrind/valgrind.h(4493) : error C2220: warning treated as error - no 'object' file generated 1>include\Valgrind/valgrind.h(4493) : warning C4100: 'format' : unreferenced formal parameter 1>include\Valgrind/valgrind.h(4531) : warning C4100: 'format' : unreferenced formal parameter See https://bugs.kde.org/show_bug.cgi?id=356817 There are multiple ways to fix this issue. 1. Complicated one: extend the existing ifdef code with special NVALGRIND function versions for gcc and MSVC, see https://bugs.kde.org/attachment.cgi?id=96139 2. Simple one: have a function or macro with only "..." argument in the NVALGRIND case that does nothing. --- a/include/valgrind.h +++ b/include/valgrind.h @@ -6746,6 +6746,18 @@ typedef is the number of characters printed, excluding the "**<pid>** " part at the start and the backtrace (if present). */ +#if defined(_MSC_VER) && defined(NVALGRIND) +static int __inline VALGRIND_PRINTF(...) +{ + return 0; +} + +static int __inline VALGRIND_PRINTF_BACKTRACE(...) +{ + return 0; +} + +#else /* _MSC_VER && NVALGRIND */ #if defined(__GNUC__) || defined(__INTEL_COMPILER) && !defined(_MSC_VER) /* Modern GCC will optimize the static routine out if unused, and unused attribute will shut down warnings about it. */ @@ -6823,7 +6835,7 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...) return (int)_qzz_res; #endif /* NVALGRIND */ } - +#endif /* _MSC_VER && NVALGRIND */ /* These requests allow control to move from the simulated CPU to the real CPU, calling an arbitary function. What do you think about this? Regards Matthias |
|
From: Florian K. <fl...@ei...> - 2016-01-13 07:43:52
|
Given that we already have some support for MSC_VER in valgrind.h it would be good to sort this out. But that file is already quite messy with all the ifdeffery that is going on in there so we should be careful adding more.. I do not understand what the issue really is. The lines mentioned in the errors point into macro definitions and offer no clue. They are unrelated to VALGRIND_PRINTF. Can you provide some context? Assume that none of us has access to that particular compiler to play around with. Florian On 12.01.2016 23:14, Matthias Schwarzott wrote: > When compiling code that includes valgrind.h on visual studio I get > compiler errors about VALGRIND_PRINTF: > 1>include\Valgrind/valgrind.h(4493) : error C2220: warning treated as > error - no 'object' file generated > 1>include\Valgrind/valgrind.h(4493) : warning C4100: 'format' : > unreferenced formal parameter > 1>include\Valgrind/valgrind.h(4531) : warning C4100: 'format' : > unreferenced formal parameter > > See https://bugs.kde.org/show_bug.cgi?id=356817 > > There are multiple ways to fix this issue. > 1. Complicated one: extend the existing ifdef code with special > NVALGRIND function versions for gcc and MSVC, see > https://bugs.kde.org/attachment.cgi?id=96139 > > 2. Simple one: have a function or macro with only "..." argument in the > NVALGRIND case that does nothing. > > --- a/include/valgrind.h > +++ b/include/valgrind.h > @@ -6746,6 +6746,18 @@ typedef > is the number of characters printed, excluding the "**<pid>** " part > at the > start and the backtrace (if present). */ > > +#if defined(_MSC_VER) && defined(NVALGRIND) > +static int __inline VALGRIND_PRINTF(...) > +{ > + return 0; > +} > + > +static int __inline VALGRIND_PRINTF_BACKTRACE(...) > +{ > + return 0; > +} > + > +#else /* _MSC_VER && NVALGRIND */ > #if defined(__GNUC__) || defined(__INTEL_COMPILER) && !defined(_MSC_VER) > /* Modern GCC will optimize the static routine out if unused, > and unused attribute will shut down warnings about it. */ > @@ -6823,7 +6835,7 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...) > return (int)_qzz_res; > #endif /* NVALGRIND */ > } > - > +#endif /* _MSC_VER && NVALGRIND */ > > /* These requests allow control to move from the simulated CPU to the > real CPU, calling an arbitary function. > > > What do you think about this? > > Regards > Matthias > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > Monitor end-to-end web transactions and take corrective actions now > Troubleshoot faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140 > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |
|
From: Matthias S. <zz...@ge...> - 2016-01-13 08:12:00
|
Hi Florian!
Ok, the line numbers relate to an older copy of valgrind.h but the issue
still exists. The first error points exactly on the definition of
VALGRIND_PRINTF (line 6759 in the current version of the file, see
below, the second on the equivalent definition of
VALGRIND_PRINTF_BACKTRACE).
The format argument is not used if NVALGRIND is defined (obvious).
For gcc the warning is disabled by adding a declaration containing the
attribute __unused__ for the format argument.
But for sure this does not apply to Visual Studio compiler.
To silence this warning one either needs to not assign a name to the
argument (only possible in c++) or only have the "..." argument (for a
function or macro). But I guess for better error checking the argument
should stay for gcc (to make use of the attribute format...).
Here is the code of VALGRIND_PRINTF of the latest valgrind.h file:
> 6749: #if defined(__GNUC__) || defined(__INTEL_COMPILER) && !defined(_MSC_VER)
> 6750: /* Modern GCC will optimize the static routine out if unused,
> 6751: and unused attribute will shut down warnings about it. */
> 6752: static int VALGRIND_PRINTF(const char *format, ...)
> 6753: __attribute__((format(__printf__, 1, 2), __unused__));
> 6754: #endif
> 6755: static int
> 6756: #if defined(_MSC_VER)
> 6757: __inline
> 6758: #endif
> 6759: VALGRIND_PRINTF(const char *format, ...)
> 6760: {
> 6761: #if defined(NVALGRIND)
> 6762: return 0;
> 6763: #else /* NVALGRIND */
> 6764: #if defined(_MSC_VER) || defined(__MINGW64__)
> 6765: uintptr_t _qzz_res;
> 6766: #else
> 6767: unsigned long _qzz_res;
> 6768: #endif
> 6769: va_list vargs;
> 6770: va_start(vargs, format);
> 6771: #if defined(_MSC_VER) || defined(__MINGW64__)
> 6772: _qzz_res = VALGRIND_DO_CLIENT_REQUEST_EXPR(0,
> 6773: VG_USERREQ__PRINTF_VALIST_BY_REF,
> 6774: (uintptr_t)format,
> 6775: (uintptr_t)&vargs,
> 6776: 0, 0, 0);
> 6777: #else
> 6778: _qzz_res = VALGRIND_DO_CLIENT_REQUEST_EXPR(0,
> 6779: VG_USERREQ__PRINTF_VALIST_BY_REF,
> 6780: (unsigned long)format,
> 6781: (unsigned long)&vargs,
> 6782: 0, 0, 0);
> 6783: #endif
> 6784: va_end(vargs);
> 6785: return (int)_qzz_res;
> 6786: #endif /* NVALGRIND */
> 6787: }
Regards
Matthias
Am 13.01.2016 um 08:18 schrieb Florian Krohm:
> Given that we already have some support for MSC_VER in valgrind.h it
> would be good to sort this out. But that file is already quite messy
> with all the ifdeffery that is going on in there so we should be careful
> adding more..
>
> I do not understand what the issue really is. The lines mentioned in the
> errors point into macro definitions and offer no clue. They are
> unrelated to VALGRIND_PRINTF. Can you provide some context? Assume that
> none of us has access to that particular compiler to play around with.
>
> Florian
>
>
> On 12.01.2016 23:14, Matthias Schwarzott wrote:
>> When compiling code that includes valgrind.h on visual studio I get
>> compiler errors about VALGRIND_PRINTF:
>> 1>include\Valgrind/valgrind.h(4493) : error C2220: warning treated as
>> error - no 'object' file generated
>> 1>include\Valgrind/valgrind.h(4493) : warning C4100: 'format' :
>> unreferenced formal parameter
>> 1>include\Valgrind/valgrind.h(4531) : warning C4100: 'format' :
>> unreferenced formal parameter
>>
>> See https://bugs.kde.org/show_bug.cgi?id=356817
>>
>> There are multiple ways to fix this issue.
>> 1. Complicated one: extend the existing ifdef code with special
>> NVALGRIND function versions for gcc and MSVC, see
>> https://bugs.kde.org/attachment.cgi?id=96139
>>
>> 2. Simple one: have a function or macro with only "..." argument in the
>> NVALGRIND case that does nothing.
>>
>> --- a/include/valgrind.h
>> +++ b/include/valgrind.h
>> @@ -6746,6 +6746,18 @@ typedef
>> is the number of characters printed, excluding the "**<pid>** " part
>> at the
>> start and the backtrace (if present). */
>>
>> +#if defined(_MSC_VER) && defined(NVALGRIND)
>> +static int __inline VALGRIND_PRINTF(...)
>> +{
>> + return 0;
>> +}
>> +
>> +static int __inline VALGRIND_PRINTF_BACKTRACE(...)
>> +{
>> + return 0;
>> +}
>> +
>> +#else /* _MSC_VER && NVALGRIND */
>> #if defined(__GNUC__) || defined(__INTEL_COMPILER) && !defined(_MSC_VER)
>> /* Modern GCC will optimize the static routine out if unused,
>> and unused attribute will shut down warnings about it. */
>> @@ -6823,7 +6835,7 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...)
>> return (int)_qzz_res;
>> #endif /* NVALGRIND */
>> }
>> -
>> +#endif /* _MSC_VER && NVALGRIND */
>>
>> /* These requests allow control to move from the simulated CPU to the
>> real CPU, calling an arbitary function.
>>
>>
>> What do you think about this?
>>
>> Regards
>> Matthias
>>
>> ------------------------------------------------------------------------------
>> Site24x7 APM Insight: Get Deep Visibility into Application Performance
>> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
>> Monitor end-to-end web transactions and take corrective actions now
>> Troubleshoot faster and improve end-user experience. Signup Now!
>> http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
>> _______________________________________________
>> Valgrind-developers mailing list
>> Val...@li...
>> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
>>
>
>
|
|
From: Matthias S. <zz...@ge...> - 2016-01-13 09:51:10
|
Am 13.01.2016 um 08:18 schrieb Florian Krohm: > Given that we already have some support for MSC_VER in valgrind.h it > would be good to sort this out. But that file is already quite messy > with all the ifdeffery that is going on in there so we should be careful > adding more.. > The attached patch is a draft version of a different way to cleanup VALGRIND_PRINTF. Regards Matthias |
|
From: Florian K. <fl...@ei...> - 2016-01-14 09:19:37
|
On 13.01.2016 10:50, Matthias Schwarzott wrote:
> Am 13.01.2016 um 08:18 schrieb Florian Krohm:
>> Given that we already have some support for MSC_VER in valgrind.h it
>> would be good to sort this out. But that file is already quite messy
>> with all the ifdeffery that is going on in there so we should be careful
>> adding more..
>>
> The attached patch is a draft version of a different way to cleanup
> VALGRIND_PRINTF.
>
Thanks for the patch.
Simplifying that file is a good thing. But I'd like to understand
whether we can get rid of some of the special casing.
__inline:
I presume this is needed to keep MSC_VER from complaining that the
function is possibly unused. Right?
uintptr_t:
I do not unerstand why this type is needed and we cannot use unsigned
long instead. svn archeology did not reveal anything except that Bart
might know.
Your #define __VALGRIND_PRINTF_ARGLIST ... will cause warnings with
most compilers because C does not allow such parameter lists.
To suppress the warning about the unused parameter we could use:
Index: include/valgrind.h
===================================================================
--- include/valgrind.h (revision 15754)
+++ include/valgrind.h (working copy)
@@ -6759,6 +6759,7 @@
VALGRIND_PRINTF(const char *format, ...)
{
#if defined(NVALGRIND)
+ if (format) *(volatile const *)format; // avoid compiler warning
return 0;
#else /* NVALGRIND */
#if defined(_MSC_VER) || defined(__MINGW64__)
@@ -6797,6 +6798,7 @@
VALGRIND_PRINTF_BACKTRACE(const char *format, ...)
{
#if defined(NVALGRIND)
+ if (format) *(volatile const *)format; // avoid compiler warning
return 0;
#else /* NVALGRIND */
#if defined(_MSC_VER) || defined(__MINGW64__)
Florian
|
|
From: Matthias S. <zz...@ge...> - 2016-01-17 14:04:50
|
Am 14.01.2016 um 10:19 schrieb Florian Krohm: > On 13.01.2016 10:50, Matthias Schwarzott wrote: >> Am 13.01.2016 um 08:18 schrieb Florian Krohm: >>> Given that we already have some support for MSC_VER in valgrind.h it >>> would be good to sort this out. But that file is already quite messy >>> with all the ifdeffery that is going on in there so we should be careful >>> adding more.. >>> >> The attached patch is a draft version of a different way to cleanup >> VALGRIND_PRINTF. >> > > Thanks for the patch. > Simplifying that file is a good thing. But I'd like to understand > whether we can get rid of some of the special casing. > > __inline: > I presume this is needed to keep MSC_VER from complaining that the > function is possibly unused. Right? I don't know for sure, but testing shows it is needed. But when we are already defining some inline things for MSVC we could also use the __inline__ keyword for gcc. > > uintptr_t: > I do not unerstand why this type is needed and we cannot use unsigned > long instead. I also don't understand why it is uintptr_t and not unsigned long, but going through the file, each architecture seems to use a different type for the argument types of the client requests. Some other macros like CALL_FN_W_5W macro always use "unsigned long". I do not get the difference. Could there be a static assert to check the correct size. I assume this must be valid for "unsigned long" to be usable: sizeof(unsigned long) == sizeof(void*) > svn archeology did not reveal anything except that Bart > might know. I found commit 11314 "Suppressed a few warnings reported by the Microsoft C Compiler." by Bart that started to switch from "unsigned long" to "ptrdiff_t" and some commit later later it was changed to "uintptr_t". > > Your #define __VALGRIND_PRINTF_ARGLIST ... will cause warnings with > most compilers because C does not allow such parameter lists. > ok, then go for a different solution. > To suppress the warning about the unused parameter we could use: > I suggest to use "format = format". This compiles fine for MSVC and gcc. I retried the original file with gcc with -Wall -Wextra and then gcc also warns about unused variable format. Regards Matthias |
|
From: Florian K. <fl...@ei...> - 2016-01-17 16:20:49
|
On 17.01.2016 15:04, Matthias Schwarzott wrote:
>>
>> uintptr_t:
>> I do not unerstand why this type is needed and we cannot use unsigned
>> long instead.
>
[ snip ]
> Could there be a static assert to check the correct size.
> I assume this must be valid for "unsigned long" to be usable:
> sizeof(unsigned long) == sizeof(void*)
We rely on this to be true all over the place and I'm sure it is
asserted somewhere.
>
>
>> svn archeology did not reveal anything except that Bart
>> might know.
>
> I found commit 11314 "Suppressed a few warnings reported by the
> Microsoft C Compiler." by Bart that started to switch from "unsigned
> long" to "ptrdiff_t" and some commit later later it was changed to
> "uintptr_t".
No. r11314 fixed the warning by adding __inline for MSVC and also added
a version of VALGRIND_DO_CLIENT_REQUEST for MSVC that was using
ptrdiff_t. That type being a signed type was clearly the wrong choice
and then corrected in r11317 to uintptr_t. I doubt that
VALGRIND_DO_CLIENT_REQUEST was added to fix a warning.
Perhaps Bart can double check and clean this up. That would be good.
>
>>
>> Your #define __VALGRIND_PRINTF_ARGLIST ... will cause warnings with
>> most compilers because C does not allow such parameter lists.
>>
> ok, then go for a different solution.
>
>> To suppress the warning about the unused parameter we could use:
>>
> I suggest to use "format = format". This compiles fine for MSVC and gcc.
Yes but with clang 3.7 and -Wall you get warnings about self assignments.
So what I'm going to do is to check in the change I suggested. That
fixes your warning. For the cleanup stuff we need Bart's help.
Florian
|
|
From: Matthias S. <zz...@ge...> - 2016-01-17 19:42:43
Attachments:
valgrind-fix-unused-parameter-15762.patch
|
Am 17.01.2016 um 17:20 schrieb Florian Krohm:
> On 17.01.2016 15:04, Matthias Schwarzott wrote:
>> I suggest to use "format = format". This compiles fine for MSVC and gcc.
>
> Yes but with clang 3.7 and -Wall you get warnings about self assignments.
>
> So what I'm going to do is to check in the change I suggested. That
> fixes your warning. For the cleanup stuff we need Bart's help.
>
Hi Florian,
your commit does not fix all issues:
1. This is what I get (when compiling vgprintf.c with NVALGRIND):
../../include/valgrind.h: In function 'VALGRIND_PRINTF':
../../include/valgrind.h:6762:4: warning: type defaults to 'int' in type
name [-Wimplicit-int]
if (format) *(volatile const *)format; // avoid compiler warning
To fix this I needed to modify the cast to (volatile const char*).
Second: the comment must be c style instead of c++.
Third: The compiler is not allowed to optimize away the read access to
format. so we should get rid of the volatile.
The simpler solution to cast format to void works for me.
I could today only check with gcc-5.3.0 and clang-3.7 but these two were
happy about this solution:
(void)format;
I hope msvc will like it too.
Regards
Matthias
|
|
From: Florian K. <fl...@ei...> - 2016-01-17 20:33:00
|
Hi Matthias,
thanks for your review.
On 17.01.2016 20:42, Matthias Schwarzott wrote:
>
> your commit does not fix all issues:
> 1. This is what I get (when compiling vgprintf.c with NVALGRIND):
>
> ../../include/valgrind.h: In function 'VALGRIND_PRINTF':
> ../../include/valgrind.h:6762:4: warning: type defaults to 'int' in type
> name [-Wimplicit-int]
> if (format) *(volatile const *)format; // avoid compiler warning
>
> To fix this I needed to modify the cast to (volatile const char*).
Yes, true. I forgot the type.
>
> Second: the comment must be c style instead of c++.
Right again.
>
> Third: The compiler is not allowed to optimize away the read access to
> format. so we should get rid of the volatile.
>
> The simpler solution to cast format to void works for me.
> I could today only check with gcc-5.3.0 and clang-3.7 but these two were
> happy about this solution:
>
> (void)format;
>
> I hope msvc will like it too.
Maybe msvc likes that. But static analysis tools may say that
(void)format; is a statement with no effect which are known to be
symptoms of bugs (sometimes).
I was tempted for a moment to use: if (format) return;
which would kill your warning as well, but then VALGRIND_PRINTF would be
a function without effect and I know at least one checker that would
complain about that.
So I'm leaving the volatile in there. It'll shut up any compiler for good.
Florian
|
|
From: Matthias S. <zz...@ge...> - 2016-01-17 20:57:50
|
Am 17.01.2016 um 21:32 schrieb Florian Krohm: > Hi Matthias, > >> >> Third: The compiler is not allowed to optimize away the read access to >> format. so we should get rid of the volatile. >> >> The simpler solution to cast format to void works for me. >> I could today only check with gcc-5.3.0 and clang-3.7 but these two were >> happy about this solution: >> >> (void)format; >> >> I hope msvc will like it too. > > Maybe msvc likes that. But static analysis tools may say that > (void)format; is a statement with no effect which are known to be > symptoms of bugs (sometimes). > I was tempted for a moment to use: if (format) return; > which would kill your warning as well, but then VALGRIND_PRINTF would be > a function without effect and I know at least one checker that would > complain about that. > So I'm leaving the volatile in there. It'll shut up any compiler for good. > My only remaining point is that until now there was the guarantee that absolutely no code is emitted when NVALGRIND is defined. This is no longer valid with the commited solution. There is the different solution: Add the gcc attribute unused to the parameter. I hope this can be applied for clang also, but I do not understand the current ifdef around the declaration with __attribute__ (#if defined(__GNUC__) || defined(__INTEL_COMPILER) && !defined(_MSC_VER)). The attribute code could also be cleaned by defining a macro __attribute__ if the compiler does not support it (or defining a macro __VALGRIND_ATTRIBUTE to not pollute the namespace). And then add some other code like that (without volatile) to silence MSVC: #if defined(_MSC_VER) (void)format; #endif Regards Matthias |
|
From: Florian K. <fl...@ei...> - 2016-01-18 08:08:12
|
On 18.01.2016 03:52, Bart Van Assche wrote: > >> uintptr_t: >> I do not unerstand why this type is needed and we cannot use unsigned >> long instead. svn archeology did not reveal anything except that Bart >> might know. > > MSVC uses the LLP64 model. This means that the "long long" type is 64 > bits and also that the type "long" is only 32 bits wide. See e.g. > https://blogs.msdn.microsoft.com/oldnewthing/20050131-00/?p=36563. > I'm confused. coregrind/m_execontext.c has this: vg_assert(sizeof(void*) == sizeof(UWord)); and include/pub/tool_basics.h has this: typedef unsigned long UWord; How can that assert not fail on LLP64 ? The function containing the assert will for sure be exercised by the testsuite. Florian |
|
From: Matthias S. <zz...@ge...> - 2016-01-19 20:47:57
|
Am 18.01.2016 um 08:54 schrieb Florian Krohm: > On 18.01.2016 03:52, Bart Van Assche wrote: >> >>> uintptr_t: >>> I do not unerstand why this type is needed and we cannot use unsigned >>> long instead. svn archeology did not reveal anything except that Bart >>> might know. >> >> MSVC uses the LLP64 model. This means that the "long long" type is 64 >> bits and also that the type "long" is only 32 bits wide. See e.g. >> https://blogs.msdn.microsoft.com/oldnewthing/20050131-00/?p=36563. >> > > I'm confused. coregrind/m_execontext.c has this: > > vg_assert(sizeof(void*) == sizeof(UWord)); > To make this assert checked for all platforms either it need to be part of valgrind.h or we need a test-build that compiles this assert for all relevant platforms. > and include/pub/tool_basics.h has this: > > typedef unsigned long UWord; > Does it make sense to have a valgrind.h-specific define for the unsigned type with size matching a pointer? Regards |
|
From: Matthias S. <zz...@ge...> - 2016-01-19 20:51:08
Attachments:
valgrind-improve-unused-parameter-on-r15763.patch
|
Am 17.01.2016 um 21:57 schrieb Matthias Schwarzott: > Am 17.01.2016 um 21:32 schrieb Florian Krohm: >> Hi Matthias, >> >>> >>> Third: The compiler is not allowed to optimize away the read access to >>> format. so we should get rid of the volatile. >>> >>> The simpler solution to cast format to void works for me. >>> I could today only check with gcc-5.3.0 and clang-3.7 but these two were >>> happy about this solution: >>> >>> (void)format; >>> >>> I hope msvc will like it too. >> >> Maybe msvc likes that. But static analysis tools may say that >> (void)format; is a statement with no effect which are known to be >> symptoms of bugs (sometimes). >> I was tempted for a moment to use: if (format) return; >> which would kill your warning as well, but then VALGRIND_PRINTF would be >> a function without effect and I know at least one checker that would >> complain about that. >> So I'm leaving the volatile in there. It'll shut up any compiler for good. >> > My only remaining point is that until now there was the guarantee that > absolutely no code is emitted when NVALGRIND is defined. > This is no longer valid with the commited solution. > > There is the different solution: > > Add the gcc attribute unused to the parameter. I hope this can be > applied for clang also, but I do not understand the current ifdef around > the declaration with __attribute__ (#if defined(__GNUC__) || > defined(__INTEL_COMPILER) && !defined(_MSC_VER)). > > The attribute code could also be cleaned by defining a macro > __attribute__ if the compiler does not support it (or defining a macro > __VALGRIND_ATTRIBUTE to not pollute the namespace). > > And then add some other code like that (without volatile) to silence MSVC: > > #if defined(_MSC_VER) > (void)format; > #endif > The attached patch implements exactly this (with just defining __attribute__ for not-supporting compilers). Matthias |
|
From: Matthias S. <zz...@ge...> - 2016-01-28 19:50:41
|
Am 19.01.2016 um 21:50 schrieb Matthias Schwarzott: > Am 17.01.2016 um 21:57 schrieb Matthias Schwarzott: >> My only remaining point is that until now there was the guarantee that >> absolutely no code is emitted when NVALGRIND is defined. >> This is no longer valid with the commited solution. >> >> There is the different solution: >> >> Add the gcc attribute unused to the parameter. I hope this can be >> applied for clang also, but I do not understand the current ifdef around >> the declaration with __attribute__ (#if defined(__GNUC__) || >> defined(__INTEL_COMPILER) && !defined(_MSC_VER)). >> >> The attribute code could also be cleaned by defining a macro >> __attribute__ if the compiler does not support it (or defining a macro >> __VALGRIND_ATTRIBUTE to not pollute the namespace). >> >> And then add some other code like that (without volatile) to silence MSVC: >> >> #if defined(_MSC_VER) >> (void)format; >> #endif >> > The attached patch implements exactly this (with just defining > __attribute__ for not-supporting compilers). > > I created a bug for this to not get lost: https://bugs.kde.org/show_bug.cgi?id=358697 Regards Matthias |
|
From: Greg P. <gp...@ap...> - 2016-01-19 21:54:06
|
> On Jan 17, 2016, at 12:32 PM, Florian Krohm <fl...@ei...> wrote: > > But static analysis tools may say that (void)format; is a statement with no effect which are known to be symptoms of bugs (sometimes). Which static analyzer does that? The tools I know of consider an explicit cast to (void) to indicate a deliberately unused value. They would not warn about such an expression. -- Greg Parker gp...@ap... Runtime Wrangler |
|
From: Florian K. <fl...@ei...> - 2016-01-20 15:39:20
|
On 19.01.2016 22:55, Greg Parker wrote:
>
>> On Jan 17, 2016, at 12:32 PM, Florian Krohm <fl...@ei...> wrote:
>>
>> But static analysis tools may say that (void)format; is a statement with no effect which are known to be symptoms of bugs (sometimes).
>
> Which static analyzer does that? The tools I know of consider an explicit cast to (void) to indicate a deliberately unused value. They would not warn about such an expression.
>
>
I do not recall which one it was. I presume one of those lint-like
checkers (if you consider those static analysers). For sure it was
neither Coverity nor the IBM checker.
Florian
|