Big endian fixes for dotnet runtime #47981

nealef · 2021-02-07T23:57:34Z

A series of fixes that enable big endian running of the dotnet command. With these fixes and fixes in dotnet/runtimelab#650 the s390x run of the test suite results in 240 of 240 tests passing.

System.Private.CoreLib/src/System/Text/ASCIIUtility.cs
-- Big endian handling of widening to UTF16
System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Helpers.cs
-- Correct big endian extraction of UTF8 from two/four byte sequences
-- Correct detection of overlong rep for BE
System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Transcoding.cs
-- Correct transcoding to UTF8 for BE
System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs
-- Remove debug message as we have run and verified the code
System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryParser.cs
-- Remove debug message as we have run and verified the code
System.Text.Encoding.CodePages/src/System.Text.Encoding.CodePages.csproj
-- Include reference for use in BE code
System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.cs
-- Add code for BE reading of file header, code page header, and code page index
-- Remove debug message as we have run and verified the code
System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.netcoreapp.cs
-- Add code for BE reading of code page index
System.Text.Encoding.CodePages/src/System/Text/DBCSCodePageEncoding.cs
-- Provide endian aware character reading
System.Text.Encoding.CodePages/src/System/Text/SBCSCodePageEncoding.cs
-- Endian aware code to map unicode to bytes

These fixes also address the problem described in issue #44805.

@eiriktsarpalis @layomia

Suggestion: area-System.Text.Encodings

dotnet-issue-labeler · 2021-02-07T23:57:38Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

nealef · 2021-02-08T00:38:46Z

I note failures such as:

Check failure on line 103 in src/libraries/System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.cs

Azure Pipelines
/ runtime-dev-innerloop (Build Linux x64 release SourceBuild)
src/libraries/System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.cs#L103
src/libraries/System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.cs(103,79): error CS0246: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'Span<>' could not be found (are you missing a using directive or an assembly reference?)

However, I had built runtime on my own 64-bit system with this patches and got a clean bill:

Build succeeded.
    0 Warning(s)
    0 Error(s)

I am not sure why this is happening.

stephentoub · 2021-02-08T01:43:01Z

I am not sure why this is happening.

That project builds for various configurations that don't have a built-in Span:

runtime/src/libraries/System.Text.Encoding.CodePages/src/System.Text.Encoding.CodePages.csproj

Line 5 in 45287ed

    
           <TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-windows;netstandard2.0;netcoreapp2.0-windows;netstandard2.0-windows;net461-windows</TargetFrameworks>

You could try adding a reference to System.Memory (it looks like you tried adding one, but it's in a section conditioned to only apply to some of the targets).

nealef · 2021-02-08T01:46:57Z

I am not sure why this is happening.

That project builds for various configurations that don't have a built-in Span:

runtime/src/libraries/System.Text.Encoding.CodePages/src/System.Text.Encoding.CodePages.csproj

Line 5 in 45287ed

<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-windows;netstandard2.0;netcoreapp2.0-windows;netstandard2.0-windows;net461-windows</TargetFrameworks>

You could try adding a reference to System.Memory.

Thanks, what about things like:

src\libraries\System.Text.Encoding.CodePages\src\System\Text\BaseCodePageEncoding.cs(4,14): error CS0234: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'Buffers' does not exist in the namespace 'System' (are you missing an assembly reference?)

That seems a bit harder to fix if that reference isn't present.

stephentoub · 2021-02-08T01:48:35Z

That seems a bit harder to fix if that reference isn't present.

That's why you'd add the reference :) System.Memory provides the BinaryPrimitives type.

stephentoub · 2021-02-08T01:59:49Z

And dispense with that using System.Buffers.Binary altogether? [Excuse my naivety]

No. System.Memory.dll includes the System.Buffers.Binary.BinaryPrimitives type. You would need to add something like:

  <ItemGroup Condition="!$(TargetFramework.StartsWith('$(NetCoreAppCurrent)'))">
    <PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
  </ItemGroup>

to the project file, to include a package reference to the System.Memory package for the targets that don't have System.Memory in-box.

