[cfe-commits] [PATCH] Fix for PR7218, and analyzer support for calloc()

Jordy Rose jediknil at belkadan.com
Sun May 30 15:13:35 PDT 2010


On Sun, 30 May 2010 14:51:50 -0700, Ted Kremenek <kremenek at apple.com>
wrote:
> On May 30, 2010, at 12:46 AM, Jordy Rose wrote:
> 
>> Binding a symbolic region whose type is a reference shows up when the
>> reference is an argument, like so:
>> 
>> char t3 (char& r) {
>>  r = 'c';
>>  if (r) return r;
>>  return '0';
>> }
>> 
>> The reason for the SymbolicRegion section in canHaveDirectBinding(),
>> though, was originally more about having a way to set default values by
>> taking advantages of a fact about SymbolicRegions (if you're accessing
>> them
>> directly, it's either *p or a reference, or an explicit call to
Bind()),
>> not enforcing a rule.
> 
> Hi Jordy,
> 
> A fundamental invariant in the memory region design is that symbolic
> regions don't have bindings since they represent typeless blobs of
memory. 
> All bindings need to be done via ElementRegions or FieldRegions layered
on
> top of them.  Changing this would subtly break many things.  The
> ElementRegion design allows us to express all bindings as being typed.
> 
> Ted

Right, that makes sense -- I meant "passing a SymbolicRegion to Bind()
when you really mean binding to element 0", which Bind() currently rewrites
using GetElementZeroRegion().

That's what I was trying to take advantage of, by defining "binding to an
untyped region" as "setting a byte-fill value" for malloc() and calloc().
For *p, I moved the GetElementZeroRegion() construction up into
GRExprEngine.cpp, and for references I just omitted it, since it gets
stripped off by BindingKey::Make anyway. Perhaps that is breaking an
abstraction barrier, though.

The more the thread goes, though, the more I dislike this overloading of
Bind() -- again, I realize I should have thought more about design
beforehand, not just "make it work". Do either of the other designs make
more sense to you?
1. New method on StoreManager for setting default values.
2. Adding a parameter to SetExtent() for setting default values.

Or is there a better idea I'm missing?
Jordy



More information about the cfe-commits mailing list