Simplified a bit:
- spinlock_t is not essential
- mempool_t is not essential
- snic_log_q_error_err_status variable is not necessary
- __attribute__ ((__aligned__)) can be dropped too
- struct vnic_wq can be folded
OTOH:
- struct vnic_wq_ctrl wrapping of int variable is necessary
- wq_lock[1] unused member is necessary (makes gcc "know for sure" that wq[1] is 1-element array)
- each of -O2 -fno-reorder-blocks -fsanitize-coverage=trace-pc are necessary


extern unsigned int ioread32(void *);
struct vnic_wq_ctrl {
    unsigned int error_status;
};
struct snic {
    unsigned int wq_count;
    struct vnic_wq_ctrl *wq[1];
    int wq_lock[1];
};
void snic_log_q_error(struct snic *snic)
{
    unsigned int i;
    for (i = 0; i < snic->wq_count; i++)
        ioread32(&snic->wq[i]->error_status);
}


<snic_log_q_error>:
   0:   53                      push   %rbx
   1:   48 89 fb                mov    %rdi,%rbx
   4:   e8 00 00 00 00          callq  __sanitizer_cov_trace_pc
   9:   8b 03                   mov    (%rbx),%eax
   b:   85 c0                   test   %eax,%eax  # snic->wq_count==0?
   d:   75 09                   jne    18
   f:   5b                      pop    %rbx # yes, 0
  10:   e9 00 00 00 00          jmpq   __sanitizer_cov_trace_pc #tail call
  15:   0f 1f 00                nopl   (%rax)

  18:   e8 00 00 00 00          callq  __sanitizer_cov_trace_pc
  1d:   48 8b 7b 08             mov    0x8(%rbx),%rdi
  21:   e8 00 00 00 00          callq  ioread32
  26:   83 3b 01                cmpl   $0x1,(%rbx) # snic->wq_count<=1?
  29:   76 e4                   jbe    f
  2b:   e8 00 00 00 00          callq  __sanitizer_cov_trace_pc


Looks like gcc thinks that the loop can execute only zero or one time
(or else we run off wq[1]). So when it iterated once:

  21:   e8 00 00 00 00          callq  ioread32

it checks that snic->wq_count <= 1

  26:   83 3b 01                cmpl   $0x1,(%rbx)
  29:   76 e4                   jbe    f

and if not, we are in "impossible" land and just stop codegen. -fsanitize-coverage=trace-pc generator twitches one last time:

  2b:   e8 00 00 00 00          callq  __sanitizer_cov_trace_pc
<end>

Without -fsanitize-coverage=trace-pc, the second, redundant check "snic->wq_count<=1?" is not generated. This eliminates the hanging "impossible" code path:

<snic_log_q_error>:
   0:   8b 07                   mov    (%rdi),%eax
   2:   85 c0                   test   %eax,%eax
   4:   74 09                   je     f
   6:   48 8b 7f 08             mov    0x8(%rdi),%rdi
   a:   e9 00 00 00 00          jmpq   ioread32
   f:   c3                      retq

Hello.

You are right, GCC assumes that the loop can iterate at most 1 times:

$ head pr77966.c.*ivcanon*
;; Function snic_log_q_error (snic_log_q_error, funcdef_no=0, decl_uid=2082, cgraph_uid=0, symbol_order=0)

Induction variable (unsigned int) 0 + 1 * iteration does not wrap in statement _6 = snic_4(D)->wq[i_13];
 in loop 1.
Statement _6 = snic_4(D)->wq[i_13];
 is executed at most 0 (bounded by 0) + 1 times in loop 1.
Loop 1 iterates at most 1 times.
snic_log_q_error (struct snic * snic)

That's because struct vnic_wq_ctrl *wq[1]; is not a trailing array in a struct.
Thus last BB contains:

  <bb 5>:
  __builtin___sanitizer_cov_trace_pc ();
  __builtin_unreachable ();

That's reason why no return insn is emitted (occurrence of __builtin_unreachable).
Thus I think the PR is invalid. Please re-open if having a different opinion.

