openssl
openssl copied to clipboard
openssl
Enable some disabled __owurs
Fixes #15902
Checklist
- [ ] documentation is added or updated
- [ ] tests are added or updated
Frankly speaking, I treat the commented out __owurs as a bug, though I am ready to adjust the checks to make Coverity happy.
Frankly speaking, I treat the commented out
__owurs as a bug, though I am ready to adjust the checks to make Coverity happy.
It will be quite hard not to introduce additional warnings on modern compilers unless you really add proper error checking in speed. Is that something that should go in 3.0? Not sure, IMO rather not.
Currently my practice show that at least Init functions should be __owur-ed. If their result is not checked for the unavailable algorithms, the crash occurs when Update is called. So I'd like at least to enforce these checks.
We can discuss this going into 3.0 at the OTC call. Post 3.0, I think it should go in.
OTC voted against including this in 3.0. I guess there's no reason it can't be merged post 3.0.
I think we have discussed it a year ago, see https://github.com/openssl/openssl/pull/15905#issuecomment-872082988
Hmm. I see we discussed not including it into 3.0. We did not really discuss whether it can be included in 3.1.
OTC: Is adding __owur for functions as in this PR OK for master branch I.E., next minor release?
OTC: This is OK to go into 3.1.
One of the reasons is because __owur is defined to be empty unless --strict-warnings is used which applies only to compiling openssl itself and thus it does not really affect external users. They would have to define UNUSEDRESULT_DEBUG.
It seems some of the original comments on this PR from when it is was first opened are still relevant and should be looked at or responded to.
This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago
This PR is in a state where it requires action by @openssl/otc but the last update was 61 days ago
This PR is in a state where it requires action by @openssl/otc but the last update was 92 days ago
Temporary reverted all apps/speed.c changes. Local test passes, let's see the CIs result.
Did you verify that openssl speed still works as before (i.e. even the measurements are not affected)?
I'm not sure how to test it properly. BTW, speed measurements in case of failure provide false information.
I do not mean to check the failure case but just to verify that openssl speed with this patch still measures roughly the same values by the affected cipher loops.
master results:
type 16 bytes 64 bytes 256 bytes 1024 bytes 8192 bytes 16384 bytes
md5 91772.69k 257389.44k 533931.75k 707116.37k 816830.07k 827539.95k
sha1 84025.47k 240990.05k 567302.51k 857841.44k 973689.06k 983352.34k
rmd160 46301.62k 113790.82k 211919.71k 267129.06k 294384.98k 299035.49k
sha256 55175.94k 142183.77k 307577.84k 420504.06k 461638.44k 472122.22k
sha512 41781.01k 162284.56k 306821.14k 516779.58k 637521.17k 645326.85k
hmac(md5) 42626.68k 140568.51k 359760.57k 581000.13k 737553.98k 741537.65k
des-ede3 28208.99k 30115.40k 30465.11k 30593.63k 30436.43k 30170.67k
aes-128-cbc 958764.46k 1329055.10k 1426154.42k 1454299.53k 1455299.21k 1459951.39k
aes-192-cbc 900652.74k 1118750.37k 1201432.06k 1224710.85k 1223125.88k 1222794.36k
aes-256-cbc 801394.05k 951672.27k 1012161.50k 1046445.12k 1048751.35k 1057387.19k
camellia-128-cbc 114136.98k 177445.03k 209379.49k 219330.53k 222370.33k 220850.86k
camellia-192-cbc 97546.57k 141597.54k 158839.61k 164845.16k 166533.22k 166103.07k
camellia-256-cbc 97689.86k 141149.97k 157672.45k 163126.97k 161697.48k 164349.60k
my results:
type 16 bytes 64 bytes 256 bytes 1024 bytes 8192 bytes 16384 bytes
md5 88924.93k 235466.46k 502023.88k 651251.67k 773883.72k 766028.97k
sha1 75003.91k 216793.86k 504076.50k 808832.60k 964836.77k 933288.72k
rmd160 42344.06k 108339.88k 204763.18k 261897.42k 288681.70k 289996.80k
sha256 52682.93k 132354.75k 287863.44k 396939.95k 443976.16k 417085.13k
sha512 38796.09k 157492.44k 302009.21k 489177.13k 606746.80k 615495.92k
hmac(md5) 42532.11k 137884.34k 350451.07k 567857.66k 721331.63k 733997.72k
des-ede3 28754.39k 28934.66k 29631.70k 30028.89k 29951.49k 30269.30k
aes-128-cbc 900809.60k 1306924.80k 1405518.26k 1433256.96k 1444170.15k 1442898.88k
aes-192-cbc 844049.87k 1105085.99k 1172450.72k 1198948.86k 1209804.97k 1185291.99k
aes-256-cbc 784697.56k 967036.87k 973483.41k 994751.74k 1025403.56k 1017820.26k
camellia-128-cbc 108197.38k 173425.70k 204015.90k 213402.28k 210674.13k 208334.34k
camellia-192-cbc 90880.47k 133707.56k 155732.27k 160278.60k 161491.99k 160398.26k
camellia-256-cbc 91916.18k 135697.17k 152320.00k 157895.66k 156527.68k 152760.25k
Don't think the difference really matters as hash algorithms also show some performance loss but the processing didn't change.
Don't think the difference really matters as hash algorithms also show some performance loss but the processing didn't change.
Hmm, did you try running both clean master and the same just with this PR rebased on top of it? There should not be any significant difference.
This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago
