Re: [Patch 0/X] HWASAN v3

[Kostya Serebryany via gcc-patches]

[asan/hwasan co-author here, with clearly biased opinions]

On Android, HWASAN is already a fully usable testing tool.
We apply it to the kernel, user space system libraries, and select apps.
A phone with HWASAN-ified system is fully usable (I carry one as my
primary device since March 2019).
HWASAN has discovered over 120 bugs by now (heap-use-after-free,
heap/stack buffer overflows, stack-use-after-return, double free).
Many of the bugs were discovered during the everyday use (as opposed
to testing in the lab).
The overhead is low enough that on a top-tier CPU the user will rarely
notice any slowdown
(the increased battery drain *is* noticeable - compiler
instrumentation is not a substitute for hardware).
HWASAN has also helped discover 4 instances of future incompatibility
with MTE, all fixed.

The main benefit of HWASAN over ASAN is, as Matthew correctly
explains, the memory usage.
On embedded devices, this is often the difference between "can't
deploy" and "can deploy"
because, unlike in the server land, you can't install more RAM.

The other, more subtle benefit, is that HWASAN is more sensitive to
some types of bugs,
such as buffer-overflow-far-from-bounds or use-after-long-ago-free, etc.

MTE hardware is years away. Even once we have it in major new devices,
many smaller devices will still be running on Arm v8, for a decade or two.
As with ASAN/TSAN/UBSAN, having this sanitizer implemented in GCC will
vastly extend its user base and applicability and thus contribute to
the overall code quality and security.

Whether HWASAN should intercept libc functions or libc itself should
support HWASAN...
My strong opinion is that today the interception approach can only be
seen as a way to prototype.
ASAN, implemented in 2011, had to use interception because we needed
to get a new idea working fast.
However, over these 9 years, the interception caused an enormous
amount of complexity and user dissatisfaction.
The Android implementation of HWASAN (with hooks in the Bionic libc
and no interceptors) is
many times simpler, robust, and complete.
We need to do the same for other LIBCs, eventually, but we don't have
to do it immediately.

--kcc





On Wed, Jan 8, 2020 at 3:26 AM Matthew Malcomson
 wrote:
>
> Hi everyone,
>
> I'm writing this email to summarise & publicise the state of this patch
> series, especially the difficulties around approval for GCC 10 mentioned
> on IRC.
>
>
> The main obstacle seems to be that no maintainer feels they have enough
> knowledge about hwasan and justification that it's worthwhile to approve
> the patch series.
>
> Similarly, Martin has given a review of the parts of the code he can
> (thanks!), but doesn't feel he can do a deep review of the code related
> to the RTL hooks and stack expansion -- hence that part is as yet not
> reviewed in-depth.
>
>
>
> The questions around justification raised on IRC are mainly that it
> seems like a proof-of-concept for MTE rather than a stand-alone useable
> sanitizer.  Especially since in the GNU world hwasan instrumented code
> is not really ready for production since we can only use the
> less-"interceptor ABI" rather than the "platform ABI".  This restriction
> is because there is no version of glibc with the required modifications
> to provide the "platform ABI".
>
> (n.b. that since https://reviews.llvm.org/D69574 the code-generation for
> these ABI's is the same).
>
>
>  From my perspective the reasons that make HWASAN useful in itself are:
>
> 1) Much less memory usage.
>
>  From a back-of-the-envelope calculation based on the hwasan paper's
> table of memory overhead from over-alignment
> https://arxiv.org/pdf/1802.09517.pdf  I guess hwasan instrumented code
> has an overhead of about 1.1x (~4% from overalignment and ~6.25% from
> shadow memory), while asan seems to have an overhead somewhere in the
> range 1.5x - 3x.
>
> Maybe there's some data out there comparing total overheads that I
> haven't found? (I'd appreciate a reference if anyone has that info).
>
>
>
> 2) Available on more architectures that MTE.
>
> HWASAN only requires TBI, which is a feature of all AArch64 machines,
> while MTE will be an optional extension and only available on certain
> architectures.
>
>
> 3) This enables using hwasan in the kernel.
>
> While instrumented user-space applications will be using the
> "interceptor ABI" and hence are likely not production-quality, the
> biggest aim of implementing hwasan in GCC is to allow building the Linux
> kernel with tag-based sanitization using GCC.
>
> Instrumented kernel code uses hooks in the kernel itself, so this ABI
> distinction is no longer relevant, and this sanitizer should produce a
> production-quality kernel binary.
>
>
>
>
> I'm hoping I can find a maintainer willing to review and ACK this patch
> series -- especially with stage3 coming to a close soon.  If there's
> anything else I could do to help get someone willing up-to-speed then
> please just ask.
>
>
> Cheers,
> Matthew
>
>
>
> On 

