Skip to content

Conversation

@waskyo
Copy link
Contributor

@waskyo waskyo commented May 1, 2025

With these changes we no longer have a ton of open POs when programs
use command line arguments.

These fixes both issues when accessing argv[0] directly (which is given
special treatment) and accessing other command line arguments after
checking that those exist (by looking at the value of argc).

Before these changes, these were all the open POs for a simple hello world
program:

int main(int argc, char **argv) {
  if (argc != 2) {
    printf("ERROR: usage: %s <name>\n", argv[0]);
--------------------------------------------------------------------------------
|  null-terminated((*(argv + 0):((char *) *)))                                 |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  initialized-range((*(argv + 0):((char *) *)), len:cnapp(ntp((*(argv + 0):((char *) *))))|
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  ptr-upper-bound((*(argv + 0):((char *) *)), cnapp(ntp((*(argv + 0):((char *) *))), op:pluspi, typ: char)|
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  Preconditions:                                                              |
|    initialized((*argv))                                                      |
--------------------------------------------------------------------------------
    return 1;
  }
  printf("Hello world %s\n", argv[1]);
--------------------------------------------------------------------------------
|  initialized-range((*(argv + 1):((char *) *)), len:cnapp(ntp((*(argv + 1):((char *) *))))|
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  valid-mem((*(argv + 1):((char *) *)))                                       |
|  [augv[call]:$fn-entry$(-1):calls]:none                                      |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  null-terminated((*(argv + 1):((char *) *)))                                 |
|  no invariants found for *(((lval (argv) +i 1):((char*)*))                   |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  upper-bound(char,(*(argv + 1):((char *) *)))                                |
|  no invariants for *(((lval (argv) +i 1):((char*)*))                         |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  lower-bound(char,(*(argv + 1):((char *) *)))                                |
|  no invariants found for *(((lval (argv) +i 1):((char*)*))                   |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  ptr-upper-bound((*(argv + 1):((char *) *)), cnapp(ntp((*(argv + 1):((char *) *))), op:pluspi, typ: char)|
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  in-scope((*(argv + 1):((char *) *)))                                        |
|  no invariants found for *(((lval (argv) +i 1):((char*)*))                   |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  Preconditions:                                                              |
|    ptr-upper-bound-deref(argv, 1, op:indexpi, typ: (char *))                 |
--------------------------------------------------------------------------------
  return 0;
}

Two pending things which I will tackle on the next set of commits:

  • There's still one open pre-condition that I haven't figured out how to
    close:
  printf("Hello world %s\n", argv[1]);
--------------------------------------------------------------------------------
|  Preconditions:                                                              |
|    ptr-upper-bound-deref(argv, 1, op:indexpi, typ: (char *))                 |
--------------------------------------------------------------------------------
  • If the program does not check the value of argc before accessing
    argv then the diagnostics all relate to the fact that we don't have
    invariants to verify whether the access is correct or not, which makes it
    sound like there's a bug in CHC. For example:
  printf("Hello world %s\n", argv[1]);
--------------------------------------------------------------------------------
|  upper-bound(char,(*(argv + 1):((char *) *)))                                |
|  no invariant found for argument count; unable to validate access of command-line argument 1|
|  no invariants for *(((lval (argv) +i 1):((char*)*))                         |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  not-null((*(argv + 1):((char *) *)))                                        |
|  no invariant found for argument count; unable to validate not-null of command-line argument 1|
--------------------------------------------------------------------------------

waskyo added 3 commits April 30, 2025 10:01
We already had some logic to deal with this but not for all pointer cases.
I copy/pasted that logic over to the cases where the current version of CHC
complains for a simple hello world program that passes argv[0] to printf.

There's still an open issue where there's a precondition for 'initialized(*argv)'
that I have not addressed
…world program

I couldn't figure out how to do it within the cCHCheckValid.ml match
logic so I added the logic to the actual initialized checker. It now
has a specific path that figures out if what we're looking at is
argv[0] and marks it safe, closes the PO, and does not mark it for
delegation.

Probably not the cleanest solution, but it will do for now.
The logic was already in both cCHPOCheckNotNull.ml and in
cCHPOQuery.ml, although cCHPOCheckNotNull.ml was using
its own function instead of the one in the POQuery object.

This adds calls to the util function in the POQuery object and
nukes the private function in CheckNotNull.

With these changes, a simple hello world program no longer gets a
ton of open POs:

```
int main(int argc, char **argv) {
  if (argc != 2) {
    printf("ERROR: usage: %s <name>\n", argv[0]);
    return 1;
  }
  printf("Hello world %s\n", argv[1]);
--------------------------------------------------------------------------------
|  initialized-range((*(argv + 1):((char *) *)), len:cnapp(ntp((*(argv + 1):((char *) *))))|
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  valid-mem((*(argv + 1):((char *) *)))                                       |
|  [augv[call]:$fn-entry$(-1):calls]:none                                      |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  null-terminated((*(argv + 1):((char *) *)))                                 |
|  no invariants found for *(((lval (argv) +i 1):((char*)*))                   |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  upper-bound(char,(*(argv + 1):((char *) *)))                                |
|  no invariants for *(((lval (argv) +i 1):((char*)*))                         |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  lower-bound(char,(*(argv + 1):((char *) *)))                                |
|  no invariants found for *(((lval (argv) +i 1):((char*)*))                   |
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  ptr-upper-bound((*(argv + 1):((char *) *)), cnapp(ntp((*(argv + 1):((char *) *))), op:pluspi, typ: char)|
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
|  in-scope((*(argv + 1):((char *) *)))                                        |
|  no invariants found for *(((lval (argv) +i 1):((char*)*))                   |
--------------------------------------------------------------------------------
```

There's still one open pre-condition that I haven't figured out how to
close:
--------------------------------------------------------------------------------
|  Preconditions:                                                              |
|    ptr-upper-bound-deref(argv, 1, op:indexpi, typ: (char *))                 |
--------------------------------------------------------------------------------
Copy link
Contributor

@sipma sipma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

This is a good solution to handle these proof obligations.

@sipma sipma merged commit b7151cf into static-analysis-engineering:master May 2, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants