NativeAOT: Quote whole commands to make it easier to consume linker args by Xamarin

[filipnavara]

Workaround for xamarin/xamarin-macios#18531. I realise it's fragile, the general integration issues are tracked in #87187.

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Workaround for xamarin/xamarin-macios#18531. I realise it's fragile, the general integration issues are tracked in #87187.

Author:	filipnavara
Assignees:	-
Labels:	community-contribution, area-NativeAOT-coreclr
Milestone:	-

[jkotas]

Is this going to reintroduce problem fixed by #87669 ?

[filipnavara]

Is this going to reintroduce problem fixed by #87669 ?

D'oh. Possibly yes. #87669 was actually merged after the Xamarin code was written which introduced the error there... :-/

[filipnavara]

@MichalStrehovsky Thoughts on how to handle this?

[MichalStrehovsky]

@MichalStrehovsky Thoughts on how to handle this?

I don't know. I don't even understand what the problem was that I fixed in #87669. The quoting is all very confusing even without Xamarin rewriting in the picture. I don't understand the transformation Xamarin is doing there.

It feels like the real solution would be to extract the NativeAOT-common linker args into some separate ItemGroup that Xamarin can consume. I doubt Xamarin wants us to control the RPath to begin with, so that would take care of this specific argument.

[filipnavara]

I don't understand the transformation Xamarin is doing there.

Both ILC and Xamarin linker step have the linker arguments as ItemGroup. The difference is that Xamarin expect to do the quoting itself for each item, while ILC expects the content of each item to be already quoted. Ideally we would unify the approach since the transformation between these two is painful and error prone.

[rolfbjarne]

I don't understand the transformation Xamarin is doing there.

Both ILC and Xamarin linker step have the linker arguments as ItemGroup. The difference is that Xamarin expect to do the quoting itself for each item, while ILC expects the content of each item to be already quoted. Ideally we would unify the approach since the transformation between these two is painful and error prone.

I agree that unifying the approaches for quoting would be desirable.

Using Xamarin's approach will also solve make sure there aren't any issues with paths with spaces (because we don't do any parsing, we only quote when needed).

For instance this doesn't work if there's a space in any of the paths in @(NativeLibrary):

https://github.com/filipnavara/runtime/blob/479daad2b7638f66265d06fe5ad32fa5de9f4b3d/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets#L110

[MichalStrehovsky]

Which LinkerArgs that we have here are actually important for Xamarin?

There's name of the object file (we have that in a property). Then there's the .a libraries that are our implementation details. We have those in an itemgroup. Then there's list of Apple frameworks we need - those are also in itemgroups.

I'm wondering if we could construct the linker command line on the Xamarin side from these, without having to hardcode NativeAOT implementation details (the fact that we depend on "some native libraries" and "some frameworks" from an ItemGroup would be fine to hardcode and we can leave a comment on the ItemGroup that this is now a contract).

We could put the really odd ones like -Wl,-segprot,__THUNKS,rx,rx into a LinkerArgCommon item group that Xamarin would also consume but hopefully there's no quoting problems with those.

[rolfbjarne]

Which LinkerArgs that we have here are actually important for Xamarin?

IMHO it's the other way around: which linker flags does NativeAOT require? I'm assuming all of the linker flags in LinkerArgs - because if there's a LinkerArgs that's not needed, then why's it in LinkerArgs in the first place?

[MichalStrehovsky]

Which LinkerArgs that we have here are actually important for Xamarin?

IMHO it's the other way around: which linker flags does NativeAOT require? I'm assuming all of the linker flags in LinkerArgs - because if there's a LinkerArgs that's not needed, then why's it in LinkerArgs in the first place?

There's a whole category of switches that are needed for compliance - I assume if Xamarin runs the same compliance tooling these would be already present for the non-NativeAOT scenarios and can be reused. Such as:

runtime/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets

Lines 139 to 145 in bb69c0b

	<!-- binskim warning BA3001 PIE disabled on executable -->
	<LinkerArg Include="-pie -Wl,-pie" Condition="'$(_IsApplePlatform)' != 'true' and '$(NativeLib)' == '' and '$(StaticExecutable)' != 'true' and '$(PositionIndependentExecutable)' != 'false'" />
	<LinkerArg Include="-Wl,-no-pie" Condition="'$(_IsApplePlatform)' != 'true' and '$(NativeLib)' == '' and '$(StaticExecutable)' != 'true' and '$(PositionIndependentExecutable)' == 'false'" />
	<!-- binskim warning BA3010 The GNU_RELRO segment is missing -->
	<LinkerArg Include="-Wl,-z,relro" Condition="'$(_IsApplePlatform)' != 'true'" />
	<!-- binskim warning BA3011 The BIND_NOW flag is missing -->
	<LinkerArg Include="-Wl,-z,now" Condition="'$(_IsApplePlatform)' != 'true'" />

Then we have switches that kind of make sense for the embedded to control, since we don't control where native dependencies go to in Xamarin scenarios, like the rpath parameter this PR is touching.

I don't really see much else than these categories. Most of the extra switches are excluded for Apple platforms. Maybe we're really only talking about the categories I enumerated above.

[MichalStrehovsky]

I just realized the compliance ones are also excluded on apple and nobody probably ran binskim on apple executables to see what is the compliance requirement there (if binskim even supports macho)

[MichalStrehovsky]

So I looked at the effective list of things in LinkerArg we're ending up with for a console app (so that I don't need to decypher it from .targets).

