diff --git a/include/analysis/IRValueUtils.hpp b/include/analysis/IRValueUtils.hpp index 467f120f..23ee3ab7 100644 --- a/include/analysis/IRValueUtils.hpp +++ b/include/analysis/IRValueUtils.hpp @@ -7,6 +7,7 @@ namespace llvm { class AllocaInst; + class CallBase; class ConstantInt; class Function; class StoreInst; @@ -35,6 +36,12 @@ namespace ctrace::stack::analysis Unknown }; + /// The function a call reaches directly, through pointer casts and aliases: on ELF targets, + /// clang emits a complete-object constructor or destructor defined out of its class as an + /// alias of the base-object one, and calls the alias. Null for an indirect call, and for a + /// call through an alias that the link may replace (weak): its target is not known. + llvm::Function* directCallee(const llvm::CallBase& call); + std::string deriveAllocaName(const llvm::AllocaInst* AI); bool isLikelyCompilerTemporaryName(llvm::StringRef name); diff --git a/run_test.py b/run_test.py index 276eedb5..02c0eee9 100755 --- a/run_test.py +++ b/run_test.py @@ -2347,7 +2347,7 @@ def checked_output(inputs): # Old summaries have no uncertainty metadata and must be rebuilt, not reused. for path, summary in zip(cache_files, summaries): - summary["schema"] = "resource-summary-cache-v3" + summary["schema"] = "resource-summary-cache-v4" for fn in summary["functions"]: for kind in ("normal", "exceptional"): exit_summary = fn["ownership"][kind] @@ -2357,7 +2357,7 @@ def checked_output(inputs): if checked_output(inputs) != first: return fail_check("obsolete summaries changed the wrapper diagnostics") # Intermediate fixpoint keys need not be reused; active keys must be rewritten. - if not any(json.loads(path.read_text())["schema"] == "resource-summary-cache-v4" for path in cache_files): + if not any(json.loads(path.read_text())["schema"] == "resource-summary-cache-v5" for path in cache_files): return fail_check("obsolete ownership summaries were not rebuilt") print(" ✅ ownership wrapper metadata preserved locally, across TUs and in the cache\n") return True @@ -3890,6 +3890,241 @@ def check_diagnostic_paths_follow_the_input() -> bool: return ok +def check_calls_through_aliases() -> bool: + """ + #179: on ELF targets, clang calls a constructor or a destructor defined out of its class + through an alias (C1 -> C2, D1 -> D2), a direct call that Mach-O makes without one. The + findings, and whether each max stack is known, must be the same for x86-64 and AArch64 Linux + as for macOS: an object that only reaches its constructor does not escape, the call does not + hide that the constructor initializes it, and a constructor that keeps this is reported. A + constructor defined in another file must resolve through its alias too. The max stacks + themselves may differ: Mach-O keeps C1 as a function that calls C2, a frame that ELF folds + into the alias. + """ + print("=== Testing calls through constructor and destructor aliases, per target ===") + targets = ("x86_64-unknown-linux-gnu", "aarch64-unknown-linux-gnu", "arm64-apple-macosx") + smt_args = [ + "--smt=on", + "--smt-backend=z3", + "--smt-mode=single", + f"--smt-rules={','.join(_all_smt_rules())}", + f"--smt-timeout-ms={SMT_FIXTURE_TIMEOUT_MS}", + ] + + def observed(sources: list[Path], target: str, extra: list[str]): + result = run_analyzer( + [*map(str, sources), "--format=json", f"--compile-arg=--target={target}", *extra] + ) + try: + payload = json.loads(result.stdout or "") + except json.JSONDecodeError: + return None + # Constructors and destructors are left out: Mach-O lists C1 and D1, ELF only aliases. + stacks = { + fn.get("name"): "known" if fn.get("maxStackUnknown") is False else "unknown" + for fn in payload.get("functions", []) + if not str(fn.get("name", "")).startswith("_ZN") + } + findings = sorted( + { + (str(d.get("ruleId", "")).split(".", 1)[0], d.get("location", {}).get("function")) + for d in payload.get("diagnostics", []) + } + ) + return stacks, findings + + with tempfile.TemporaryDirectory(prefix="ct_alias_cross_file_") as tmp: + cross = Path(tmp) + (cross / "resolver.hpp").write_text( + "struct Resolver\n{\n explicit Resolver(int value);\n int seed;\n};\n" + ) + (cross / "resolver.cpp").write_text( + '#include "resolver.hpp"\n\nResolver::Resolver(int value) : seed(value) {}\n' + ) + (cross / "run.cpp").write_text( + '#include "resolver.hpp"\n\nint run(int value)\n{\n' + " const Resolver resolver(value);\n return resolver.seed;\n}\n" + ) + escape = "StackPointerEscape" + # (label, sources, extra arguments, functions whose max stack must be known, findings + # expected on every target). + cases = [ + ("escape-stack/ctor-alias-no-escape.cpp", None, [], ["_Z3runi"], []), + ( + "false-positive-repro/ctor-dtor-alias.cpp", + None, + [], + ["_Z3usev", "_Z5framev"], + [("UninitializedLocalRead", "_Z3usev")], + ), + ("escape-stack/ctor-alias-stores-this.cpp", None, [], ["_Z3runi"], [(escape, "_Z3runi")]), + ( + "escape-stack/ctor-alias-passes-this.cpp", + None, + [], + [], + [(escape, "_Z3runiPFvP8ResolverE")], + ), + ( + "constructor defined in another file", + [cross / "resolver.cpp", cross / "run.cpp"], + [f"--compile-arg=-I{cross}"], + ["_Z3runi"], + [], + ), + ] + ok = True + for pass_name, pass_args in (("default", []), ("smt-z3", smt_args)): + for name, sources, case_args, known_stacks, expected in cases: + label = f"{pass_name}, {name}" + files = sources or [RUN_CONFIG.test_dir / name] + runs = {t: observed(files, t, [*case_args, *pass_args]) for t in targets} + problems = [] + if any(run is None for run in runs.values()): + missing = sorted(t for t, r in runs.items() if r is None) + problems.append(f"no JSON output: {missing}") + else: + first = runs[targets[0]] + for target in targets[1:]: + if runs[target] != first: + problems.append(f"{target}: {runs[target]}, {targets[0]}: {first}") + for target, (stacks, findings) in runs.items(): + unknown = [f for f in known_stacks if stacks.get(f) != "known"] + if unknown: + problems.append(f"{target}: max stack not known for {unknown}") + if findings != expected: + problems.append(f"{target}: findings {findings}, expected {expected}") + if problems: + ok = False + print(f" ❌ {label}") + for problem in problems: + print(f" {problem}") + else: + print(f" ✅ {label}: {runs[targets[0]][1]}") + + # Aliases the source declares, which clang supports on ELF targets only. An alias the link + # may replace (weak) does not designate its aliasee for sure: the caller's max stack stays + # unknown, whether or not the definition that replaces it is analyzed too. A plain alias does. + with tempfile.TemporaryDirectory(prefix="ct_alias_attribute_") as tmp: + root = Path(tmp) + (root / "weak-alias.c").write_text( + "int fallback(int n) { return n; }\n" + 'int replaceable(int n) __attribute__((weak, alias("fallback")));\n' + "int caller(int n) { return replaceable(n); }\n" + ) + (root / "plain-alias.c").write_text( + "int fallback(int n) { return n; }\n" + 'int replaceable(int n) __attribute__((alias("fallback")));\n' + "int caller(int n) { return replaceable(n); }\n" + ) + (root / "override.c").write_text( + "int replaceable(int n)\n{\n volatile unsigned char buffer[256];\n" + " buffer[0] = (unsigned char)n;\n return buffer[0];\n}\n" + ) + alias_cases = [ + ("weak alias and the definition that replaces it", ["weak-alias.c", "override.c"], "unknown"), + ("weak alias alone", ["weak-alias.c"], "unknown"), + ("plain alias", ["plain-alias.c"], "known"), + ] + for pass_name, pass_args in (("default", []), ("smt-z3", smt_args)): + for name, files, status in alias_cases: + for target in targets[:2]: + run = observed([root / f for f in files], target, pass_args) + got = run[0].get("caller") if run else "no JSON output" + if got == status: + print(f" ✅ {pass_name}, {name}, {target}: caller max stack {got}") + else: + ok = False + print(f" ❌ {pass_name}, {name}, {target}: caller max stack {got}, expected {status}") + print() + return ok + + +def check_resource_cache_rebuilds_alias_summaries() -> bool: + """ + Following a call through an alias changes the resource summaries (#179): a wrapper that + returns what an alias of an acquiring function returns now acquires. Summaries cached by a + previous version of the analyzer, under the previous cache schema, must be rebuilt: the + summaries a run with that cache writes must be those a run with an empty cache writes, the + wrapper's acquisition included, not only the same diagnostics. Entries for intermediate + rounds of the fixed point that a run does not read again may stay as they were. The cache + keys hash the IR as printed, which differs between a fresh compile and the compile IR cache: + a first run fills that cache, so that both runs compared take their IR from it. + """ + print("=== Testing that resource summaries cached by a previous schema are rebuilt ===") + ok = True + with tempfile.TemporaryDirectory(prefix="ct_alias_resource_cache_") as tmp: + root = Path(tmp) + (root / "alias.c").write_text( + "static int storage;\n" + "void *create_impl(void) { return &storage; }\n" + 'void *alias_create(void) __attribute__((alias("create_impl")));\n' + "void *wrapper(void) { return alias_create(); }\n" + ) + (root / "caller.c").write_text( + "void *wrapper(void);\nvoid release_handle(void *);\n" + "void consumer(void) { release_handle(wrapper()); }\n" + ) + model = root / "model.txt" + model.write_text("acquire_ret create_impl Handle\nrelease_arg release_handle 0 Handle\n") + + def acquires(summary: dict) -> bool: + return any( + e.get("action") == "acquire_ret" + for fn in summary["functions"] if fn["name"] == "wrapper" for e in fn["effects"] + ) + + for target in ("x86_64-unknown-linux-gnu", "aarch64-unknown-linux-gnu"): + cold, stale = root / f"cold-{target}", root / f"stale-{target}" + + def run(cache: Path): + return run_analyzer_uncached( + [str(root / "alias.c"), str(root / "caller.c"), "--format=json", "--jobs=1", + f"--compile-arg=--target={target}", f"--resource-model={model}", + f"--compile-ir-cache-dir={root / f'ir-{target}'}", + f"--resource-summary-cache-dir={cache}"] + ) + + run(root / f"warm-up-{target}") + first = run(cold) + expected = {path.name: json.loads(path.read_text()) for path in cold.glob("*.json")} + if first.returncode != 0 or not any(acquires(s) for s in expected.values()): + ok = False + print(f" ❌ {target}: the wrapper's summary does not acquire with an empty cache") + continue + # The cache a previous version left: the previous schema, and a wrapper that does + # not acquire, as it was summarized without following the alias. + stale.mkdir() + for path in cold.glob("*.json"): + summary = json.loads(path.read_text()) + summary["schema"] = "resource-summary-cache-v4" + for fn in summary["functions"]: + if fn["name"] == "wrapper": + fn["effects"] = [] + fn["ownership"]["normal"]["returns"] = "unknown" + fn["ownership"]["normal"]["returnsKind"] = "" + (stale / path.name).write_text(json.dumps(summary)) + second = run(stale) + current = next(iter(expected.values()))["schema"] + written = { + path.name: summary + for path in stale.glob("*.json") + if (summary := json.loads(path.read_text()))["schema"] == current + } + if second.returncode != 0 or second.stdout != first.stdout: + ok = False + print(f" ❌ {target}: the output with the previous cache differs from the one with an empty cache") + elif any(summary != expected.get(name) for name, summary in written.items()) or not any( + acquires(s) for s in written.values() + ): + ok = False + print(f" ❌ {target}: the summaries written with the previous cache are not those of an empty one: {written}") + else: + print(f" ✅ {target}: the previous cache is rebuilt to the summaries of an empty one") + print() + return ok + + def check_const_param_abi_split_struct() -> bool: """ ConstParameterNotModified names the same source parameters whatever the ABI does with a @@ -4838,6 +5073,8 @@ def record_ok(ok: bool): check_diagnostic_cwe_coverage, check_diagnostic_paths_follow_the_input, check_const_param_abi_split_struct, + check_calls_through_aliases, + check_resource_cache_rebuilds_alias_summaries, check_uninitialized_receiver_abi_split_struct, check_escape_through_returned_value, check_sarif_rule_cwe_tags, diff --git a/src/analysis/CommandInjectionAnalysis.cpp b/src/analysis/CommandInjectionAnalysis.cpp index a1992c11..4dc6a535 100644 --- a/src/analysis/CommandInjectionAnalysis.cpp +++ b/src/analysis/CommandInjectionAnalysis.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/CommandInjectionAnalysis.hpp" +#include "analysis/IRValueUtils.hpp" #include "analysis/AnalyzerUtils.hpp" @@ -16,15 +17,6 @@ namespace ctrace::stack::analysis { namespace { - static const llvm::Function* getDirectCallee(const llvm::CallBase& call) - { - if (const llvm::Function* direct = call.getCalledFunction()) - return direct; - const llvm::Value* called = call.getCalledOperand(); - if (!called) - return nullptr; - return llvm::dyn_cast(called->stripPointerCasts()); - } static llvm::StringRef canonicalCalleeName(llvm::StringRef name) { @@ -106,7 +98,7 @@ namespace ctrace::stack::analysis if (!call) continue; - const llvm::Function* callee = getDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); if (!callee) continue; @@ -142,7 +134,7 @@ namespace ctrace::stack::analysis { for (const CallBaseT* call : callBases) { - const llvm::Function* callee = getDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); if (!callee) continue; diff --git a/src/analysis/ConstParamAnalysis.cpp b/src/analysis/ConstParamAnalysis.cpp index 9771aceb..f55f9eb5 100644 --- a/src/analysis/ConstParamAnalysis.cpp +++ b/src/analysis/ConstParamAnalysis.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/ConstParamAnalysis.hpp" +#include "analysis/IRValueUtils.hpp" #include "analysis/AnalyzerUtils.hpp" #include @@ -328,14 +329,7 @@ namespace ctrace::stack::analysis { using namespace llvm; - const Function* callee = CB.getCalledFunction(); - if (!callee) - { - const Value* called = CB.getCalledOperand(); - if (called) - called = called->stripPointerCasts(); - callee = dyn_cast(called); - } + const Function* callee = directCallee(CB); if (!callee) return ParamWriteState::Unknown; @@ -578,7 +572,7 @@ namespace ctrace::stack::analysis continue; if (CB->isCallee(&use)) return true; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = directCallee(*CB); if (!callee) continue; if (const llvm::DISubprogram* SP = callee->getSubprogram()) diff --git a/src/analysis/DuplicateIfCondition.cpp b/src/analysis/DuplicateIfCondition.cpp index 2162a5ae..0ccd5574 100644 --- a/src/analysis/DuplicateIfCondition.cpp +++ b/src/analysis/DuplicateIfCondition.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/DuplicateIfCondition.hpp" +#include "analysis/IRValueUtils.hpp" #include #include @@ -338,11 +339,10 @@ namespace ctrace::stack::analysis // counts when the other modules make it deterministic. static const llvm::Function* calledFunction(const llvm::CallBase& call) { - if (const llvm::Function* callee = call.getCalledFunction()) + const llvm::Function* callee = directCallee(call); + if (!callee || callee->getFunctionType() == call.getFunctionType()) return callee; - const auto* declared = - llvm::dyn_cast(call.getCalledOperand()->stripPointerCasts()); - return declared && isDeterministicElsewhere(*declared) ? declared : nullptr; + return isDeterministicElsewhere(*callee) ? callee : nullptr; } static llvm::Value* stripCasts(llvm::Value* v) diff --git a/src/analysis/IRValueUtils.cpp b/src/analysis/IRValueUtils.cpp index 293903f9..c2d76f27 100644 --- a/src/analysis/IRValueUtils.cpp +++ b/src/analysis/IRValueUtils.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,19 @@ namespace ctrace::stack::analysis return name.split('$').first; } + llvm::Function* directCallee(const llvm::CallBase& call) + { + llvm::Value* callee = call.getCalledOperand()->stripPointerCasts(); + while (auto* alias = llvm::dyn_cast(callee)) + { + // The link may replace an interposable alias, a weak one, by another definition. + if (alias->isInterposable()) + return nullptr; + callee = alias->getAliasee()->stripPointerCasts(); + } + return llvm::dyn_cast(callee); + } + const llvm::StoreInst* findUniqueStoreToSlot(const llvm::AllocaInst& slot) { const llvm::StoreInst* uniqueStore = nullptr; diff --git a/src/analysis/IntegerOverflowAnalysis.cpp b/src/analysis/IntegerOverflowAnalysis.cpp index fcfc6d30..f3bcf9fb 100644 --- a/src/analysis/IntegerOverflowAnalysis.cpp +++ b/src/analysis/IntegerOverflowAnalysis.cpp @@ -113,16 +113,6 @@ namespace ctrace::stack::analysis } }; - static const llvm::Function* getDirectCallee(const llvm::CallBase& call) - { - if (const llvm::Function* direct = call.getCalledFunction()) - return direct; - const llvm::Value* called = call.getCalledOperand(); - if (!called) - return nullptr; - return llvm::dyn_cast(called->stripPointerCasts()); - } - static llvm::StringRef canonicalCalleeName(llvm::StringRef name) { if (!name.empty() && name.front() == '\1') @@ -855,7 +845,7 @@ namespace ctrace::stack::analysis std::optional lookup(const llvm::CallBase& call) { - const llvm::Function* callee = getDirectCallee(call); + const llvm::Function* callee = directCallee(call); if (!callee) return std::nullopt; if (auto sink = resolveLengthOnlySink(canonicalCalleeName(callee->getName()))) @@ -886,7 +876,7 @@ namespace ctrace::stack::analysis std::optional sink = resolveIntrinsicSizeSink(*call); if (!sink) { - if (const llvm::Function* callee = getDirectCallee(*call)) + if (const llvm::Function* callee = directCallee(*call)) sink = resolveSizeSink(canonicalCalleeName(callee->getName())); } if (!sink) @@ -984,7 +974,7 @@ namespace ctrace::stack::analysis llvm::StringRef sinkName; if (!sink) { - const llvm::Function* callee = getDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); if (!callee) continue; sinkName = canonicalCalleeName(callee->getName()); diff --git a/src/analysis/MemIntrinsicOverflow.cpp b/src/analysis/MemIntrinsicOverflow.cpp index 96d51723..d76474f1 100644 --- a/src/analysis/MemIntrinsicOverflow.cpp +++ b/src/analysis/MemIntrinsicOverflow.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/MemIntrinsicOverflow.hpp" +#include "analysis/IRValueUtils.hpp" #include "analysis/BufferWriteModel.hpp" #include @@ -51,19 +52,6 @@ namespace ctrace::stack::analysis return std::nullopt; } - static const llvm::Function* resolveDirectCallee(const llvm::CallBase* CB) - { - using namespace llvm; - if (!CB) - return nullptr; - if (const Function* direct = CB->getCalledFunction()) - return direct; - const Value* callee = CB->getCalledOperand(); - if (!callee) - return nullptr; - return dyn_cast(callee->stripPointerCasts()); - } - static ResolvedSink resolveBuiltInSink(llvm::CallBase* CB) { using namespace llvm; @@ -101,7 +89,7 @@ namespace ctrace::stack::analysis } } - const Function* callee = resolveDirectCallee(CB); + const Function* callee = directCallee(*CB); if (!callee) return sink; @@ -145,7 +133,7 @@ namespace ctrace::stack::analysis if (!CB || !model || !matcher) return sink; - const llvm::Function* callee = resolveDirectCallee(CB); + const llvm::Function* callee = directCallee(*CB); if (!callee) return sink; diff --git a/src/analysis/NullDerefAnalysis.cpp b/src/analysis/NullDerefAnalysis.cpp index 16c27530..5625006a 100644 --- a/src/analysis/NullDerefAnalysis.cpp +++ b/src/analysis/NullDerefAnalysis.cpp @@ -105,7 +105,7 @@ namespace ctrace::stack::analysis if (!call) return false; - const llvm::Function* callee = call->getCalledFunction(); + const llvm::Function* callee = directCallee(*call); if (!callee || !callee->isDeclaration()) return false; diff --git a/src/analysis/OOBReadAnalysis.cpp b/src/analysis/OOBReadAnalysis.cpp index 56e37c5a..760a219e 100644 --- a/src/analysis/OOBReadAnalysis.cpp +++ b/src/analysis/OOBReadAnalysis.cpp @@ -54,16 +54,6 @@ namespace ctrace::stack::analysis std::string displayName; }; - static const llvm::Function* getDirectCallee(const llvm::CallBase& call) - { - if (const llvm::Function* direct = call.getCalledFunction()) - return direct; - const llvm::Value* called = call.getCalledOperand(); - if (!called) - return nullptr; - return llvm::dyn_cast(called->stripPointerCasts()); - } - static llvm::StringRef canonicalCalleeName(llvm::StringRef name) { if (!name.empty() && name.front() == '\1') @@ -301,7 +291,7 @@ namespace ctrace::stack::analysis { if (const auto* call = llvm::dyn_cast(&inst)) { - const llvm::Function* callee = getDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); llvm::StringRef calleeName; if (const auto* intrinsic = llvm::dyn_cast(call)) { diff --git a/src/analysis/ResourceLifetimeAnalysis.cpp b/src/analysis/ResourceLifetimeAnalysis.cpp index 33efa747..0734d893 100644 --- a/src/analysis/ResourceLifetimeAnalysis.cpp +++ b/src/analysis/ResourceLifetimeAnalysis.cpp @@ -551,8 +551,6 @@ namespace ctrace::stack::analysis return resolvePointerStorageWithExtraOffset(ptr, 0, F, DL, methodInfo); } - const llvm::Function* resolveDirectCallee(const llvm::CallBase& CB); - static void collectThisFieldOriginsFromValue( const llvm::Value* value, const llvm::Function& F, const llvm::DataLayout& DL, const MethodClassInfo& methodInfo, unsigned depth, @@ -668,7 +666,7 @@ namespace ctrace::stack::analysis if (const auto* CB = llvm::dyn_cast(stripped)) { - const llvm::Function* callee = resolveDirectCallee(*CB); + const llvm::Function* callee = directCallee(*CB); if (!callee || callee->isDeclaration()) { sawUnknownOrigin = true; @@ -861,7 +859,6 @@ namespace ctrace::stack::analysis return false; } - const llvm::Function* resolveDirectCallee(const llvm::CallBase& CB); static const llvm::Instruction* firstInstructionAnchor(const llvm::Function& F); static bool summaryStorageScopeAllowed(const StorageKey& storage) @@ -1126,7 +1123,7 @@ namespace ctrace::stack::analysis if (const auto* CB = llvm::dyn_cast(user)) { - const llvm::Function* callee = resolveDirectCallee(*CB); + const llvm::Function* callee = directCallee(*CB); if (!callee) return false; @@ -1334,7 +1331,7 @@ namespace ctrace::stack::analysis if (llvm::isa(CB)) continue; - const llvm::Function* callee = resolveDirectCallee(*CB); + const llvm::Function* callee = directCallee(*CB); bool modeledCall = false; if (callee) { @@ -1681,7 +1678,7 @@ namespace ctrace::stack::analysis if (!CB) continue; - const llvm::Function* callee = resolveDirectCallee(*CB); + const llvm::Function* callee = directCallee(*CB); if (!callee) continue; @@ -1897,17 +1894,6 @@ namespace ctrace::stack::analysis return out; } - const llvm::Function* resolveDirectCallee(const llvm::CallBase& CB) - { - if (const llvm::Function* callee = CB.getCalledFunction()) - return callee; - const llvm::Value* called = CB.getCalledOperand(); - if (!called) - return nullptr; - called = called->stripPointerCasts(); - return llvm::dyn_cast(called); - } - static const llvm::Instruction* firstInstructionAnchor(const llvm::Function& F) { const llvm::Instruction* first = nullptr; @@ -2940,7 +2926,7 @@ namespace ctrace::stack::analysis if (!CB) continue; - const llvm::Function* callee = resolveDirectCallee(*CB); + const llvm::Function* callee = directCallee(*CB); if (!callee) continue; diff --git a/src/analysis/ResourceLifetimeInternal.hpp b/src/analysis/ResourceLifetimeInternal.hpp index 8fd5eeb4..2515179c 100644 --- a/src/analysis/ResourceLifetimeInternal.hpp +++ b/src/analysis/ResourceLifetimeInternal.hpp @@ -57,7 +57,6 @@ namespace ctrace::stack::analysis::lifetime_detail const llvm::DataLayout& DL, const MethodClassInfo& methodInfo); StorageKey resolveHandleStorage(const llvm::Value* handleValue, const llvm::Function& F, const llvm::DataLayout& DL, const MethodClassInfo& methodInfo); - const llvm::Function* resolveDirectCallee(const llvm::CallBase& CB); /// The caller-side storage an out-parameter path of `CB` designates: /// `*(arg + offset)`, or `**(arg + offset)` when viaPointerSlot. StorageKey resolveArgPathStorage(const llvm::CallBase& CB, unsigned argIndex, diff --git a/src/analysis/SizeMinusKWrites.cpp b/src/analysis/SizeMinusKWrites.cpp index 94d68d48..b5883c09 100644 --- a/src/analysis/SizeMinusKWrites.cpp +++ b/src/analysis/SizeMinusKWrites.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/SizeMinusKWrites.hpp" +#include "analysis/IRValueUtils.hpp" #include "analysis/AnalyzerUtils.hpp" #include "analysis/IntRanges.hpp" @@ -272,45 +273,40 @@ namespace ctrace::stack::analysis bool matched = false; StringRef name; - Value* callee = CB->getCalledOperand(); - if (callee) + if (Function* fn = directCallee(*CB)) { - callee = callee->stripPointerCasts(); - if (auto* fn = dyn_cast(callee)) + LibFunc lf; + if (TLI.getLibFunc(*fn, lf)) { - LibFunc lf; - if (TLI.getLibFunc(*fn, lf)) + switch (lf) { - switch (lf) - { - case LibFunc_memcpy: - case LibFunc_memmove: - case LibFunc_memset: - case LibFunc_strncpy: - case LibFunc_strncat: - case LibFunc_stpncpy: - dst = 0; - len = 2; - name = fn->getName(); - matched = true; - break; - default: - break; - } + case LibFunc_memcpy: + case LibFunc_memmove: + case LibFunc_memset: + case LibFunc_strncpy: + case LibFunc_strncat: + case LibFunc_stpncpy: + dst = 0; + len = 2; + name = fn->getName(); + matched = true; + break; + default: + break; } + } - if (!matched) + if (!matched) + { + StringRef fnName = fn->getName(); + if (fnName.contains("memcpy") || fnName.contains("memmove") || + fnName.contains("memset") || fnName.contains("strncpy") || + fnName.contains("strncat") || fnName.contains("stpncpy")) { - StringRef fnName = fn->getName(); - if (fnName.contains("memcpy") || fnName.contains("memmove") || - fnName.contains("memset") || fnName.contains("strncpy") || - fnName.contains("strncat") || fnName.contains("stpncpy")) - { - dst = 0; - len = 2; - name = fnName; - matched = true; - } + dst = 0; + len = 2; + name = fnName; + matched = true; } } } @@ -354,15 +350,13 @@ namespace ctrace::stack::analysis const SizeMinusKSummaryMap& summaries, const SizeMinusKWrapperIndex* elsewhere) { - if (const llvm::Function* callee = CB.getCalledFunction(); - callee && !callee->isDeclaration()) + const llvm::Function* declared = directCallee(CB); + if (declared && !declared->isDeclaration()) { - const auto it = summaries.find(callee); + const auto it = summaries.find(declared); return it == summaries.end() ? std::vector{} : it->second; } - const auto* declared = - llvm::dyn_cast(CB.getCalledOperand()->stripPointerCasts()); - if (!elsewhere || !declared || !declared->isDeclaration()) + if (!elsewhere || !declared) return {}; const auto it = elsewhere->functions.find(linkerSymbolName(*declared)); if (it == elsewhere->functions.end() || it->second.params != CB.arg_size()) diff --git a/src/analysis/StackComputation.cpp b/src/analysis/StackComputation.cpp index 45b0fff1..62972eac 100644 --- a/src/analysis/StackComputation.cpp +++ b/src/analysis/StackComputation.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "analysis/StackComputation.hpp" +#include "analysis/IRValueUtils.hpp" #include #include @@ -46,7 +47,7 @@ namespace ctrace::stack::analysis const auto& CB = llvm::cast(I); if (resolver) return resolver->resolve(CB); - const llvm::Function* callee = CB.getCalledFunction(); + const llvm::Function* callee = directCallee(CB); return callee && !callee->isDeclaration() ? callee : nullptr; } @@ -75,7 +76,7 @@ namespace ctrace::stack::analysis { if (CB.isInlineAsm()) return false; - const llvm::Function* callee = CB.getCalledFunction(); + const llvm::Function* callee = directCallee(CB); if (callee && callee->isIntrinsic()) return false; if (resolver && resolver->resolve(CB)) @@ -925,15 +926,8 @@ namespace ctrace::stack::analysis for (llvm::Instruction& I : BB) { const llvm::Function* Callee = nullptr; - - if (auto* CI = llvm::dyn_cast(&I)) - { - Callee = CI->getCalledFunction(); - } - else if (auto* II = llvm::dyn_cast(&I)) - { - Callee = II->getCalledFunction(); - } + if (llvm::isa(I) || llvm::isa(I)) + Callee = directCallee(llvm::cast(I)); if (Callee && !Callee->isDeclaration()) { @@ -950,28 +944,37 @@ namespace ctrace::stack::analysis { // Only the symbols that a single definition of the run defines, exactly, are resolved. std::unordered_map definitionCount; + const auto define = [&](const llvm::GlobalValue& symbolValue, const llvm::Function& F) + { + const std::string symbol = linkerSymbolName(symbolValue); + if (++definitionCount[symbol] == 1 && symbolValue.hasExactDefinition()) + definitions_[symbol] = &F; + else + definitions_.erase(symbol); + }; for (const llvm::Module* M : modules) { for (const llvm::Function& F : *M) { - if (F.isDeclaration() || F.hasLocalLinkage()) - continue; - const std::string symbol = linkerSymbolName(F); - if (++definitionCount[symbol] == 1 && F.hasExactDefinition()) - definitions_[symbol] = &F; - else - definitions_.erase(symbol); + if (!F.isDeclaration() && !F.hasLocalLinkage()) + define(F, F); + } + // A symbol defined as an alias of a function, such as a complete-object constructor + // on ELF targets, defines that function for the other modules. + for (const llvm::GlobalAlias& alias : M->aliases()) + { + const auto* F = llvm::dyn_cast(alias.getAliaseeObject()); + if (F && !F->isDeclaration() && !alias.hasLocalLinkage()) + define(alias, *F); } } } const llvm::Function* CallResolver::resolve(const llvm::CallBase& call) const { - if (const llvm::Function* callee = call.getCalledFunction(); - callee && !callee->isDeclaration()) - return callee; - const auto* declared = - llvm::dyn_cast(call.getCalledOperand()->stripPointerCasts()); + const llvm::Function* declared = directCallee(call); + if (declared && !declared->isDeclaration()) + return declared; if (!declared || !declared->isDeclaration() || declared->isIntrinsic()) return nullptr; const auto it = definitions_.find(linkerSymbolName(*declared)); diff --git a/src/analysis/StackPointerEscape.cpp b/src/analysis/StackPointerEscape.cpp index 9158d405..c56f5c33 100644 --- a/src/analysis/StackPointerEscape.cpp +++ b/src/analysis/StackPointerEscape.cpp @@ -270,11 +270,7 @@ namespace ctrace::stack::analysis if (const auto* CB = llvm::dyn_cast(stripped)) { - const llvm::Value* calledVal = CB->getCalledOperand(); - const llvm::Value* calledStripped = - calledVal ? calledVal->stripPointerCasts() : nullptr; - const llvm::Function* directCallee = - calledStripped ? llvm::dyn_cast(calledStripped) : nullptr; + const llvm::Function* directCallee = analysis::directCallee(*CB); std::optional returnedArg = getReturnedArgIndexFromCall(*CB, directCallee, returnedArgAliases); @@ -534,11 +530,7 @@ namespace ctrace::stack::analysis if (CB->getArgOperand(argIndex) != V) continue; - const Value* calledVal = CB->getCalledOperand(); - const Value* calledStripped = - calledVal ? calledVal->stripPointerCasts() : nullptr; - const Function* directCallee = - calledStripped ? dyn_cast(calledStripped) : nullptr; + const Function* directCallee = analysis::directCallee(*CB); if (callParamHasNonCaptureLikeAttr(*CB, argIndex)) continue; @@ -1097,12 +1089,7 @@ namespace ctrace::stack::analysis continue; } - const Value* calledVal = CB->getCalledOperand(); - const Value* calledStripped = - calledVal ? calledVal->stripPointerCasts() : nullptr; - if (const auto* directCallee = - calledStripped ? dyn_cast(calledStripped) - : nullptr) + if (const auto* directCallee = analysis::directCallee(*CB)) { applyCallee(*directCallee, *CB, argIndex); continue; @@ -1336,12 +1323,7 @@ namespace ctrace::stack::analysis if (CB->getArgOperand(argIndex) != V) continue; - const Value* calledVal = CB->getCalledOperand(); - const Value* calledStripped = - calledVal ? calledVal->stripPointerCasts() : nullptr; - const Function* directCallee = - calledStripped ? dyn_cast(calledStripped) - : nullptr; + const Function* directCallee = analysis::directCallee(*CB); if (callParamHasNonCaptureLikeAttr(*CB, argIndex)) { continue; diff --git a/src/analysis/TOCTOUAnalysis.cpp b/src/analysis/TOCTOUAnalysis.cpp index ff42e19a..226b4f9f 100644 --- a/src/analysis/TOCTOUAnalysis.cpp +++ b/src/analysis/TOCTOUAnalysis.cpp @@ -31,16 +31,6 @@ namespace ctrace::stack::analysis unsigned reserved = 0; }; - static const llvm::Function* getDirectCallee(const llvm::CallBase& call) - { - if (const llvm::Function* direct = call.getCalledFunction()) - return direct; - const llvm::Value* called = call.getCalledOperand(); - if (!called) - return nullptr; - return llvm::dyn_cast(called->stripPointerCasts()); - } - static llvm::StringRef canonicalCalleeName(llvm::StringRef name) { if (!name.empty() && name.front() == '\1') @@ -146,7 +136,7 @@ namespace ctrace::stack::analysis if (!call) continue; - const llvm::Function* callee = getDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); if (!callee) continue; @@ -212,7 +202,7 @@ namespace ctrace::stack::analysis { ++order; - const llvm::Function* callee = getDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); if (!callee) continue; diff --git a/src/analysis/UninitializedVarAnalysis.cpp b/src/analysis/UninitializedVarAnalysis.cpp index 49d08239..f7344c77 100644 --- a/src/analysis/UninitializedVarAnalysis.cpp +++ b/src/analysis/UninitializedVarAnalysis.cpp @@ -3140,7 +3140,7 @@ namespace ctrace::stack::analysis if (!CB) return; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = directCallee(*CB); const FunctionSummary* calleeSummary = nullptr; bool imported = false; if (callee) @@ -3256,7 +3256,7 @@ namespace ctrace::stack::analysis const auto* CB = llvm::dyn_cast(&I); if (!CB) continue; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = directCallee(*CB); if (!callee) continue; if (summaries.find(callee) != summaries.end()) @@ -3496,7 +3496,7 @@ namespace ctrace::stack::analysis const auto* CB = llvm::dyn_cast(&I); if (!CB) continue; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = directCallee(*CB); if (callee && summaries.find(callee) != summaries.end()) callerOf[callee].push_back(func); } @@ -3575,7 +3575,7 @@ namespace ctrace::stack::analysis const auto* CB = llvm::dyn_cast(&I); if (!CB) continue; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = directCallee(*CB); if (!callee || callee->isDeclaration()) continue; if (!shouldIncludeInSummaryScope(*callee, shouldAnalyze)) @@ -3606,7 +3606,7 @@ namespace ctrace::stack::analysis const auto* CB = llvm::dyn_cast(&I); if (!CB) continue; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = directCallee(*CB); if (!callee) continue; if (!callee->hasName() || callee->getName().empty()) diff --git a/src/analysis/ownership/ResourceFactCollector.cpp b/src/analysis/ownership/ResourceFactCollector.cpp index 6c023638..6e5ec8a5 100644 --- a/src/analysis/ownership/ResourceFactCollector.cpp +++ b/src/analysis/ownership/ResourceFactCollector.cpp @@ -313,7 +313,7 @@ namespace ctrace::stack::analysis::ownership const auto* call = llvm::dyn_cast(user); if (!call) return false; - const llvm::Function* callee = lifetime_detail::resolveDirectCallee(*call); + const llvm::Function* callee = directCallee(*call); if (!callee) return false; bool modelledOut = false; @@ -734,7 +734,7 @@ namespace ctrace::stack::analysis::ownership { if (llvm::isa(&call) || call.isInlineAsm()) return; - const llvm::Function* callee = lifetime_detail::resolveDirectCallee(call); + const llvm::Function* callee = directCallee(call); // A plain call that may throw is an exceptional exit taken before any of // its effects; the absence of an invoke proves nothing. Whether a call that diff --git a/src/analyzer/ModulePreparationService.cpp b/src/analyzer/ModulePreparationService.cpp index de121c81..566166a3 100644 --- a/src/analyzer/ModulePreparationService.cpp +++ b/src/analyzer/ModulePreparationService.cpp @@ -3,6 +3,7 @@ #include "analyzer/HotspotProfiler.hpp" #include "analysis/FunctionFilter.hpp" +#include "analysis/IRValueUtils.hpp" #include #include @@ -138,10 +139,8 @@ namespace ctrace::stack::analyzer for (llvm::Instruction& I : BB) { const llvm::Function* callee = nullptr; - if (auto* CI = llvm::dyn_cast(&I)) - callee = CI->getCalledFunction(); - else if (auto* II = llvm::dyn_cast(&I)) - callee = II->getCalledFunction(); + if (llvm::isa(I) || llvm::isa(I)) + callee = analysis::directCallee(llvm::cast(I)); if (callee && !callee->isDeclaration() && ctx.isDefined(*callee)) callees.push_back(callee); diff --git a/src/app/AnalyzerApp.cpp b/src/app/AnalyzerApp.cpp index c081863e..7a037ede 100644 --- a/src/app/AnalyzerApp.cpp +++ b/src/app/AnalyzerApp.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "app/AnalyzerApp.hpp" +#include "analysis/IRValueUtils.hpp" #include "CrossTUSummaryDriver.hpp" #include "StackUsageAnalyzer.hpp" @@ -1802,7 +1803,7 @@ static bool writeSummaryCacheFile(const std::filesystem::path& cacheFile, } llvm::json::Object root; - root["schema"] = "resource-summary-cache-v4"; + root["schema"] = "resource-summary-cache-v5"; root["functions"] = std::move(functionArray); std::ofstream out(cacheFile, std::ios::out | std::ios::trunc | std::ios::binary); @@ -1833,7 +1834,7 @@ readSummaryCacheFile(const std::filesystem::path& cacheFile) if (!obj) return std::nullopt; auto schema = obj->getString("schema"); - if (!schema || *schema != "resource-summary-cache-v4") + if (!schema || *schema != "resource-summary-cache-v5") return std::nullopt; const auto* functions = obj->getArray("functions"); @@ -1950,7 +1951,7 @@ struct ResourceSummaryOperations std::string cacheKey(std::size_t module, const std::string& externalHash) const { // Keep the schema and key components identical: this refactor changes no summaries. - return md5Hex("cross-tu-resource-summary-v4|" + modelHash + "|" + externalHash + "|" + + return md5Hex("cross-tu-resource-summary-v5|" + modelHash + "|" + externalHash + "|" + filterHash + "|" + compileArgsHashes[module] + "|" + irHashes[module]); } bool usesDiskCache() const @@ -2159,7 +2160,7 @@ buildCrossTUSummaryIndex(const std::vector& loadedModules, const auto* CB = llvm::dyn_cast(&I); if (!CB) continue; - const llvm::Function* callee = CB->getCalledFunction(); + const llvm::Function* callee = analysis::directCallee(*CB); if (!callee || !callee->hasName() || callee->getName().empty()) continue; const std::string canon = diff --git a/src/passes/ModulePasses.cpp b/src/passes/ModulePasses.cpp index 2972873a..6530cdc9 100644 --- a/src/passes/ModulePasses.cpp +++ b/src/passes/ModulePasses.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "passes/ModulePasses.hpp" +#include "analysis/IRValueUtils.hpp" #include #include @@ -7,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -39,7 +41,7 @@ namespace ctrace::stack bool neverReturnsFrom(const llvm::CallBase& call, const FunctionSet& neverReturn) { - const llvm::Function* callee = call.getCalledFunction(); + const llvm::Function* callee = analysis::directCallee(call); return call.doesNotReturn() || (callee && neverReturn.contains(callee)); } @@ -99,13 +101,16 @@ namespace ctrace::stack } llvm::DenseMap> callers; - for (llvm::Function* F : worklist) + // A call through an alias is a user of the alias, not of the function: every call + // of the module is looked at. + for (llvm::Function& caller : mod) { - for (llvm::User* user : F->users()) + for (llvm::Instruction& I : llvm::instructions(caller)) { - auto* call = llvm::dyn_cast(user); - if (call && call->getCalledFunction() == F) - callers[F].push_back(call->getFunction()); + const auto* call = llvm::dyn_cast(&I); + const llvm::Function* callee = call ? analysis::directCallee(*call) : nullptr; + if (callee && neverReturn.contains(callee)) + callers[callee].push_back(&caller); } } diff --git a/test/escape-stack/ctor-alias-no-escape.cpp b/test/escape-stack/ctor-alias-no-escape.cpp new file mode 100644 index 00000000..e8ddf1b0 --- /dev/null +++ b/test/escape-stack/ctor-alias-no-escape.cpp @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// The object built by an out-of-class constructor of the same file only reaches that constructor +// and a reader (#179). On ELF targets, clang calls the constructor through an alias (C1 -> C2): +// the call is direct, and the address does not escape, on every target. + +struct Resolver +{ + explicit Resolver(int value); + int seed; +}; + +Resolver::Resolver(int value) : seed(value) {} + +static int lookup(const Resolver* resolver) +{ + return resolver->seed; +} + +int run(int value) +{ + const Resolver resolver(value); + return lookup(&resolver); +} diff --git a/test/escape-stack/ctor-alias-passes-this.cpp b/test/escape-stack/ctor-alias-passes-this.cpp new file mode 100644 index 00000000..c364c142 --- /dev/null +++ b/test/escape-stack/ctor-alias-passes-this.cpp @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A constructor that passes this to an unknown callback (#179). The callback may keep it: it must +// be reported on every target, also on ELF ones, where the call goes through an alias (C1 -> C2). + +struct Resolver; +using Hook = void (*)(Resolver*); + +struct Resolver +{ + Resolver(int value, Hook hook); + int seed; +}; + +Resolver::Resolver(int value, Hook hook) : seed(value) +{ + hook(this); +} + +int run(int value, Hook hook) +{ + const Resolver resolver(value, hook); + return resolver.seed; +} + +// at line 22, column 20 +// [ !!Warn ] stack pointer escape: address of variable 'resolver' escapes this function +// ↳ address passed as argument to function 'Resolver::Resolver(int, void (*)(Resolver*))' (callee may capture the pointer beyond this function) + +// strict-expectation-details: true diff --git a/test/escape-stack/ctor-alias-stores-this.cpp b/test/escape-stack/ctor-alias-stores-this.cpp new file mode 100644 index 00000000..77aa8186 --- /dev/null +++ b/test/escape-stack/ctor-alias-stores-this.cpp @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A constructor that stores this in a global (#179). The address escapes: the constructor is +// defined in the same file, and keeps it. It must be reported on every target, also on ELF ones, +// where the call goes through an alias (C1 -> C2). + +struct Resolver; +Resolver* lastResolver; + +struct Resolver +{ + explicit Resolver(int value); + int seed; +}; + +Resolver::Resolver(int value) : seed(value) +{ + lastResolver = this; +} + +static int lookup(const Resolver* resolver) +{ + return resolver->seed; +} + +int run(int value) +{ + const Resolver resolver(value); + return lookup(&resolver); +} + +// at line 28, column 20 +// [ !!Warn ] stack pointer escape: address of variable 'resolver' escapes this function +// ↳ address passed as argument to function 'Resolver::Resolver(int)' (callee may capture the pointer beyond this function) + +// strict-expectation-details: true diff --git a/test/false-positive-repro/ctor-dtor-alias.cpp b/test/false-positive-repro/ctor-dtor-alias.cpp new file mode 100644 index 00000000..7a531287 --- /dev/null +++ b/test/false-positive-repro/ctor-dtor-alias.cpp @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A constructor and a destructor defined out of their class (#179). On ELF targets, clang calls +// them through aliases (C1 -> C2, D1 -> D2). The calls are direct, and every target must agree: +// the max stack of frame() and use() is known, use() reads v uninitialized, frame() reads a +// member that the constructor sets, and box does not escape. + +struct Box +{ + explicit Box(const int* out); + ~Box(); + int value; +}; + +Box::Box(const int* out) : value(0) +{ + (void)out; +} + +Box::~Box() {} + +int use() +{ + int v; + Box box(&v); + return v + box.value; +} + +int frame() +{ + Box box(nullptr); + return box.value; +} + +// at line 26, column 12 +// [ !!Warn ] potential read of uninitialized local variable 'v' +// ↳ this load may execute before any definite initialization on all control-flow paths