From 6c093258385a51f237437f15f76f4b8aec247b59 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Wed, 26 Nov 2025 13:05:12 +0100 Subject: [PATCH] C/C++ Overlay: Preserve entities that have at least one location in an unchanged file Previously, an entity would be discarded if it had any location in a changed file. This caused issues for entities with multiple declaration entries, such as extern variables declared in one file and defined in another. For example, given: // a.c (changed) // b.c (unchanged) extern int x; int x; The variable `x` should be preserved because it has a location in the unchanged file b.c, even though it also has a location in the changed file a.c. --- .../lib/semmle/code/cpp/internal/Overlay.qll | 56 +++++++++++++------ 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll index 0c8fd9439ac..432dca34550 100644 --- a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll +++ b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll @@ -16,25 +16,49 @@ private string getLocationFilePath(@location_default loc) { exists(@file file | locations_default(loc, file, _, _, _, _) | files(file, result)) } -/** - * Gets the file path for an element in the base variant. - */ overlay[local] -private string getElementPathInBase(@element e) { - not isOverlay() and - exists(@location_default loc | - // Direct location (declarations) - var_decls(e, _, _, _, loc) - or - // Indirect location (entities) - exists(@var_decl vd | var_decls(vd, e, _, _, _) | var_decls(vd, _, _, _, loc)) - | - result = getLocationFilePath(loc) - ) +private class DiscardableEntityBase extends @element { + /** Gets the path to the file in which this element occurs. */ + abstract string getFilePath(); + + /** Holds if this element exists in the base variant. */ + predicate existsInBase() { not isOverlay() } + + /** Gets a textual representation of this discardable element. */ + string toString() { none() } } /** - * Discard any element from the base that is in a changed file. + * Discard an entity from the base if all its locations are in changed files. + * Entities with at least one location in an unchanged file are kept. */ overlay[discard_entity] -private predicate discardElement(@element e) { overlayChangedFiles(getElementPathInBase(e)) } +private predicate discardEntity(@element e) { + e = + any(DiscardableEntityBase de | + de.existsInBase() and + overlayChangedFiles(de.getFilePath()) and + // Only discard if ALL file paths are in changed files + forall(string path | path = de.getFilePath() | overlayChangedFiles(path)) + ) +} + +/** A discardable variable declaration entry. */ +overlay[local] +private class DiscardableVarDecl extends DiscardableEntityBase instanceof @var_decl { + override string getFilePath() { + exists(@location_default loc | var_decls(this, _, _, _, loc) | + result = getLocationFilePath(loc) + ) + } +} + +/** A discardable variable. */ +overlay[local] +private class DiscardableVariable extends DiscardableEntityBase instanceof @variable { + override string getFilePath() { + exists(@var_decl vd, @location_default loc | var_decls(vd, this, _, _, loc) | + result = getLocationFilePath(loc) + ) + } +}