Re: [Patch 0/X] [WIP][RFC][libsanitizer] Introduce HWASAN to GCC

[Kostya Serebryany via gcc-patches]

+Peter Collingbourne +Evgeniy Stepanov (the main developers of HWASAN
in LLVM,  FYI)
Please note that Peter has recently implemented support for globals in
LLVM's HWASAN.

--kcc

On Mon, Sep 9, 2019 at 8:55 AM Matthew Malcomson
 wrote:
>
> On 09/09/19 11:47, Martin Liška wrote:
> > On 9/6/19 4:46 PM, Matthew Malcomson wrote:
> >> Hello,
> >>
> >> This patch series is a WORK-IN-PROGRESS towards porting the LLVM hardware
> >> address sanitizer (HWASAN) in GCC.  The document describing HWASAN can be 
> >> found
> >> here 
> >> http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html.
> >
> > Hello.
> >
> > I'm happy that you are working on the functionality for GCC and I can 
> > provide
> > my knowledge that I have with ASAN. I briefly read the patch series and I 
> > have
> > multiple questions (and observations):
> >
> > 1) Is the ambition of the patchset to be a software emulation of MTE that 
> > can
> > work targets that do not support MTE? Is it something what clang
> > names hwasan-abi=interceptor?
>
> The ambition is to provide a software emulation of MTE for AArch64
> targets that don't support MTE.
> I also hope to have the framework set up so that enabling for other
> architectures is relatively easy and can be done by those interested.
>
> As I understand it, `hwasan-abi=interceptor` vs `platform` is about
> adding such MTE emulation for "application code" or "platform code (e.g.
> kernel)" respectively.
>
> >
> > 2) Do you have a real aarch64 hardware that has MTE support? Would it be 
> > possible
> > for the future to give such a machine to GCC Compile Farm for testing 
> > purpose?
>
> No our team doesn't have real MTE hardware, I have been testing on an
> AArch64 machine that has TBI, other work in the team that requires MTE
> support is being tested on the Arm "Fast Models" emulator.
>
> >
> > 3) I like the idea of sharing of internal functions like 
> > ASAN_CHECK/HWASAN_CHECK.
> > We should benefit from that in the future.
> >
> > 4) Am I correct that due to escape of "tagged" pointers, one needs to have 
> > an entire
> > DSO (dynamic shared object) built with hwasan enabled? Otherwise, a 
> > dereference of
> > a tagged pointer will lead to a segfault (except TBI feature on aarch64)?
>
>
> Yes, one needs to take pains to avoid the escape of tagged pointers on
> architectures other than AArch64.
>
> I don't believe that compiling the entire DSO with HWASAN enabled is
> enough, since pointers can be passed across DSO boundaries.
> I haven't yet looked into how to handle this.
>
> There's an even more fundamental problem of accesses within the
> instrumented binary -- I haven't yet figured out how to remove the tag
> before accesses on architectures without the AArch64 TBI feature.
>
>
> >
> > 5) Is there a documentation/definition of how shadow memory for memory 
> > tagging looks like?
> > Is it similar to ASAN, where one can get to tag with:
> > u8 memory_tag = *((PTR >> TG) + SHADOW_OFFSET) & 0xf?
> >
>
> Yes, it's similar.
>
>  From the libhwasan code, the function to fetch a pointer to the shadow
> memory byte corresponding to a memory address is MemToShadow.
>
> constexpr uptr kShadowScale = 4;
> inline uptr MemToShadow(uptr untagged_addr) {
>return (untagged_addr >> kShadowScale) +
>   __hwasan_shadow_memory_dynamic_address;
> }
>
> https://github.com/llvm-mirror/compiler-rt/blob/99ce9876124e910475c627829bf14326b8073a9d/lib/hwasan/hwasan_mapping.h#L42
>
>
> > 6) Note that thing like memtag_tag_size, memtag_granule_size define an ABI 
> > of libsanitizer
> >
>
> Yes, the size of these values define an ABI.
>
> Those particular hooks are added as a demonstration for how something
> like MTE would be implemented on top of this framework (where the
> backend would specify the tag and granule size to match their targets
> architecture).
>
> HWASAN itself would use the hard-coded tag and granule size that matches
> what libsanitizer uses.
> https://github.com/llvm-mirror/compiler-rt/blob/99ce9876124e910475c627829bf14326b8073a9d/lib/hwasan/hwasan_mapping.h#L36
>
> I define these as `HWASAN_TAG_SIZE` and `HWASAN_TAG_GRANULE_SIZE` in
> asan.h, and when using the sanitizer library the macro
> `HARDWARE_MEMORY_TAGGING` would be false so their values would be constant.
>
>
> >>
> >> The current patch series is far from complete, but I'm posting the current 
> >> state
> >> to provide something to discuss at the Cauldron next week.
> >>
> >> In its current state, this sanitizer only works on AArch64 with a custom 
> >> kernel
> >> to allow tagged pointers in system calls.  This is discussed in the below 
> >> link
> >> https://source.android.com/devices/tech/debug/hwasan -- the custom kernel 
> >> allows
> >> tagged pointers in syscalls.
> >
> > Can you be please more specific. Is the MTE in upstream linux kernel? If so,
> > starting from which version?
>
> I find I can only make complicated statements remotely clear in bullet
> points ;-)
>
> What I 