That's not going to solve all of the issues you're hitting, though. For example, you've written code to use an overload of Stream.Read that simply doesn't exist in those previous versions.

nealef · 2021-02-08T02:13:35Z

That's not going to solve all of the issues you're hitting, though. For example, you've written code to use an overload of Stream.Read that simply doesn't exist in those previous versions.

Thanks again. I'll add those lines. As I, presumably, can't emulate that previous build environment is it acceptable to amend this PR to generate another around of building and debug from that or is that a gratuitous use of the CI environment?

stephentoub · 2021-02-08T02:19:05Z

As I, presumably, can't emulate that previous build environment

You can. When you build from root, by default you're just building one specific target. You can either build the whole repo for multiple targets (e.g. build.cmd -allconfigurations), or you can cd into the relevant directory here (cd src\libraries\System.Text.Encoding.CodePages\src) and do a build just of that project (dotnet build) which will build all of its configurations.

nealef · 2021-02-08T02:20:57Z

Perfect! Thanks yet again.

- Use Stream.Read(byte[] target, int offset, int length) instead of Stream.Read(Span) - Include System.Memory for older build levels - Enable BaseCodePageEncoding.netcoreapp.cs to use the Stream.Read(Span) method by giving it it's own ReadCodePageIndex() method

nealef · 2021-02-08T04:16:36Z

That's not going to solve all of the issues you're hitting, though. For example, you've written code to use an overload of Stream.Read that simply doesn't exist in those previous versions.

I found those errors to which you referred and have pushed some changes. I got a clean build using the dotnet build in that directory. I then tried a build using the -allconfigurations option and it built for sometime before failing here:

CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/ISsmlParser.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/SsmlParser.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/TtsEvent.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/TtsVoice.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/SrgsCompiler/SrgsCompiler.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/TtsEngineProxy.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/ISsmlParser.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/SsmlParser.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/TtsEngineProxy.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/TtsEvent.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/Synthesis/TtsVoice.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]
CSC : error CS2001: Source file '/home/neale/runtime/src/libraries/System.Speech/src/Internal/SrgsCompiler/SrgsCompiler.cs' could not be found. [/home/neale/runtime/src/libraries/System.Speech/src/System.Speech.csproj]

I am making the probably rash assumption that this is due to something missing on my build machine. I'll await the latest checks to see how it goes there.

...ation.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs

ghost · 2021-02-08T11:13:07Z

Tagging subscribers to this area: @tarekgh, @krwq, @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

A series of fixes that enable big endian running of the dotnet command. With these fixes and fixes in dotnet/runtimelab#650 the s390x run of the test suite results in 240 of 240 tests passing.

System.Private.CoreLib/src/System/Text/ASCIIUtility.cs
-- Big endian handling of widening to UTF16
System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Helpers.cs
-- Correct big endian extraction of UTF8 from two/four byte sequences
-- Correct detection of overlong rep for BE
System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Transcoding.cs
-- Correct transcoding to UTF8 for BE
System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs
-- Remove debug message as we have run and verified the code
System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryParser.cs
-- Remove debug message as we have run and verified the code
System.Text.Encoding.CodePages/src/System.Text.Encoding.CodePages.csproj
-- Include reference for use in BE code
System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.cs
-- Add code for BE reading of file header, code page header, and code page index
-- Remove debug message as we have run and verified the code
System.Text.Encoding.CodePages/src/System/Text/BaseCodePageEncoding.netcoreapp.cs
-- Add code for BE reading of code page index
System.Text.Encoding.CodePages/src/System/Text/DBCSCodePageEncoding.cs
-- Provide endian aware character reading
System.Text.Encoding.CodePages/src/System/Text/SBCSCodePageEncoding.cs
-- Endian aware code to map unicode to bytes

These fixes also address the problem described in issue #44805.

@eiriktsarpalis @layomia

Suggestion: area-System.Text.Encodings

Author:	nealef
Assignees:	-
Labels:	`arch-s390x`, `area-System.Text.Encoding`
Milestone:	-

src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Transcoding.cs

src/libraries/System.Text.Encoding.CodePages/src/System/Text/SBCSCodePageEncoding.cs

GrabYourPitchforks · 2021-02-09T18:30:04Z

/azp run runtime

azure-pipelines · 2021-02-09T18:30:33Z

Azure Pipelines successfully started running 1 pipeline(s).

GrabYourPitchforks · 2021-02-09T18:34:12Z

Previous CI run lost the artifacts somehow, so kicked off a new run. Hoping things will pass so that we can merge this in today.

GrabYourPitchforks · 2021-02-09T19:44:32Z

The Libraries Build windows allConfigurations x64 Debug failure is a known issue (fixed pending via #48074) and the build engineering team is on it. I'll ignore it for now while waiting for the rest to finish.

GrabYourPitchforks · 2021-02-09T21:54:57Z

Got it in. Thanks @nealef for your contribution! 🚀

nealef · 2021-02-09T22:04:39Z

Thanks! I think there'll be another installment after some more testing.

Big endian fixes for dotnet runtime

a92eaff

nealef mentioned this pull request Feb 8, 2021

s390x fixes that enable 100% test suite pass - Part 1 dotnet/runtimelab#650

Merged

jkotas reviewed Feb 8, 2021

View reviewed changes

...ation.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterWriter.cs Outdated Show resolved Hide resolved

Delete Debug review message

bdc71bd

marek-safar added arch-s390x Related to s390x architecture (unsupported) area-System.Text.Encoding labels Feb 8, 2021

stephentoub requested a review from GrabYourPitchforks February 8, 2021 17:12

GrabYourPitchforks approved these changes Feb 8, 2021

View reviewed changes

Conform to style conventions

f146076

GrabYourPitchforks merged commit 6f38724 into dotnet:master Feb 9, 2021

GrabYourPitchforks mentioned this pull request Feb 9, 2021

PEReader - Big Endian issues when getting a string #44805

Closed

nealef deleted the big-endian branch February 9, 2021 22:05

ghost locked as resolved and limited conversation to collaborators Mar 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Big endian fixes for dotnet runtime #47981

Big endian fixes for dotnet runtime #47981

nealef commented Feb 7, 2021 •

edited

Loading

dotnet-issue-labeler bot commented Feb 7, 2021

nealef commented Feb 8, 2021

stephentoub commented Feb 8, 2021 •

edited

Loading

nealef commented Feb 8, 2021

stephentoub commented Feb 8, 2021

stephentoub commented Feb 8, 2021

nealef commented Feb 8, 2021

stephentoub commented Feb 8, 2021 •

edited

Loading

nealef commented Feb 8, 2021

nealef commented Feb 8, 2021

ghost commented Feb 8, 2021

GrabYourPitchforks commented Feb 9, 2021

azure-pipelines bot commented Feb 9, 2021

GrabYourPitchforks commented Feb 9, 2021

GrabYourPitchforks commented Feb 9, 2021 •

edited

Loading

GrabYourPitchforks commented Feb 9, 2021

nealef commented Feb 9, 2021

Big endian fixes for dotnet runtime #47981

Big endian fixes for dotnet runtime #47981

Conversation

nealef commented Feb 7, 2021 • edited Loading

dotnet-issue-labeler bot commented Feb 7, 2021

nealef commented Feb 8, 2021

stephentoub commented Feb 8, 2021 • edited Loading

nealef commented Feb 8, 2021

stephentoub commented Feb 8, 2021

stephentoub commented Feb 8, 2021

nealef commented Feb 8, 2021

stephentoub commented Feb 8, 2021 • edited Loading

nealef commented Feb 8, 2021

nealef commented Feb 8, 2021

ghost commented Feb 8, 2021

GrabYourPitchforks commented Feb 9, 2021

azure-pipelines bot commented Feb 9, 2021

GrabYourPitchforks commented Feb 9, 2021

GrabYourPitchforks commented Feb 9, 2021 • edited Loading

GrabYourPitchforks commented Feb 9, 2021

nealef commented Feb 9, 2021

nealef commented Feb 7, 2021 •

edited

Loading

stephentoub commented Feb 8, 2021 •

edited

Loading

stephentoub commented Feb 8, 2021 •

edited

Loading

GrabYourPitchforks commented Feb 9, 2021 •

edited

Loading