Re: [PATCH 2/2] x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Mon, Mar 19, 2018 at 04:22:55PM -0700, Matthias Kaehlcke wrote:
> arch/x86/mm/pti.o: warning: objtool: pti_init() falls through to next
> function pti_user_pagetable_walk_pmd()
> s/debugfs/file.o: warning: objtool: full_proxy_llseek() falls through to next function full_proxy_read()
> fs/debugfs/file.o: warning: objtool: full_proxy_read() falls through to next function full_proxy_write()
> fs/debugfs/file.o: warning: objtool: full_proxy_write() falls through to next function full_proxy_poll()
> fs/debugfs/file.o: warning: objtool: full_proxy_poll() falls through to next function full_proxy_unlocked_ioctl()
> fs/debugfs/file.o: warning: objtool: full_proxy_unlocked_ioctl() falls
> through to next function fops_u8_open()

Ok, I did a little more digging.  Surprise, these objtool warnings are
actually correct.  Somehow the WARN macros are causing Clang to produce
bad code.  In some cases, Clang is assuming that a WARN is "noreturn",
i.e. that the UD2 doesn't return from the exception.

As an example, here's the full_proxy_llseek() function and its compiled
code:

#define FULL_PROXY_FUNC(name, ret_type, filp, proto, args)		\
static ret_type full_proxy_ ## name(proto)				\
{									\
	struct dentry *dentry = F_DENTRY(filp);			\
	const struct file_operations *real_fops;			\
	ret_type r;							\
									\
	r = debugfs_file_get(dentry);					\
	if (unlikely(r))						\
		return r;						\
	real_fops = debugfs_real_fops(filp);				\
	r = real_fops->name(args);					\
	debugfs_file_put(dentry);					\
	return r;							\
}

FULL_PROXY_FUNC(llseek, loff_t, filp,
		PROTO(struct file *filp, loff_t offset, int whence),
		ARGS(filp, offset, whence));


0000000000000ca0 <full_proxy_llseek>:
     ca0:       55                      push   %rbp
     ca1:       41 57                   push   %r15
     ca3:       41 56                   push   %r14
     ca5:       53                      push   %rbx
     ca6:       48 83 ec 10             sub    $0x10,%rsp
     caa:       41 89 d6                mov    %edx,%r14d
     cad:       49 89 f7                mov    %rsi,%r15
     cb0:       48 89 fd                mov    %rdi,%rbp
     cb3:       65 48 8b 04 25 28 00    mov    %gs:0x28,%rax
     cba:       00 00
     cbc:       48 89 44 24 08          mov    %rax,0x8(%rsp)
     cc1:       48 8b 5d 18             mov    0x18(%rbp),%rbx
     cc5:       48 89 df                mov    %rbx,%rdi
     cc8:       e8 00 00 00 00          callq  ccd <full_proxy_llseek+0x2d>
                        cc9: R_X86_64_PC32      debugfs_file_get-0x4
     ccd:       85 c0                   test   %eax,%eax
     ccf:       75 65                   jne    d36 <full_proxy_llseek+0x96>
     cd1:       48 8b 45 18             mov    0x18(%rbp),%rax
     cd5:       48 8b 40 78             mov    0x78(%rax),%rax
     cd9:       a8 01                   test   $0x1,%al
     cdb:       75 63                   jne    d40 <full_proxy_llseek+0xa0>
     ...
     d40:       0f 0b                   ud2
     d42:       0f 1f 40 00             nopl   0x0(%rax)
     d46:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
     d4d:       00 00 00

0000000000000d50 <full_proxy_read>:


After the debugfs_file_get() call, the debugfs_real_fops() call is
inlined.  Here it is:

const struct file_operations *debugfs_real_fops(const struct file *filp)
{
	struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;

	if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) {
		/*
		 * Urgh, we've been called w/o a protecting
		 * debugfs_file_get().
		 */
		WARN_ON(1);
		return NULL;
	}

	return fsd->real_fops;
}

The WARN_ON() is the UD2 at offset d40.  Notice that, as objtool found,
it just falls through to the next function instead of "returning" to
full_proxy_llseek().


At first I thought this might be some kind of "optimization", since, if
you look closely, you'll see that the warning can never happen in this
situation.  But no, I downloaded the Clang binary, changed the 

  if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) /* DEBUGFS_FSDATA_IS_REAL_FOPS_BIT == 1 */

to

  if ((unsigned long)fsd & 0x2)

and still got the same issue.


Then I figured that Clang must be peeking into the inline asm, and is
assuming that UD2 is a dead end.  But no, I changed ASM_UD2 to a 2-byte
NOP, and got the same issue.


So unless I'm missing some "undefined behavior", this looks like a Clang
bug to me.  Somehow it doesn't like the WARN macros (but apparently only
in a small number of cases).

-- 
Josh