Re: [PATCH] Call REAL(swapcontext) with indirect_return attribute on x86

[Kostya Serebryany via gcc-patches]

On Wed, Jul 18, 2018 at 12:29 PM H.J. Lu  wrote:
>
> On Wed, Jul 18, 2018 at 11:45 AM, Kostya Serebryany  wrote:
> > On Wed, Jul 18, 2018 at 11:40 AM H.J. Lu  wrote:
> >>
> >> On Wed, Jul 18, 2018 at 11:18 AM, Kostya Serebryany  
> >> wrote:
> >> > What's ENDBR and do we really need to have it in compiler-rt?
> >>
> >> When shadow stack from Intel CET is enabled,  the first instruction of all
> >> indirect branch targets must be a special instruction, ENDBR.  In this 
> >> case,
> >
> > I am confused.
> > CET is a security mitigation feature (and ENDBR is a pretty weak form of 
> > such),
> > while ASAN is a testing tool, rarely used in production is almost
> > never as a mitigation (which it is not!).
> > Why would anyone need to combine CET and ASAN in one process?
> >
>
> CET is transparent to ASAN.  It is perfectly OK to use -fcf-protection to
> enable CET together with ASAN.

It is ok, but does it make any sense?
If anything, the current ASAN's intereceptors are a large blob of
security vulnerabilities.
If we ever want to use ASAN (or, more likely, HWASAN) as a security
mitigation feature,
we will need to get rid of these interceptors entirely.


>
> > Also, CET doesn't exist in the hardware yet, at least not publicly 
> > available.
> > Which means there should be no rush (am I wrong?) and we can do things
> > in the correct order:
> > implement the Clang/LLVM support, make the compiler-rt change in LLVM,
> > merge back to GCC.
>
> I am working with our LLVM people to address this.

Cool!


