From 39136f38276a93c89545a64c15f2ea53e298c256 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Tue, 25 Nov 2025 14:50:22 +0100 Subject: [PATCH 1/5] C/C++ overlay: Add basic Overlay.qll file --- cpp/ql/lib/cpp.qll | 1 + .../lib/semmle/code/cpp/internal/Overlay.qll | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll diff --git a/cpp/ql/lib/cpp.qll b/cpp/ql/lib/cpp.qll index 46c651daf579..560a4444bfad 100644 --- a/cpp/ql/lib/cpp.qll +++ b/cpp/ql/lib/cpp.qll @@ -74,3 +74,4 @@ import semmle.code.cpp.Preprocessor import semmle.code.cpp.Iteration import semmle.code.cpp.NameQualifiers import DefaultOptions +private import semmle.code.cpp.internal.Overlay diff --git a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll new file mode 100644 index 000000000000..0c8fd9439aca --- /dev/null +++ b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll @@ -0,0 +1,40 @@ +/** + * Defines entity discard predicates for C++ overlay analysis. + */ + +/** + * Holds always for the overlay variant and never for the base variant. + * This local predicate is used to define local predicates that behave + * differently for the base and overlay variant. + */ +overlay[local] +predicate isOverlay() { databaseMetadata("isOverlay", "true") } + +/** Gets the file path for a location. */ +overlay[local] +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) + ) +} + +/** + * Discard any element from the base that is in a changed file. + */ +overlay[discard_entity] +private predicate discardElement(@element e) { overlayChangedFiles(getElementPathInBase(e)) } From 6c093258385a51f237437f15f76f4b8aec247b59 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Wed, 26 Nov 2025 13:05:12 +0100 Subject: [PATCH 2/5] 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 0c8fd9439aca..432dca345505 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) + ) + } +} From 3d692863829b09a13179f39c1bfd497b00cf4161 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Wed, 26 Nov 2025 14:56:13 +0100 Subject: [PATCH 3/5] C/C++ overlay: Address review comments --- .../lib/semmle/code/cpp/internal/Overlay.qll | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll index 432dca345505..1e01282a42e7 100644 --- a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll +++ b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll @@ -10,42 +10,52 @@ overlay[local] predicate isOverlay() { databaseMetadata("isOverlay", "true") } -/** Gets the file path for a location. */ overlay[local] private string getLocationFilePath(@location_default loc) { exists(@file file | locations_default(loc, file, _, _, _, _) | files(file, result)) } +/** + * An element with a single location. Discard if in a changed file. + */ overlay[local] -private class DiscardableEntityBase extends @element { - /** Gets the path to the file in which this element occurs. */ +abstract private class Discardable extends @element { 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() } } +overlay[discard_entity] +private predicate discardable(@element e) { + e = any(Discardable d | d.existsInBase() and overlayChangedFiles(d.getFilePath())) +} + /** - * 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. + * An element with potentially multiple locations, e.g., variables, functions and types. + * Discard only if all locations are in changed files. */ +overlay[local] +abstract private class MultiDiscardable extends @element { + abstract string getFilePath(); + + predicate existsInBase() { not isOverlay() } + + string toString() { none() } +} + overlay[discard_entity] -private predicate discardEntity(@element e) { +private predicate multiDiscardable(@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)) + any(MultiDiscardable d | + d.existsInBase() and + forall(string path | path = d.getFilePath() | overlayChangedFiles(path)) ) } -/** A discardable variable declaration entry. */ overlay[local] -private class DiscardableVarDecl extends DiscardableEntityBase instanceof @var_decl { +private class DiscardableVarDecl extends Discardable instanceof @var_decl { override string getFilePath() { exists(@location_default loc | var_decls(this, _, _, _, loc) | result = getLocationFilePath(loc) @@ -53,9 +63,8 @@ private class DiscardableVarDecl extends DiscardableEntityBase instanceof @var_d } } -/** A discardable variable. */ overlay[local] -private class DiscardableVariable extends DiscardableEntityBase instanceof @variable { +private class DiscardableVariable extends MultiDiscardable instanceof @variable { override string getFilePath() { exists(@var_decl vd, @location_default loc | var_decls(vd, this, _, _, loc) | result = getLocationFilePath(loc) From eac06ddd8f31b8f2ae47b1ea802a6cc9f52d40ed Mon Sep 17 00:00:00 2001 From: idrissrio Date: Fri, 28 Nov 2025 11:28:26 +0100 Subject: [PATCH 4/5] C/C++ overlay: Address review comments Split the discard predicate into two: one for single-location elements and one for multi-location elements. --- .../lib/semmle/code/cpp/internal/Overlay.qll | 73 ++++++++----------- 1 file changed, 32 insertions(+), 41 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll index 1e01282a42e7..ed0f0445697e 100644 --- a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll +++ b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll @@ -16,58 +16,49 @@ private string getLocationFilePath(@location_default loc) { } /** - * An element with a single location. Discard if in a changed file. + * Gets the file path for an element with a single location. */ overlay[local] -abstract private class Discardable extends @element { - abstract string getFilePath(); - - predicate existsInBase() { not isOverlay() } - - string toString() { none() } -} - -overlay[discard_entity] -private predicate discardable(@element e) { - e = any(Discardable d | d.existsInBase() and overlayChangedFiles(d.getFilePath())) +private string getSingleLocationFilePath(@element e) { + // @var_decl has a direct location in the var_decls relation + exists(@location_default loc | var_decls(e, _, _, _, loc) | result = getLocationFilePath(loc)) + //TODO: add other kinds of elements with single locations } /** - * An element with potentially multiple locations, e.g., variables, functions and types. - * Discard only if all locations are in changed files. + * Gets the file path for an element with potentially multiple locations. */ overlay[local] -abstract private class MultiDiscardable extends @element { - abstract string getFilePath(); - - predicate existsInBase() { not isOverlay() } - - string toString() { none() } +private string getMultiLocationFilePath(@element e) { + // @variable gets its location(s) from its @var_decl(s) + exists(@var_decl vd, @location_default loc | var_decls(vd, e, _, _, loc) | + result = getLocationFilePath(loc) + ) + //TODO: add other kinds of elements with multiple locations } -overlay[discard_entity] -private predicate multiDiscardable(@element e) { - e = - any(MultiDiscardable d | - d.existsInBase() and - forall(string path | path = d.getFilePath() | overlayChangedFiles(path)) - ) +/** Holds if `e` exists in the base variant. */ +overlay[local] +private predicate existsInBase(@element e) { + not isOverlay() and + (exists(getSingleLocationFilePath(e)) or exists(getMultiLocationFilePath(e))) } -overlay[local] -private class DiscardableVarDecl extends Discardable instanceof @var_decl { - override string getFilePath() { - exists(@location_default loc | var_decls(this, _, _, _, loc) | - result = getLocationFilePath(loc) - ) - } +/** + * Discard an element with a single location if it is in a changed file. + */ +overlay[discard_entity] +private predicate discardSingleLocationElement(@element e) { + existsInBase(e) and + overlayChangedFiles(getSingleLocationFilePath(e)) } -overlay[local] -private class DiscardableVariable extends MultiDiscardable instanceof @variable { - override string getFilePath() { - exists(@var_decl vd, @location_default loc | var_decls(vd, this, _, _, loc) | - result = getLocationFilePath(loc) - ) - } +/** + * Discard an element with multiple locations only if all its locations are in changed files. + */ +overlay[discard_entity] +private predicate discardMultiLocationElement(@element e) { + existsInBase(e) and + exists(getMultiLocationFilePath(e)) and + forall(string path | path = getMultiLocationFilePath(e) | overlayChangedFiles(path)) } From 4ad25e4d92c760f37a6200af2d3f7014463f74c5 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Fri, 28 Nov 2025 14:16:10 +0100 Subject: [PATCH 5/5] C/C++ overlay: Address review comments --- .../lib/semmle/code/cpp/internal/Overlay.qll | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll index ed0f0445697e..571f034d85b1 100644 --- a/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll +++ b/cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll @@ -37,28 +37,24 @@ private string getMultiLocationFilePath(@element e) { //TODO: add other kinds of elements with multiple locations } -/** Holds if `e` exists in the base variant. */ -overlay[local] -private predicate existsInBase(@element e) { - not isOverlay() and - (exists(getSingleLocationFilePath(e)) or exists(getMultiLocationFilePath(e))) -} - /** - * Discard an element with a single location if it is in a changed file. + * A local helper predicate that holds in the base variant and never in the + * overlay variant. */ -overlay[discard_entity] -private predicate discardSingleLocationElement(@element e) { - existsInBase(e) and - overlayChangedFiles(getSingleLocationFilePath(e)) -} +overlay[local] +private predicate holdsInBase() { not isOverlay() } /** - * Discard an element with multiple locations only if all its locations are in changed files. + * Discards an element from the base variant if: + * - It has a single location in a changed file, or + * - All of its locations are in changed files. */ overlay[discard_entity] -private predicate discardMultiLocationElement(@element e) { - existsInBase(e) and - exists(getMultiLocationFilePath(e)) and - forall(string path | path = getMultiLocationFilePath(e) | overlayChangedFiles(path)) +private predicate discardElement(@element e) { + holdsInBase() and + ( + overlayChangedFiles(getSingleLocationFilePath(e)) + or + forex(string path | path = getMultiLocationFilePath(e) | overlayChangedFiles(path)) + ) }