search

correctcomputation / checkedc-clang

This is the primary development repository for 3C, a tool for automatically converting legacy C code to the Checked C extension of C, which aims to enforce spatial memory safety. This repository is a fork of Checked C's.
14 stars 5 forks source link

Many root causes have no source location #679

Open mattmccutchen-cci opened 3 years ago

mattmccutchen-cci commented 3 years ago

I noticed that at the current stage of Zane's port of Icecast, many of the root-cause warnings in PerWildPtrStats.json (as nicely formatted by our internal root_cause tool) have no source location: 118 out of 1369 in total, including some of the highest-score root causes. Are there already known reasons why this can happen? Do we want to try to investigate and fix some of these cases (and file individual issues as appropriate)?

This issue overlaps with #612, but it may be a higher priority to add source locations for root causes for which 3C currently reports no source location at all than to add constraint source locations to root causes that are already covered by the AtomSourceMap fallback. See also https://github.com/correctcomputation/checkedc-clang/pull/666#discussion_r684493577 and #674 related to testing of root causes with no source location.

mattmccutchen-cci commented 2 years ago

One case that came up: the external PVConstraint of a function parameter doesn't make it into the AtomSourceMap because ProgramInfo::Variables can only hold one ConstraintVariable for a given PSL, and for whatever reason, we chose the internal PVConstraint rather than the external one: https://github.com/correctcomputation/checkedc-clang/blob/48ebd6cb9ff487d4004214491499b14cf623f173/clang/lib/3C/ProgramInfo.cpp#L663 There's a similar problem with function declarations in macros (#603). It should be straightforward to fix the root-cause locations in these "PSL collision" cases by constructing a map from ConstraintVariable to PSL in the first place so we can represent any number of ConstraintVariables with the same PSL. But then we still have the problem that RewriteConsumer::emitRootCauseDiagnostics deduplicates root-cause warnings seen in different translation units by PSL; it should probably deduplicate them by atom ID or something like that. This work might become obsolete if we end up doing #612, but it might make sense to go ahead and do it anyway.