linux-kernel - Re: [tip:locking/core 6/10] arch/x86/include/asm/rmwcc.h:23:17: error: jump into statement expression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 22 Oct 2018 11:22:55 +0100
From:   Will Deacon <will.deacon@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     kbuild test robot <lkp@...el.com>, kbuild-all@...org,
        linux-kernel@...r.kernel.org, tipbuild@...or.com,
        Ingo Molnar <mingo@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [tip:locking/core 6/10] arch/x86/include/asm/rmwcc.h:23:17:
 error: jump into statement expression

On Mon, Oct 22, 2018 at 12:08:30PM +0200, Peter Zijlstra wrote:
> ARGH; so, this:
> 
> #define __GEN_RMWcc(fullop, _var, cc, clobbers, ...)			\
> ({									\
> 	bool c = false;							\
> 	asm_volatile_goto (fullop "; j" #cc " %l[cc_label]"		\
> 			: : [var] "m" (_var), ## __VA_ARGS__		\
> 			: clobbers : cc_label);				\
> 	if (0) {							\
> cc_label:	c = true;						\
> 	}								\
> 	c;								\
> })
> 
> static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lock)
> {
> 	u32 val = 0;
> 
> 	if (GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", lock->val.counter, c,
> 			     "I", _Q_PENDING_OFFSET))
> 		val |= _Q_PENDING_VAL;
> 
> 	val |= atomic_read(&lock->val) & ~_Q_PENDING_MASK;
> 
> 	return val;
> }
> 
> fails to compile when combined with this:
> 
> #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )

Rosteeeedt!

> #define __trace_if(cond) \
> 	if (__builtin_constant_p(!!(cond)) ? !!(cond) :			\
> 	({								\
> 		int ______r;						\
> 		static struct ftrace_branch_data			\
> 			__attribute__((__aligned__(4)))			\
> 			__attribute__((section("_ftrace_branch")))	\
> 			______f = {					\
> 				.func = __func__,			\
> 				.file = __FILE__,			\
> 				.line = __LINE__,			\
> 			};						\
> 		______r = !!(cond);					\
> 		______f.miss_hit[______r]++;				\
> 		______r;						\
> 	}))
> 
> Because that moves the __GEN_RMWcc into a statement expression and GCC
> apparently doesn't like labels inside statement expressions.
> 
> If we avoid if() and rewrite queued_fetch_set_pending_acquire() like so:
> 
> static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lock)
> {
> 	bool pending;
> 	u32 val;
> 
> 	pending = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", lock->val.counter, c,
> 				   "I", _Q_PENDING_OFFSET);
> 
> 	val = pending * _Q_PENDING_VAL;
> 	val |= atomic_read(&lock->val) & ~_Q_PENDING_MASK;
> 
> 	return val;
> }
> 
> then it compiles again; but urgh.
> 
> Anybody see a better solution?

No, that looks about right to me, but please throw in a comment so that we
don't "fix" this in the future.

Will