Skip to content

Improve and modernize logging for Process::CompleteAttach() #82717

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

adrian-prantl
Copy link
Collaborator

Target::SetArchitecture() does not necessarily set the triple that is being passed in, and will unconditionally log the real architecture to the log channel. By flipping the order between the log outputs, the resulting combined log makes a lot more sense to read.

Target::SetArchitecture() does not necessarily set the triple that is
being passed in, and will unconditionally log the real architecture to
the log channel. By flipping the order between the log outputs, the
resulting combined log makes a lot more sense to read.
@adrian-prantl adrian-prantl requested review from jasonmolenda and removed request for JDevlieghere February 23, 2024 01:06
@llvmbot llvmbot added the lldb label Feb 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 23, 2024

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

Target::SetArchitecture() does not necessarily set the triple that is being passed in, and will unconditionally log the real architecture to the log channel. By flipping the order between the log outputs, the resulting combined log makes a lot more sense to read.


Full diff: https://github.com/llvm/llvm-project/pull/82717.diff

1 Files Affected:

  • (modified) lldb/source/Target/Process.cpp (+4-7)
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 23a8a66645c02d..137795cb8cec9e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2937,14 +2937,11 @@ void Process::CompleteAttach() {
   DidAttach(process_arch);
 
   if (process_arch.IsValid()) {
+    LLDB_LOG(log,
+             "Process::{0} replacing process architecture with DidAttach() "
+             "architecture: \"{1}\"",
+             __FUNCTION__, process_arch.GetTriple().getTriple());
     GetTarget().SetArchitecture(process_arch);
-    if (log) {
-      const char *triple_str = process_arch.GetTriple().getTriple().c_str();
-      LLDB_LOGF(log,
-                "Process::%s replacing process architecture with DidAttach() "
-                "architecture: %s",
-                __FUNCTION__, triple_str ? triple_str : "<null>");
-    }
   }
 
   // We just attached.  If we have a platform, ask it for the process

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adrian-prantl adrian-prantl merged commit 55bc048 into llvm:main Feb 23, 2024
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request Feb 23, 2024
Target::SetArchitecture() does not necessarily set the triple that is
being passed in, and will unconditionally log the real architecture to
the log channel. By flipping the order between the log outputs, the
resulting combined log makes a lot more sense to read.

(cherry picked from commit 55bc048)
adrian-prantl added a commit to swiftlang/llvm-project that referenced this pull request Feb 26, 2024
…25-Improve-and-modernize-logging-for-Process-CompleteAttach-82717

[Cherry-pick into stable/20230725] Improve and modernize logging for Process::CompleteAttach() (llvm#82717)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants