Re: Linux and Windows generate different binaries
On Thu, Jul 13, 2017 at 4:56 AM, Klaus Kruse Pedersen (Klaus) wrote: > On Wed, 2017-07-12 at 08:57 -0600, Sandra Loosemore wrote: >> On 07/12/2017 05:07 AM, Klaus Kruse Pedersen (Klaus) wrote: >> > I have seen reproducible builds being discussed here, but what is >> > the >> > position on inter c-lib and OS reproducible builds? >> >> I think we consider unstable sort problems bugs and have fixed them >> in >> the past. Bugzilla search turned up #28964 and I remember at least >> one >> more recent instance of this as well (although not the details any >> more). > > > Yes, 28964 is similar to the issue I was hit by. > > By extension, does that mean that all qsort compare/rank functions that > can return 0, should be considered buggy? > > I went through a some of the 140'ish rank functions - and it does > indeed look like considerable effort went into returning only +1 and > -1. > > A general pattern seem to be: > > return da ? 1 : -1; > > And comments like: > > /* If regs are equally good, sort by their numbers, so that the > results of qsort leave nothing to chance. */ > > > But there are exceptions, all rank functions in > > tree-ssa-loop-ivopts.c, > tree-ssa-loop-niter.c > tree-ssa-loop-im.c > > can return 0. One more issue with some of qsort callbacks is that they do not always satisfy ordering axioms which in practice may result in random variations in output. I once reported this in https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02141.html but didn't follow up. -Y
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: Getting spurious FAILS in testsuite?
Wow, your patch fixed the spurious fails for me on Ubuntu 16.04 with Linux 4.4.
Just to make sure, I ran the ubsan tests with: make -k -j4 check-gcc
RUNTESTFLAGS=“ubsan.exp”
Thanks
Dominik
> On 12 Jul 2017, at 15:40, Bernd Edlinger wrote:
>
> On 07/11/17 22:28, Bernd Edlinger wrote:
>> On 07/11/17 21:42, Andrew Pinski wrote:
>>> On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski
>>> wrote:
On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski
wrote:
> On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
> wrote:
>> Hi,
>>
>> I see this now as well on Ubuntu 16.04, but I doubt that the Kernel is
>> to blame.
>
> I don't see these failures when I use a 4.11 kernel. Only with a
> 4.4 kernel.
> Also the guality testsuite does not run at all with a 4.4 kernel, it
> does run when using a 4.11 kernel; I suspect this is the same symptom
> of the bug.
4.11 kernel results (with CentOS 7.4 glibc):
https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
4.4 kernel results (with ubuntu 1604 glibc):
https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
Note the glibc does not matter here as I get a similar testsuite
results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
the 4.11 kernel.
Also note I get a similar results with a plain 4.11 kernel compared to
the above kernel.
>>>
>>>
>>> Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
>>> 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
>>> issues.
>>>
>>
>> As Georg-Johann points out here:
>> https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
>> updating the kernel alone does not fix the issue.
>>
>> These patches do all circle around pty and tty devices,
>> but in the strace file I see a pipe(2) command
>> creating the fileno 10 and the sequence of events is
>> as follows:
>>
>> pipe([7, 10]) = 0 (not in my previous e-mail)
>> clone() = 16141
>> clone() = 16142
>> write(4, "spawn: returns {0}\r\n", 20) = 20
>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
>> read(10,...,4096) = 4096
>> read(10,...,4096) = 2144
>> read(10, "", 4096) = 0
>> close(10) = 0
>> wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
>> wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
>>
>> All data arrive at the expect process, but apparently too quickly.
>>
>>
>> Thanks
>> Bernd.
>
>
>
> Oh, I think the true reason is this patch:
> Author: Upstream
> Description: Report and fix both by Nils Carlson.
> Replaced a cc==0 check with proper Tcl_Eof() check.
> Last-Modified: Fri, 23 Oct 2015 11:09:07 +0300
> Bug: http://sourceforge.net/p/expect/patches/18/
> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799301
>
> --- a/expect.c
> +++ b/expect.c
> @@ -1860,19 +1860,11 @@
> if (cc == EXP_DATA_NEW) {
> /* try to read it */
> cc = expIRead(interp,esPtr,timeout,tcl_set_flags);
> -
> - /* the meaning of 0 from i_read means eof. Muck with it a */
> - /* little, so that from now on it means "no new data arrived */
> - /* but it should be looked at again anyway". */
> - if (cc == 0) {
> +
> + if (Tcl_Eof(esPtr->channel)) {
> cc = EXP_EOF;
> - } else if (cc > 0) {
> - /* successfully read data */
> - } else {
> - /* failed to read data - some sort of error was encountered such as
> - * an interrupt with that forced an error return
> - */
> }
> +
> } else if (cc == EXP_DATA_OLD) {
> cc = 0;
> } else if (cc == EXP_RECONFIGURE) {
>
>
> The correct way to fix this would be something like this:
>
> if (cc == 0 && Tcl_Eof(esPtr->channel)) {
> cc = EXP_EOF;
> }
>
> What happens for me is cc > 0 AND Tcl_Eof at the same time.
> That makes dejagnu ignore the last few lines, because it does
> not expect EOF and data at the same time.
>
> Apparently tcl starts to read ahead because the default match
> buffer size is unreasonably small like 2000 bytes.
>
> I can work around it by increasing the buffer size like this:
>
> ndex: gcc/testsuite/lib/gcc-dg.exp
> ===
> --- gcc/testsuite/lib/gcc-dg.exp (revision 250150)
> +++ gcc/testsuite/lib/gcc-dg.exp (working copy)
> @@ -43,6 +43,9 @@
>setenv LANG C.ASCII
> }
>
> +# Avoid sporadic data-losses with expect
> +match_max -d 1
> +
> # Ensure GCC_COLORS is unset, for the rare testcases that verify
> # how output is colorized.
> if [info exists ::env(GCC_COLORS) ] {
>
>
> What do you think?
> Can debian/ubuntu people reproduce and fix this?
> Should we increase the match buffer size on the gcc side?
>
>
>
> Thanks
> Bernd.
signature.asc
Description: Message signed with OpenPGP using GPGMail
Re: Add support to trace comparison instructions and switch statements
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:
>> + *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:
Re: Add support to trace comparison instructions and switch statements
Hi
In fact, under linux with "return address" and file "/proc/self/maps",
we can give unique id for every comparison.
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:
>>> + *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-
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: [patch] RFC: Hook for insn costs (v2)?
Hi, this is a bit different proposal. Instead of using some
magic value to indicate that the hook returns nothing useful,
the hook has not a 4th argument of bool* to ship that information.
The goal and usage is the same as with the first proposal:
* Allow the back-end to compute correct costs and to see the
full pattern. Reason: middle-end currently hides parts of
patterns that might be relevant and implements strange logic
in some places, e.g. in insn_rtx_costs.
* Don't introduce any performance degradation by changing
current usage / assumptions of rtx_costs.
Would such a change be in order in principle?
Ideas to improve it?
I would then round it up and propose it as a patch.
Johann
On 12.07.2017 15:15, Georg-Johann Lay wrote:
Hi,
the current cost computations in rtlanal.c and maybe other places
suffer from the fact that they are hiding parts of the expressions
from the back-end, like SET_DESTs of single_set or the anatomy of
PARALELLs.
Would it be in order to have a hook like the one attached?
I am aware of that, in an ideal world, there wouldn't be more
than one hook to get rtx costs. But well...
Whilst rtx_costs does in the majority of cases, there are cases
where hiding information leads to performance degradation,
for example when insn combine cooks up a set zero_extract.
combine.c does actually the right thing as it uses insn_rtx_costs,
but insn_rtx_cost is already a lie because it only uses SET_SRC
and digs into PARALELL without exposing the whole story.
The patch just uses a new targetm.insn_cost hook. If the
back-end doesn't come up with something useful, the classic
functions with rtx_costs for sub-rtxes are called.
The purpose is to allow a friendly transition and not no
raise any performance degradations which would be very likely
if we just called rtx_costs with outer_code = INSN.
If a back-end finds it useful to implement this hook and need
the whole story, they can do so. Otherwise, or if it is too
lazy to analyse a specific rtx, they can switch to the old
infrastructure.
Returning a magic value for "unknown" is just an implementation
detail; it could just as well be some bool* that would be set
to true or false depending on whether or not the computation
returned something useful or not.
The patch only touches seq_cost insn_rtx_cost of rtlanal.c.
Would something like this be in order, or is a new hook just
a complete no-go?
Index: target.def
===
--- target.def (revision 250090)
+++ target.def (working copy)
@@ -3558,6 +3558,41 @@ DEFHOOKPOD
appropriately.",
unsigned int, INVALID_REGNUM)
+/* Compute the cost of an insn resp. something that might become an insn. */
+DEFHOOK
+(insn_costs,
+"This target hook describes the relative costs of an insn.\n\
+\n\
+@var{insn} might be NULL or an @code{INSN_P}.\n\
+@var{pattern} is the pattern of @var{insn} or an rtx that is supposed\n\
+to be used as the pattern of an insn the remainder of the compilation.\n\
+\n\
+In implementing this hook, you can use the construct\n\
+@code{COSTS_N_INSNS (@var{n})} to specify a cost equal to @var{n}\n\
+instructions.\n\
+When optimizing for execution speed, i.e.@: when @var{speed} is\n\
+true, this target hook should be used to estimate the relative\n\
+execution cost of the pattern, and the size cost of the pattern if\n\
+@var{speed} is false.\n\
+\n\
+Use this pattern if a @code{SET_DEST} is needed to calculate the correct\n\
+costs or when you want to see the whole of a @code{PARALLEL} and not only\n\
+parts of it. Notice that for a @code{single_set} you might see a\n\
+@code{PARALLEL} @var{pattern} that contains a @code{SET} together with\n\
+@code{COBBER}s.\n\
+\n\
+If the hook computed the cost, set @var{*cost_computed} to true.\n\
+If the hook implementation choses not to compute the cost for some reason,\n\
+set @var{*cost_computed} to false. This directs the middle-end to use\n\
+other approaches to get the respective costs like calling\n\
+@code{TARGET_RTX_COSTS} for sub-rtxes of @var{pattern}.\n\
+\n\
+Don't implement this hook if it would always set @var{*cost_computed} to\n\
+false. Even if this hook handles all cases, you still need to implement\n\
+@code{TARGET_RTX_COSTS}.",
+ int, (const rtx_insn *insn, rtx pattern, bool speed, bool *cost_computed),
+ default_insn_costs)
+
/* Compute a (partial) cost for rtx X. Return true if the complete
cost has been computed, and false if subexpressions should be
scanned. In either case, *TOTAL contains the cost result. */
Index: targhooks.c
===
--- targhooks.c (revision 250090)
+++ targhooks.c (working copy)
@@ -2108,4 +2108,11 @@ default_excess_precision (enum excess_pr
return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT;
}
+int
+default_insn_costs (const rtx_insn*, rtx, bool, bool *cost_computed)
+{
+ * cost_computed = false;
+ return 0;
+}
+
#include "gt-targhooks.h"
Index: ta
Re: Getting spurious FAILS in testsuite?
On 12.07.2017 15:40, Bernd Edlinger wrote:
On 07/11/17 22:28, Bernd Edlinger wrote:
On 07/11/17 21:42, Andrew Pinski wrote:
On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski
wrote:
On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski
wrote:
On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
wrote:
Hi,
I see this now as well on Ubuntu 16.04, but I doubt that the Kernel is
to blame.
I don't see these failures when I use a 4.11 kernel. Only with a
4.4 kernel.
Also the guality testsuite does not run at all with a 4.4 kernel, it
does run when using a 4.11 kernel; I suspect this is the same symptom
of the bug.
4.11 kernel results (with CentOS 7.4 glibc):
https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
4.4 kernel results (with ubuntu 1604 glibc):
https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
Note the glibc does not matter here as I get a similar testsuite
results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
the 4.11 kernel.
Also note I get a similar results with a plain 4.11 kernel compared to
the above kernel.
Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
issues.
As Georg-Johann points out here:
https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
updating the kernel alone does not fix the issue.
These patches do all circle around pty and tty devices,
but in the strace file I see a pipe(2) command
creating the fileno 10 and the sequence of events is
as follows:
pipe([7, 10]) = 0 (not in my previous e-mail)
clone() = 16141
clone() = 16142
write(4, "spawn: returns {0}\r\n", 20) = 20
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
read(10,...,4096) = 4096
read(10,...,4096) = 2144
read(10, "", 4096) = 0
close(10) = 0
wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
All data arrive at the expect process, but apparently too quickly.
Thanks
Bernd.
Oh, I think the true reason is this patch:
Author: Upstream
Description: Report and fix both by Nils Carlson.
Replaced a cc==0 check with proper Tcl_Eof() check.
Last-Modified: Fri, 23 Oct 2015 11:09:07 +0300
Bug: http://sourceforge.net/p/expect/patches/18/
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799301
--- a/expect.c
+++ b/expect.c
@@ -1860,19 +1860,11 @@
if (cc == EXP_DATA_NEW) {
/* try to read it */
cc = expIRead(interp,esPtr,timeout,tcl_set_flags);
-
- /* the meaning of 0 from i_read means eof. Muck with it a */
- /* little, so that from now on it means "no new data arrived */
- /* but it should be looked at again anyway". */
- if (cc == 0) {
+
+ if (Tcl_Eof(esPtr->channel)) {
cc = EXP_EOF;
- } else if (cc > 0) {
- /* successfully read data */
- } else {
- /* failed to read data - some sort of error was encountered such as
-* an interrupt with that forced an error return
-*/
}
+
} else if (cc == EXP_DATA_OLD) {
cc = 0;
} else if (cc == EXP_RECONFIGURE) {
The correct way to fix this would be something like this:
if (cc == 0 && Tcl_Eof(esPtr->channel)) {
cc = EXP_EOF;
}
What happens for me is cc > 0 AND Tcl_Eof at the same time.
That makes dejagnu ignore the last few lines, because it does
not expect EOF and data at the same time.
Apparently tcl starts to read ahead because the default match
buffer size is unreasonably small like 2000 bytes.
I can work around it by increasing the buffer size like this:
ndex: gcc/testsuite/lib/gcc-dg.exp
===
--- gcc/testsuite/lib/gcc-dg.exp(revision 250150)
+++ gcc/testsuite/lib/gcc-dg.exp(working copy)
@@ -43,6 +43,9 @@
setenv LANG C.ASCII
}
+# Avoid sporadic data-losses with expect
+match_max -d 1
+
# Ensure GCC_COLORS is unset, for the rare testcases that verify
# how output is colorized.
if [info exists ::env(GCC_COLORS) ] {
What do you think?
Can debian/ubuntu people reproduce and fix this?
Should we increase the match buffer size on the gcc side?
Thanks
Bernd.
Sigh, I am still getting these spurious fails.
$ sudo apt-get install tcl-expect
Reading package lists... Done
Building dependency tree
Reading state information... Done
tcl-expect is already the newest version (5.45-7).
...
So I have "5.45-7"
$ apt-get changelog tcl-expect
expect (5.45-7) unstable; urgency=medium
* Applied a few fixes by upstream which were included in Expect 5.45.3
never released as a tarball (closes: #799301).
* Bumped standards version to 3.9.6.
-- Sergei Golovan Fri, 23 Oct 2015 11:27:59 +0300
...
$ expect -v
expect version 5.45
Not showing the bugfix level as of configure:PACKAGE_VERSION='5.45'.
I also tried
Re: Getting spurious FAILS in testsuite?
On 07/13/17 16:31, Georg-Johann Lay wrote:
> On 12.07.2017 15:40, Bernd Edlinger wrote:
>> On 07/11/17 22:28, Bernd Edlinger wrote:
>>> On 07/11/17 21:42, Andrew Pinski wrote:
On Tue, Jul 11, 2017 at 12:31 PM, Andrew Pinski
wrote:
> On Tue, Jul 11, 2017 at 12:27 PM, Andrew Pinski
> wrote:
>> On Tue, Jul 11, 2017 at 12:15 PM, Bernd Edlinger
>> wrote:
>>> Hi,
>>>
>>> I see this now as well on Ubuntu 16.04, but I doubt that the
>>> Kernel is
>>> to blame.
>>
>> I don't see these failures when I use a 4.11 kernel. Only with a
>> 4.4 kernel.
>> Also the guality testsuite does not run at all with a 4.4 kernel, it
>> does run when using a 4.11 kernel; I suspect this is the same symptom
>> of the bug.
>
>
> 4.11 kernel results (with CentOS 7.4 glibc):
> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg00998.html
>
> 4.4 kernel results (with ubuntu 1604 glibc):
> https://gcc.gnu.org/ml/gcc-testresults/2017-07/msg01009.html
>
> Note the glibc does not matter here as I get a similar testsuite
> results as the CentOS 7.4 glibc using the Ubuntu 1604 glibc but with
> the 4.11 kernel.
>
> Also note I get a similar results with a plain 4.11 kernel compared to
> the above kernel.
Maybe commit 6a7e6f78c235975cc14d4e141fa088afffe7062c or
0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa to the kernel fixes both
issues.
>>>
>>> As Georg-Johann points out here:
>>> https://gcc.gnu.org/ml/gcc/2017-07/msg00028.html
>>> updating the kernel alone does not fix the issue.
>>>
>>> These patches do all circle around pty and tty devices,
>>> but in the strace file I see a pipe(2) command
>>> creating the fileno 10 and the sequence of events is
>>> as follows:
>>>
>>> pipe([7, 10]) = 0 (not in my previous e-mail)
>>> clone() = 16141
>>> clone() = 16142
>>> write(4, "spawn: returns {0}\r\n", 20) = 20
>>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16141,
>>> --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=16142,
>>> read(10,...,4096) = 4096
>>> read(10,...,4096) = 2144
>>> read(10, "", 4096) = 0
>>> close(10) = 0
>>> wait4(16141, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16141
>>> wait4(16142, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 16142
>>>
>>> All data arrive at the expect process, but apparently too quickly.
>>>
>>>
>>> Thanks
>>> Bernd.
>>
>>
>>
>> Oh, I think the true reason is this patch:
>> Author: Upstream
>> Description: Report and fix both by Nils Carlson.
>>Replaced a cc==0 check with proper Tcl_Eof() check.
>> Last-Modified: Fri, 23 Oct 2015 11:09:07 +0300
>> Bug: http://sourceforge.net/p/expect/patches/18/
>> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=799301
>>
>> --- a/expect.c
>> +++ b/expect.c
>> @@ -1860,19 +1860,11 @@
>>if (cc == EXP_DATA_NEW) {
>>/* try to read it */
>>cc = expIRead(interp,esPtr,timeout,tcl_set_flags);
>> -
>> -/* the meaning of 0 from i_read means eof. Muck with it a */
>> -/* little, so that from now on it means "no new data arrived */
>> -/* but it should be looked at again anyway". */
>> -if (cc == 0) {
>> +
>> +if (Tcl_Eof(esPtr->channel)) {
>>cc = EXP_EOF;
>> -} else if (cc > 0) {
>> -/* successfully read data */
>> -} else {
>> -/* failed to read data - some sort of error was encountered
>> such as
>> - * an interrupt with that forced an error return
>> - */
>>}
>> +
>>} else if (cc == EXP_DATA_OLD) {
>>cc = 0;
>>} else if (cc == EXP_RECONFIGURE) {
>>
>>
>> The correct way to fix this would be something like this:
>>
>> if (cc == 0 && Tcl_Eof(esPtr->channel)) {
>> cc = EXP_EOF;
>> }
>>
>> What happens for me is cc > 0 AND Tcl_Eof at the same time.
>> That makes dejagnu ignore the last few lines, because it does
>> not expect EOF and data at the same time.
>>
>> Apparently tcl starts to read ahead because the default match
>> buffer size is unreasonably small like 2000 bytes.
>>
>> I can work around it by increasing the buffer size like this:
>>
>> ndex: gcc/testsuite/lib/gcc-dg.exp
>> ===
>> --- gcc/testsuite/lib/gcc-dg.exp(revision 250150)
>> +++ gcc/testsuite/lib/gcc-dg.exp(working copy)
>> @@ -43,6 +43,9 @@
>> setenv LANG C.ASCII
>>}
>>
>> +# Avoid sporadic data-losses with expect
>> +match_max -d 1
>> +
>># Ensure GCC_COLORS is unset, for the rare testcases that verify
>># how output is colorized.
>>if [info exists ::env(GCC_COLORS) ] {
>>
>>
>> What do you think?
>> Can debian/ubuntu people reproduce and fix this?
>> Should we increase the match buffer size on the gcc side?
>>
>> Thanks
>> Bernd.
>
> Sigh, I am still getting these spurious fails.
>
> $ sudo apt-get install tcl-expect
> Reading package lists... Done
> Buil
gcc-7-20170713 is now available
Snapshot gcc-7-20170713 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/7-20170713/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 7 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-7-branch revision 250189 You'll find: gcc-7-20170713.tar.xzComplete GCC SHA256=105bf4bfda70e456cc99a0de030fc863e97487fe62d6ca870d963e34fd33ef09 SHA1=cd49c291209fae98554b935cf23ed291d06b237c Diffs from 7-20170706 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-7 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