This confuses object code sanity analysis tools which check that every function ends "properly", i.e. with a return or jump (possibly padded with nops).

Can gcc get an option like -finsert-stop-insn-when-unreachable[=insn], making bad programs crash if they do reach "impossible" code, rather than happily running off and executing random stuff?

For x86, one-byte INT3, INT1, HLT or two-byte UD2 insn would be a good choice.

I checked the test case using "-fsanitize=unreachable" and that avoids the problem.

Josh, should we set that whenever we enable objtool in the kernel?

(In reply to Arnd Bergmann from comment #5)
> I checked the test case using "-fsanitize=unreachable" and that avoids the
> problem.
> 
> Josh, should we set that whenever we enable objtool in the kernel?

In theory, adding -fsanitize=unreachable might be a workable option for allowing objtool to detect such unreachable blocks.

However, in practice, that option doesn't seem to work as advertised.  It seems to change the control flow unexpectedly.  When adding it to the test case, it doesn't add a __ubsan_handle_builtin_unreachable() call to the unreachable block.  Instead, it treats it as a normal loop, and removes the assumption that the loop can only run one time.

Here's the same test case from comment #1, with -fsanitize-unreachable added:

0000000000000000 <snic_log_q_error>:
   0:	55                   	push   %rbp
   1:	53                   	push   %rbx
   2:	48 89 fd             	mov    %rdi,%rbp
   5:	31 db                	xor    %ebx,%ebx
   7:	48 83 ec 08          	sub    $0x8,%rsp
   b:	e8 00 00 00 00       	callq  10 <snic_log_q_error+0x10>
			c: R_X86_64_PC32	__sanitizer_cov_trace_pc-0x4
  10:	8b 45 00             	mov    0x0(%rbp),%eax
  13:	85 c0                	test   %eax,%eax
  15:	75 11                	jne    28 <snic_log_q_error+0x28>
  17:	48 83 c4 08          	add    $0x8,%rsp
  1b:	5b                   	pop    %rbx
  1c:	5d                   	pop    %rbp
  1d:	e9 00 00 00 00       	jmpq   22 <snic_log_q_error+0x22>
			1e: R_X86_64_PC32	__sanitizer_cov_trace_pc-0x4
  22:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)
  28:	e8 00 00 00 00       	callq  2d <snic_log_q_error+0x2d>
			29: R_X86_64_PC32	__sanitizer_cov_trace_pc-0x4
  2d:	89 d8                	mov    %ebx,%eax
  2f:	83 c3 01             	add    $0x1,%ebx
  32:	48 8b 7c c5 08       	mov    0x8(%rbp,%rax,8),%rdi
  37:	e8 00 00 00 00       	callq  3c <snic_log_q_error+0x3c>
			38: R_X86_64_PC32	ioread32-0x4
  3c:	39 5d 00             	cmp    %ebx,0x0(%rbp)
  3f:	77 e7                	ja     28 <snic_log_q_error+0x28>
  41:	eb d4                	jmp    17 <snic_log_q_error+0x17>

Yes, the reason is that aggressive loop optimizations are disabled if a -fsanitize is enabled.

If you want to catch the out of bounds access to wq_lock array, you can use -fsanitize=undefined.

Ok, after discussion with Jakub, I'm going to create a patch that would enable aggressive loop optimizations when -fsanitize=leak or -fsanitize=unreachable is enabled (a different sanitization disable them).

Author: marxin
Date: Mon Oct 24 07:00:53 2016
New Revision: 241463

URL: https://gcc.gnu.org/viewcvs?rev=241463&root=gcc&view=rev
Log:
Do not disable aggressive loop opts for

	PR sanitizer/77966
	* opts.c (finish_options): Skip conditionally.
	PR sanitizer/77966
	* c-c++-common/ubsan/unreachable-3.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/ubsan/unreachable-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/opts.c
    trunk/gcc/testsuite/ChangeLog

Ok, with the patch applied (r241463), you should be given expected output.
Please combine that with -fsanitize-undefined-trap-on-error and the binary will be instrumented as desired in #c4.