[
https://issues.apache.org/jira/browse/TS-1278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13294885#comment-13294885
]
Igor Galić commented on TS-1278:
--------------------------------
{noformat}
09:56:58 <@baldrick> jMCg: how about this: turn
09:56:58 <@baldrick> ink_assert(c->mutex->thread_holding);
09:57:00 <@baldrick> into:
09:57:10 <@baldrick> #ifdef DEBUG
09:57:24 <@baldrick> ink_assert(c->mutex->thread_holding);
09:57:27 <@baldrick> #endif
09:57:46 < jMCg> >_o
09:58:07 <@baldrick> this is based on the following observations:
09:58:07 < jMCg> baldrick: isn't that what the macro does anyway.. no, it's
not. Hmnm.
09:58:25 <@baldrick> (1) reading c->mutex->thread_holding is in fact harmless:
it won't fire off any nuclear missiles
09:58:40 < jMCg> (Hopefully)
09:58:52 <@baldrick> (2) the compiler will never eliminate a read from it
because it doesn't and can't know that
09:59:33 < jMCg> baldrick: is there a way to define the macro such that it
doesn't read the variable?
09:59:39 <@baldrick> (3) thus when debugging is turned off you get better code
if you don't read it here
09:59:56 < Axman6> hmm, is this really the definition that want? #define
ink_assert(EX) (void)(EX). won't that cause things to be executed that you
probably don't want executed? (i guess if there's a lot of
that ink_assert(foo = bar) going on, then I guess you do want it, but
it's gross)
10:00:09 <@baldrick> conclusion: it is safe to condition this on DEBUG because
reading or not reading it won't make a difference
10:00:13 < Axman6> that they*
10:00:58 < jMCg> Axman6: true, true.
10:01:19 < jMCg> The thing is, an assert shouldn't have any side-effects
10:01:45 <@baldrick> but by definition volatile reads do have side effects...
10:06:13 < jMCg> Yeah, I keep putting that detail off.
{noformat}
> Clang warns: Volatile fields read but results discarded
> -------------------------------------------------------
>
> Key: TS-1278
> URL: https://issues.apache.org/jira/browse/TS-1278
> Project: Traffic Server
> Issue Type: Bug
> Components: Core
> Environment: clang version 3.2 (trunk 157601)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> Reporter: Igor Galić
> Attachments: volatile.patch
>
>
> {noformat}
> Making all in eventsystem
> make[2]: Entering directory
> `/home/igalic/src/asf/trafficserver/BUILD/iocore/eventsystem'
> CXX EventSystem.o
> In file included from ../../../iocore/eventsystem/EventSystem.cc:31:
> In file included from ../../../iocore/eventsystem/P_EventSystem.h:41:
> In file included from ../../../iocore/eventsystem/I_EventSystem.h:35:
> In file included from ../../../iocore/eventsystem/I_Action.h:30:
> In file included from ../../../iocore/eventsystem/I_Continuation.h:40:
> ../../../iocore/eventsystem/I_Lock.h:404:19: error: expression result unused;
> assign into a variable to force a volatile load
> [-Werror,-Wunused-volatile-lvalue]
> ink_assert(m->thread_holding);
> ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
> ../../../lib/ts/ink_assert.h:54:31: note: expanded from macro 'ink_assert'
> #define ink_assert(EX) (void)(EX)
> ^
> 1 error generated.
> make[2]: *** [EventSystem.o] Error 1
> {noformat}
> Discussion from {{#llvm}} on IRC:
> {noformat}
> < jMCg> volatile EThreadPtr thread_holding;
> <@baldrick> the clang warning sounds very sensible then
> < jMCg>
> http://git-wip-us.apache.org/repos/asf?p=trafficserver.git;a=blob;f=iocore/eventsystem/I_Lock.h#l80
> < jMCg> The comment is great. "You must not modify or set this value
> directly." -- that's why it's public!
> < jMCg> baldrick: can you still help me understand it?
> <@baldrick> jMCg: reading a volatile variable may have side-effects (that's
> what volatile is for). For example, if it is mapped to some I/O area, each
> read could fire off a nuclear warhead for example.
> <@baldrick> jMCg: thus it seems sensible to warn if it looks like someone is
> readying it but not using the result.
> < jMCg> Oh. Now I'm back on track. *Reading* a volatile variable may have
> *side-effects* - sometimes I'm slow.
> < jMCg> baldrick: I'll open a Bug in our project. Thank you very much.
> {noformat}
> This is it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
