[Bug target/77966] Corrupt function with -fsanitize-coverage=trace-pc

2016-10-17 Thread marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966

--- Comment #7 from Martin Liška  ---
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.

[Bug target/77966] Corrupt function with -fsanitize-coverage=trace-pc

2016-10-14 Thread jpoimboe at redhat dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966

--- Comment #6 from Josh Poimboeuf  ---
(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:

 :
   0:   55  push   %rbp
   1:   53  push   %rbx
   2:   48 89 fdmov%rdi,%rbp
   5:   31 db   xor%ebx,%ebx
   7:   48 83 ec 08 sub$0x8,%rsp
   b:   e8 00 00 00 00  callq  10 
c: R_X86_64_PC32__sanitizer_cov_trace_pc-0x4
  10:   8b 45 00mov0x0(%rbp),%eax
  13:   85 c0   test   %eax,%eax
  15:   75 11   jne28 
  17:   48 83 c4 08 add$0x8,%rsp
  1b:   5b  pop%rbx
  1c:   5d  pop%rbp
  1d:   e9 00 00 00 00  jmpq   22 
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 
29: R_X86_64_PC32   __sanitizer_cov_trace_pc-0x4
  2d:   89 d8   mov%ebx,%eax
  2f:   83 c3 01add$0x1,%ebx
  32:   48 8b 7c c5 08  mov0x8(%rbp,%rax,8),%rdi
  37:   e8 00 00 00 00  callq  3c 
38: R_X86_64_PC32   ioread32-0x4
  3c:   39 5d 00cmp%ebx,0x0(%rbp)
  3f:   77 e7   ja 28 
  41:   eb d4   jmp17 

[Bug target/77966] Corrupt function with -fsanitize-coverage=trace-pc

2016-10-14 Thread arnd at linaro dot org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966

--- Comment #5 from Arnd Bergmann  ---
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?

[Bug target/77966] Corrupt function with -fsanitize-coverage=trace-pc

2016-10-14 Thread vda.linux at googlemail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966

--- Comment #4 from Denis Vlasenko  ---
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.

[Bug target/77966] Corrupt function with -fsanitize-coverage=trace-pc

2016-10-14 Thread marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77966

Martin Liška  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #3 from Martin Liška  ---
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:

  :
  __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.