Missing root-cause warning for function parameter in macro

[mattmccutchen-cci]

Given root-cause-function-macro.c:

#define IDENTITY(x) x

IDENTITY(int *p;)
IDENTITY(void foo(int *q);)

3c -warn-all-root-cause root-cause-function-macro.c -- >/dev/null prints:

/home/matt/test/root-cause-function-macro.c:3:1: warning: Root cause for 1 unchecked pointer: Pointer in Macro declaration.
IDENTITY(int *p;)
^

We get a root-cause warning for the global variable but not for the function parameter. This happens even with the root-cause-warning fix that is currently included in #598 (but may be moved to a separate PR), so it is a separate problem.

Some debugging suggests that the function parameter has neither a PSL nor an APSL here:

checkedc-clang/clang/lib/3C/ProgramInfo.cpp

Lines 1048 to 1061 in e240faa

	auto &WildPtrsReason = CState.RootWildAtomsWithReason;
	for (auto *CurrC : CS.getConstraints()) {
	if (Geq *EC = dyn_cast<Geq>(CurrC)) {
	VarAtom *VLhs = dyn_cast<VarAtom>(EC->getLHS());
	if (EC->constraintIsChecked() && dyn_cast<WildAtom>(EC->getRHS())) {
	PersistentSourceLoc PSL = EC->getLocation();
	const PersistentSourceLoc *APSL = CState.AtomSourceMap[VLhs->getLoc()];
	if (!PSL.valid() && APSL && APSL->valid())
	PSL = *APSL;
	WildPointerInferenceInfo Info(EC->getReason(), PSL);
	WildPtrsReason.insert(std::make_pair(VLhs->getLoc(), Info));
	}
	}
	}

My understanding is that the PSL is specified when the constraint is added, but we don't specify one here:

checkedc-clang/clang/lib/3C/ProgramInfo.cpp

Line 683 in e240faa

constrainWildIfMacro(PVExternal, PVD->getLocation());

or here:

checkedc-clang/clang/lib/3C/ProgramInfo.cpp

Line 739 in e240faa

constrainWildIfMacro(NewCV, D->getLocation());

And the APSL is supposed to be added here:

checkedc-clang/clang/lib/3C/ProgramInfo.cpp

Lines 1040 to 1046 in e240faa

	for (const auto &I : Variables)
	insertIntoPtrSourceMap(&(I.first), I.second);
	for (const auto &I : ExprConstraintVars) {
	PersistentSourceLoc *PSL = &ExprLocations[I.first];
	for (auto *J : I.second.first)
	insertIntoPtrSourceMap(PSL, J);
	}

However, for a FunctionVariableConstraint, insertIntoPtrSourceMap processes only the return value under the assumption that the parameter PointerVariableConstraints will have their own entries in Variables. Normally they do, but when the entire function is inside a macro, all of those ConstraintVariables get the same PSL, and under the current implementation of ProgramInfo::addVariable, the FunctionVariableConstraint overwrites the parameter PointerVariableConstraints in the map: oops.

I drafted a code change to add the PSL to the constrainWildIfMacro calls (since I don't see an easy way to fix the APSL), and that seemed to be enough to fix the problem; I'll probably submit that change as a PR later. But it might be worth thinking if there is a way we can detect "missing root-cause warning" bugs earlier in the 3C development process (or verify that none remain other than this one) via review of existing code or internal warnings or assertions rather than waiting until we notice that a root-cause warning is missing, since the lack of the warning here confused me as I investigated #604.

[mattmccutchen-cci]

But it might be worth thinking if there is a way we can detect "missing root-cause warning" bugs earlier in the 3C development process (or verify that none remain other than this one) via review of existing code or internal warnings or assertions rather than waiting until we notice that a root-cause warning is missing, since the lack of the warning here confused me as I investigated #604.

Now that I've filed #612, let's move any discussion of general solutions there and leave this issue just for function parameters defined in macros.

[mattmccutchen-cci]

This problem no longer occurs, presumably because #708 added the PSL to the constrainWildIfMacro calls like I was originally planning to do here. Work on general solutions to ensure we don't miss root-cause warnings can continue in #679 or #612.