rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
192 stars 54 forks source link

runtime error: call to function ffi_pass_callback_file #1329

Open ni4 opened 3 years ago

ni4 commented 3 years ago

Description

Following error appears on centos sanitize build:

/__w/rnp/rnp/src/lib/rnp.cpp:1080:7: runtime error: call to function ffi_pass_callback_file(rnp_ffi_st*, void*, rnp_key_handle_st*, char const*, char*, unsigned long) through pointer to incorrect function type 'bool (*)(rnp_ffi_st *, void *, rnp_key_handle_st *, const char *, char *, unsigned long)'
2020-10-16T16:39:09.8868329Z 201: /__w/rnp/rnp/src/rnp/fficli.cpp:315: note: ffi_pass_callback_file(rnp_ffi_st*, void*, rnp_key_handle_st*, char const*, char*, unsigned long) defined here
ni4 commented 3 years ago

Strange since ffi_pass_callback_file is defined correctly:

static bool
ffi_pass_callback_file(rnp_ffi_t        ffi,
                       void *           app_ctx,
                       rnp_key_handle_t key,
                       const char *     pgp_context,
                       char             buf[],
                       size_t           buf_len)
ni4 commented 2 years ago

This could be false positive, as the issue with stdout_writer, see PR #1709. Possible solution:

#if defined(__clang__)
__attribute__((no_sanitize("undefined")))
#endif
andrey-utkin commented 2 years ago

It seems like a trait of outdated Clang shipped with CentOS 7.

In PR 1792, CI checks on centos-based systems with "mode sanitize" and clang:

So I'd suggest to disregard the combination centos7 + clang + sanitize.

ronaldtse commented 2 years ago

@andrey-utkin I think it would be the right time to drop CentOS 7, especially since CentOS 8 has already been discontinued. Maybe make the next release the last release for CentOS 7?

ni4 commented 2 years ago

@ronaldtse CentOS 7 has EOL 2024, while 8 had end of 2021, is it okay to drop 2-year support timeline? Also, then should not we add support for CentOS stream 9 instead?

ronaldtse commented 2 years ago

@ni4 whoa, I did not realise that CentOS 8 was EOLed before CentOS 7. In this case we should keep CentOS 7... And yes we should support the CentOS Stream version. I think deprecating CentOS 8 and adding support for CentOS 9 can be independent steps.

ni4 commented 2 years ago

Oh, I forgot - we already use CentOS 8 Stream images in CI, but those are unofficial (there are no official on the Docker hub, they use Quay). Will file an issue for CentOS 9 Stream.