>
> H.J.
> > --kcc
> >
> >>
> >> int res = REAL(swapcontext)(oucp, ucp);
> >>     This function may be
> >> returned via an indirect branch.
> >>
> >> Here compiler must insert ENDBR after call, like
> >>
> >> call *bar(%rip)
> >> endbr64
> >>
> >> > As usual, I am opposed to any gcc compiler-rt that bypass upstream.
> >>
> >> We want it to be fixed in upstream.  That is why I opened an LLVM bug.
> >>
> >>
> >> > --kcc
> >> >
> >> > On Wed, Jul 18, 2018 at 8:37 AM H.J. Lu  wrote:
> >> >>
> >> >> asan/asan_interceptors.cc has
> >> >>
> >> >> ...
> >> >>   int res = REAL(swapcontext)(oucp, ucp);
> >> >> ...
> >> >>
> >> >> REAL(swapcontext) is a function pointer to swapcontext in libc.  Since
> >> >> swapcontext may return via indirect branch on x86 when shadow stack is
> >> >> enabled, we need to call REAL(swapcontext) with indirect_return 
> >> >> attribute
> >> >> on x86 so that compiler can insert ENDBR after REAL(swapcontext) call.
> >> >>
> >> >> I opened an LLVM bug:
> >> >>
> >> >> https://bugs.llvm.org/show_bug.cgi?id=38207
> >> >>
> >> >> But it won't get fixed before indirect_return attribute is added to
> >> >> LLVM.  I'd like to get it fixed in GCC first.
> >> >>
> >> >> Tested on i386 and x86-64.  OK for trunk after
> >> >>
> >> >> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01007.html
> >> >>
> >> >> is approved?
> >> >>
> >> >> Thanks.
> >> >>
> >> >>
> >> >> H.J.
> >> >> ---
> >> >> PR target/86560
> >> >> * asan/asan_interceptors.cc (swapcontext): Call 
> >> >> REAL(swapcontext)
> >> >> with indirect_return attribute on x86.
> >> >> ---
> >> >>  libsanitizer/asan/asan_interceptors.cc | 6 ++
> >> >>  1 file changed, 6 insertions(+)
> >> >>
> >> >> diff --git a/libsanitizer/asan/asan_interceptors.cc 
> >> >> b/libsanitizer/asan/asan_interceptors.cc
> >> >> index a8f4b72723f..b8dde4f19c5 100644
> >> >> --- a/libsanitizer/asan/asan_interceptors.cc
> >> >> +++ b/libsanitizer/asan/asan_interceptors.cc
> >> >> @@ -267,7 +267,13 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t 
> >> >> *oucp,
> >> >>uptr stack, ssize;
> >> >>ReadContextStack(ucp, , );
> >> >>ClearShadowMemoryForContextStack(stack, ssize);
> >> >> +#if defined(__x86_64__) || defined(__i386__)
> >> >> +  int (*real_swapcontext) (struct ucontext_t *, struct ucontext_t *)
> >> >> +__attribute__((__indirect_return__)) = REAL(swapcontext);
> >> >> +  int res = real_swapcontext(oucp, ucp);
> >> >> +#else
> >> >>int res = REAL(swapcontext)(oucp, ucp);
> >> >> +#endif
> >> >>// swapcontext technically does not return, but program may swap 
> >> >> context to
> >> >>// "oucp" later, that would look as if swapcontext() returned 0.
> >> >>// We need to clear shadow for ucp once again, as it may be in 
> >> >> arbitrary
> >> >> --
> >> >> 2.17.1
> >> >>
> >>
> >>
> >>
> >> --
> >> H.J.
>
>
>
> --
> H.J.

Re: [PATCH] Call REAL(swapcontext) with indirect_return attribute on x86

[Kostya Serebryany via gcc-patches]

On Wed, Jul 18, 2018 at 11:40 AM H.J. Lu  wrote:
>
> On Wed, Jul 18, 2018 at 11:18 AM, Kostya Serebryany  wrote:
> > What's ENDBR and do we really need to have it in compiler-rt?
>
> When shadow stack from Intel CET is enabled,  the first instruction of all
> indirect branch targets must be a special instruction, ENDBR.  In this case,

