Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
john-h-kastner opened this issue Feb 18, 2021 · 15 comments
Labels

Comments

@john-h-kastner
Copy link
Collaborator

john-h-kastner commented Feb 18, 2021

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
Copy link
Member

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

@john-h-kastner
Copy link
Collaborator Author

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

@mwhicks1
Copy link
Member

So weird! What codebase actually does this?

@john-h-kastner
Copy link
Collaborator Author

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
Copy link
Member

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
Copy link
Collaborator Author

john-h-kastner commented Feb 19, 2021

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
Copy link
Member

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

@john-h-kastner
Copy link
Collaborator Author

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
Copy link
Collaborator Author

john-h-kastner commented Mar 9, 2021

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
Copy link
Member

mwhicks1 commented Mar 9, 2021

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
Copy link
Collaborator Author

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
Copy link
Member

mwhicks1 commented Mar 9, 2021

Yes, sounds good to me.

@kyleheadley
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants