[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

Jakub Jelinek  changed:

   What|Removed |Added

 Status|WAITING |RESOLVED
 Resolution|--- |INVALID

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #5 from Jiri Slaby  ---
(In reply to Jakub Jelinek from comment #4)
> What gcc options are you using on the preprocessed source to trigger this?

By default this:
gcc-6 -nostdinc -fno-strict-aliasing -fno-common -std=gnu89 -mno-sse -mno-mmx
-mno-sse2 -mno-3dnow -mno-avx -m64 -falign-jumps=1 -falign-loops=1 -mno-80387
-mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -mtune=generic
-mno-red-zone -mcmodel=kernel -funit-at-a-time -maccumulate-outgoing-args -pipe
-fno-delete-null-pointer-checks -O2 --param=allow-store-data-races=0 
-fstack-protector -Wno-unused-but-set-variable -fno-omit-frame-pointer
-fno-optimize-sibling-calls -fno-var-tracking-assignments
-fasynchronous-unwind-tables -pg -mfentry  -fno-inline-functions-called-once 
-fno-strict-overflow -fconserve-stack  -fsanitize=kernel-address
-fasan-shadow-offset=0xdc00 --param asan-stack=1 --param
asan-globals=1 --param asan-instrumentation-with-call-threshold=1
-fsanitize-coverage=trace-pc   -S -o - af_netlink.i

And this simplified one produces the same around the call:
gcc-6 -nostdinc -O2 -std=gnu89   -fsanitize=kernel-address
-fasan-shadow-offset=0xdc00 --param asan-stack=1 --param
asan-globals=1 --param asan-instrumentation-with-call-threshold=1
-fsanitize-coverage=trace-pc   -S -o - af_netlink.i

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #9 from Jiri Slaby  ---
(In reply to Dmitry Vyukov from comment #8)
> First of all, are you sure that r12 is not 0 before the call?

Yes.

> Deference of 0xdc00 is how KASAN reacts on NULL deref, it does
> shadow check before the memory accesses. If original address is NULL, the
> shadow check will go to 0xdc00. I see such GPFs quite
> frequently, so that's what I would assume first.

I know, I thought so first too. But later, I debugged that to a gcc bug :).

> If you just switched to gcc6, then it can be some latent bug (undefined
> behavior), which started to fire with a new compiler.

W/ CONFIG_KCOV=n (i.e. no -fsanitize-coverage), it works, apparently.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread dvyukov at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #16 from Dmitry Vyukov  ---
> Could you please push that to the syzkaller tree [1] then?

Sorry, syzkaller page referred to outdated patch. I was hoping that Andrew will
take it soon, so that I can update the link to a more respected location.
Updated now:
https://github.com/dvyukov/linux/commit/33787098ffaaa83b8a7ccf519913ac5fd6125931

If you will have any other issue feel free to contact
syzkal...@googlegroups.com.

Re the original issue.

We could call a special __sanitizer_cov_trace_pc_special callback which would
save/restore all registers when a file is compiled with -fcall-saved*. But it
probably does not worth it while we have a single case.

We could also add a finer-grained function attibute which disables kcov
instrumentation of a single function. But the same reasoning applies.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #7 from Jiri Slaby  ---
(In reply to Dmitry Vyukov from comment #6)
> Also what gcc version?

$ gcc-6 --version
gcc-6 (SUSE Linux) 6.0.0 20160121 (experimental) [trunk revision 232670]

> I've tried:
> gcc version 6.0.0 20160105 (experimental) (GCC) 
> $ gcc /tmp/af_netlink.c -c -O2 -fsanitize-coverage=trace-pc
> -fsanitize=kernel-address --param asan-stack=1 --param asan-globals=1
> --param asan-instrumentation-with-call-threshold=1
> -fasan-shadow-offset=0xdc00

With this I see that too:
movq%r12, %rdx
#APP
# 28 "../arch/x86/include/asm/arch_hweight.h" 1
661:
call __sw_hweight32
662:
#...more ALTINST crap
#NO_APP
addl720(%rbx), %eax
shrq$3, %rdx
movl%eax, %r13d
movabsq $-2305847407260205056, %rax
cmpb$0, (%rdx,%rax)

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #12 from Jiri Slaby  ---
(In reply to Jiri Slaby from comment #11)
> __sw_hweight32 changes only retval (rax) and parameter (rdi).

... and rdi is stored to and restored from stack.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread dvyukov at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #14 from Dmitry Vyukov  ---
Wait, I already disabled instrumentation of hweight.c for because of this:

+# Kernel does not boot if we instrument this file as it uses custom calling
+# convention (see CONFIG_ARCH_HWEIGHT_CFLAGS).
+KCOV_INSTRUMENT_hweight.o := n

If you apply the latest kcov patch "[PATCH v6] kernel: add kcov code coverage",
it should work.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread dvyukov at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #17 from Dmitry Vyukov  ---
Jakub, I guess you can close this.
Sorry again.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #11 from Jiri Slaby  ---
(In reply to Jakub Jelinek from comment #10)
> If you are calling a function (__sw_hweight32) without letting gcc know you
> do that, are you sure that function call does not modify any registers other
> than "flags" and "rax"?

Not at all, I suppose. See attachment #37552. __sw_hweight32 changes only
retval (rax) and parameter (rdi).

But __sw_hweight32 proper is as well instrumented by the coverage hook. And it
changes whatever... 

In any way, what is the proper way of annotating asm() directive when there is
a call inside?

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #13 from Jakub Jelinek  ---
Seems hweight.c is compiled with
-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx
-fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11
but that of course expects that all the functions in there are leaf, if they
aren't leaf, you'd need to compile with the same flags also all the functions
that might be called from there.
So bet you want to arrange for hweight.c from being compiled without
-fsanitize-coverage=trace-pc and without other options that might introduce
calls.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread dvyukov at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #6 from Dmitry Vyukov  ---
Also what gcc version?

I've tried:
gcc version 6.0.0 20160105 (experimental) (GCC) 
$ gcc /tmp/af_netlink.c -c -O2 -fsanitize-coverage=trace-pc
-fsanitize=kernel-address --param asan-stack=1 --param asan-globals=1 --param
asan-instrumentation-with-call-threshold=1
-fasan-shadow-offset=0xdc00

(which should resemble what kernel uses), but I don't see any similar code
fragments.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread dvyukov at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #8 from Dmitry Vyukov  ---
First of all, are you sure that r12 is not 0 before the call?

Deference of 0xdc00 is how KASAN reacts on NULL deref, it does
shadow check before the memory accesses. If original address is NULL, the
shadow check will go to 0xdc00. I see such GPFs quite frequently,
so that's what I would assume first.

If you just switched to gcc6, then it can be some latent bug (undefined
behavior), which started to fire with a new compiler.

p.s. I can reproduce the generated code now.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

Jakub Jelinek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |WAITING
   Last reconfirmed||2016-02-02
 Ever confirmed|0   |1

--- Comment #4 from Jakub Jelinek  ---
What gcc options are you using on the preprocessed source to trigger this?

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jakub at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #10 from Jakub Jelinek  ---
If you are calling a function (__sw_hweight32) without letting gcc know you do
that, are you sure that function call does not modify any registers other than
"flags" and "rax"?

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #15 from Jiri Slaby  ---
(In reply to Dmitry Vyukov from comment #14)
> If you apply the latest kcov patch "[PATCH v6] kernel: add kcov code
> coverage", it should work.

Could you please push that to the syzkaller tree [1] then?

[1] https://github.com/dvyukov/linux/commits/kcov

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #1 from Jiri Slaby  ---
Created attachment 37552
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37552=edit
__sw_hweight32 assembly

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #2 from Jiri Slaby  ---
Created attachment 37553
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37553=edit
__sanitizer_cov_trace_pc implementation

This guys actually changes rdx.

[Bug c/69624] sanitize-coverage=trace-pc miscompiles kernel

2016-02-02 Thread jirislaby at gmail dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69624

--- Comment #3 from Jiri Slaby  ---
Preprocessed code:
http://www.fi.muni.cz/~xslaby/sklad/af_netlink.i

This one results in the code from initial description. I.e. rdx is loaded
before a call.