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

Fix cache collision with object file and precompiled headers #2268

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

jimis
Copy link
Contributor

@jimis jimis commented Oct 4, 2024

When compiling with PCH enabled, it happens that some times object files end up with PCH content, or that .gch files end up with object code.

Simplest way to reproduce the problem:

$ touch empty.c
$ gcc -c -o out1 empty.c
$ gcc -x c-header -c -o out2 empty.c
$ file out1 out2
out1: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
out2: GCC precompiled header (version 014) for C

The two files are different. But if we feed these compilations to sccache, they lead to the same result:

$ sccache gcc -c -o out3 empty.c
$ sccache gcc -x c-header -c -o out4 empty.c
$ file out3 out4
out3: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
out4: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped

Same thing is reproducible with c++ compiler and -x c++-header argument.

The reason is that the hash string that identifies each command line is the same.

With this patch, compilation of C files is always differentiated from compilations of same C-Header files. And compilation of C++ files is always differentiated from compilations of same C++-Header files.

Fixes #1851.

@jimis jimis force-pushed the miscompilation-gh1851 branch from 0ad9102 to 5926a1b Compare October 4, 2024 16:38
When compiling with PCH enabled, it happens that some times object files
end up with PCH content, or that .gch files end up with object code.

Simplest way to reproduce the problem:

  $ touch empty.c
  $ gcc              -c -o out1 empty.c
  $ gcc  -x c-header -c -o out2 empty.c
  $ file out1 out2
  out1: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
  out2: GCC precompiled header (version 014) for C

The two files are different. But if we feed these compilations to
sccache, they lead to the same result:

  $ sccache gcc              -c -o out3 empty.c
  $ sccache gcc  -x c-header -c -o out4 empty.c
  $ file out3 out4
  out3: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
  out4: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped

Same thing is reproducible with c++ compiler and -x c++-header argument.

The reason is that the hash string that identifies each command line is
the same.

With this patch, compilation of C files is always differentiated from
compilations of same C-Header files. And compilation of C++ files is
always differentiated from compilations of same C++-Header files.

Fixes mozilla#1851.
@jimis jimis force-pushed the miscompilation-gh1851 branch from 5926a1b to b881869 Compare October 4, 2024 22:50
@jimis
Copy link
Contributor Author

jimis commented Oct 4, 2024

This is ready for review.

The weirdest thing happened while I was running the integration tests on my fork: The "clang" integration test failed (the last --stats invocation showed 0 cache hits). Then I click "rerun failed tests" without changing anything and it passed (it then showed 2 cache hits).
Are the integration tests known to be flaky?

@sylvestre
Copy link
Collaborator

Are the integration tests known to be flaky?

yes, the cache might be full at time

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 40.61%. Comparing base (0cc0c62) to head (b881869).
Report is 80 commits behind head on main.

Files with missing lines Patch % Lines
src/compiler/c.rs 33.33% 2 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2268      +/-   ##
==========================================
+ Coverage   30.91%   40.61%   +9.70%     
==========================================
  Files          53       54       +1     
  Lines       20112    20746     +634     
  Branches     9755     9641     -114     
==========================================
+ Hits         6217     8427    +2210     
- Misses       7922     8150     +228     
+ Partials     5973     4169    -1804     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sylvestre
Copy link
Collaborator

nice easy fix, well done :)

@sylvestre sylvestre merged commit af5ea09 into mozilla:main Oct 5, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sccache fails to compile Qt: hash collision between precompiled headers and object files
3 participants