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

review windows warnings #966

Closed
chipitsine opened this issue Dec 12, 2023 · 9 comments · Fixed by #967
Closed

review windows warnings #966

chipitsine opened this issue Dec 12, 2023 · 9 comments · Fixed by #967

Comments

@chipitsine
Copy link
Contributor

while working on automatic release, I noticed a lot of build warnings under windows.
let's review them.

either we can

  1. leave as is
  2. fix them and make fatal
  3. mute them
   crypto_init.c
D:\a\portable\portable\crypto\..\include\compat\pthread.h(36,1): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\crypto_init.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\..\include\compat\pthread.h(36,1): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\cryptlib.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\..\include\compat\pthread.h(44,71): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\crypto_init.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\..\include\compat\pthread.h(44,71): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\cryptlib.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
  cversion.c
botovq added a commit to botovq/libressl-portable that referenced this issue Dec 12, 2023
You can't pass a function pointer through a void pointer.
So wrap the pthread callback in a struct.

Fixes libressl#966
@chipitsine
Copy link
Contributor Author

one more like that

D:\a\portable\portable\crypto\x509\x509_vpm.c(98,8): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\x509\x509_vpm.c(99,7): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\x509\x509_vpm.c(316,49): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\x509\x509_vpm.c(316,59): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

@chipitsine
Copy link
Contributor Author

... and more