I am confused.
CET is a security mitigation feature (and ENDBR is a pretty weak form of such),
while ASAN is a testing tool, rarely used in production is almost
never as a mitigation (which it is not!).
Why would anyone need to combine CET and ASAN in one process?

Also, CET doesn't exist in the hardware yet, at least not publicly available.
Which means there should be no rush (am I wrong?) and we can do things
in the correct order:
implement the Clang/LLVM support, make the compiler-rt change in LLVM,
merge back to GCC.

--kcc

>
> int res = REAL(swapcontext)(oucp, ucp);
>     This function may be
> returned via an indirect branch.
>
> Here compiler must insert ENDBR after call, like
>
> call *bar(%rip)
> endbr64
>
> > As usual, I am opposed to any gcc compiler-rt that bypass upstream.
>
> We want it to be fixed in upstream.  That is why I opened an LLVM bug.
>
>
> > --kcc
> >
> > On Wed, Jul 18, 2018 at 8:37 AM H.J. Lu  wrote:
> >>
> >> asan/asan_interceptors.cc has
> >>
> >> ...
> >>   int res = REAL(swapcontext)(oucp, ucp);
> >> ...
> >>
> >> REAL(swapcontext) is a function pointer to swapcontext in libc.  Since
> >> swapcontext may return via indirect branch on x86 when shadow stack is
> >> enabled, we need to call REAL(swapcontext) with indirect_return attribute
> >> on x86 so that compiler can insert ENDBR after REAL(swapcontext) call.
> >>
> >> I opened an LLVM bug:
> >>
> >> https://bugs.llvm.org/show_bug.cgi?id=38207
> >>
> >> But it won't get fixed before indirect_return attribute is added to
> >> LLVM.  I'd like to get it fixed in GCC first.
> >>
> >> Tested on i386 and x86-64.  OK for trunk after
> >>
> >> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01007.html
> >>
> >> is approved?
> >>
> >> Thanks.
> >>
> >>
> >> H.J.
> >> ---
> >> PR target/86560
> >> * asan/asan_interceptors.cc (swapcontext): Call REAL(swapcontext)
> >> with indirect_return attribute on x86.
> >> ---
> >>  libsanitizer/asan/asan_interceptors.cc | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/libsanitizer/asan/asan_interceptors.cc 
> >> b/libsanitizer/asan/asan_interceptors.cc
> >> index a8f4b72723f..b8dde4f19c5 100644
> >> --- a/libsanitizer/asan/asan_interceptors.cc
> >> +++ b/libsanitizer/asan/asan_interceptors.cc
> >> @@ -267,7 +267,13 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
> >>uptr stack, ssize;
> >>ReadContextStack(ucp, , );
> >>ClearShadowMemoryForContextStack(stack, ssize);
> >> +#if defined(__x86_64__) || defined(__i386__)
> >> +  int (*real_swapcontext) (struct ucontext_t *, struct ucontext_t *)
> >> +__attribute__((__indirect_return__)) = REAL(swapcontext);
> >> +  int res = real_swapcontext(oucp, ucp);
> >> +#else
> >>int res = REAL(swapcontext)(oucp, ucp);
> >> +#endif
> >>// swapcontext technically does not return, but program may swap 
> >> context to
> >>// "oucp" later, that would look as if swapcontext() returned 0.
> >>// We need to clear shadow for ucp once again, as it may be in arbitrary
> >> --
> >> 2.17.1
> >>
>
>
>
> --
> H.J.

Re: [PATCH] Call REAL(swapcontext) with indirect_return attribute on x86

[Kostya Serebryany via gcc-patches]

What's ENDBR and do we really need to have it in compiler-rt?

As usual, I am opposed to any gcc compiler-rt that bypass upstream.

--kcc

