[android][coreclr] Make coreclr build specify HOST_ANDROID and include Android logging APIs

[elinor-fung]

• Define HOST_ANDROID for coreclr builds and link to Android logging APIs
• Make jitprintf use Android logging APIs if printing to stdout
  • This allows us to get JIT disassembly output via logcat

Copilot wasn't able to review any files in this pull request.

Files not reviewed (3)

• src/coreclr/clrdefinitions.cmake: Language not supported
• src/coreclr/jit/CMakeLists.txt: Language not supported
• src/coreclr/jit/ee_il_dll.cpp: Language not supported

[dotnet-policy-service]

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

[jkoritzinsky]

Can we use HOST_ANDROID here instead of TARGET_ANDROID? That way it won't influence the cross-targeting JIT builds that target Android but don't run on it.

[jakobbotsch]

This is going to mean DOTNET_JitStdOutFile does not work on android. Is there a way to still keep it working?

[jakobbotsch]

There are also a number of other uses of jitstdout() throughout the JIT. Is there no way to reuse what's already there and redirect a file to this log facility instead?

[jakobbotsch]

From some reading around the web:

1. logcat appends its own newlines, so the output is going to look odd (JIT submits lots of separate printf's that are not expected to be printed with newlines)
2. A lot of people suggest https://codelab.wordpress.com/2014/11/03/how-to-use-standard-output-streams-for-logging-in-android-apps/, which might be workable as a more generic solution since we could then just change what jitstdoutInit is doing

However, I am also ok with this solution if you just add a check for jitstdout() == procstdout().

[grendello]

From some reading around the web:

1. logcat appends its own newlines, so the output is going to look odd (JIT submits lots of separate `printf`'s that are not expected to be printed with newlines)

2. A lot of people suggest https://codelab.wordpress.com/2014/11/03/how-to-use-standard-output-streams-for-logging-in-android-apps/, which might be workable as a more generic solution since we could then just change what `jitstdoutInit` is doing

However, I am also ok with this solution if you just add a check for jitstdout() == procstdout().

I don't think we should do that in a framework. This is a disruptive change to the way applications work and the way they expect things to work. The most important part is that the application has no way of fixing it if it proves to be a problem for it, without breaking something else.

The application could, of course, undo the changes (if they knew about them) but that would, in turn, break the runtime logging so there would be no win-win outcome here.

I think such tricks belong in applications and should be the very last resort in frameworks.

[jakobbotsch]

@grendello We would not be making any changes to the process stdout. I agree that is not workable.
We would be changing jitstdout to a new file and then start a thread that redirects anything written to that to the logcat facility.

[jakobbotsch]

Basically something like:

diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp
index ff190a64c75..104a88aa4b2 100644
--- a/src/coreclr/jit/ee_il_dll.cpp
+++ b/src/coreclr/jit/ee_il_dll.cpp
@@ -112,6 +112,18 @@ static FILE* jitstdoutInit()
     }
 #endif // !HOST_UNIX
 
+#ifdef HOST_ANDROID
+    if (file == nullptr)
+    {
+        int fds[2];
+        if (pipe(fds) >= 0)
+        {
+            file = fdopen(fds[1], "w");
+            // start a thread to read whole lines from fds[0] and write them to logcat
+        }
+    }
+#endif
+
     if (file == nullptr)
     {
         file = procstdout();

[grendello]

That's workable, but I wonder - wouldn't it be better to replace all the *printf* output function calls with x*printf* wrappers that would be signature-compatible and do the platform-specific thing in the implementation? In their simplest form, they would live in some header file and be optimized to the original call. For more complex cases, like Android, they would map the call to the __android_*log* equivalent, possibly with some pre-processing. I the case of this PR, for instance, vprintf would become xvprintf and minimize the number of ifdefs at the call site.

[jakobbotsch]

Isn't that what this PR is doing? printf's in the JIT already become jitprintf which is what this PR is changing to go to __android_log. The problem is that it has the downsides pointed out above.

[grendello]

I meant it for the entire runtime, a global change - to expand what this PR is doing (there are a lot of places where *printf* functions are called)

[elinor-fung]

For now, I've left it just changing jitprintf but with the check for jitstdout() == procstdout(). I think that small change now helps with diagnostics/development without precluding a much broader change later.