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

Ptr<struct> passed to external struct * param in benchmarks #731

Open
kyleheadley opened this issue Oct 20, 2021 · 5 comments
Open

Ptr<struct> passed to external struct * param in benchmarks #731

kyleheadley opened this issue Oct 20, 2021 · 5 comments
Labels
benchmark failure A bug causing a failure in our nightly benchmark tests cast incomplete description

Comments

@kyleheadley
Copy link
Member

libarchive and icecast have errors like:

fserve.c:175:19: error (parameter_incompatible): passing '_Array_ptr<struct pollfd>' to parameter of incompatible type 'struct pollfd *'
    else if (poll(ufds, fserve_clients, 200) > 0)
                  ^~~~

which call an external library (in this case sys/poll.h) with unchecked parameters, but either allow the argument to be checked, or don't add the cast. My oversimplified test shows 3c behaving properly:

#include <sys/poll.h>

// _Ptr<struct pollfd> f = 0; // cast added to poll
struct pollfd *f = 0; // doesn't convert because of poll

void test() {
  poll(f,2,200);
}

So the benchmark examples must be doing something more complex here.

@kyleheadley kyleheadley added cast benchmark failure A bug causing a failure in our nightly benchmark tests incomplete description labels Oct 20, 2021
@john-h-kastner
Copy link
Collaborator

john-h-kastner commented Oct 20, 2021

It looks like this is another instance of #507. fserve.c includes sys/poll.h which doesn't have a checked header. There is a header for poll.h included by net/sock.c. The checked version of poll.h has the declaration of poll which is being used here even though it isn't visible in the fserve.c translation unit.

@kyleheadley
Copy link
Member Author

Thanks. libarchive has the same error with functions like EVP_DigestUpdate and HMAC_Update which I'm not familiar with and don't appear in the rest of the benchmark code. Do you know where these come from and if it's the same issue?

../libarchive/archive_hmac.c:243:14: error (parameter_incompatible_struct): passing 'archive_hmac_sha1_ctx' (aka '_Ptr<struct hmac_ctx_st>') to parameter of incompatible type 'HMAC_CTX *' (aka 'struct hmac_ctx_st *')
        HMAC_Update(*ctx, data, data_len);
                    ^~~~

@john-h-kastner
Copy link
Collaborator

I'm not sure about that. HMAC_Update seems to come from openssl, but that shouldn't have any checked headers, so the error shouldn't be possible. Grepping for HMAC_Update doesn't show any local definition that could be messing things up either.

The type of ctx is a pointer to a typedef, and I've previously noticed a bug in how these are converted (#696). The same thing could possible by happening here.

@mattmccutchen-cci
Copy link
Member

It looks like this is another instance of #507. fserve.c includes sys/poll.h which doesn't have a checked header. There is a header for poll.h included by net/sock.c. The checked version of poll.h has the declaration of poll which is being used here even though it isn't visible in the fserve.c translation unit.

If we want to support code that does #include <sys/poll.h>, I guess we need to modify the checked headers so that #include <sys/poll.h> brings in the necessary checked declarations. Alternatively, we could take the position that programs should follow POSIX and use #include <poll.h>, but practically speaking, it will probably make our lives and our customers' lives easier if we try to accommodate these variations whenever we can.

If on all systems we care about, the system's poll.h just does #include <sys/poll.h> (as on Ubuntu), then we can just move the checked headers from poll{,_checked}.h to sys/poll{,_checked}.h. If some systems don't have sys/poll.h at all and have the declarations directly in poll.h, then we'll have to do something more complicated so that programs that #include <poll.h> continue to work on those systems. It would probably work to just duplicate the declarations in poll_checked.h and sys/poll_checked.h (or put them in an auxiliary checked header file that is loaded by both poll_checked.h and sys/poll_checked.h). Then, on a system where poll.h delegates to sys/poll.h, user code that does #include <poll.h> would load the checked declarations twice, but that's probably harmless.

@kyleheadley
Copy link
Member Author

kyleheadley commented Oct 21, 2021

Since this already needs to be broken down into multiple sources of error, I'll add another libarchive(no macro,no -alltypes) failure from conflicting struct pointers:

../libarchive/archive_string.c:871:34: error: assigning to 'int (*)(_Ptr<struct archive_string>, const void *, size_t, _Ptr<struct archive_string_conv>)' (aka 'int (*)(_Ptr<struct archive_string>, const void *, unsigned long, _Ptr<struct archive_string_conv>)') from incompatible type 'int ((*)(struct archive_string *, const void *, size_t, struct archive_string_conv *))' (aka 'int (*)(struct archive_string *, const void *, unsigned long, struct archive_string_conv *)'): type mismatch at 1st parameter ('_Ptr<struct archive_string>' vs 'struct archive_string *')
        sc->converter[sc->nconverter++] = converter;
                                        ^ ~~~~~~~~~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark failure A bug causing a failure in our nightly benchmark tests cast incomplete description
Projects
None yet
Development

No branches or pull requests

3 participants