Re: Add support to trace comparison instructions and switch statements
On Sat, Aug 5, 2017 at 11:53 AM, 吴潍浠(此彼) wrote: > Hi all > Is it worth adding my codes to gcc ? Are there some steps I need to do ? > Could somebody tell me the progress ? FYI, we've mailed a Linux kernel change that uses this instrumentation: https://groups.google.com/forum/#!topic/syzkaller/r0ARNVV-Bhg Another reason to have this in gcc. Can somebody from gcc maintainers take a look at this? Jakub? Thanks > Maybe there should be a project like libfuzzer to solve bugs in program. > > Wish Wu > -- > From:Wish Wu > Time:2017 Jul 21 (Fri) 13:38 > To:gcc ; gcc-patches ; Jeff Law > > Cc:wishwu007 > Subject:Re: Add support to trace comparison instructions and switch statements > > > Hi Jeff > > I have signed the copyright assignment, and used the name 'Wish Wu' . > Should I send you a copy of my assignment ? > > The attachment is my new patch with small changes. > Codes are checked by ./contrib/check_GNU_style.sh, except some special files. > > With > > -- > From:Jeff Law > Time:2017 Jul 14 (Fri) 15:37 > To:Wish Wu ; gcc ; gcc-patches > > Cc:wishwu007 > Subject:Re: Add support to trace comparison instructions and switch statements > > > On 07/10/2017 06:07 AM, 吴潍浠(此彼) wrote: >> Hi >> >> I write some codes to make gcc support comparison-guided fuzzing. >> It is very like >> http://clang.llvm.org/docs/SanitizerCoverage.html#tracing-data-flow . >> With -fsanitize-coverage=trace-cmp the compiler will insert extra >> instrumentation around comparison instructions and switch statements. >> I think it is useful for fuzzing. :D >> >> Patch is below, I may supply test cases later. > Before anyone can really look at this code you'll need to get a > copyright assignment on file with the FSF. > > See: > https://gcc.gnu.org/contribute.html > > If you've already done this, please let me know and I'll confirm with > the FSF copyright clerk. > > Jeff
Re: Add support to trace comparison instructions and switch statements
On Fri, Sep 1, 2017 at 6:23 PM, Jakub Jelinek wrote: > On Fri, Jul 21, 2017 at 01:38:17PM +0800, 吴潍浠(此彼) wrote: >> Hi Jeff >> >> I have signed the copyright assignment, and used the name 'Wish Wu' . >> Should I send you a copy of my assignment ? >> >> The attachment is my new patch with small changes. >> Codes are checked by ./contrib/check_GNU_style.sh, except some special files. > > Please provide a ChangeLog entry, you can use ./contrib/mklog as a start. > > @@ -975,6 +974,10 @@ fsanitize= > Common Driver Report Joined > Select what to sanitize. > > +fsanitize-coverage= > +Common Driver Report Joined > +Select what to coverage sanitize. > + > > Why Driver? The reason fsanitize= needs it is that say for > -fsanitize=address we add libraries in the driver, etc., but that > isn't the case for the coverage, right? Yes, there is no compiler-provided library that provides implementation of the emitted instrumentation. User is meant to provide them (or, use a third-party fuzzer that provides them). > --- gcc/flag-types.h(revision 250199) > +++ gcc/flag-types.h(working copy) > @@ -250,6 +250,14 @@ enum sanitize_code { > | SANITIZE_BOUNDS_STRICT > }; > > +/* Different trace modes. */ > +enum sanitize_coverage_code { > + /* Trace PC. */ > + SANITIZE_COV_TRACE_PC = 1UL << 0, > + /* Trace Compare. */ > + SANITIZE_COV_TRACE_CMP = 1UL << 1 > +}; > > No need for UL suffixes, the reason sanitize_code uses them is > that it includes 1 << 16 and above and might be included even in target code > (for host we require 32-bit integers, for target code it might be just > 16-bit). > > --- gcc/opts.c (revision 250199) > +++ gcc/opts.c (working copy) > @@ -1519,6 +1519,17 @@ const struct sanitizer_opts_s sanitizer_opts[] = >{ NULL, 0U, 0UL, false } > }; > > +/* -f{,no-}sanitize-coverage= suboptions. */ > +const struct sanitizer_opts_s coverage_sanitizer_opts[] = > +{ > +#define SANITIZER_OPT(name, flags, recover) \ > +{ #name, flags, sizeof #name - 1, recover } > + SANITIZER_OPT (trace-pc, SANITIZE_COV_TRACE_PC, false), > + SANITIZER_OPT (trace-cmp, SANITIZE_COV_TRACE_CMP, false), > +#undef SANITIZER_OPT > + { NULL, 0U, 0UL, false } > > No need to have the recover argument for the macro, just add false to it > (unless you want to use a different struct type that wouldn't even include > that member). > > +/* Given ARG, an unrecognized coverage sanitizer option, return the best > + matching coverage sanitizer option, or NULL if there isn't one. */ > + > +static const char * > +get_closest_coverage_sanitizer_option (const string_fragment &arg) > +{ > + best_match bm (arg); > + for (int i = 0; coverage_sanitizer_opts[i].name != NULL; ++i) > +{ > + bm.consider (coverage_sanitizer_opts[i].name); > +} > > Body which contains just one line shouldn't be wrapped in {}s, just use > for (int i = 0; coverage_sanitizer_opts[i].name != NULL; ++i) > bm.consider (coverage_sanitizer_opts[i].name); > > +unsigned int > +parse_coverage_sanitizer_options (const char *p, location_t loc, > +unsigned int flags, int value, bool complain) > > Wrong formatting, unsigned int should go below const char *, like: > > parse_coverage_sanitizer_options (const char *p, location_t loc, > unsigned int flags, int value, bool > complain) > > +{ > + while (*p != 0) > +{ > + size_t len, i; > + bool found = false; > + const char *comma = strchr (p, ','); > + > + if (comma == NULL) > + len = strlen (p); > + else > + len = comma - p; > + if (len == 0) > + { > + p = comma + 1; > + continue; > + } > + > + /* Check to see if the string matches an option class name. */ > + for (i = 0; coverage_sanitizer_opts[i].name != NULL; ++i) > + if (len == coverage_sanitizer_opts[i].len > + && memcmp (p, coverage_sanitizer_opts[i].name, len) == 0) > + { > + if (value) > + flags |= coverage_sanitizer_opts[i].flag; > + else > + flags &= ~coverage_sanitizer_opts[i].flag; > + found = true; > + break; > + } > + > + if (! found && complain) > + { > + const char *hint > + = get_closest_coverage_sanitizer_option (string_fragment (p, > len)); > + > + if (hint) > + error_at (loc, > + "unrecognized argument to " > + "-f%ssanitize-coverage= option: %q.*s;" > + " did you mean %qs?", > + value ? "" : "no-", > + (int) len, p, hint); > + else > + error_at (loc, > + "unrecognized argument to " > + "-f%ssanitize-coverage= option: %q.*s", > + value ? "" : "no-", > + (int) len, p); > + } > + > + if (comma == NULL) > +
Re: Add support to trace comparison instructions and switch statements
On Sun, Sep 3, 2017 at 12:01 PM, Jakub Jelinek 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).
Re: Add support to trace comparison instructions and switch statements
On Sun, Sep 3, 2017 at 12:19 PM, Dmitry Vyukov wrote: > On Sun, Sep 3, 2017 at 12:01 PM, Jakub Jelinek 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 progress.
Re: Add support to trace comparison instructions and switch statements
On Sun, Sep 3, 2017 at 12:38 PM, 吴潍浠(此彼) wrote: > Hi > 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 > Time:2017 Sep 3 (Sun) 18:21 > To:Jakub Jelinek > Cc:Wish Wu ; gcc ; gcc-patches > ; Jeff Law ; wishwu007 > > Subject:Re: Add support to trace comparison instructions and switch statements > > > On Sun, Sep 3, 2017 at 12:19 PM, Dmitry Vyukov wrote: >> On Sun, Sep 3, 2017 at 12:01 PM, Jakub Jelinek 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 > progress.
Re: Add support to trace comparison instructions and switch statements
On Tue, Jul 11, 2017 at 1:59 PM, Wish Wu 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? 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: void __sanitizer_cov_trace_cmp(uint64 arg1, uint64 arg2, uint64 flags); But I wonder if 3 uint64 args will be too inefficient for 32 bit archs?... If we create a global per comparison then we could put the flags into the global: void __sanitizer_cov_trace_cmp(uint64 arg1, uint64 arg2, something_t *global); Thoughts? > Index: gcc/testsuite/gcc.dg/sancov/basic3.c > === > --- gcc/testsuite/gcc.dg/sancov/basic3.c (nonexistent) > +++ gcc/testsuite/gcc.dg/sancov/basic3.c (working copy) > @@ -0,0 +1,42 @@ > +/* Basic test on number of inserted callbacks. */ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize-coverage=trace-cmp -fdump-tree-optimized" } */ > + > +void foo(char *a, short *b, int *c, long long *d, float *e, double *f) > +{ > + if (*a) > +*a += 1; > + if (*b) > +*b = *a; > + if (*c) > +*c += 1; > + if(*d) > +*d = *c; > + if(*e == *c) > +*e = *c; > + if(*f == *e) > +*f = *e; > + switch(*a) > +{ > +case 2: > + *b += 2; > + break; > +default: > + break; > +} > + switch(*d) > +{ > +case 3: > + *d += 3; > +case -4: > + *d -= 4; > +} > +} > + > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_cmp1 \\(" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_cmp2 \\(" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_cmp4 \\(" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_cmp8 \\(" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_cmpf \\(" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_cmpd \\(" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times > "__builtin___sanitizer_cov_trace_switch \\(" 2 "optimized" } } */ > > > With Regards > Wish Wu > > On Mon, Jul 10, 2017 at 8:07 PM, 吴潍浠(此彼) wrote: >> Hi >> >> I write some codes to make gcc support comparison-guided fuzzing. >> It is very like >> http://clang.llvm.org/docs/SanitizerCoverage.html#tracing-data-flow . >> With -fsanitize-coverage=trace-cmp the compiler will insert extra >> instrumentation around comparison instructions and switch statements. >> I think it is useful for fuzzing. :D >> >> Patch is below, I may supply test cases later. >> >> With Regards >> Wish Wu >> >> Index: gcc/asan.c >> === >> --- gcc/asan.c (revision 250082) >> +++ gcc/asan.c (working copy) >> @@ -2705,6 +2705,29 @@ initialize_sanitizer_builtins (void) >>tree BT_FN_SIZE_CONST_PTR_INT >> = build_function_type_list (size_type_node, const_ptr_type_node, >> integer_type_node, NULL_TREE); >> + >> + tree BT_FN_VOID_UINT8_UINT8 >> += build_function_type_list (void_type_node, unsigned_char_type_node, >> + unsigned_char_type_node, NULL_TREE); >> + tree BT_FN_VOID_UINT16_UINT16 >> += build_function_type_list (void_type_node, uint16_type_node, >> + uint16_type_node, NULL_TREE); >> + tree BT_FN_VOID_UINT32_UINT32 >> +=
Re: Add support to trace comparison instructions and switch statements
On Thu, Jul 13, 2017 at 12:41 PM, Wish Wu wrote: > Hi > > In fact, under linux with "return address" and file "/proc/self/maps", > we can give unique id for every comparison. Yes, it's doable. But you expressed worries about performance hit of merging callbacks for different sizes. Mapping pc + info from /proc/self/maps to a unique id via an external map is an order of magnitude slower than the hit of merged callbacks. > For fuzzing, we may give 3 bits for every comparison as marker of if > "<", "==" or ">" is showed. :D > > With Regards > Wish Wu of Ant-financial Light-Year Security Lab > > On Thu, Jul 13, 2017 at 6:04 PM, Wish Wu wrote: >> Hi >> >> In my perspective: >> >> 1. Do we need to assign unique id for every comparison ? >> Yes, I suggest to implement it like -fsanitize-coverage=trace-pc-guard . >> Because some fuzzing targets may invoke dlopen() like functions to >> load libraries(modules) after fork(), while these libraries are >> compiled with trace-cmp as well. >> With ALSR enabled by linker and/or kernel, return address can't be >> a unique id for every comparison. >> >> 2. Should we merge cmp1(),cmp2(),cmp4(),cmp8(),cmpf(),cmpd() into one cmp() ? >> No, It may reduce the performance of fuzzing. It may wastes >> registers. But the number "switch" statements are much less than "if", >> I forgive "switch"'s wasting behaviors. >> >> 3.Should we record operands(<,>,==,<= ..) ? >> Probably no. As comparison,"<" , "==" and ">" all of them are >> meaningful, because programmers must have some reasons to do that. As >> practice , "==" is more meaningful. >> >> 4.Should we record comparisons for counting loop checks ? >> Not sure. >> >> With Regards >> Wish Wu of Ant-financial Light-Year Security Lab >> >> On Thu, Jul 13, 2017 at 4:09 PM, Dmitry Vyukov wrote: >>> On Tue, Jul 11, 2017 at 1:59 PM, Wish Wu 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? >>> >>> 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: >>> >>> void __sanitizer_cov_trace_cmp(uint64 arg1, uint64 arg2, uint64 flags); >>> >>> But I wonder if 3 uint64 args will be too inefficient for 32 bit archs?... >>> >>> If we create a global per comparison then we could put the flags into >>> the global: >>> >>> void __sanitizer_cov_trace_cmp(uint64 arg1, uint64 arg2, something_t >>> *global); >>> >>> Thoughts? >>> >>> >>> >>> Index: gcc/testsuite/gcc.dg/sancov/basic3.c === --- gcc/testsuite/gcc.dg/sancov/basic3.c (nonexistent) +++ gcc/testsuite/gcc.dg/sancov/basic3.c (working copy) @@ -0,0 +1,42 @@ +/* Basic test on number of inserted callbacks. */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize-coverage=trace-cmp -fdump-tree-optimized" } */ + +void foo(char *a, short *b, int *c, long long *d, float *e, double *f) +{ + if (*a) +*a += 1; + if (*b) +*b = *a; + if (*c) +*c += 1; + if(*d) +*d = *c; + if(*e == *c) +*e = *c; + if(*f == *e) +*f = *e; + switch(*a) +{ +case 2: + *b += 2; + break; +default: + break; +} + switch(*d) +{ +case 3:
Re: Add support to trace comparison instructions and switch statements
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 weak aliases form __sanitizer_cov_trace_cmp_const/dyn1 to the old __sanitizer_cov_trace_cmp1, which makes it backwards compatible. New runtimes that implement __sanitizer_cov_trace_cmp_const/dyn1, also need to provide the old __sanitizer
Re: Add support to trace comparison instructions and switch statements
On Fri, Jul 14, 2017 at 11:17 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
Re: Add support to trace comparison instructions and switch statements
On Sat, Jul 15, 2017 at 9:21 AM, 吴潍浠(此彼) wrote: > Hi > > Implementing __sanitizer_cov_trace_cmp[1248]_const is OK . > And I will try to find some determinate way to judge this comparison is for > loop or not. > Because all the loops(for() or while()) seem to be transformed to "if" and > "goto" before running sancov pass. > Does it necessary to include APIs for float and double comparison ? like > __sanitizer_cov_trace_cmpf(float, float) > Why you do not include them ? Note you don't need to implement any of this right now, since we can make it [almost] backwards compatible. Unless, of course, it's simple and you find it useful and want to do this. I think it's better to get a first version of the change in as is (implement current clang API), and then do improvements in subsequent patches. I would expect that detecting consts should be simple. Re loops, I don't know, I would expect that gcc has some existing analysis for loop (it does lots of optimization with counting loop). Re floats/doubles, I am not sure, we managed to live without them in go-fuzz and then in libfuzzer, and I still don't see lots of value in them. Anyway, it should be a separate patch. > Now, I am following some suggests about my codes from Gribow. Final patch is > still on processing. > I am also waiting for copyright assignment of FSF which is requested by Jeff. > > With Regards > Wish Wu > > -- > From:Dmitry Vyukov > Time:2017 Jul 15 (Sat) 13:41 > To:Kostya Serebryany > Cc:Wish Wu ; gcc ; gcc-patches > ; Wish Wu ; Alexander > Potapenko ; andreyknvl ; Victor > Chibotaru ; Yuri Gribov > Subject:Re: Add support to trace comparison instructions and switch statements > > > On Fri, Jul 14, 2017 at 11:17 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
Re: Add support to trace comparison instructions and switch statements
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. > > 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. > > 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". 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. > > 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. >> 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. > 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)). > > 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 input, it won't be able to do so. On the other hand, it does not seem to be harmful, fuzzers that are not interested in them can just add empty callbacks. Re trace-pc-guard, I also don't have strong preference. Global variables should work for kernel, but we probably will not use them in kernel, because even aslr aside we would need to establish some global numbering of these globals across multiple different machines. But then it's much easier and simper to just use PCs as identifiers. Re __sanitizer_cov_trace_pc_{enter,exit}, I don't think we ever experimented/evaluated this idea. Do you have any data that it's useful? I suspect that it can grow corpus too much.
Re: Add support to trace comparison instructions and switch statements
Some stats from kernel build for number of trace_cmp callbacks: gcc non-const: 38051 const: 272726 total: 310777 clang: non-const: 45944 const: 266299 total: 312243 The total is quite close. Gcc seems to emit more const callbacks, which is good. On Tue, Sep 12, 2017 at 4:32 PM, 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. >> >> 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. >> >> 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". 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. >> >> 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. >>> 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. >> 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)). >> >> 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 input, it > won't be able to do so. > On the other hand, it does not seem to be harmful, fuzzers that are > not interested in them can just add empty callbacks. > > Re trace-pc-guard, I also don't have strong preference. Global > variables should work for kernel, but we probably will not use them in > kernel, because even aslr aside we would need to establish some global > numbering of these globals across multiple different machines. But > then it's much easier and simper to just use PCs as identifiers. > > Re __sanitizer_cov_trace_pc_{enter,exit}, I don't think we ever > experimented/evaluated this idea. Do you have any data that it's > useful? I suspect that it can grow corpus too much.
Re: [PATCH] tsan: Add optional support for distinguishing volatiles
On Thu, Apr 23, 2020 at 5:43 PM Marco Elver wrote: > > Add support to optionally emit different instrumentation for accesses to > volatile variables. While the default TSAN runtime likely will never > require this feature, other runtimes for different environments that > have subtly different memory models or assumptions may require > distinguishing volatiles. > > One such environment are OS kernels, where volatile is still used in > various places for various reasons, and often declare volatile to be > "safe enough" even in multi-threaded contexts. One such example is the > Linux kernel, which implements various synchronization primitives using > volatile (READ_ONCE(), WRITE_ONCE()). Here the Kernel Concurrency > Sanitizer (KCSAN) [1], is a runtime that uses TSAN instrumentation but > otherwise implements a very different approach to race detection from > TSAN. > > While in the Linux kernel it is generally discouraged to use volatiles > explicitly, the topic will likely come up again, and we will eventually > need to distinguish volatile accesses [2]. The other use-case is > ignoring data races on specially marked variables in the kernel, for > example bit-flags (here we may hide 'volatile' behind a different name > such as 'no_data_race'). > > [1] https://github.com/google/ktsan/wiki/KCSAN > [2] > https://lkml.kernel.org/r/CANpmjNOfXNE-Zh3MNP=-gmnhvKbsfUfTtWkyg_=vqtxs4nn...@mail.gmail.com Hi Jakub, FWIW this is: Acked-by: Dmitry Vyukov We just landed a similar change to llvm: https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814 Do you have any objections? Yes, I know volatile is not related to threading :) But 5 years we have a similar patch for gcc for another race detector prototype: https://gist.github.com/xairy/862ba3260348efe23a37decb93aa79e9 So this is not the first time this comes up. Thanks > 2020-04-23 Marco Elver > > gcc/ > * params.opt: Define --param=tsan-distinguish-volatile=[0,1]. > * sanitizer.def (BUILT_IN_TSAN_VOLATILE_READ1): Define new > builtin for volatile instrumentation of reads/writes. > (BUILT_IN_TSAN_VOLATILE_READ2): Likewise. > (BUILT_IN_TSAN_VOLATILE_READ4): Likewise. > (BUILT_IN_TSAN_VOLATILE_READ8): Likewise. > (BUILT_IN_TSAN_VOLATILE_READ16): Likewise. > (BUILT_IN_TSAN_VOLATILE_WRITE1): Likewise. > (BUILT_IN_TSAN_VOLATILE_WRITE2): Likewise. > (BUILT_IN_TSAN_VOLATILE_WRITE4): Likewise. > (BUILT_IN_TSAN_VOLATILE_WRITE8): Likewise. > (BUILT_IN_TSAN_VOLATILE_WRITE16): Likewise. > * tsan.c (get_memory_access_decl): Argument if access is > volatile. If param tsan-distinguish-volatile is non-zero, and > access if volatile, return volatile instrumentation decl. > (instrument_expr): Check if access is volatile. > > gcc/testsuite/ > * c-c++-common/tsan/volatile.c: New test. > --- > gcc/ChangeLog | 19 +++ > gcc/params.opt | 4 ++ > gcc/sanitizer.def | 21 > gcc/testsuite/ChangeLog| 4 ++ > gcc/testsuite/c-c++-common/tsan/volatile.c | 62 ++ > gcc/tsan.c | 53 -- > 6 files changed, 146 insertions(+), 17 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/tsan/volatile.c > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index 5f299e463db..aa2bb98ae05 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,22 @@ > +2020-04-23 Marco Elver > + > + * params.opt: Define --param=tsan-distinguish-volatile=[0,1]. > + * sanitizer.def (BUILT_IN_TSAN_VOLATILE_READ1): Define new > + builtin for volatile instrumentation of reads/writes. > + (BUILT_IN_TSAN_VOLATILE_READ2): Likewise. > + (BUILT_IN_TSAN_VOLATILE_READ4): Likewise. > + (BUILT_IN_TSAN_VOLATILE_READ8): Likewise. > + (BUILT_IN_TSAN_VOLATILE_READ16): Likewise. > + (BUILT_IN_TSAN_VOLATILE_WRITE1): Likewise. > + (BUILT_IN_TSAN_VOLATILE_WRITE2): Likewise. > + (BUILT_IN_TSAN_VOLATILE_WRITE4): Likewise. > + (BUILT_IN_TSAN_VOLATILE_WRITE8): Likewise. > + (BUILT_IN_TSAN_VOLATILE_WRITE16): Likewise. > + * tsan.c (get_memory_access_decl): Argument if access is > + volatile. If param tsan-distinguish-volatile is non-zero, and > + access if volatile, return volatile instrumentation decl. > + (instrument_expr): Check if access is volatile. > + > 2020-04-23 Srinath Parvathaneni > > * config/arm/arm_mve.h (__arm_vbicq_n_u16): Modify function > parameter's > diff --git a/gcc/params.opt b/gcc/params.opt > index 4aec480798b..9b564bb046c 100644 > --- a/gcc/params.opt > +++ b/gcc/params.opt > @@ -908,6 +908,10 @@ Stop reverse growth if the reverse probability of best > edge is less than this th > Common Joined UInteger Var(param_tree_reassoc_width) Param Optimization
Re: [PATCH] tsan: Add optional support for distinguishing volatiles
On Tue, Apr 28, 2020 at 4:55 PM Jakub Jelinek wrote: > > On Tue, Apr 28, 2020 at 04:48:31PM +0200, Dmitry Vyukov wrote: > > FWIW this is: > > > > Acked-by: Dmitry Vyukov > > > > We just landed a similar change to llvm: > > https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814 > > > > Do you have any objections? > > I don't have objections or anything right now, we are just trying to > finalize GCC 10 and once it branches, patches like this can be > reviewed/committed for GCC11. Thanks for clarification! Then we will just wait.
Re: [PATCH] tsan: Add optional support for distinguishing volatiles
On Wed, May 13, 2020 at 12:48 PM Marco Elver wrote: > > Hello, Jakub, > > > > On Tue, 28 Apr 2020 at 16:58, Dmitry Vyukov wrote: > > > > > > On Tue, Apr 28, 2020 at 4:55 PM Jakub Jelinek wrote: > > > > > > > > On Tue, Apr 28, 2020 at 04:48:31PM +0200, Dmitry Vyukov wrote: > > > > > FWIW this is: > > > > > > > > > > Acked-by: Dmitry Vyukov > > > > > > > > > > We just landed a similar change to llvm: > > > > > https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814 > > > > > > > > > > Do you have any objections? > > > > > > > > I don't have objections or anything right now, we are just trying to > > > > finalize GCC 10 and once it branches, patches like this can be > > > > reviewed/committed for GCC11. > > > > > > Thanks for clarification! > > > Then we will just wait. > > > > Just saw the announcement that GCC11 is in development stage 1 [1]. In > > case it is still too early, do let us know what time window we shall > > follow up. > > > > Would it be useful to rebase and resend the patch? > > So, it's starting to look like we're really going to need this sooner > than later. Given the feature is guarded behind a flag, and otherwise > does not affect anything else, would it be possible to take this for > GCC11? What do we need to do to make this happen? > > Thanks, > -- Marco > > > [1] https://gcc.gnu.org/pipermail/gcc/2020-April/000505.html Jakub, could you please give some update. Do we just wait? That's fine, just want to understand because there are some interesting discussions in the kernel re bumping compiler requirements. Thanks