gcc: don't require .gcno for assembly compiled with coverage flags - #2845
Open
cstrahan wants to merge 1 commit into
Open
gcc: don't require .gcno for assembly compiled with coverage flags#2845cstrahan wants to merge 1 commit into
cstrahan wants to merge 1 commit into
Conversation
Assembly inputs (.s/.S) accept --coverage / -ftest-coverage but never emit a .gcno note file. Assembly became cacheable in the "Assembly language support" change, after which sccache aborts caching such a compile with "failed to zip up compiler outputs": the .gcno is added to the expected outputs as required (optional: false), but the file does not exist. Mark the .gcno output optional for the assembly languages; C/C++/ObjC keep requiring it, so a genuinely-missing note file there is still an error. Adds a regression test. Fixes mozilla#2275
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2845 +/- ##
==========================================
- Coverage 76.14% 67.46% -8.69%
==========================================
Files 72 72
Lines 39807 38625 -1182
==========================================
- Hits 30313 26060 -4253
- Misses 9494 12565 +3071 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Caching an assembly compile that carries the gcov/coverage flags aborts the build:
--coverage/-ftest-coveragesetoutputs_gcno = true, and ingcc::parse_argumentsthe
.gcnonote file is then added to the expected outputs as required(
optional: false). For C/C++/ObjC the compiler does emit a.gcno, but an assemblyinput (
.s/.S) accepts the coverage flags and produces no.gcno. When sccachepackages the compiler outputs for caching it can't find the file and fails the whole
compile.
This only surfaces because assembly is cacheable: the "Assembly language support" change
(d9d2eb6) added
Language::Assembler/Language::AssemblerToPreprocess, so assemblycompiles now go through output packaging. Before that they were
CannotCacheand ran thecompiler directly, so the missing
.gcnonever mattered. It's the same class of bug asthe missing-
.gcnocase in #2275.Real-world hit: any project that builds a
.Sunder-Db_coverage/--coverage(e.g. breakpad's
breakpad_getcontext.S) can no longer be built through sccache with acoverage build.
Fix
Mark the
.gcnooutputoptionalfor assembly languages. C/C++/ObjC are unchanged — the.gcnostays required there, so a genuinely-missing note file for those languages is stillan error.
Test
Added
test_parse_arguments_coverage_assembly_gcno_optional, asserting that--coverage -c foo.Syields agcnooutput withoptional: true(the existingtest_parse_arguments_coverage_outputs_gcnocontinues to assertoptional: falseforfoo.cpp).Verified end to end with a from-source build:
sccache cc --coverage -c foo.Snow succeeds and is cached;sccache cc --coverage -c foo.cstill emits and cachesfoo.gcno(restored on a cachehit), i.e. no regression for C/C++ coverage.
Fixes #2275.