https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/187080
From f976ae61849fe88b76e5685c746a205bdce4d4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Tue, 17 Mar 2026 18:50:11 +0100 Subject: [PATCH 1/4] [analyzer] Don't rule out symbolic pointer pointing to stack Ensure that the analyzer doesn't rule out the equality (or guarantee disequality) of a pointer to the stack and a symbolic pointer in unknown space. Previously the analyzer incorrectly assumed that stack pointers cannot be equal to symbolic pointers in unknown space. It is true that functions cannot validly return pointers to their own stack frame, but they can easily return a pointer to some other stack frame (e.g. a function can return a pointer recieved as an argument). The old behavior was introduced intentionally in 2012 by commit 3563fde6a02c2a75d0b4ba629d80c5511056a688, but it causes incorrect analysis, e.g. it prevents the correct handling of some testcases from the Juliet suite because it rules out the "fgets succeeds" branch. --- .../StaticAnalyzer/Core/SimpleSValBuilder.cpp | 8 +-- ...allow-equality-of-stack-and-symbolic-ptr.c | 62 +++++++++++++++++++ clang/test/Analysis/ptr-arith.c | 2 +- 3 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 6108931f737d4..53ceabc621c19 100644 --- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -952,12 +952,8 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state, const MemSpaceRegion *RightMS = RightBase->getMemorySpace(state); const MemSpaceRegion *UnknownMS = MemMgr.getUnknownRegion(); - // If the two regions are from different known memory spaces they cannot be - // equal. Also, assume that no symbolic region (whose memory space is - // unknown) is on the stack. - if (LeftMS != RightMS && - ((LeftMS != UnknownMS && RightMS != UnknownMS) || - (isa<StackSpaceRegion>(LeftMS) || isa<StackSpaceRegion>(RightMS)))) { + // Two regions from different known memory spaces cannot be equal. + if (LeftMS != RightMS && LeftMS != UnknownMS && RightMS != UnknownMS) { switch (op) { default: return UnknownVal(); diff --git a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c new file mode 100644 index 0000000000000..0e58b885cd283 --- /dev/null +++ b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c @@ -0,0 +1,62 @@ +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -triple x86_64-unknown-linux-gnu \ +// RUN: -verify + +#include "Inputs/std-c-library-functions-POSIX.h" +#define NULL ((void*)0) + +void clang_analyzer_eval(int); +void clang_analyzer_warnIfReached(); + + +// An opaque function that returns a symbolic pointer in unknown space. +int *opaque_function(int *p); + +void test_simple(void) { + // Validate that the analyzer doesn't rule out the equality (or disequality) + // of a pointer to the stack (&x) and a symbolic pointer in unknown space. + // Previously the analyzer incorrectly assumed that stack pointers cannot be + // equal to symbolic pointers, which is obviously nonsense. It is true that + // functions cannot validly return pointers to _their own stack frame_, but + // they can easily return a pointer to some other stack frame (e.g. in this + // example 'opaque_function' could return its argument). + int x = 0; + if (&x == opaque_function(&x)) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } +} + +void test_local_not_leaked(void) { + // In this situation a very smart analyzer could theoretically deduce that + // the address of the local 'x' cannot leak from this function, so the + // call to 'opaque_function' cannot return it. + int x = 0; + if (&x == opaque_function(NULL)) { + // This branch is practically unreachable; however, we cannot fault the + // analyzer for not deducing this. + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } +} + +void test_fgets_can_succeed(FILE *infile) { + // The modeling of `fgets` in StdCLibraryFunctions splits two branches: one + // where the return value is assumed to be equal to the first argument, and + // one where the return value is assumed to be null. However, if the target + // buffer was on the stack, then 'evalBinOp' rejected the possibility that + // the return value (a symbolic pointer) can be equal to the first argument + // (a pointer to the stack), so the analyzer was unable to enter the + // 'success' branch. + char buffer[100]; + if (fgets(buffer, 100, infile) != NULL) { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } else { + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + } +} diff --git a/clang/test/Analysis/ptr-arith.c b/clang/test/Analysis/ptr-arith.c index 51a13b8cfb723..2855e1761e673 100644 --- a/clang/test/Analysis/ptr-arith.c +++ b/clang/test/Analysis/ptr-arith.c @@ -146,7 +146,7 @@ void mixed_region_types(void) { void symbolic_region(int *p) { int a; - clang_analyzer_eval(&a != p); // expected-warning{{TRUE}} + clang_analyzer_eval(&a != p); // expected-warning{{UNKNOWN}} clang_analyzer_eval(&a > p); // expected-warning{{UNKNOWN}} clang_analyzer_eval(&a >= p); // expected-warning{{UNKNOWN}} } From d331c34efbf7505fd5c33193b2ec7f6dff981e13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 18 Mar 2026 14:36:33 +0100 Subject: [PATCH 2/4] Unpin target triple in new test file I wrote this test file by copying and reducing one of the test files for `unix.StdCLibraryFunctions`; the pin was "inherited" from there. Let's hope that it is not necessary in this smaller test file, which only uses `fgets` from the standard library functions. --- clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c index 0e58b885cd283..bdd6dba4036a3 100644 --- a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c +++ b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c @@ -2,7 +2,6 @@ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix.StdCLibraryFunctions \ // RUN: -analyzer-checker=debug.ExprInspection \ -// RUN: -triple x86_64-unknown-linux-gnu \ // RUN: -verify #include "Inputs/std-c-library-functions-POSIX.h" From 0f61781c7c0af9fb247d050b5dcf8d31a46a7dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 18 Mar 2026 14:40:07 +0100 Subject: [PATCH 3/4] Tweak phrasing: s/fault/blame/ in comment --- clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c index bdd6dba4036a3..1d01e8c3c45c2 100644 --- a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c +++ b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c @@ -36,7 +36,7 @@ void test_local_not_leaked(void) { // call to 'opaque_function' cannot return it. int x = 0; if (&x == opaque_function(NULL)) { - // This branch is practically unreachable; however, we cannot fault the + // This branch is practically unreachable; however, we cannot blame the // analyzer for not deducing this. clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} } else { From 208d4db927fc572f6113245fe6819eaf7455281c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <[email protected]> Date: Wed, 18 Mar 2026 14:44:36 +0100 Subject: [PATCH 4/4] Tweak comments in new test file --- .../allow-equality-of-stack-and-symbolic-ptr.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c index 1d01e8c3c45c2..964c860e460b2 100644 --- a/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c +++ b/clang/test/Analysis/allow-equality-of-stack-and-symbolic-ptr.c @@ -16,10 +16,10 @@ int *opaque_function(int *p); void test_simple(void) { // Validate that the analyzer doesn't rule out the equality (or disequality) - // of a pointer to the stack (&x) and a symbolic pointer in unknown space. + // of a pointer to the stack ('&x') and a symbolic pointer in unknown space. // Previously the analyzer incorrectly assumed that stack pointers cannot be // equal to symbolic pointers, which is obviously nonsense. It is true that - // functions cannot validly return pointers to _their own stack frame_, but + // functions cannot validly return pointers to their own stack frame, but // they can easily return a pointer to some other stack frame (e.g. in this // example 'opaque_function' could return its argument). int x = 0; @@ -36,8 +36,8 @@ void test_local_not_leaked(void) { // call to 'opaque_function' cannot return it. int x = 0; if (&x == opaque_function(NULL)) { - // This branch is practically unreachable; however, we cannot blame the - // analyzer for not deducing this. + // This branch is unreachable (without non-standard-compliant tricks); + // however, we cannot blame the analyzer for not deducing this. clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} } else { clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} @@ -45,13 +45,13 @@ void test_local_not_leaked(void) { } void test_fgets_can_succeed(FILE *infile) { - // The modeling of `fgets` in StdCLibraryFunctions splits two branches: one + // The modeling of 'fgets' in StdCLibraryFunctions splits two branches: one // where the return value is assumed to be equal to the first argument, and // one where the return value is assumed to be null. However, if the target // buffer was on the stack, then 'evalBinOp' rejected the possibility that // the return value (a symbolic pointer) can be equal to the first argument // (a pointer to the stack), so the analyzer was unable to enter the - // 'success' branch. + // "success" branch. char buffer[100]; if (fgets(buffer, 100, infile) != NULL) { clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
