Skip to content

fix(c): invalidate cache for .include and .incbin directives - #2795

Open
bleggett wants to merge 2 commits into
mozilla:mainfrom
edera-dev:pr/incbin-cachebust
Open

fix(c): invalidate cache for .include and .incbin directives#2795
bleggett wants to merge 2 commits into
mozilla:mainfrom
edera-dev:pr/incbin-cachebust

Conversation

@bleggett

@bleggett bleggett commented Aug 5, 2026

Copy link
Copy Markdown

This fixes a really nasty bug with sccache for C code where .incbin and .include directives weren't fully checked for cache consistency before being returned in the build cache.

This led to a case where a Linux kernel build that used sccache ended up with kmods signed with the wrong signing key, because the kernel's certs/system_certificates.S embeds the cert via assembler .incbin, and sccache would mistakenly return a stale signing key from a previous build 🙃

Ex:
certs/system_certificates.S embeds the cert via assembler .incbin:

  asm
  __module_cert_start:
        .incbin "certs/signing_key.x509"
  __module_cert_end:
        .incbin "certs/x509_certificate_list"

The sizes come from label arithmetic, not from the source text. So the preprocessed text of that file is byte-identical on every build regardless of which key is in signing_key.x509. sccache hashes the preprocessed output, gets a hit, and hands back an object containing the old certificate.

ccache refuses to cache any translation unit containing .incbin precisely because of this (ccache/ccache#136, documented in the ccache manual https://ccache.dev/manual/4.13.6.html).

sccache has the detector but only wires it into preprocessor cache mode, which isn't enough.

Tested with a kernel build using this SHA, mismatch issue goes away. Also added some more unit tests.

I also did a pass to see if there are other similarly-shaped problems where the cache hashing/keying might be broken and return the wrong thing and found a few more minor ones, mostly around plugins/specs/struct layout randomization, those are fixed in the second commit.

@bleggett bleggett changed the title fix: invalidate cache for .include and .incbin directives fix(c): invalidate cache for .include and .incbin directives Aug 5, 2026
@AJIOB

AJIOB commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This also should fix #2700

@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.14388% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.25%. Comparing base (46e96ab) to head (0bb81ee).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/compiler/c.rs 91.50% 22 Missing ⚠️
src/compiler/gcc.rs 94.23% 3 Missing ⚠️
src/compiler/compiler.rs 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2795      +/-   ##
==========================================
+ Coverage   72.90%   73.25%   +0.35%     
==========================================
  Files          72       72              
  Lines       37275    37819     +544     
==========================================
+ Hits        27176    27705     +529     
- Misses      10099    10114      +15     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AJIOB

AJIOB commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Hi @bleggett ,

Looks like you have conflicts. Please, rebase your changes

@bleggett
bleggett force-pushed the pr/incbin-cachebust branch from 9e62ec3 to 0bb81ee Compare August 6, 2026 17:19
@bleggett

bleggett commented Aug 6, 2026

Copy link
Copy Markdown
Author

Hi @bleggett ,

Looks like you have conflicts. Please, rebase your changes

Rebased, thanks!

@AJIOB

AJIOB commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fixes, @bleggett

Is it ok for you, @sylvestre ?

Comment thread src/compiler/c.rs Outdated
Comment thread src/compiler/c.rs
@bleggett
bleggett force-pushed the pr/incbin-cachebust branch from 0bb81ee to bd909f4 Compare August 27, 2026 16:12
@bleggett
bleggett force-pushed the pr/incbin-cachebust branch from bd909f4 to f864db1 Compare August 27, 2026 16:14
@bleggett
bleggett requested a review from sylvestre August 27, 2026 16:15
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.

4 participants