On Wed, Jul 18, 2018 at 8:37 AM H.J. Lu  wrote:
>
> asan/asan_interceptors.cc has
>
> ...
>   int res = REAL(swapcontext)(oucp, ucp);
> ...
>
> REAL(swapcontext) is a function pointer to swapcontext in libc.  Since
> swapcontext may return via indirect branch on x86 when shadow stack is
> enabled, we need to call REAL(swapcontext) with indirect_return attribute
> on x86 so that compiler can insert ENDBR after REAL(swapcontext) call.
>
> I opened an LLVM bug:
>
> https://bugs.llvm.org/show_bug.cgi?id=38207
>
> But it won't get fixed before indirect_return attribute is added to
> LLVM.  I'd like to get it fixed in GCC first.
>
> Tested on i386 and x86-64.  OK for trunk after
>
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01007.html
>
> is approved?
>
> Thanks.
>
>
> H.J.
> ---
> PR target/86560
> * asan/asan_interceptors.cc (swapcontext): Call REAL(swapcontext)
> with indirect_return attribute on x86.
> ---
>  libsanitizer/asan/asan_interceptors.cc | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libsanitizer/asan/asan_interceptors.cc 
> b/libsanitizer/asan/asan_interceptors.cc
> index a8f4b72723f..b8dde4f19c5 100644
> --- a/libsanitizer/asan/asan_interceptors.cc
> +++ b/libsanitizer/asan/asan_interceptors.cc
> @@ -267,7 +267,13 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
>uptr stack, ssize;
>ReadContextStack(ucp, , );
>ClearShadowMemoryForContextStack(stack, ssize);
> +#if defined(__x86_64__) || defined(__i386__)
> +  int (*real_swapcontext) (struct ucontext_t *, struct ucontext_t *)
> +__attribute__((__indirect_return__)) = REAL(swapcontext);
> +  int res = real_swapcontext(oucp, ucp);
> +#else
>int res = REAL(swapcontext)(oucp, ucp);
> +#endif
>// swapcontext technically does not return, but program may swap context to
>// "oucp" later, that would look as if swapcontext() returned 0.
>// We need to clear shadow for ucp once again, as it may be in arbitrary
> --
> 2.17.1
>

Re: Add support to trace comparison instructions and switch statements

[Kostya Serebryany via gcc-patches]

On Tue, Sep 12, 2017 at 7:32 AM, Dmitry Vyukov  wrote:
> On Thu, Sep 7, 2017 at 9:02 AM, 吴潍浠(此彼)  wrote:
>> Hi
>> The trace-div and trace-gep options seems be used to evaluate corpus
>> to trigger specific kind of bugs. And they don't have strong effect to 
>> coverage.

These are used for what I call data-flow-driven mutations.
If we see x/y we tried to drive the inputs toward y==1.
Similarly, when we see a[idx], we try to drive towards idx being large
or negative.
When combined with value profiling, these do affect "generalized"
coverage and thus the corpus size.
They don't directly affect the regular edge coverage.

>>
>> The trace-pc-guard is useful, but it may be much more complex than trace-pc.
>> I think the best benefit of trace-pc-guard is avoiding dealing ASLR of 
>> programs,
>> especially programs with dlopen(). Seems hard to implement it in Linux 
>> kernel.

One of the benefits of trace-pc-guard is that you have to deal with
consecutive integers, not PCs (that are indeed affected by ARLR).

>>
>> The inline-8bit-counters and pc-table is too close to implementation of fuzz 
>> tool and
>> option trace-pc-guard .
>>
>> I am interesting in "stack-depth" and "func".

stack-depth is fully independent of all others.

"func" is a modifier that tells to insert callbacks only at the function entry.
It applies to trace-pc, trace-pc-guard, inline-8bit-counters, and pc-table
Other modifiers are bb (basic blocks) and edge (split critical edges,
then apply instrumentation to all BBs)

>> If we trace user-defined functions enter and exit,
>> we can calculate stack-depth and difference level of stack to past existed 
>> stack.
>> Adding __sanitizer_cov_trace_pc_{enter,exit} is easy , but it is not 
>> standard of llvm.

__sanitizer_cov_trace_pc_enter would be equivalent to trace-pc,func

