- Notifications
You must be signed in to change notification settings - Fork564
[native] Ensure function_name() is part of log message#10302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[native] Ensure function_name() is part of log message#10302
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Context:dotnet/runtime#117733When a .NET for Android app aborts, the abort message should containthe file name, line number, column, and function name of the functionwhich triggered the abort.Insteda, we get the filename, line number, and column, but we get`%s` for the function name: F monodroid: Abort at mono-log-adapter.cc:46:3 ('%s')Update the `log_fatal()` invocation within `mono-log-adapter.cc`to use `{}` instead of `%s`, in accordance with C++ `std::format`.This should cause `sloc.function_name()` to be properly part of theabort message.
jonathanpeppers commentedJul 16, 2025
/azp run |
| Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR fixes a formatting issue in native logging code where function names were not being properly displayed in abort messages. Instead of showing the actual function name, abort messages were displaying the literal string%s.
- Updates the log format string to use C++
std::formatsyntax with{}instead of%s - Ensures that
sloc.function_name()is properly included in abort messages for better debugging
| log_fatal ( | ||
| category, | ||
| "Abort at {}:{}:{} ('%s')", | ||
| "Abort at {}:{}:{} ('{}')", |
CopilotAIJul 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The format string has been updated to use{} for the function name, but there's no corresponding argument being passed for this placeholder. The diff shows the format string change but doesn't show the function_name argument being added to the log_fatal call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I seestd.function_name:
android/src/native/mono/shared/helpers.cc
Lines 36 to 43 in92d48f4
| log_fatal ( | |
| category, | |
| "Abort at {}:{}:{} ('{}')", | |
| file_name, | |
| sloc.line (), | |
| sloc.column (), | |
| sloc.function_name () | |
| ); |
Because this is on multiple lines, I bet the AI didn't actually see the full method...
jonathanpeppers left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The test failures are due to coming from a fork:
'InstallAndroidDependenciesTestGoogleV2' requires a commercial build of .NET for Android.We probably want those to just ignore instead of fail.
fa23828 intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
Context:dotnet/runtime#117733
When a .NET for Android app aborts, the abort message should contain the file name, line number, column, and function name of the function which triggered the abort.
Insteda, we get the filename, line number, and column, but we get
%sfor the function name:Update the
log_fatal()invocation withinmono-log-adapter.ccto use{}instead of%s, in accordance with C++std::format. This should causesloc.function_name()to be properly part of the abort message.