D:\a\portable\portable\tests\x509_asn1.c(36,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(37,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(38,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(39,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(40,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(41,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(42,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(43,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(44,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(45,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(46,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(47,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(48,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(95,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(111,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(124,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(185,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(198,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]

@botovq
Copy link
Contributor

botovq commented Dec 12, 2023

Yes, thanks. I saw them

D:\a\portable\portable\crypto\x509\x509_vpm.c(98,8): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

sk_deep_copy() should just use the proper function pointers passed in its only caller instead of being written in this pseudo generality. But that needs to be done in the OpenBSD repo.

D:\a\portable\portable\tests\x509_asn1.c(36,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]

this test needs reworking. I've added it to my to do list.

@botovq
Copy link
Contributor

botovq commented Dec 12, 2023

  • static const not necessarily ininitialized to 0?

    D:\a\portable\portable\crypto\evp\e_chacha20poly1305.c(107,43): warning C4132: 'zero_pad16': const object should be initialized [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

    unsure about this one. I would have expected this to be fine.

    Added: It was confirmed to me that tihs is ok as it is (according to C2x 6.7.9, clause 10), so a false positive. We won't work around htis.

  • freeing of const object

    D:\a\portable\portable\crypto\objects\o_names.c(317,15): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

    This code will hopefully be replaced soon.

  • potential use of uninitialized

    D:\a\portable\portable\crypto\pem\pem_lib.c(424): warning C4701: potentially uninitialized local variable 'j' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
    D:\a\portable\portable\crypto\pem\pem_lib.c(475): warning C4701: potentially uninitialized local variable 'i' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

    This looks like a correct warning and should be fixed. It is actually a false positive. i and j will only be used when they were initialized. The code is horrible and should be reworked. This will likely happen as part of a related cleanup soon.

  • Unreachable code

    D:\a\portable\portable\crypto\pkcs7\pk7_doit.c(657): warning C4702: unreachable code [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

    Ugly code. Harmless.

  • passing a function pointer through void

    D:\a\portable\portable\crypto\x509\x509_vpm.c(98,1): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

    As mentioned above, sk_deep_copy() should be fixed. I have a diff that I will send out for review soon.

  • initializing an array with a longer string constant

    D:\a\portable\portable\crypto\compat\chacha_private.h(51,49): warning C4295: 'sigma': array is too small to include a terminating null character (compiling source file D:\a\portable\portable\crypto\compat\arc4random.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

    Already discussed. Correct C, reasonable warning.

  • unreachable code

    D:\a\portable\portable\ssl\d1_pkt.c(1000): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
    D:\a\portable\portable\ssl\s3_cbc.c(464): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
    D:\a\portable\portable\ssl\s3_cbc.c(465): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
    D:\a\portable\portable\ssl\s3_cbc.c(466): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

    abuse of OPENSSL_assert(0).

  • const issue with tlsext_build_order. Need to think about how I want to fix this. Harmless.

    D:\a\portable\portable\ssl\ssl_lib.c(571,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
    D:\a\portable\portable\ssl\ssl_tlsext.c(2256,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
    D:\a\portable\portable\ssl\ssl_tlsext.c(2305,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

  • whining about the empty empty.c

    D:\a\portable\portable\crypto\empty.c(1,1): warning C4206: nonstandard extension used: translation unit is empty [D:\a\portable\portable\build\crypto\crypto.vcxproj]

    I have no idea what empty.c is about. It was introduced in 80eb145 but I don't understand what that is doing/fixing.

  • invalid octal

    D:\a\portable\portable\tests\constraints.c(98,2): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\constraints.vcxproj]
    D:\a\portable\portable\tests\tlsexttest.c(4370,15): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\tlsexttest.vcxproj]

    this is nonsense. will fix.

  • unsure what this is about

    D:\a\portable\portable\tests\freenull.c(214,2): warning C4115: 'lhash_st_FUNCTION': named type definition in parentheses [D:\a\portable\portable\build\tests\freenull.vcxproj]

    The offending line can probably be deleted.

  • double const due to ugly typedef

    D:\a\portable\portable\tests\rfc3779.c(364,28): warning C4114: same type qualifier used more than once [D:\a\portable\portable\build\tests\rfc3779.vcxproj]

    typedef const ASN1_ITEM ASN1_ITEM_EXP... sigh.

  • passing a const pointer through void

    D:\a\portable\portable\tests\x509_asn1.c(36,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]

@chipitsine
Copy link
Contributor Author

D:\a\portable\portable\crypto\evp\e_chacha20poly1305.c(107,43): warning C4132: 'zero_pad16': const object should be initialized [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

unsure about this one. I would have expected this to be fine.

D:\a\portable\portable\crypto\objects\o_names.c(317,15): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

This code will hopefully be replaced soon.

D:\a\portable\portable\crypto\pem\pem_lib.c(424): warning C4701: potentially uninitialized local variable 'j' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\pem\pem_lib.c(475): warning C4701: potentially uninitialized local variable 'i' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

This looks like a correct warning and should be fixed.

D:\a\portable\portable\crypto\pkcs7\pk7_doit.c(657): warning C4702: unreachable code [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

Ugly code. Harmless.

D:\a\portable\portable\crypto\x509\x509_vpm.c(98,1): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

As mentioned above, sk_deep_copy() should be fixed.

D:\a\portable\portable\crypto\compat\chacha_private.h(51,49): warning C4295: 'sigma': array is too small to include a terminating null character (compiling source file D:\a\portable\portable\crypto\compat\arc4random.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

Already discussed. Correct C, reasonable warning.

D:\a\portable\portable\ssl\d1_pkt.c(1000): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\s3_cbc.c(464): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\s3_cbc.c(465): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\s3_cbc.c(466): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

abuse of OPENSSL_assert(0).

D:\a\portable\portable\ssl\ssl_lib.c(571,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\ssl_tlsext.c(2256,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\ssl_tlsext.c(2305,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

const issue with tlsext_build_order. Need to think about how I want to fix this. Harmless.

D:\a\portable\portable\crypto\empty.c(1,1): warning C4206: nonstandard extension used: translation unit is empty [D:\a\portable\portable\build\crypto\crypto.vcxproj]

I have no idea what empty.c is about.

I've added build artifacts, entire build folder is added to artifacts

image

no idea what empty.c is either.
but we can have a look. it might be something automatically generated ...

D:\a\portable\portable\tests\constraints.c(98,2): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\constraints.vcxproj]
D:\a\portable\portable\tests\tlsexttest.c(4370,15): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\tlsexttest.vcxproj]

this is nonsense. will fix.

D:\a\portable\portable\tests\freenull.c(214,2): warning C4115: 'lhash_st_FUNCTION': named type definition in parentheses [D:\a\portable\portable\build\tests\freenull.vcxproj]

This should probably be deleted.

D:\a\portable\portable\tests\rfc3779.c(364,28): warning C4114: same type qualifier used more than once [D:\a\portable\portable\build\tests\rfc3779.vcxproj]

typedef const ASN1_ITEM ASN1_ITEM_EXP... sigh.

@vszakats
Copy link
Contributor

vszakats commented Dec 13, 2023

To silence the chacha warnings, something like this can be done:

- static const char sigma[16] = "expand 32-byte k";
+ static const char sigma[16] = { 'e', 'x', 'p', 'a', 'n', 'd', ' ', '3',
+                                 '2', '-', 'b', 'y', 't', 'e', ' ', 'k' };

@botovq
Copy link
Contributor

botovq commented Dec 13, 2023 via email

@chipitsine
Copy link
Contributor Author

for ugly things we have "pragma" ))

@vszakats
Copy link
Contributor

To me the ugly solution reduces the ambigiuty when reading the code. The string is just a bunch of bytes, not meant for reading or grepping. Another solution is to bump its size to [] or 17 and adjust any sizeof()s. What's against both is the diverging from the reference source. FWIW clang and gcc is silent about it.

As for this one:

D:\a\portable\portable\crypto\empty.c(1,1): warning C4206: nonstandard extension used: translation unit is empty

It'd be useful to know what exact CMake issue (and which Generator) made this necessary. Other than that, this warning may happen when the whole source content is guarded out legitimately, leaving an empty object. It think it's safe to silence, or ignore. If that's the only source affected, it'd be better to fix the CMake root issue though.

@botovq botovq closed this as completed in 0491aef Dec 14, 2023
busterb pushed a commit to busterb/portable that referenced this issue Mar 3, 2024
You can't pass a function pointer through a void pointer.
So wrap the pthread callback in a struct.

Fixes libressl#966
busterb pushed a commit to busterb/portable that referenced this issue Mar 3, 2024
You can't pass a function pointer through a void pointer.
So wrap the pthread callback in a struct.

Fixes libressl#966
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 a pull request may close this issue.

3 participants