Here's the full list that I broke down into categories:

1. Things that are implementation details needed by NativeAOT:

"obj\Release\net8.0\ios-x64\native\NAotHello.o" 
/runtimes/ios-x64/\native\libbootstrapper.o
/runtimes/ios-x64/\native\libRuntime.WorkstationGC.a
/runtimes/ios-x64/\native\libeventpipe-disabled.a
/runtimes/ios-x64/\native\libstdc++compat.a
/runtimes/ios-x64/\native\libSystem.Native.a
/runtimes/ios-x64/\native\libSystem.IO.Compression.Native.a
/runtimes/ios-x64/\native\libSystem.Net.Security.Native.a
/runtimes/ios-x64/\native\libSystem.Security.Cryptography.Native.Apple.a
-ldl
-lobjc
-lz
-licucore
-lm
-framework CoreFoundation
-framework Foundation
-framework Security
-framework GSS
-exported_symbols_list /dev/null
-Wl,-segprot,__THUNKS,rx,rx 

2. Things that Xamarin would want to control and we should not really be passing when invoked as part of Xamarin build

-o "bin\Release\net8.0\ios-x64\native\NAotHello"
-Wl,-rpath,"@executable_path"

3. Things that are common sense and Xamarin would probably already include when people want debuggable code.

-g

So basically we're only talking about category 1 above.

Things in category 1 are mostly already exposed in other properties/ItemGroup. I would be happy to make those part of the official protocol instead of the LinkerArg kitchen sink.

The items are (in order):

$(NativeObject)
@(NativeLibrary)
- need to expose system libraries like dl, objc, z, icucore, m in an item group, so this needs a PR with a fix
@(NativeFramework)
$(ExportsFile)
-Wl,-segprot,__THUNKS,rx,rx

@filipnavara is there any chance the THUNKS could live in the TEXT section so that we don't need the extra linker argument? If not, it's not a huge problem, we could introduce a @(ExtraLinkerArg) itemgroup that would have things like this, everything unquoted, and without the -Wl, prefix. It would be much less of a kitchen sink than LinkerArg that has everything.

It is possible to construct a linker command line from this on the Xamarin side and I think this protocol is much cleaner - it avoids the issue with quoting or the -Wl, prefix.

[filipnavara]

is there any chance the THUNKS could live in the TEXT section so that we don't need the extra linker argument?

Unfortunately not. The DATA and TEXT parts of THUNKS have to be next to each other in the final executable, and there's no way to guarantee that with TEXT section since its contents are linked together from multiple object files.

[rolfbjarne]

Things in category 1 are mostly already exposed in other properties/ItemGroup. I would be happy to make those part of the official protocol instead of the LinkerArg kitchen sink.

That would be great!

[MichalStrehovsky]

Things in category 1 are mostly already exposed in other properties/ItemGroup. I would be happy to make those part of the official protocol instead of the LinkerArg kitchen sink.

That would be great!

Ok, took at shot at this in #89916.

[MichalStrehovsky]

Closing since we shouldn't need this after #89916.

We may also be able to close #87187 and xamarin/xamarin-macios#18531.