This is the mail archive of the
mailing list for the GCC project.
Re: Add support to trace comparison instructions and switch statements
- From: "Dmitry Vyukov via gcc" <gcc at gcc dot gnu dot org>
- To: 吴潍浠(此彼) <weixi dot wwx at antfin dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, gcc <gcc at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Jeff Law <law at redhat dot com>, wishwu007 <wishwu007 at gmail dot com>
- Date: Sun, 3 Sep 2017 13:04:47 +0200
- Subject: Re: Add support to trace comparison instructions and switch statements
- Authentication-results: sourceware.org; auth=none
- References: <firstname.lastname@example.org> <email@example.com> <20170901162318.GN2323@tucnak> <CACT4Y+Y-NUgTakh1t4p+f-6YKuQcULDR0g+nE6iyAud1kMB44g@mail.gmail.com> <20170903100121.GU2323@tucnak> <CACT4Y+bf3CT3T1uh+B8hUORtheX+owUPha7+Cme++PP3B=9wQA@mail.gmail.com> <firstname.lastname@example.org>
- Reply-to: Dmitry Vyukov <dvyukov at google dot com>
On Sun, Sep 3, 2017 at 12:38 PM, 吴潍浠(此彼) <email@example.com> wrote:
> I will update the patch according to your requirements, and with some my suggestions.
> It will take me one or two days.
Thanks! No hurry, just wanted to make sure you still want to pursue this.
> Wish Wu
> From:Dmitry Vyukov <firstname.lastname@example.org>
> Time:2017 Sep 3 (Sun) 18:21
> To:Jakub Jelinek <email@example.com>
> Cc:Wish Wu <firstname.lastname@example.org>; gcc <email@example.com>; gcc-patches <firstname.lastname@example.org>; Jeff Law <email@example.com>; wishwu007 <firstname.lastname@example.org>
> Subject:Re: Add support to trace comparison instructions and switch statements
> On Sun, Sep 3, 2017 at 12:19 PM, Dmitry Vyukov <email@example.com> wrote:
>> On Sun, Sep 3, 2017 at 12:01 PM, Jakub Jelinek <firstname.lastname@example.org> wrote:
>>> On Sun, Sep 03, 2017 at 10:50:16AM +0200, Dmitry Vyukov wrote:
>>>> What we instrument in LLVM is _comparisons_ rather than control
>>>> structures. So that would be:
>>>> _4 = x_8(D) == 98;
>>>> For example, result of the comparison can be stored into a bool struct
>>>> field, and then used in branching long time after. We still want to
>>>> intercept this comparison.
>>> Then we need to instrument not just GIMPLE_COND, which is the stmt
>>> where the comparison decides to which of the two basic block successors to
>>> jump, but also GIMPLE_ASSIGN with tcc_comparison class
>>> gimple_assign_rhs_code (the comparison above), and maybe also
>>> GIMPLE_ASSIGN with COND_EXPR comparison code (that is say
>>> _4 = x_1 == y_2 ? 23 : _3;
>>>> > Perhaps for -fsanitize-coverage= it might be a good idea to force
>>>> > LOGICAL_OP_NON_SHORT_CIRCUIT/BRANCH_COST or whatever affects GIMPLE
>>>> > decisions mentioned above so that the IL is closer to what the user wrote.
>>>> If we recurse down to comparison operations and instrument them, this
>>>> will not be so important, right?
>>> Well, if you just handle tcc_comparison GIMPLE_ASSIGN and not GIMPLE_COND,
>>> then you don't handle many comparisons from the source code. And if you
>>> handle both, some of the GIMPLE_CONDs might be just artificial comparisons.
>>> By pretending small branch cost for the tracing case you get fewer
>>> artificial comparisons.
>> Are these artificial comparisons on BOOLEAN_TYPE? I think BOOLEAN_TYPE
>> needs to be ignored entirely, there is just like 2 combinations of
>> possible values.
>> If not, then what it is? Is it a dup of previous comparisons?
>> I am not saying these modes should not be enabled. You know much
>> better. I just wanted to point that that integer comparisons is what
>> we should be handling.
>> Your example:
>> _1 = x_8(D) == 21;
>> _2 = x_8(D) == 64;
>> _3 = _1 | _2;
>> if (_3 != 0)
>> raises another point. Most likely we don't want to see speculative
>> comparisons. At least not yet (we will see them once we get through
>> the first comparison). So that may be another reason to enable these
>> modes (make compiler stick closer to original code).
> Wait, it is not speculative in this case as branch is on _1 | _2. But
> still, it just makes it harder for fuzzer to get through as it needs
> to guess both values at the same time rather then doing incremental