Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Fix -int.ToString() for locales (#7941)
Browse files Browse the repository at this point in the history
Fixes: #7939

Context: dotnet/runtime#13363
Context: https://www.unicode.org/charts/PDF/U2200.pdf (page 2)
Context: 5271f3e

The LLVM IR generator (5271f3e) outputs a number of integer values,
relying on the standard `int.ToString()` method to convert the value
to a string.  However, since .NET 6, this conversion is culture-
sensitive and in certain cultures (list below) it will output the
minus sign not as the "standard" ASCII \u002d character `-` but
instead as something else (see complete list below for details).

This breaks LLVM LLC:

	llc: environment.arm64-v8a.ll:554:7: error: expected value token
	  stderr |                 i32 −1, ; apk_fd

Fix the problem by using invariant culture when converting ***all***
integers and unknown objects to strings.  The reason all of them are
converted in this way is to avoid future changes to ICU and/or .NET
to-string conversion that might affect the resulting integer format.

Workaround: export `$LANG` to a locale that uses 0x2D `-` for
negation, e.g. `LANG=C`.

Locales/cultures affected by this issue are as follows:

  * ARABIC LETTER MARK + HYPHEN-MINUS `؜-` (\u061c \u002d): 26 cultures
    * ar
    * ar-001
    * ar-BH
    * ar-DJ
    * ar-EG
    * ar-ER
    * ar-IL
    * ar-IQ
    * ar-JO
    * ar-KM
    * ar-KW
    * ar-LB
    * ar-MR
    * ar-OM
    * ar-PS
    * ar-QA
    * ar-SA
    * ar-SD
    * ar-SO
    * ar-SS
    * ar-SY
    * ar-TD
    * ar-YE
    * sd
    * sd-Arab
    * sd-Arab-PK
  * LEFT-TO-RIGHT MARK + HYPHEN-MINUS `‎-` (\u200e \u002d): 10 cultures
    * ar-AE
    * ar-DZ
    * ar-EH
    * ar-LY
    * ar-MA
    * ar-TN
    * he
    * he-IL
    * ur
    * ur-PK
  * RIGHT-TO-LEFT MARK + HYPHEN-MINUS `‏-` (\u200f \u002d): 3 cultures
    * ckb
    * ckb-IQ
    * ckb-IR
  * MINUS SIGN `−` (\u2212): 38 cultures
    * et
    * et-EE
    * eu
    * eu-ES
    * fi
    * fi-FI
    * fo
    * fo-DK
    * fo-FO
    * gsw
    * gsw-CH
    * gsw-FR
    * gsw-LI
    * hr
    * hr-BA
    * hr-HR
    * ksh
    * ksh-DE
    * lt
    * lt-LT
    * nb
    * nb-NO
    * nb-SJ
    * nn
    * nn-NO
    * no
    * rm
    * rm-CH
    * se
    * se-FI
    * se-NO
    * se-SE
    * sl
    * sl-SI
    * sv
    * sv-AX
    * sv-FI
    * sv-SE
  * LEFT-TO-RIGHT MARK + MINUS SIGN `‎−` (\u200e \u2212): 3 cultures
    * fa
    * fa-AF
    * fa-IR
  * LEFT-TO-RIGHT MARK + HYPHEN-MINUS + LEFT-TO-RIGHT MARK `‎-‎` (\u200e \u002d \u002e): 16 cultures
    * ks
    * ks-Arab
    * ks-Arab-IN
    * lrc
    * lrc-IQ
    * lrc-IR
    * mzn
    * mzn-IR
    * pa-Arab
    * pa-Arab-PK
    * ps
    * ps-AF
    * ps-PK
    * ur-IN
    * uz-Arab
    * uz-Arab-AF

Update `BuildTest2.BuildBasicApplication()` to export
`LANG=sv_SE.UTF-8` as part of the `dotnet build` command to test this
scenario on macOS and Linux.  (This change is ignored on Windows.)
  • Loading branch information
grendello committed Apr 17, 2023
1 parent bff7242 commit b1079a0
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,22 @@ public static string [] SupportedTargetFrameworks ()
}

[Test]
public void BuildBasicApplication ([ValueSource (nameof (SupportedTargetFrameworks))] string tfv, [Values (true, false)] bool isRelease)
public void BuildBasicApplication ([ValueSource (nameof (SupportedTargetFrameworks))] string tfv, [Values (true, false)] bool isRelease, [Values ("", "en_US.UTF-8", "sv_SE.UTF-8")] string langEnvironmentVariable)
{
var proj = new XamarinAndroidApplicationProject {
IsRelease = isRelease,
TargetFrameworkVersion = tfv,
};

Dictionary<string, string> envvar = null;
if (!String.IsNullOrEmpty (langEnvironmentVariable)) {
envvar = new Dictionary<string, string> (StringComparer.OrdinalIgnoreCase) {
{"LANG", langEnvironmentVariable},
};
}

using (var b = CreateApkBuilder ()) {
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
Assert.IsTrue (b.Build (proj, environmentVariables: envvar), "Build should have succeeded.");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;

using Java.Interop.Tools.TypeNameMappings;
Expand Down Expand Up @@ -253,7 +254,7 @@ List<StructureInstance<DSOCacheEntry>> InitDSOCache ()
continue;
}

dsos.Add ((name, $"dsoName{dsos.Count}", ELFHelper.IsEmptyAOTLibrary (log, item.ItemSpec)));
dsos.Add ((name, $"dsoName{dsos.Count.ToString (CultureInfo.InvariantCulture)}", ELFHelper.IsEmptyAOTLibrary (log, item.ItemSpec)));
}

var dsoCache = new List<StructureInstance<DSOCacheEntry>> ();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Text;
using System.Globalization;

