From 1963e3bd40fb9cc13b89c3b31b05d940c9f83e5c Mon Sep 17 00:00:00 2001 From: Jerome Haltom Date: Wed, 20 Mar 2024 10:06:04 -0500 Subject: [PATCH] Add explicit support for separators and multiple values as metadata. This allows us to escape properly. --- src/IKVM.Clang.Sdk.Tasks/ClangExe.cs | 72 +++++++++++++- .../Project/Executable/Executable.clangproj | 1 + .../SharedLibrary1/SharedLibrary1.clangproj | 1 + .../SharedLibrary2/SharedLibrary2.clangproj | 1 + src/IKVM.Clang.Sdk.Tests/ProjectTests.cs | 9 +- .../Sdk/targets/IKVM.Clang.Core.targets | 99 ++++++++++++------- 6 files changed, 141 insertions(+), 42 deletions(-) diff --git a/src/IKVM.Clang.Sdk.Tasks/ClangExe.cs b/src/IKVM.Clang.Sdk.Tasks/ClangExe.cs index 2470671..d808134 100644 --- a/src/IKVM.Clang.Sdk.Tasks/ClangExe.cs +++ b/src/IKVM.Clang.Sdk.Tasks/ClangExe.cs @@ -34,11 +34,81 @@ protected override string GenerateResponseFileCommands() var sb = new StringBuilder(); foreach (var arg in Arguments) - sb.AppendLine(arg.ItemSpec); + { + if (arg.ItemSpec.Length > 0) + { + AppendEscaped(sb, arg.ItemSpec); + + if (arg.GetMetadata("Value") is string value && value.Length > 0) + { + // separate the value + if (arg.GetMetadata("Seperator") is string seperator && seperator.Length > 0) + sb.Append(seperator); + + AppendEscaped(sb, value); + + // additional Value/Seperator metadata + for (int i = 2; i < 128; i++) + { + if (arg.GetMetadata($"Value{i}") is string nextValue && nextValue.Length > 0) + { + // seperate the second value + if (arg.GetMetadata($"Seperator{i}") is string nextSeperator && nextSeperator.Length > 0) + sb.Append(nextSeperator); + + AppendEscaped(sb, nextValue); + } + else + { + // no more values, exit early + break; + } + } + } + + sb.AppendLine(); + } + } return sb.ToString(); } + /// + /// Escapes the specified string if it contains any characters the require escaping. + /// + /// + /// + /// + static StringBuilder AppendEscaped(StringBuilder sb, string value) + { + var ws = false; + + foreach (var c in value) + { + switch (c) + { + case '\"': + sb.Append('\\').Append(c); + break; + case '\\': + sb.Append("\\\\"); + break; + default: + ws |= char.IsWhiteSpace(c); + sb.Append(c); + break; + } + } + + if (ws) + { + sb.Insert(0, '\"'); + sb.Append('\"'); + } + + return sb; + } + } } \ No newline at end of file diff --git a/src/IKVM.Clang.Sdk.Tests/Project/Executable/Executable.clangproj b/src/IKVM.Clang.Sdk.Tests/Project/Executable/Executable.clangproj index f0551b4..824ef29 100644 --- a/src/IKVM.Clang.Sdk.Tests/Project/Executable/Executable.clangproj +++ b/src/IKVM.Clang.Sdk.Tests/Project/Executable/Executable.clangproj @@ -6,6 +6,7 @@ Exe x86_64-pc-windows-msvc;i686-pc-windows-msvc;thumbv7-pc-windows-msvc;aarch64-pc-windows-msvc + true console diff --git a/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary1/SharedLibrary1.clangproj b/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary1/SharedLibrary1.clangproj index 5242d1c..ef66a30 100644 --- a/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary1/SharedLibrary1.clangproj +++ b/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary1/SharedLibrary1.clangproj @@ -6,6 +6,7 @@ Library x86_64-pc-windows-msvc;i686-pc-windows-msvc;thumbv7-pc-windows-msvc;aarch64-pc-windows-msvc + true diff --git a/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary2/SharedLibrary2.clangproj b/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary2/SharedLibrary2.clangproj index 317d1bd..cd05e50 100644 --- a/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary2/SharedLibrary2.clangproj +++ b/src/IKVM.Clang.Sdk.Tests/Project/SharedLibrary2/SharedLibrary2.clangproj @@ -6,6 +6,7 @@ Library x86_64-pc-windows-msvc;i686-pc-windows-msvc;thumbv7-pc-windows-msvc;aarch64-pc-windows-msvc + true diff --git a/src/IKVM.Clang.Sdk.Tests/ProjectTests.cs b/src/IKVM.Clang.Sdk.Tests/ProjectTests.cs index 2c55483..353fd18 100644 --- a/src/IKVM.Clang.Sdk.Tests/ProjectTests.cs +++ b/src/IKVM.Clang.Sdk.Tests/ProjectTests.cs @@ -125,9 +125,9 @@ public static void ClassInitialize(TestContext context) options.TargetsToBuild.Add("Clean"); options.TargetsToBuild.Add("Restore"); options.Arguments.Add("/v:d"); - analyzer.Build(options).OverallSuccess.Should().Be(true); - + var result = analyzer.Build(options); context.AddResultFile(Path.Combine(WorkRoot, "msbuild.binlog")); + result.OverallSuccess.Should().Be(true); } [DataTestMethod] @@ -166,9 +166,10 @@ public void CanBuildTestProject(EnvironmentPreference env, string tid) options.TargetsToBuild.Add("Clean"); options.TargetsToBuild.Add("Build"); options.Arguments.Add("/v:d"); - analyzer.Build(options).OverallSuccess.Should().BeTrue(); - + var result = analyzer.Build(options); TestContext.AddResultFile(Path.Combine(WorkRoot, $"{tid}-msbuild.binlog")); + result.OverallSuccess.Should().BeTrue(); + } } diff --git a/src/IKVM.Clang.Sdk/Sdk/targets/IKVM.Clang.Core.targets b/src/IKVM.Clang.Sdk/Sdk/targets/IKVM.Clang.Core.targets index fa851cd..31162dd 100644 --- a/src/IKVM.Clang.Sdk/Sdk/targets/IKVM.Clang.Core.targets +++ b/src/IKVM.Clang.Sdk/Sdk/targets/IKVM.Clang.Core.targets @@ -418,50 +418,65 @@ <_Args Remove="@(_Args)" /> <_Args Include="-c" /> <_Args Include="-v" Condition=" '$(Verbose)' == 'true' " /> - <_Args Include="--target=$(TargetTriple)" /> + <_Args Include="--target" Value="$(TargetTriple)" Seperator="=" /> <_Args Include="-g" Condition=" '$(_DebugSymbols)' == 'true' " /> - <_Args Include="-std=$(_LanguageStandard)" Condition=" '$(_LanguageStandard)' != '' " /> + <_Args Include="-std" Value="$(_LanguageStandard)" Seperator="=" Condition=" '$(_LanguageStandard)' != '' " /> <_Args Include="-fPIC" Condition=" '$(_PositionIndependentCode)' == 'true' " /> <_Args Include="-fms-compatibility" Condition=" '$(_MsCompatibility)' == 'true' " /> - <_Args Include="-fms-compatibility-version=$(_MsCompatibilityVersion)" Condition=" '$(_MsCompatibilityVersion)' != '' " /> + <_Args Include="-fms-compatibility-version" Value="$(_MsCompatibilityVersion)" Seperator="=" Condition=" '$(_MsCompatibilityVersion)' != '' " /> - <_Args Include="-x;c" Condition=" '$(_Language)' == 'C' " /> - <_Args Include="-x;c++" Condition=" '$(_Language)' == 'C++' " /> + <_Args Include="-xc" Condition=" '$(_Language)' == 'C' " /> + <_Args Include="-xc++" Condition=" '$(_Language)' == 'C++' " /> <_SystemRootDirectories Remove="@(_SystemRootDirectories)" /> <_SystemRootDirectories Include="%(Compile.SystemRootDirectories)" /> <_SystemRootDirectories Include="@(SystemRootDirectories)" /> <_SystemRootDirectories Include="$(SystemRootDirectories)" /> - <_Args Include="@(_SystemRootDirectories->Distinct()->Replace('\', '\\')->Replace('%22', '%5c%22')->'--sysroot "%(Identity)"')" Condition=" '@(_SystemRootDirectories)' != '' " /> + <_SystemRootDirectoriesTemp Remove="@(_SystemRootDirectoriesTemp)" /> + <_SystemRootDirectoriesTemp Include="@(_SystemRootDirectories->Distinct())" /> + <_SystemRootDirectoriesArgs Remove="@(_SystemRootDirectoriesArgs)" /> + <_SystemRootDirectoriesArgs Include="@(_SystemRootDirectoriesTemp->'--sysroot')" Value="%(_SystemRootDirectoriesTemp.Identity)" Seperator=" " /> + <_Args Include="@(_SystemRootDirectoriesArgs)" /> <_IncludeSystemRootDirectories Remove="@(_IncludeSystemRootDirectories)" /> <_IncludeSystemRootDirectories Include="%(Compile.IncludeSystemRootDirectories)" /> <_IncludeSystemRootDirectories Include="@(IncludeSystemRootDirectories)" /> <_IncludeSystemRootDirectories Include="$(IncludeSystemRootDirectories)" /> - <_Args Include="@(_IncludeSystemRootDirectories->Distinct()->Replace('\', '\\')->Replace('%22', '%5c%22')->'-isysroot "%(Identity)"')" Condition=" '@(_IncludeSystemRootDirectories)' != '' " /> + <_IncludeSystemRootDirectoriesTemp Remove="@(_IncludeSystemRootDirectoriesTemp)" /> + <_IncludeSystemRootDirectoriesTemp Include="@(_IncludeSystemRootDirectories->Distinct())" /> + <_IncludeSystemRootDirectoriesArgs Remove="@(_IncludeSystemRootDirectoriesArgs)" /> + <_IncludeSystemRootDirectoriesArgs Include="@(_IncludeSystemRootDirectoriesTemp->'--isysroot')" Value="%(_IncludeSystemRootDirectoriesTemp.Identity)" Seperator=" " /> + <_Args Include="@(_IncludeSystemRootDirectoriesArgs)" /> <_SystemIncludeDirectories Remove="@(_SystemIncludeDirectories)" /> <_SystemIncludeDirectories Include="%(Compile.SystemIncludeDirectories)" /> <_SystemIncludeDirectories Include="@(SystemIncludeDirectories)" /> <_SystemIncludeDirectories Include="$(SystemIncludeDirectories)" /> - <_Args Include="@(_SystemIncludeDirectories->Distinct()->Replace('\', '\\')->Replace('%22', '%5c%22')->'-isystem "%(Identity)"')" Condition=" '@(_SystemIncludeDirectories)' != '' " /> + <_SystemIncludeDirectoriesTemp Remove="@(_SystemIncludeDirectoriesTemp)" /> + <_SystemIncludeDirectoriesTemp Include="@(_SystemIncludeDirectories->Distinct())" /> + <_SystemIncludeDirectoriesArgs Remove="@(_SystemIncludeDirectoriesArgs)" /> + <_SystemIncludeDirectoriesArgs Include="@(_SystemIncludeDirectoriesTemp->'-isystem')" Value="%(_SystemIncludeDirectoriesTemp.Identity)" Seperator=" " /> + <_Args Include="@(_SystemIncludeDirectoriesArgs)" /> <_IncludeDirectories Remove="@(_IncludeDirectories)" /> <_IncludeDirectories Include="%(Compile.IncludeDirectories)" /> <_IncludeDirectories Include="@(IncludeDirectories)" /> <_IncludeDirectories Include="$(IncludeDirectories)" /> <_IncludeDirectories Include="@(ImportedIncludeDirectories)" /> - <_Args Include="@(_IncludeDirectories->Distinct()->Replace('\', '\\')->Replace('%22', '%5c%22')->'-I "%(Identity)"')" Condition=" '@(_IncludeDirectories)' != '' " /> + <_IncludeDirectoriesTemp Remove="@(_IncludeDirectoriesTemp)" /> + <_IncludeDirectoriesTemp Include="@(_IncludeDirectories->Distinct())" /> + <_IncludeDirectoriesArgs Remove="@(_IncludeDirectoriesArgs)" /> + <_IncludeDirectoriesArgs Include="@(_IncludeDirectoriesTemp->'-I')" Value="%(_IncludeDirectoriesTemp.Identity)" Seperator="" /> + <_Args Include="@(_IncludeDirectoriesArgs)" /> <_PreprocessorDefinitions Remove="@(_PreprocessorDefinitions)" /> <_PreprocessorDefinitions Include="%(Compile.PreprocessorDefinitions)" /> <_PreprocessorDefinitions Include="@(PreprocessorDefinitions)" /> <_PreprocessorDefinitions Include="$(PreprocessorDefinitions)" /> - <_PreprocessorDefinitionsEscaped Remove="@(_PreprocessorDefinitionsEscaped)" /> - <_PreprocessorDefinitionsEscaped Include="@(_PreprocessorDefinitions)" EscapedValue="$([System.String]::Copy('%(_PreprocessorDefinitions.Value)').Replace('%22', '%5c%22'))" /> - <_Args Include="@(_PreprocessorDefinitionsEscaped->Distinct()->'-D %(Identity)')" Condition=" '@(_PreprocessorDefinitionsEscaped)' != '' And %(_PreprocessorDefinitionsEscaped.EscapedValue) == '' " /> - <_Args Include="@(_PreprocessorDefinitionsEscaped->Distinct()->'-D %(Identity)=%(EscapedValue)')" Condition=" '@(_PreprocessorDefinitionsEscaped)' != '' And %(_PreprocessorDefinitionsEscaped.EscapedValue) != '' " /> + <_PreprocessorDefinitionsArgs Remove="@(_PreprocessorDefinitionsArgs)" /> + <_PreprocessorDefinitionsArgs Include="@(_PreprocessorDefinitions->'-D')" Value="%(_PreprocessorDefinitions.Identity)" Seperator="" Value2="%(_PreprocessorDefinitions.Value)" Seperator2="=" /> + <_Args Include="@(_PreprocessorDefinitionsArgs)" /> <_AdditionalOptions Remove="@(_AdditionalOptions)" /> <_AdditionalOptions Include="%(Compile.AdditionalCompileOptions)" /> @@ -469,21 +484,17 @@ <_AdditionalOptions Include="$(AdditionalCompileOptions)" /> <_Args Include="@(_AdditionalOptions)" /> - <_SourcePath Remove="@(_SourcePath)" /> - <_SourcePath Include="$(_SourcePath)" /> - <_Args Include="@(_SourcePath->Distinct()->Replace('\', '\\')->'"%(Identity)"')" Condition=" '@(_SourcePath)' != '' " /> + <_Args Include="$(_SourcePath)" /> - <_ObjectPath Remove="@(_ObjectPath)" /> - <_ObjectPath Include="$(_ObjectPath)" /> - <_Args Include="@(_ObjectPath->Distinct()->Replace('\', '\\')->'-o "%(Identity)"')" Condition=" '@(_ObjectPath)' != '' " /> + <_Args Include="-o" Value="$(_ObjectPath)" Seperator=" " /> - + - + - + @@ -511,56 +522,70 @@ <_Args Include="-shared" Condition=" '$(OutputType)' == 'library' " /> <_Args Include="-static" Condition=" '$(OutputType)' == 'staticlibrary' " /> <_Args Include="-v" Condition=" '$(Verbose)' == 'true' " /> - <_Args Include="--target=$(TargetTriple)" /> - <_Args Include="-fuse-ld=lld" Condition=" '$(UseLld)' == 'true' " /> + <_Args Include="--target" Value="$(TargetTriple)" Seperator="=" /> + <_Args Include="-fuse-ld" Value="lld" Seperator="=" Condition=" '$(UseLld)' == 'true' " /> <_Args Include="-g" Condition=" '$(DebugSymbols)' == 'true' " /> - <_Args Include="-Xlinker /subsystem:$(Subsystem)" Condition=" '$(Subsystem)' != '' " /> + <_Args Include="-Wl,/subsystem:$(Subsystem)" Condition=" '$(Subsystem)' != '' " /> <_SystemRootDirectories Remove="@(_SystemRootDirectories)" /> + <_SystemRootDirectories Include="%(Compile.SystemRootDirectories)" /> <_SystemRootDirectories Include="@(SystemRootDirectories)" /> <_SystemRootDirectories Include="$(SystemRootDirectories)" /> - <_Args Include="@(_SystemRootDirectories->Distinct()->Replace('%5c', '%5c%5c')->Replace('%22', '%5c%22')->'--sysroot "%(Identity)"')" Condition=" '@(_SystemRootDirectories)' != '' " /> + <_SystemRootDirectoriesTemp Remove="@(_SystemRootDirectoriesTemp)" /> + <_SystemRootDirectoriesTemp Include="@(_SystemRootDirectories->Distinct())" /> + <_SystemRootDirectoriesArgs Remove="@(_SystemRootDirectoriesArgs)" /> + <_SystemRootDirectoriesArgs Include="@(_SystemRootDirectoriesTemp->'--sysroot')" Value="%(_SystemRootDirectoriesTemp.Identity)" Seperator=" " /> + <_Args Include="@(_SystemRootDirectoriesArgs)" /> <_IncludeSystemRootDirectories Remove="@(_IncludeSystemRootDirectories)" /> + <_IncludeSystemRootDirectories Include="%(Compile.IncludeSystemRootDirectories)" /> <_IncludeSystemRootDirectories Include="@(IncludeSystemRootDirectories)" /> <_IncludeSystemRootDirectories Include="$(IncludeSystemRootDirectories)" /> - <_Args Include="@(_IncludeSystemRootDirectories->Distinct()->Replace('%5c', '%5c%5c')->Replace('%22', '%5c%22')->'-isysroot "%(Identity)"')" Condition=" '@(_IncludeSystemRootDirectories)' != '' " /> + <_IncludeSystemRootDirectoriesTemp Remove="@(_IncludeSystemRootDirectoriesTemp)" /> + <_IncludeSystemRootDirectoriesTemp Include="@(_IncludeSystemRootDirectories->Distinct())" /> + <_IncludeSystemRootDirectoriesArgs Remove="@(_IncludeSystemRootDirectoriesArgs)" /> + <_IncludeSystemRootDirectoriesArgs Include="@(_IncludeSystemRootDirectoriesTemp->'--isysroot')" Value="%(_IncludeSystemRootDirectoriesTemp.Identity)" Seperator=" " /> + <_Args Include="@(_IncludeSystemRootDirectoriesArgs)" /> <_LibraryDirectories Remove="@(_LibraryDirectories)" /> <_LibraryDirectories Include="@(LibraryDirectories)" /> <_LibraryDirectories Include="$(LibraryDirectories)" /> <_LibraryDirectories Include="@(ImportedLibraryDirectories)" /> - <_Args Include="@(_LibraryDirectories->Distinct()->Replace('%5c', '%5c%5c')->Replace('%22', '%5c%22')->'-L "%(Identity)"')" Condition=" '@(_LibraryDirectories)' != '' " /> + <_LibraryDirectoriesTemp Remove="@(_LibraryDirectoriesTemp)" /> + <_LibraryDirectoriesTemp Include="@(_LibraryDirectories->Distinct())" /> + <_LibraryDirectoriesArgs Remove="@(_LibraryDirectoriesArgs)" /> + <_LibraryDirectoriesArgs Include="@(_LibraryDirectoriesTemp->'-L')" Value="%(_LibraryDirectoriesTemp.Identity)" Seperator="" /> + <_Args Include="@(_LibraryDirectoriesArgs)" /> <_Dependencies Remove="@(_Dependencies)" /> <_Dependencies Include="@(Dependencies)" /> <_Dependencies Include="$(Dependencies)" /> <_Dependencies Include="@(ImportedDependencies)" /> - <_Args Include="@(_Dependencies->Distinct()->Replace('%5c', '%5c%5c')->Replace('%22', '%5c%22')->'-l "%(Identity)"')" Condition=" '@(_Dependencies)' != '' " /> + <_DependenciesTemp Remove="@(_DependenciesTemp)" /> + <_DependenciesTemp Include="@(_Dependencies->Distinct())" /> + <_DependenciesArgs Remove="@(_DependenciesArgs)" /> + <_DependenciesArgs Include="@(_DependenciesTemp->'-l')" Value="%(_DependenciesTemp.Identity)" Seperator="" /> + <_Args Include="@(_DependenciesArgs)" /> <_AdditionalOptions Remove="@(_AdditionalOptions)" /> <_AdditionalOptions Include="@(AdditionalLinkOptions)" /> <_AdditionalOptions Include="$(AdditionalLinkOptions)" /> <_Args Include="@(_AdditionalOptions)" /> - <_ObjectPath Remove="@(_ObjectPath)" /> - <_ObjectPath Include="@(Compile->'%(ObjectPath)')" /> - <_Args Include="@(_ObjectPath->Distinct()->Replace('%5c', '%5c%5c')->Replace('%22', '%5c%22')->'"%(Identity)"')" /> + <_Args Include="@(Compile->'%(ObjectPath)')" /> - <_Output Remove="@(_Output)" /> - <_Output Include="$(IntermediateOutputPath)$(TargetFileName)" /> - <_Args Include="@(_Output->Replace('%5c', '%5c%5c')->Replace('%22', '%5c%22')->'-o "%(Identity)"')" /> + <_Args Include="-o" Value="$(IntermediateOutputPath)$(TargetFileName)" Seperator=" " /> - + - +