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.
15 stars 5 forks source link

Constraint aren't generated correctly for functions declared by typedef #437

Open john-h-kastner opened 3 years ago

john-h-kastner commented 3 years ago

Functions bar and baz are declared using the same typedef, so their types should be equated and the their pre-declarations should continue to use the typedef.

typedef void foo(int*);

foo bar;
void bar(int *a){};

foo baz;
void baz(int *a){
  a =1;
};

PR #436 address a rewriting error in this example (issue #430), but even after this fix, the typedef is ignored.

typedef void foo(int*);

void bar(_Ptr<int> a);
void bar(_Ptr<int> a){};

void baz(int *a : itype(_Ptr<int>));
void baz(int *a : itype(_Ptr<int>)){
  a =1;
};
mattmccutchen-cci commented 3 years ago

Might #408 help with this? (Just a quick reminder; you're probably more familiar with the details than I am.)

john-h-kastner commented 3 years ago

It's definitely relevant, but I don't believe it will fix the issue.

mwhicks1 commented 3 years ago

So weird! What codebase actually does this?

john-h-kastner commented 3 years ago

This is from libarchive.

typedef dev_t pack_t(int, unsigned long [], const char **);

pack_t  *pack_find(const char *);
pack_t   pack_native;

https://github.com/plum-umd/checkedc-eval-libarchive/blob/836fe7878a681184f38cb349530595e9b6d62bfb/libarchive/archive_pack_dev.h#L37-L40

mwhicks1 commented 3 years ago

But there's now way to define an actual function of this type. E.g., you can't do

pack_t pack_native {
  ... code here ...
}

because there's no way to name the parameters, is that right? So: Seems a bit useless to make a typedef since you have to hand-write the type anyway.

Also: What is pack_find doing? Is that returning a pointer to a pack_t i.e., a function pointer?

john-h-kastner commented 3 years ago

pack_native is declared without the typedef, so, yeah, kind of pointless from that perspective.

dev_t
pack_native(int n, unsigned long numbers[], const char **error)
{
// .......
}

pack_find and a few other functions return pointers to pack_t . I suppose that's the real purpose of the typedef.

mwhicks1 commented 3 years ago

OK that makes sense. Can we handle pack_find (now) but it's the other case that we can't?

john-h-kastner commented 3 years ago

I don't think it correct for either yet. We do get it correct if the pack_t typdef is a pointer.

typedef dev_t (*pack_t)(int, unsigned long [], const char **);

but if it's not a pointer like in libarchive, then the typedef is inlined for for pack_find.

john-h-kastner commented 3 years ago

Update on this issue following the merge of #408: Typedef declarations are now represented by a ConstraintVariable, so the foo typedef solves and is rewritten to a checked type.

typedef void foo(int*);
// converts to 
typedef void foo(_Ptr<int> );

Constraints are still not created between the typedef declaration and subsequent function declarations, so functions bar and baz solve exactly as before.

mwhicks1 commented 3 years ago

Constraints should be created, right? That is, when I have

foo baz;
void baz(int *a){
  a =1;
};

I should create a FVConstraint for baz, due to the definition and our new pre-pass that unifies the constraint variable for all declarations/definitions. Then the fact that baz is also given type foo, I should equate the foo constraint variable to baz's definition's FVConstraint, right?

john-h-kastner commented 3 years ago

Yes, equating foo and baz would ensure that functions declared using the typedef solve to the same type. If we do that the only change left would be to not rewrite the declaration foo baz so that it still uses the typedef instead of expanding it to void baz(int *a : itype(_Ptr<int>));.

mwhicks1 commented 3 years ago

Yes, sounds good to me.

kyleheadley commented 3 years ago

In #505 I stop rewriting the declaration so that the typedef is used. The merger works now because the definition's FVC is used as the constraint target. The FVC for the typedef's declaration is constructed differently (probably a bug). I didn't add any constaints between typedef and declaration. So that's a good reason to keep this open if the current solution is not enough.

mattmccutchen-cci commented 3 years ago

I forgot we already had this open issue for function typedefs and it is not a new problem in #505. Then I'm not worried about what #505 does in this case. Thanks.

mattmccutchen-cci commented 3 years ago

A more extreme example to test any potential fix for this issue:

typedef int foo1_t(int);
typedef int foo2_t(int);
foo1_t foo_func;
foo2_t foo_func;
int foo_func(int y) { return 1; }