Skip to content
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

[CN] specs on declarations have a different syntax from specs on definitions #371

Open
peterohanley opened this issue Jul 9, 2024 · 8 comments
Assignees
Labels
blocking cn language Related to design of the CN language

Comments

@peterohanley
Copy link

In particular accesses does not work but trusted also does not.

int x;

void f(void)
/*@ accesses x; @*/
{

}

void g(void);
/*@ spec g();
    accesses x;
@*/
% cn accesses_spec.c
accesses_spec.c:10:5: error: unexpected token after ';' and before 'accesses'
parsing "cn_fun_spec": seen "CN_SPEC LNAME VARIABLE LPAREN cn_args RPAREN SEMICOLON", expecting "CN_REQUIRES nonempty_list(condition) CN_ENSURES nonempty_list(condition)"
    accesses x;
    ^~~~~~~~ 

Looking at the grammar in the documentation, it looks like a declaration spec should refer to function_spec rather than hardcode only requires and ensures.

@septract
Copy link
Collaborator

septract commented Aug 6, 2024

I think the ideal way to resolve this would be to eliminate the distinction between spec and trusted - see #467

@cp526
Copy link
Collaborator

cp526 commented Aug 7, 2024

Agreed, we should fix the CN frontend so the syntax for spec includes all the features that specifications on defined functions support.

As commented on #467, trusted and spec have a different purpose, so we should keep both.

@peterohanley
Copy link
Author

The accesses part of this is now impeding TA2 work, I would appreciate a fix even for just this part.

@dc-mak dc-mak added language Related to design of the CN language blocking labels Nov 4, 2024
@septract
Copy link
Collaborator

Bumping this issue as it's now also blocking work on the TA2 Mission Key Management example.

Here's a distilled example of the problem (derived from policy_add in mission_key_management/policy.c, here). Suppose we have three files: libfile.c, libfile.h, clientfile.c.

libfile.c:

#include "libfile.h"

int foo(int i) 
/*@
accesses myval; 
requires myval == 1i32; 
         i == 1i32; 
ensures  return == 0i32; 
@*/
{
  if (i == myval) return 0; 
  else return 1; 
}

clientfile.c:

#include "libfile.h" 

int bar() 
/*@ 
accesses myval; 
@*/
{ 
  myval = 0; 
  int i = foo(0); 
}

libfile.h

int myval = 0; 

int foo(int i); 
// Should be able to write spec here 

We can't verify clientfile.c because foo() doesn't have a specification in scope. But if we try to add a specification to libfile.h, we can't because it would require an accesses annotation.

@cp526 @dc-mak and others in the CN team - this is a blocker on TA2 work for both @peterohanley and myself. Could you look at this and make an assessment about how difficult it would be to fix?

@dc-mak dc-mak self-assigned this Feb 26, 2025
@dc-mak
Copy link
Contributor

dc-mak commented Feb 26, 2025

On it. Apologies for the inconvenience. It's not just a grammar change - code needs to be moved from the frontend to the backend. Nothing conceptually difficult, best guess is that it will probably take a few (2-3) days.

@septract
Copy link
Collaborator

Thanks @dc-mak - once you have this in prototype, I have an example in the OpenSUT we can use to test it.

@septract
Copy link
Collaborator

@dc-mak also note there was some talk of removing the accesses notation altogether.

dc-mak added a commit that referenced this issue Feb 27, 2025
This is in preparation for unifying function definition and declaration
specification parsing, as needed by #371.
dc-mak added a commit to dc-mak/cerberus that referenced this issue Feb 27, 2025
DO NOT MERGE: temporary commit to share and test.

This commit unifies the handling of definition and declaration
specifications. Previously, decl specs did not support accesses,
cn_function, or trusted. The requires and ensures clauses they
did support was desugared in the frontend Lem code.

This commit moves the desugaring of the decl specs to backend/cn,
specifically Core_to_mucore.normalise_fun_map_decl, alongside the
definition specs parsing and desugaring. It also moves the logic
for combining multiple function specs into the same module, outside
of Parse.

Extra special care needs to be taken in figuring out which desugaring
state to use. I'm not exactly sure about this bit, but it seems to be
working (I think thanks to the add_spec_arg_renames).
dc-mak added a commit to dc-mak/cerberus that referenced this issue Feb 27, 2025
DO NOT MERGE: temporary commit to share and test.

This commit unifies the handling of definition and declaration
specifications. Previously, decl specs did not support accesses,
cn_function, or trusted. The requires and ensures clauses they
did support was desugared in the frontend Lem code.

This commit moves the desugaring of the decl specs to backend/cn,
specifically Core_to_mucore.normalise_fun_map_decl, alongside the
definition specs parsing and desugaring. It also moves the logic
for combining multiple function specs into the same module, outside
of Parse.

Extra special care needs to be taken in figuring out which desugaring
state to use. I'm not exactly sure about this bit, but it seems to be
working (I think thanks to the add_spec_arg_renames).
@dc-mak
Copy link
Contributor

dc-mak commented Feb 28, 2025

Can you check out #892 and test it locally? The code's ready to go but I still need to add a few tests for the new behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking cn language Related to design of the CN language
Projects
None yet
Development

No branches or pull requests

5 participants