we don't have __sanitizer_cov_trace_pc_exit. It's not very useful for
fuzzing (afaict).
I had one or two requests to implement __sanitizer_cov_trace_pc_exit
but at the end I was able to convince the folks that they don't need
it.

With pc-table and trace-pc[-guard] we already can distinguish func
entry from other blocks.
Can probably add special handling for exit, if someone explains why
this is interesting (in a separate thread, perhaps).

Also, gcc already has -finstrument-functions


>>
>> How do you think Dmitry ?
>>
>> Wish Wu
>>
>> --
>> From:Jakub Jelinek 
>> Time:2017 Sep 6 (Wed) 22:37
>> To:Wish Wu 
>> Cc:Dmitry Vyukov ; gcc-patches 
>> ; Jeff Law ; wishwu007 
>> 
>> Subject:Re: Add support to trace comparison instructions and switch 
>> statements
>>
>>
>> On Wed, Sep 06, 2017 at 07:47:29PM +0800, 吴潍浠(此彼) wrote:
>>> Hi Jakub
>>> I compiled libjpeg-turbo and libdng_sdk with options "-g -O3 -Wall 
>>> -fsanitize-coverage=trace-pc,trace-cmp -fsanitize=address".
>>> And run my fuzzer with pc and cmp feedbacks for hours. It works fine.
>>> About __sanitizer_cov_trace_cmp{f,d} , yes, it  isn't provided by llvm. But 
>>> once we trace integer comparisons, why not real type comparisons.

But why would you need to trace floats?
"Just for completeness" is not good enough.
It's extremely easy to add, but I don't want to pollute the code &
docs with something nobody needs.


>>> I remember Dmitry said it is not enough useful to trace real type 
>>> comparisons because it is rare to see them in programs.
>>> But libdng_sdk really has real type comparisons. So I want to keep them and 
>>> implementing __sanitizer_cov_trace_const_cmp{f,d} may be necessary.
>>
>> Ok.  Please make sure those entrypoints make it into the various example
>> __sanitier_cov_trace* fuzzer implementations though, so that people using
>> -fsanitize-coverage=trace-cmp in GCC will not need to hack stuff themselves.

Yes. It would be lovely if we can keep both LLVM and GCC in sync wrt
the interface.

>> At least it should be added to sanitizer_common (both in LLVM and GCC).
>>
>> BTW, https://clang.llvm.org/docs/SanitizerCoverage.html shows various other
>> -fsanitize-coverage= options, some of them terribly misnamed (e.g. trace-gep
>> using some weirdo LLVM IL acronym instead of being named by what it really
>> traces (trace-array-idx or something similar)).

I don't mind renaming trace-gep.

>>
>> Any plans to implement some or all of those?
>
>
> Thanks, Jakub!
>
> I've tested it on Linux kernel. Compiler does not crash, code is instrumented.
>
> Re  terribly misnamed trace-gep, dunno, I will leave this to Kostya.
>
> Re __sanitizer_cov_trace_cmp{f,d}, I am still not sure.
>
>> But libdng_sdk really has real type comparisons.
>
> Do they come from input data? In what format? How do you want to use
> them? E.g. if they come from input but with using some non-trivial
> transformation and the fuzzer will try to find them in the 

Re: Add support to trace comparison instructions and switch statements

[Kostya Serebryany via gcc-patches]

On Fri, Jul 14, 2017 at 5:23 AM, Dmitry Vyukov  wrote:
> On Thu, Jul 13, 2017 at 11:18 PM, Kostya Serebryany  wrote:
>>> > Hi
>>> >
>>> > I wrote a test for "-fsanitize-coverage=trace-cmp" .
>>> >
>>> > Is there anybody tells me if these codes could be merged into gcc ?
>>>
>>>
>>> Nice!
>>>
>>> We are currently working on Linux kernel fuzzing that use the
>>> comparison tracing. We use clang at the moment, but having this
>>> support in gcc would be great for kernel land.
>>>
>>> One concern I have: do we want to do some final refinements to the API
>>> before we implement this in both compilers?
>>>
>>> 2 things we considered from our perspective:
>>>  - communicating to the runtime which operands are constants
>>>  - communicating to the runtime which comparisons are counting loop checks
>>>
>>> First is useful if you do "find one operand in input and replace with
>>> the other one" thing. Second is useful because counting loop checks
>>> are usually not useful (at least all but one).
>>> In the original Go implementation I also conveyed signedness of
>>> operands, exact comparison operation (<, >, etc):
>>> https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-defs/defs.go#L13
>>> But I did not find any use for that.
>>> I also gave all comparisons unique IDs:
>>> https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-dep/sonar.go#L24
>>> That turned out to be useful. And there are chances we will want this
>>> for C/C++ as well.
>>>
>>> Kostya, did anything like this pop up in your work on libfuzzer?
>>> Can we still change the clang API? At least add an additional argument
>>> to the callbacks?
>>
>>
>> I'd prefer not to change the API, but extend it (new compiler flag, new
>> callbacks), if absolutely needed.
>> Probably make it trace-cmp-guard (similar to trace-pc-guard, with an extra
>> parameter that has the ID).
>> I don't like the approach with compiler-generated constant IDs.
>
> Yes, if we do it for C/C++, we need to create globals and pass pointer
> to a global to the callbacks. IDs do not work for C/C++.
>
>> Yes, it's a bit more efficient, but much less flexible in presence of
>> multiple modules, DSOs, dlopen, etc.
>>
>> I was also looking at completely inlining this instrumentation because it's
>> pretty expensive even in it's current form
>> (adding more parameters will make things worse).
>> This is going to be much less flexible, of course, so I'll attack it only
>> once I settle on the algorithm to handle CMPs in libFuzzer.
>
> This will require a new, completely different API for
> compiler<->runtime anyway, so we can put this aside for now.
>
>
>>> At the very least I would suggest that we add an additional arg that
>>> contains some flags (1/2 arg is a const, this is counting loop check,
>>> etc). If we do that we can also have just 1 callback that accepts
>>> uint64's for args because we can pass operand size in the flags:
>>
>>
>> How many flag combinations do we need and do we *really* need them?
>>
>> If the number of flag combinations is small, I'd prefer to have separate
>> callbacks (__sanitizer_cov_trace_cmp_loop_bound ?)
>>
>> Do we really need to know that one arg is a const?
>> It could well be a constant in fact, but compiler won't see it.
>>
>> int foo(int n) {   ... if (i < n) ... }
>> ...
>> foo(42);  // not inlined.
>>
>> We need to handle both cases the same way.
>
>
> Well, following this line of reasoning we would need only
> __asan_load/storeN callbacks for asan and remove
> __asan_load/store1/2/4/8, because compiler might not know the size at
> compile time. Constant-ness is only an optimization. If compiler does
> not know that something is a const, fine. Based on my experience with
> go-fuzz and our early experience with kernel, we badly need const
> hint.
> Otherwise fuzzer generates gazillions of candidates based on
> comparison arguments. Note that kernel is several order of magnitude
> larger than what people usually fuzz in user-space, inputs are more
> complex and at the same time execution speed is several order of
> magnitude lower. We can't rely on raw speed.
>
> Thinking of this more, I don't thing that globals will be useful in
> the kernel context (the main problem is that we have multiple
> transient isolated kernels). If we track per-comparison site
> information, we will probably use PCs. So I am ready to give up on
> this.
>
> Both of you expressed concerns about performance. Kostya says we
> should not break existing clang API.
> If we limit this to only constant-ness, then I think we can make this
> both forward and backward compatible, which means we don't need to
> handle it now. E.g. we can:
>  - if both operands are const (if it's possible at all), don't emit any 
> callback
>  - if only one is const, emit __sanitizer_cov_trace_cmp_const1 and
> pass the const in a known position (i.e. always first/second arg)
>  - if none are const, emit callback __sanitizer_cov_trace_cmp_dyn1
> Then compiler emits