Build against Y2038-compatible glibc for linux arm32 by sbomer · Pull Request #102410 · dotnet/runtime

sbomer · 2024-05-18T00:27:04Z

Reintroduces the fix for Y2038 support on arm32 linux (#102059), which was reverted due to problems running against openssl built with _TIME_BITS=32. Thanks to @davidwrighton for help diagnosing the issue.

Fixes #101444 (both the originally reported issue, and the test failures mentioned in #101444 (comment)).

Supports: #91826

…tnet#102059)" (dotnet#102324)" This reverts commit 9a18cf5.

sbomer · 2024-05-18T00:29:05Z

/azp run runtime-coreclr libraries-jitstress

azure-pipelines · 2024-05-18T00:29:12Z

Azure Pipelines successfully started running 1 pipeline(s).

src/native/libs/System.Security.Cryptography.Native/openssl.c

And avoid passing values that can't be represented in 32 bits when running against openssl that expects 32-bit time_t.

sbomer · 2024-05-22T04:26:51Z

/azp run runtime-coreclr libraries-jitstress

azure-pipelines · 2024-05-22T04:26:59Z

Azure Pipelines successfully started running 1 pipeline(s).

jkotas

LGTM. Thank you!

Apparently this symbol did not exist in the builds of openssl in Ubuntu 16.04.

Except in opensslshim.c, which is only built for that feature.

src/native/libs/System.Security.Cryptography.Native/opensslshim.c

src/native/libs/System.Security.Cryptography.Native/openssl.c

jkotas · 2024-05-22T18:15:45Z

We may want to simplify

runtime/src/native/libs/System.Security.Cryptography.Native/pal_x509.c

Lines 1357 to 1364 in f02a695

    
           if (sizeof(time_t) == sizeof(int64_t)) 
        
           { 
        
               *expiration = (int64_t)expiration_t; 
        
           } 
        
           else if (sizeof(time_t) == sizeof(int32_t)) 
        
           { 
        
               *expiration = (int32_t)expiration_t; 
        
           }

to assume 64-bit time_t and instead add assert that will fail the build when time_t is not 64-bit. Something like this: jkotas@8657f6d

- Revert unintentional change

sbomer · 2024-05-22T18:34:38Z

assume 64-bit time_t and instead add assert that will fail the build when time_t is not 64-bit.

Apparently the tizen-armel build doesn't use 64-bit time_t.

sbomer · 2024-05-22T19:05:36Z

/azp run runtime-coreclr libraries-jitstress

azure-pipelines · 2024-05-22T19:05:50Z

Azure Pipelines successfully started running 1 pipeline(s).

richlander · 2024-05-22T21:56:25Z

Will you provided a detailed writeup on what we had to do? I think that will be super useful.

This updates our linux arm32 build to build against a more recent glibc that supports _TIME_BITS (which we set to 64). Since openssl may be using either 32-bit or 64-bit time_t, this includes detection logic to determine which case we are in, and avoid passing time values that don't fit in 32 bits to openssl. The arm build image is updated to the latest version of the images added in dotnet/dotnet-buildtools-prereqs-docker#1037. The helix test images are updated to debian images added in dotnet/dotnet-buildtools-prereqs-docker#1041. Additional context: Additional context: Reintroduces the fix for Y2038 support on arm32 linux (dotnet#102059), which was reverted due to problems running against openssl built with _TIME_BITS=32. Fixes dotnet#101444 (both the originally reported issue, and the test failures mentioned in dotnet#101444 (comment)). Supports: dotnet#91826

sbomer · 2024-05-29T23:22:16Z

A writeup is available at https://github.com/dotnet/runtime/blob/main/docs/design/features/y2038.md.

This updates our linux arm32 build to build against a more recent glibc that supports _TIME_BITS (which we set to 64). Since openssl may be using either 32-bit or 64-bit time_t, this includes detection logic to determine which case we are in, and avoid passing time values that don't fit in 32 bits to openssl. The arm build image is updated to the latest version of the images added in dotnet/dotnet-buildtools-prereqs-docker#1037. The helix test images are updated to debian images added in dotnet/dotnet-buildtools-prereqs-docker#1041. Additional context: Additional context: Reintroduces the fix for Y2038 support on arm32 linux (dotnet#102059), which was reverted due to problems running against openssl built with _TIME_BITS=32. Fixes dotnet#101444 (both the originally reported issue, and the test failures mentioned in dotnet#101444 (comment)). Supports: dotnet#91826

sbomer added 2 commits May 18, 2024 00:18

Revert "Revert "Unpin arm build image, use Debian 12 helix images (do…

dde5a18

…tnet#102059)" (dotnet#102324)" This reverts commit 9a18cf5.

Fix ARM ABI issue calling openssl function

84bb050

sbomer requested review from agocke, davidwrighton, jkotas and richlander May 18, 2024 00:27

dotnet-policy-service bot assigned sbomer May 18, 2024

jkotas reviewed May 18, 2024

View reviewed changes

src/native/libs/System.Security.Cryptography.Native/openssl.c Outdated Show resolved Hide resolved

jkotas reviewed May 18, 2024

View reviewed changes

src/native/libs/System.Security.Cryptography.Native/openssl.c Outdated Show resolved Hide resolved

sbomer added 4 commits May 21, 2024 15:57

Detect openssl time_t bitness

e5880d0

And avoid passing values that can't be represented in 32 bits when running against openssl that expects 32-bit time_t.

Return failure code if for invalid time

f3f5058

Adjust comments

b25bb1d

Fix whitespace

45c2bc7

jkotas approved these changes May 22, 2024

View reviewed changes

This was referenced May 22, 2024

GC\Regressions\v2.0-beta2\452950\452950\452950.cmd failing on Mono minijit Windows x64 #99729

Closed

at GC_Regressions._v2_0_beta2_452950_452950_452950_._v2_0_beta2_452950_452950_452950_cmd() #100174

Closed

Abort on mono in SwiftErrorHandling #102478

Closed

Move OPENSSL_gmtime out of FOR_ALL_OPENSSL_FUNCTIONS

c5719ff

Apparently this symbol did not exist in the builds of openssl in Ubuntu 16.04.

build-analysis bot mentioned this pull request May 22, 2024

System.Net.Tests.HttpWebRequestTest_Async.GetResponseAsync_ParametersAreNotCachable_CreateNewClient test fails #100912

Closed

Check FEATURE_DISTRO_AGNOSTIC_SSL

414f24e

Except in opensslshim.c, which is only built for that feature.

jkotas reviewed May 22, 2024

View reviewed changes

src/native/libs/System.Security.Cryptography.Native/opensslshim.c Show resolved Hide resolved

jkotas reviewed May 22, 2024

View reviewed changes

src/native/libs/System.Security.Cryptography.Native/openssl.c Show resolved Hide resolved

PR feedback

bac65d4

- Revert unintentional change

sbomer merged commit a17b872 into dotnet:main May 22, 2024

github-actions bot locked and limited conversation to collaborators Jun 29, 2024

Conversation

sbomer commented May 18, 2024 • edited by richlander Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbomer commented May 18, 2024

Uh oh!

azure-pipelines bot commented May 18, 2024

Uh oh!

Uh oh!

Uh oh!

sbomer commented May 22, 2024

Uh oh!

azure-pipelines bot commented May 22, 2024

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jkotas commented May 22, 2024

Uh oh!

sbomer commented May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbomer commented May 22, 2024

Uh oh!

azure-pipelines bot commented May 22, 2024

Uh oh!

richlander commented May 22, 2024

Uh oh!

sbomer commented May 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sbomer commented May 18, 2024 •

edited by richlander

Loading

sbomer commented May 22, 2024 •

edited

Loading