namespace Xamarin.Android.Tasks.LLVMIR
{
Expand Down Expand Up @@ -113,7 +114,7 @@ public AlignstackFunctionAttribute (uint powerOfTwoAlignment)

protected override void RenderParams (StringBuilder sb)
{
sb.Append (alignment);
sb.Append (alignment.ToString (CultureInfo.InvariantCulture));
}
}

Expand Down Expand Up @@ -165,13 +166,13 @@ public AllocsizeFunctionAttribute (uint elementSize, uint? numberOfElements = nu

protected override void RenderParams (StringBuilder sb)
{
sb.Append (elementSize);
sb.Append (elementSize.ToString (CultureInfo.InvariantCulture));
if (!numberOfElements.HasValue) {
return;
}

sb.Append (", ");
sb.Append (numberOfElements.Value);
sb.Append (numberOfElements.Value.ToString (CultureInfo.InvariantCulture));
}
}

Expand Down Expand Up @@ -701,13 +702,13 @@ public VscaleRangeFunctionAttribute (uint min, uint? max = null)

protected override void RenderParams (StringBuilder sb)
{
sb.Append (min);
sb.Append (min.ToString (CultureInfo.InvariantCulture));
if (!max.HasValue) {
return;
}

sb.Append (", ");
sb.Append (max.Value);
sb.Append (max.Value.ToString (CultureInfo.InvariantCulture));
}
}

Expand All @@ -721,7 +722,7 @@ public MinLegalVectorWidthFunctionAttribute (uint size)
this.size = size;
}

protected override void RenderAssignedValue (StringBuilder sb) => sb.Append (size);
protected override void RenderAssignedValue (StringBuilder sb) => sb.Append (size.ToString (CultureInfo.InvariantCulture));
}

class StackProtectorBufferSizeFunctionAttribute : LLVMFunctionAttribute
Expand All @@ -734,7 +735,7 @@ public StackProtectorBufferSizeFunctionAttribute (uint size)
this.size = size;
}

protected override void RenderAssignedValue (StringBuilder sb) => sb.Append (size);
protected override void RenderAssignedValue (StringBuilder sb) => sb.Append (size.ToString (CultureInfo.InvariantCulture));
}

class TargetCpuFunctionAttribute : LLVMFunctionAttribute
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Globalization;
using System.Linq;
using System.Text;
using System.Reflection;

namespace Xamarin.Android.Tasks.LLVMIR
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Text;

Expand Down Expand Up @@ -52,7 +53,7 @@ public void WriteFunctionStart (LlvmIrFunction function, string? comment = null)
WriteFunctionParameters (function.Parameters, writeNames: true);
Output.Write(") local_unnamed_addr ");
if (attributes != null) {
Output.Write ($"#{function.AttributeSetID}");
Output.Write ($"#{function.AttributeSetID.ToString (CultureInfo.InvariantCulture)}");
}
Output.WriteLine ();
Output.WriteLine ("{");
Expand Down Expand Up @@ -184,7 +185,7 @@ public void EmitStoreInstruction (LlvmIrFunction function, LlvmIrFunctionLocalVa
CodeRenderType (source);
Output.Write ($" %{source.Name}, ");
CodeRenderType (destination);
Output.WriteLine ($"* {destination.Reference}, align {GetTypeSize (destination.Type)}");
Output.WriteLine ($"* {destination.Reference}, align {GetTypeSize (destination.Type).ToString (CultureInfo.InvariantCulture)}");
}

/// <summary>
Expand All @@ -201,7 +202,7 @@ public LlvmIrFunctionLocalVariable EmitLoadInstruction (LlvmIrFunction function,

string variableType = sb.ToString ();
LlvmIrFunctionLocalVariable result = function.MakeLocalVariable (source, resultVariableName);
Output.WriteLine ($"{function.Indent}%{result.Name} = load {variableType}, {variableType}* @{source.Name}, align {PointerSize}");
Output.WriteLine ($"{function.Indent}%{result.Name} = load {variableType}, {variableType}* @{source.Name}, align {PointerSize.ToString (CultureInfo.InvariantCulture)}");

return result;
}
Expand Down Expand Up @@ -378,7 +379,8 @@ public void EmitLabel (LlvmIrFunction function, string labelName)
Output.Write ("nonnull ");
}

Output.Write ($"align {PointerSize} dereferenceable({PointerSize}) ");
string ptrSize = PointerSize.ToString (CultureInfo.InvariantCulture);
Output.Write ($"align {ptrSize} dereferenceable({ptrSize}) ");

if (argument.Value is LlvmIrVariableReference variableRef) {
bool needBitcast = parameter.Type != argument.Type;
Expand Down Expand Up @@ -409,7 +411,7 @@ public void EmitLabel (LlvmIrFunction function, string labelName)
if (!FunctionAttributes.ContainsKey (AttributeSetID)) {
throw new InvalidOperationException ($"Unknown attribute set ID {AttributeSetID}");
}
Output.Write ($" #{AttributeSetID}");
Output.Write ($" #{AttributeSetID.ToString (CultureInfo.InvariantCulture)}");
}
Output.WriteLine ();

Expand Down Expand Up @@ -538,7 +540,7 @@ void WriteAttributeSets ()

void WriteSet (int id, TextWriter output)
{
output.Write ($"attributes #{id} = {{ ");
output.Write ($"attributes #{id.ToString (CultureInfo.InvariantCulture)} = {{ ");
foreach (LLVMFunctionAttribute attr in FunctionAttributes[id]) {
output.Write (attr.Render ());
output.Write (' ');
Expand Down
Loading

0 comments on commit b1079a0

Please sign in to comment.