Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"-D not hashed" test failure with gcc 6.1.1 #113

Closed
scop opened this issue Jul 14, 2016 · 11 comments
Closed

"-D not hashed" test failure with gcc 6.1.1 #113

scop opened this issue Jul 14, 2016 · 11 comments
Labels
bug Does not work as intended/documented
Milestone

Comments

@scop
Copy link
Contributor

scop commented Jul 14, 2016

This is with ccache 3.2.6 on Fedora 24 and development, gcc 6.1.1:

CC='gcc' ./test.sh
compiler: /usr/lib64/ccache/gcc
version: gcc (GCC) 6.1.1 20160621 (Red Hat 6.1.1-3) Copyright (C) 2016 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
test dir: testdir.8818
starting testsuite base
SUITE: "base", TEST: "-D not hashed" - Expected "cache hit (preprocessed)" to be 14, got 13
cache directory                     /builddir/build/BUILD/ccache-3.2.6/testdir.8818/.ccache
primary config                      /builddir/build/BUILD/ccache-3.2.6/testdir.8818/.ccache/ccache.conf
secondary config      (readonly)    /etc/ccache.conf
cache hit (direct)                     0
cache hit (preprocessed)              13
cache miss                            40
called for link                        2
called for preprocessing               2
multiple source files                  1
compiler produced stdout               1
couldn't find the compiler             1
bad compiler arguments                 1
unsupported source language            2
unsupported compiler option            1
output to a non-regular file           1
no input file                          1
files in cache                         3
cache size                          12.3 kB
max cache size                       5.0 GB
TEST FAILED
Test data and log file have been left in testdir.8818
Makefile:103: recipe for target 'test' failed

Tarball of testdir.8818 contents

@afbjorklund
Copy link
Contributor

Hmm, I was thinking this to be fixed (#96 and a218001)

@afbjorklund
Copy link
Contributor

Sure looks like the same issue:

$ gcc --version
gcc (GCC) 6.1.1 20160621 (Red Hat 6.1.1-3)
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc -DNOT_AFFECTING=1 -E test1.c
# 1 "test1.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test1.c"
int foo10(int x) { return x; }
$ gcc -E test1.c
# 1 "test1.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 31 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "<command-line>" 2
# 1 "test1.c"

Should have written a test for it.

@afbjorklund
Copy link
Contributor

Yeah, looks like I botched the refactoring. Now the "# 1" is missing, causing a cache miss...

Expected:

HASH 
HASH " 1 3 4
HASH # 1 "
HASH <command-line>
HASH " 2
HASH test1.c
HASH "
HASH 

Actual:

HASH 
HASH " 1 3 4
HASH <command-line>
HASH " 2
HASH test1.c
HASH "
HASH 

@jrosdahl : looks we'll be needing a 3.2.7 (problem affecting gcc5/gcc6 only), with a fix and a test

@afbjorklund
Copy link
Contributor

Actually turns out that it wasn't me, since 4dca108 works but a218001 does not.
So it was the backport that killed it... The same workaround is working on "master".

Have written a test case for it (cf8d693), and fixed the issue (04e36a2). PR upcoming.
@jrosdahl : it will be possible to merge this fix into master, it shouldn't hurt anything there.

@jrosdahl jrosdahl added this to the 3.2.7 milestone Jul 19, 2016
@jrosdahl
Copy link
Member

So it was the backport that killed it...

Darn. Thanks for the fix!

@afbjorklund
Copy link
Contributor

Interestingly, after the change done on master we are now reading the "numbers" that tell whether it's a system header (or not) - but we are not caching those numbers or the trailing quote any longer...
Is it important ? Probably not, but it did look a bit different. And it was also the reason why it didn't fail on master, since we don't write the trailing stuff it doesn't matter if the previous buffer is "flushed" or not.

@afbjorklund
Copy link
Contributor

i.e. we might want to change it back to caching that stuff again

@jrosdahl jrosdahl added the bug Does not work as intended/documented label Jul 19, 2016
@jrosdahl
Copy link
Member

Interestingly, after the change done on master we are now reading the "numbers" that tell whether it's a system header (or not) - but we are not caching those numbers or the trailing quote any longer...

process_preprocessed_file reads the preprocessed file data and computes a hash. It doesn't cache anything so I don't quite understand what you mean by "caching those numbers or the trailing quote". Could you give me some examples of what you think needs to be fixed after a218001?

@afbjorklund
Copy link
Contributor

I meant for computing the hash. While troubleshooting the issue (above), I listed all the hash output.
Just added some debugging logging into hash_buffer, actually. But anyway, dumped all the strings.

The (middle part of the) preprocessor output looks like:

# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2

Ccache version 3.2 hashes this as (omitting the header contents):

HASH:
# 1 "
HASH: <built-in>
HASH: "
# 1 "
HASH: <command-line>
HASH: "
# 1 "
HASH: /usr/include/stdc-predef.h
HASH: " 1 3 4
# 1 "
HASH: <command-line>
HASH: " 2

Whereas ccache 3.3/master hashes this as:

HASH: 
# 1 "
HASH: <built-in>
HASH: 
# 1 "
HASH: <command-line>
HASH: 
# 1 "
HASH: /usr/include/stdc-predef.h
HASH: 
# 1 "
HASH: <command-line>
HASH:

@jrosdahl
Copy link
Member

OK, so if I understand you correctly you by "after the change done on master we are now reading ... i.e. we might want to change it back to caching that stuff again" meant that master could need fixing but that 3.2-maint works fine. This sounds obvious in retrospect, but I was unsure if it could be interpreted as "the changes made on master regressed and they were then backported to 3.2-maint which now also has regressed".

Yes, sounds reasonable to restore hashing of the quotes and numbers again on master.

@jrosdahl
Copy link
Member

Fixed by #115, included in ccache 3.2.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Does not work as intended/documented
Projects
None yet
Development

No branches or pull requests

3 participants