[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers
ljmf00 added a comment. Meanwhile, a commit will land Linux stable branches soon to fix this issue: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=44cad52cc14ae10062f142ec16ede489bccf4469 . Do we have an alternative way to fix this, e.g. a custom XFAIL flag for specific linux versions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117928/new/ https://reviews.llvm.org/D117928 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D120595: [WIP][trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces
jj10306 marked 21 inline comments as done.
jj10306 added inline comments.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:51
+ int64_t m_time_shift;
+ int64_t m_time_zero;
+};
wallace wrote:
> jj10306 wrote:
> > What is the suggested way to serialize a u64 into JSON? I thought of two
> > approaches:
> > 1. Store two separate u32
> > 2. store the integer as a string
> >
> > @wallace wdyt?
> you don't need to use int64_t here. You can use the real types here and do
> the conversion in the fromJson and toJson methods
> What is the suggested way to serialize a u64 into JSON? I thought of two
> approaches:
> 1. Store two separate u32
> 2. store the integer as a string
>
> @wallace wdyt?
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:64-66
+ PerfTscConverter() = default;
+ PerfTscConverter(uint32_t time_mult, uint16_t time_shift, uint64_t
time_zero) :
+m_perf_conversion{time_mult, time_shift, static_cast(time_zero)}
{}
wallace wrote:
> you can get rid of the constructors a
Need the 3 arg constructor to create an instance of this class in
`IntelPTManager.cpp` and need the default constructor for when we create an
uninitialized instance of the class before mapping the values from JSON in the
`fromJSON` method.
Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.cpp:45
+std::shared_ptr IntelPTManager::tsc_converter = nullptr;
+
wallace wrote:
> remove it from here
Without a definition of the static member outside of the class, a linker error
occurs.
Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.h:238
+ static std::shared_ptr tsc_converter;
+
wallace wrote:
> static llvm::Optional g_tsc_rate; // by default this
> is None
Is the idea of wrapping the SP in an Optional here allows us to represent 3
different states?
1. `None`: we have not yet attempted to fetch/calculate the tsc rate.
2. `nullptr`: we tried to fetch the rate but one does not exist
3. ``: the rate that was successfully fetched
Without the Optional, we cannot distinguish between the first two cases.
Please let me know if my understanding is correct.
Comment at: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp:69
+ }
+ return ret;
+}
wallace wrote:
> return false; if you got here, then you have a kind that is not available
in this case should we also set the SP value to be nullptr? Otherwise that
value will still be None which, from my understanding, indicates that the
TscRate has not yet been fetched whereas nullptr indicates that the we have
tried to fetch the rate but it does not exist.
Comment at: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp:74
+ ObjectMapper o(value, path);
+ bool base_bool = o && fromJSON(value, (TraceGetStateResponse &)packet, path)
&& o.map("tsc_conversion", packet.tsc_converter);
+ // call perftscconverter
wallace wrote:
> o.mapOptional is what you need here. That will return true if
> "tsc_conversion" is not available or if it's well-formed.
{F22265358}
From looking at the source of map and mapOptional it appears that the only
difference is that map sets Out to None whereas mapOptional leaves it
unchanged, they both return true in the case that the property doesn't exist.
Given this, it's not immediately obvious to me which we should be using in this
case - do we want the tsc_conversion to be set to None or left unchanged in the
case that it's not present in the JSON?
Comment at: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp:84-86
+{"time_mult",m_perf_conversion.m_time_mult},
+{"time_shift", m_perf_conversion.m_time_shift},
+{"time_zero", m_perf_conversion.m_time_zero},
wallace wrote:
> here you should cast to int64_t
How should we handle serializing `m_time_zero` since it's `uint64_t` and the
JSON library only supports int64_t for number types?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120595/new/
https://reviews.llvm.org/D120595
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D120595: [WIP][trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces
jj10306 updated this revision to Diff 411862.
jj10306 marked 3 inline comments as done.
jj10306 edited the summary of this revision.
jj10306 added a comment.
Address comments, still need to handle uint64_t JSON serialization correctly. I
plan to proceed with decoder and schema changes now that initial feedback has
been received and addressed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120595/new/
https://reviews.llvm.org/D120595
Files:
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
lldb/source/Plugins/Process/Linux/IntelPTManager.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -7,6 +7,7 @@
//===--===//
#include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
+#include "lldb/Utility/TraceGDBRemotePackets.h"
using namespace llvm;
using namespace llvm::json;
@@ -43,4 +44,51 @@
return base;
}
+uint64_t PerfTimestampCounterRate::ToNanos(uint64_t tsc) {
+ // conversion logic here
+ return 1;
+}
+
+bool fromJSON(const llvm::json::Value &value, TimestampCounterRateSP &tsc_rate, llvm::json::Path path) {
+ std::string tsc_rate_kind;
+ ObjectMapper o(value, path);
+
+ if(!o.map("kind", tsc_rate_kind))
+return false;
+
+ if (tsc_rate_kind == "perf") {
+int a, b, c;
+if (!o.map("a", a) || !o.map("b", b) || !o.map("c", c))
+ return false;
+tsc_rate = std::make_shared((uint32_t)a, (uint16_t)b, (uint64_t)c);
+ }
+ return false;
+}
+
+bool fromJSON(const json::Value &value, TraceIntelPTGetStateResponse &packet, Path path) {
+ ObjectMapper o(value, path);
+ bool base_bool = o && fromJSON(value, (TraceGetStateResponse &)packet, path) && o.mapOptional("tsc_rate", packet.tsc_rate);
+ return base_bool;
+
+}
+
+llvm::json::Value PerfTimestampCounterRate::toJSON() {
+ return json::Value(json::Object{
+{"kind", "perf"},
+{"time_mult", (int64_t) m_time_mult},
+{"time_shift", (int64_t) m_time_shift},
+{"time_zero", (int64_t) m_time_zero}, // TODO: handle the potential lossy conversion correctly
+ });
+}
+
+json::Value toJSON(const TraceIntelPTGetStateResponse &packet) {
+ json::Value base = toJSON((const TraceGetStateResponse &)packet);
+ auto tsc_rate = packet.tsc_rate;
+ // is there a better way to check that it's not None nor nullptr?
+ if (tsc_rate && *tsc_rate) {
+json::Value tsc_converter_json = tsc_rate.getValue()->toJSON();
+base.getAsObject()->try_emplace("tsc_rate", std::move(tsc_converter_json));
+ }
+ return base;
+}
} // namespace lldb_private
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -18,6 +18,7 @@
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/Thread.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/Utility/TraceGDBRemotePackets.h"
using namespace lldb;
using namespace lldb_private;
@@ -187,28 +188,17 @@
m_stop_id = new_stop_id;
m_live_thread_data.clear();
- Expected json_string = GetLiveProcessState();
- if (!json_string) {
-DoRefreshLiveProcessState(json_string.takeError());
-return;
- }
- Expected live_process_state =
- json::parse(*json_string, "TraceGetStateResponse");
- if (!live_process_state) {
-DoRefreshLiveProcessState(live_process_state.takeError());
-return;
- }
+ if (std::unique_ptr live_process_state = DoRefreshLiveProcessState(GetLiveProcessState())) {
+for (const TraceThreadState &thread_state :
+live_process_state->tracedThreads) {
+ for (const TraceBinaryData &item : thread_state.binaryData)
+m_live_thread_data[thread_state.tid][item.kind] = item.size;
+}
- for (const TraceThreadState &thread_state :
- live_process_state->tracedThreads) {
-for (const TraceBinaryData &item : thread_state.binaryData)
- m_live_thread_data[thread_state.tid][item.kind] = item.size;
+for (const TraceBinaryData &item : live_process_state->processBinaryData)
+ m_live_process_data[item.kind] = item.size;
}
- for (const TraceBinaryData &item : live_process_state->processBinaryData)
-m_live_process_data[item.kind] = item.size;
-
- DoRefreshLiveProcessState(std::move(live_process_state));
}
uint32_t Trace::GetStopID() {
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT
[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables
zequanwu updated this revision to Diff 411898.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.
Remove unnecessary `location_list_base_addr` parameter from `GetDescription()`
as it doesn't do filtering when dumping.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119963/new/
https://reviews.llvm.org/D119963
Files:
lldb/include/lldb/Core/Address.h
lldb/include/lldb/Expression/DWARFExpression.h
lldb/include/lldb/Symbol/Variable.h
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/Options.td
lldb/source/Core/Address.cpp
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Symbol/Variable.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_loclists_base.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc_and_loclists.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwo.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
Index: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
+++ lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
@@ -14,35 +14,35 @@
# at the inlined function entry.
image lookup -v -s break_at_inlined_f_in_main
# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main
-# CHECK: name = "unused1", type = "void *", location =
-# CHECK: name = "used", type = "int", location = DW_OP_consts +42
-# CHECK: name = "unused2", type = "int", location =
-# CHECK: name = "partial", type = "int", location = DW_OP_reg4 RSI
-# CHECK: name = "unused3", type = "int", location =
+# CHECK: name = "unused1", type = "void *", valid ranges = , location =
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0011, 0x0014) -> DW_OP_consts +42
+# CHECK: name = "unused2", type = "int", valid ranges = , location =
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0011, 0x0019) -> DW_OP_reg4 RSI
+# CHECK: name = "unused3", type = "int", valid ranges = , location =
# Show variables outsid of the live range of the 'partial' parameter
# and verify that the output is as expected.
image lookup -v -s break_at_inlined_f_in_main_between_printfs
# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main_between_printfs
-# CHECK: name = "unused1", type = "void *", location =
-# CHECK: name = "used", type = "int", location = DW_OP_reg3 RBX
-# CHECK: name = "unused2", type = "int", location =
+# CHECK: name = "unused1", type = "void *", valid ranges = , location =
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0014, 0x001e) -> DW_OP_reg3 RBX
+# CHECK: name = "unused2", type = "int", valid ranges = , location =
# Note: image lookup does not show variables outside of their
# location, so |partial| is missing here.
# CHECK-NOT: partial
-# CHECK: name = "unused3", type = "int", location =
+# CHECK: name = "unused3", type = "int", valid ranges = , location =
# Check that we show parameters even if all of them are compiled away.
image lookup -v -s break_at_inlined_g_in_main
# CHECK-LABEL: image lookup -v -s break_at_inlined_g_in_main
-# CHECK: name = "unused", type = "int", location =
+# CHECK: name = "unused", type = "int", valid ranges = , location =
# Check that even the other inlined instance of f displays the correct
# parameters.
image lookup -v -s break_at_inlined_f_in_other
# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_other
-# CHECK: name = "unused1", type = "void *", location =
-# CHECK: name = "used", type = "int", location = DW_OP_consts +1
-# CHECK: name = "unused2", type = "int", location =
-# CHECK: name = "partial", type = "int", location = DW_OP_consts +2
-# CHECK: name = "unused3", type = "int", location =
+# CHECK: name = "unused1", type = "void *", valid ranges = , location =
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0001, 0x000b) -> DW_OP_consts +1
+# CHECK: name = "unused2", type = "int", valid ranges = , location =
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0001, 0x0006) -> DW_OP_consts +2
+# CHECK: name = "unused3", type = "int", valid ranges = , location =
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -12,7 +12,7 @@
# CHECK-LABEL: image lookup -v -n F1
# CHECK: CompileUnit: id = {0x0001}, file = "1.c", language = ""
# CHECK: Function: {{.*}}, name = "F1", range = [0x0001-0x0002)
-# C
[Lldb-commits] [lldb] a83cf7a - [Support] Allow the ability to change WithColor's auto detection function
Author: Jonas Devlieghere
Date: 2022-02-28T15:03:04-08:00
New Revision: a83cf7a84628a9e3a24cfd33c69f786cf74df4ec
URL:
https://github.com/llvm/llvm-project/commit/a83cf7a84628a9e3a24cfd33c69f786cf74df4ec
DIFF:
https://github.com/llvm/llvm-project/commit/a83cf7a84628a9e3a24cfd33c69f786cf74df4ec.diff
LOG: [Support] Allow the ability to change WithColor's auto detection function
WithColor has an "auto detection mode" which looks whether the
corresponding whether the corresponding cl::opt is enabled or not. While
this is great when opting into cl::opt, it's not so great for downstream
users of this utility, which might have their own competing options to
enable or disable colors. The WithColor constructor takes a color mode,
but the big benefit of the class are its static error and warning
helpers and default error handlers.
In order to allow users of this utility to enable or disable colors
globally, this patch adds the ability to specify a global auto detection
function. By default, the auto detection function behaves the way that
it does today. The benefit of this patch lies in that it can be
overwritten. In addition to a ability to change the auto detection
function, I've also made it possible to get your hands on the default
auto detection function, so you swap it back if if you so desire.
This patch allow downstream users (like LLDB) to globally disable colors
with its own command line flag.
Differential revision: https://reviews.llvm.org/D120593
Added:
Modified:
lldb/tools/driver/Driver.cpp
llvm/include/llvm/Support/WithColor.h
llvm/lib/Support/WithColor.cpp
Removed:
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 31407be200c0e..53eaf893b06e7 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -186,6 +186,13 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args,
bool &exiting) {
m_debugger.SkipLLDBInitFiles(false);
m_debugger.SkipAppInitFiles(false);
+ if (args.hasArg(OPT_no_use_colors)) {
+m_debugger.SetUseColor(false);
+WithColor::setAutoDetectFunction(
+[](const llvm::raw_ostream &OS) { return false; });
+m_option_data.m_debug_mode = true;
+ }
+
if (args.hasArg(OPT_version)) {
m_option_data.m_print_version = true;
}
@@ -227,11 +234,6 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args,
bool &exiting) {
m_debugger.GetInstanceName());
}
- if (args.hasArg(OPT_no_use_colors)) {
-m_debugger.SetUseColor(false);
-m_option_data.m_debug_mode = true;
- }
-
if (auto *arg = args.getLastArg(OPT_file)) {
auto arg_value = arg->getValue();
SBFileSpec file(arg_value);
diff --git a/llvm/include/llvm/Support/WithColor.h
b/llvm/include/llvm/Support/WithColor.h
index e772ea667f4f6..2e8e52dc88ae3 100644
--- a/llvm/include/llvm/Support/WithColor.h
+++ b/llvm/include/llvm/Support/WithColor.h
@@ -11,6 +11,8 @@
#include "llvm/Support/raw_ostream.h"
+#include
+
namespace llvm {
class Error;
@@ -54,6 +56,9 @@ class WithColor {
raw_ostream &OS;
ColorMode Mode;
+ using AutoDetectFunctionType = std::function;
+ static AutoDetectFunctionType AutoDetectFunction;
+
public:
/// To be used like this: WithColor(OS, HighlightColor::String) << "text";
/// @param OS The output stream
@@ -132,6 +137,13 @@ class WithColor {
/// Implement default handling for Warning.
/// Print "warning: " to stderr.
static void defaultWarningHandler(Error Warning);
+
+ /// Retrieve the default color auto detection function.
+ static AutoDetectFunctionType defaultAutoDetectFunction();
+
+ /// Change the global auto detection function.
+ static void
+ setAutoDetectFunction(AutoDetectFunctionType NewAutoDetectFunction);
};
} // end namespace llvm
diff --git a/llvm/lib/Support/WithColor.cpp b/llvm/lib/Support/WithColor.cpp
index b1aa709862d86..641ba2a616f31 100644
--- a/llvm/lib/Support/WithColor.cpp
+++ b/llvm/lib/Support/WithColor.cpp
@@ -33,6 +33,9 @@ struct CreateUseColor {
static ManagedStatic, CreateUseColor> UseColor;
void llvm::initWithColorOptions() { *UseColor; }
+WithColor::AutoDetectFunctionType WithColor::AutoDetectFunction =
+WithColor::defaultAutoDetectFunction();
+
WithColor::WithColor(raw_ostream &OS, HighlightColor Color, ColorMode Mode)
: OS(OS), Mode(Mode) {
// Detect color from terminal type unless the user passed the --color option.
@@ -127,8 +130,7 @@ bool WithColor::colorsEnabled() {
case ColorMode::Disable:
return false;
case ColorMode::Auto:
-return *UseColor == cl::BOU_UNSET ? OS.has_colors()
- : *UseColor == cl::BOU_TRUE;
+return AutoDetectFunction(OS);
}
llvm_unreachable("All cases handled above.");
}
@@ -159,3 +161,15 @@ void WithColor::defaultWarningHandler(Error Warning) {
WithColor::warning() <
[Lldb-commits] [PATCH] D120593: [Support] Allow the ability to change WithColor's auto detection function
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa83cf7a84628: [Support] Allow the ability to change
WithColor's auto detection function (authored by JDevlieghere).
Herald added a project: LLDB.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120593/new/
https://reviews.llvm.org/D120593
Files:
lldb/tools/driver/Driver.cpp
llvm/include/llvm/Support/WithColor.h
llvm/lib/Support/WithColor.cpp
Index: llvm/lib/Support/WithColor.cpp
===
--- llvm/lib/Support/WithColor.cpp
+++ llvm/lib/Support/WithColor.cpp
@@ -33,6 +33,9 @@
static ManagedStatic, CreateUseColor> UseColor;
void llvm::initWithColorOptions() { *UseColor; }
+WithColor::AutoDetectFunctionType WithColor::AutoDetectFunction =
+WithColor::defaultAutoDetectFunction();
+
WithColor::WithColor(raw_ostream &OS, HighlightColor Color, ColorMode Mode)
: OS(OS), Mode(Mode) {
// Detect color from terminal type unless the user passed the --color option.
@@ -127,8 +130,7 @@
case ColorMode::Disable:
return false;
case ColorMode::Auto:
-return *UseColor == cl::BOU_UNSET ? OS.has_colors()
- : *UseColor == cl::BOU_TRUE;
+return AutoDetectFunction(OS);
}
llvm_unreachable("All cases handled above.");
}
@@ -159,3 +161,15 @@
WithColor::warning() << Info.message() << '\n';
});
}
+
+WithColor::AutoDetectFunctionType WithColor::defaultAutoDetectFunction() {
+ return [](const raw_ostream &OS) {
+return *UseColor == cl::BOU_UNSET ? OS.has_colors()
+ : *UseColor == cl::BOU_TRUE;
+ };
+}
+
+void WithColor::setAutoDetectFunction(
+AutoDetectFunctionType NewAutoDetectFunction) {
+ AutoDetectFunction = std::move(NewAutoDetectFunction);
+}
Index: llvm/include/llvm/Support/WithColor.h
===
--- llvm/include/llvm/Support/WithColor.h
+++ llvm/include/llvm/Support/WithColor.h
@@ -11,6 +11,8 @@
#include "llvm/Support/raw_ostream.h"
+#include
+
namespace llvm {
class Error;
@@ -54,6 +56,9 @@
raw_ostream &OS;
ColorMode Mode;
+ using AutoDetectFunctionType = std::function;
+ static AutoDetectFunctionType AutoDetectFunction;
+
public:
/// To be used like this: WithColor(OS, HighlightColor::String) << "text";
/// @param OS The output stream
@@ -132,6 +137,13 @@
/// Implement default handling for Warning.
/// Print "warning: " to stderr.
static void defaultWarningHandler(Error Warning);
+
+ /// Retrieve the default color auto detection function.
+ static AutoDetectFunctionType defaultAutoDetectFunction();
+
+ /// Change the global auto detection function.
+ static void
+ setAutoDetectFunction(AutoDetectFunctionType NewAutoDetectFunction);
};
} // end namespace llvm
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -186,6 +186,13 @@
m_debugger.SkipLLDBInitFiles(false);
m_debugger.SkipAppInitFiles(false);
+ if (args.hasArg(OPT_no_use_colors)) {
+m_debugger.SetUseColor(false);
+WithColor::setAutoDetectFunction(
+[](const llvm::raw_ostream &OS) { return false; });
+m_option_data.m_debug_mode = true;
+ }
+
if (args.hasArg(OPT_version)) {
m_option_data.m_print_version = true;
}
@@ -227,11 +234,6 @@
m_debugger.GetInstanceName());
}
- if (args.hasArg(OPT_no_use_colors)) {
-m_debugger.SetUseColor(false);
-m_option_data.m_debug_mode = true;
- }
-
if (auto *arg = args.getLastArg(OPT_file)) {
auto arg_value = arg->getValue();
SBFileSpec file(arg_value);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3a167c4 - Revert "[Support] Allow the ability to change WithColor's auto detection function"
Author: Jonas Devlieghere
Date: 2022-02-28T15:32:15-08:00
New Revision: 3a167c4a90b0352ff3cca576392882e45b70dc36
URL:
https://github.com/llvm/llvm-project/commit/3a167c4a90b0352ff3cca576392882e45b70dc36
DIFF:
https://github.com/llvm/llvm-project/commit/3a167c4a90b0352ff3cca576392882e45b70dc36.diff
LOG: Revert "[Support] Allow the ability to change WithColor's auto detection
function"
This reverts commit a83cf7a84628a9e3a24cfd33c69f786cf74df4ec because it
breaks a bunch of build bots.
Added:
Modified:
lldb/tools/driver/Driver.cpp
llvm/include/llvm/Support/WithColor.h
llvm/lib/Support/WithColor.cpp
Removed:
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 53eaf893b06e7..31407be200c0e 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -186,13 +186,6 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args,
bool &exiting) {
m_debugger.SkipLLDBInitFiles(false);
m_debugger.SkipAppInitFiles(false);
- if (args.hasArg(OPT_no_use_colors)) {
-m_debugger.SetUseColor(false);
-WithColor::setAutoDetectFunction(
-[](const llvm::raw_ostream &OS) { return false; });
-m_option_data.m_debug_mode = true;
- }
-
if (args.hasArg(OPT_version)) {
m_option_data.m_print_version = true;
}
@@ -234,6 +227,11 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args,
bool &exiting) {
m_debugger.GetInstanceName());
}
+ if (args.hasArg(OPT_no_use_colors)) {
+m_debugger.SetUseColor(false);
+m_option_data.m_debug_mode = true;
+ }
+
if (auto *arg = args.getLastArg(OPT_file)) {
auto arg_value = arg->getValue();
SBFileSpec file(arg_value);
diff --git a/llvm/include/llvm/Support/WithColor.h
b/llvm/include/llvm/Support/WithColor.h
index 2e8e52dc88ae3..e772ea667f4f6 100644
--- a/llvm/include/llvm/Support/WithColor.h
+++ b/llvm/include/llvm/Support/WithColor.h
@@ -11,8 +11,6 @@
#include "llvm/Support/raw_ostream.h"
-#include
-
namespace llvm {
class Error;
@@ -56,9 +54,6 @@ class WithColor {
raw_ostream &OS;
ColorMode Mode;
- using AutoDetectFunctionType = std::function;
- static AutoDetectFunctionType AutoDetectFunction;
-
public:
/// To be used like this: WithColor(OS, HighlightColor::String) << "text";
/// @param OS The output stream
@@ -137,13 +132,6 @@ class WithColor {
/// Implement default handling for Warning.
/// Print "warning: " to stderr.
static void defaultWarningHandler(Error Warning);
-
- /// Retrieve the default color auto detection function.
- static AutoDetectFunctionType defaultAutoDetectFunction();
-
- /// Change the global auto detection function.
- static void
- setAutoDetectFunction(AutoDetectFunctionType NewAutoDetectFunction);
};
} // end namespace llvm
diff --git a/llvm/lib/Support/WithColor.cpp b/llvm/lib/Support/WithColor.cpp
index 641ba2a616f31..b1aa709862d86 100644
--- a/llvm/lib/Support/WithColor.cpp
+++ b/llvm/lib/Support/WithColor.cpp
@@ -33,9 +33,6 @@ struct CreateUseColor {
static ManagedStatic, CreateUseColor> UseColor;
void llvm::initWithColorOptions() { *UseColor; }
-WithColor::AutoDetectFunctionType WithColor::AutoDetectFunction =
-WithColor::defaultAutoDetectFunction();
-
WithColor::WithColor(raw_ostream &OS, HighlightColor Color, ColorMode Mode)
: OS(OS), Mode(Mode) {
// Detect color from terminal type unless the user passed the --color option.
@@ -130,7 +127,8 @@ bool WithColor::colorsEnabled() {
case ColorMode::Disable:
return false;
case ColorMode::Auto:
-return AutoDetectFunction(OS);
+return *UseColor == cl::BOU_UNSET ? OS.has_colors()
+ : *UseColor == cl::BOU_TRUE;
}
llvm_unreachable("All cases handled above.");
}
@@ -161,15 +159,3 @@ void WithColor::defaultWarningHandler(Error Warning) {
WithColor::warning() << Info.message() << '\n';
});
}
-
-WithColor::AutoDetectFunctionType WithColor::defaultAutoDetectFunction() {
- return [](const raw_ostream &OS) {
-return *UseColor == cl::BOU_UNSET ? OS.has_colors()
- : *UseColor == cl::BOU_TRUE;
- };
-}
-
-void WithColor::setAutoDetectFunction(
-AutoDetectFunctionType NewAutoDetectFunction) {
- AutoDetectFunction = std::move(NewAutoDetectFunction);
-}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4429cf1 - [Support] Allow the ability to change WithColor's auto detection function
Author: Jonas Devlieghere
Date: 2022-02-28T20:30:06-08:00
New Revision: 4429cf146e8ad393ec1d4e6bddab73b7e53957d2
URL:
https://github.com/llvm/llvm-project/commit/4429cf146e8ad393ec1d4e6bddab73b7e53957d2
DIFF:
https://github.com/llvm/llvm-project/commit/4429cf146e8ad393ec1d4e6bddab73b7e53957d2.diff
LOG: [Support] Allow the ability to change WithColor's auto detection function
WithColor has an "auto detection mode" which looks whether the
corresponding whether the corresponding cl::opt is enabled or not. While
this is great when opting into cl::opt, it's not so great for downstream
users of this utility, which might have their own competing options to
enable or disable colors. The WithColor constructor takes a color mode,
but the big benefit of the class are its static error and warning
helpers and default error handlers.
In order to allow users of this utility to enable or disable colors
globally, this patch adds the ability to specify a global auto detection
function. By default, the auto detection function behaves the way that
it does today. The benefit of this patch lies in that it can be
overwritten. In addition to a ability to change the auto detection
function, I've also made it possible to get your hands on the default
auto detection function, so you swap it back if if you so desire.
This patch allow downstream users (like LLDB) to globally disable colors
with its own command line flag.
Differential revision: https://reviews.llvm.org/D120593
Added:
Modified:
lldb/tools/driver/Driver.cpp
llvm/include/llvm/Support/WithColor.h
llvm/lib/Support/WithColor.cpp
Removed:
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 31407be200c0e..4540f06bebdb4 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -86,6 +86,8 @@ static void reset_stdin_termios();
static bool g_old_stdin_termios_is_valid = false;
static struct termios g_old_stdin_termios;
+static bool disable_color(const raw_ostream &OS) { return false; }
+
static Driver *g_driver = nullptr;
// In the Driver::MainLoop, we change the terminal settings. This function is
@@ -186,6 +188,12 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args,
bool &exiting) {
m_debugger.SkipLLDBInitFiles(false);
m_debugger.SkipAppInitFiles(false);
+ if (args.hasArg(OPT_no_use_colors)) {
+m_debugger.SetUseColor(false);
+WithColor::setAutoDetectFunction(disable_color);
+m_option_data.m_debug_mode = true;
+ }
+
if (args.hasArg(OPT_version)) {
m_option_data.m_print_version = true;
}
@@ -227,11 +235,6 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args,
bool &exiting) {
m_debugger.GetInstanceName());
}
- if (args.hasArg(OPT_no_use_colors)) {
-m_debugger.SetUseColor(false);
-m_option_data.m_debug_mode = true;
- }
-
if (auto *arg = args.getLastArg(OPT_file)) {
auto arg_value = arg->getValue();
SBFileSpec file(arg_value);
diff --git a/llvm/include/llvm/Support/WithColor.h
b/llvm/include/llvm/Support/WithColor.h
index e772ea667f4f6..b249f34da1fa6 100644
--- a/llvm/include/llvm/Support/WithColor.h
+++ b/llvm/include/llvm/Support/WithColor.h
@@ -51,10 +51,9 @@ enum class ColorMode {
/// An RAII object that temporarily switches an output stream to a specific
/// color.
class WithColor {
- raw_ostream &OS;
- ColorMode Mode;
-
public:
+ using AutoDetectFunctionType = bool (*)(const raw_ostream &OS);
+
/// To be used like this: WithColor(OS, HighlightColor::String) << "text";
/// @param OS The output stream
/// @param S Symbolic name for syntax element to color
@@ -132,6 +131,19 @@ class WithColor {
/// Implement default handling for Warning.
/// Print "warning: " to stderr.
static void defaultWarningHandler(Error Warning);
+
+ /// Retrieve the default color auto detection function.
+ static AutoDetectFunctionType defaultAutoDetectFunction();
+
+ /// Change the global auto detection function.
+ static void
+ setAutoDetectFunction(AutoDetectFunctionType NewAutoDetectFunction);
+
+private:
+ raw_ostream &OS;
+ ColorMode Mode;
+
+ static AutoDetectFunctionType AutoDetectFunction;
};
} // end namespace llvm
diff --git a/llvm/lib/Support/WithColor.cpp b/llvm/lib/Support/WithColor.cpp
index b1aa709862d86..abc9fb3e5d606 100644
--- a/llvm/lib/Support/WithColor.cpp
+++ b/llvm/lib/Support/WithColor.cpp
@@ -33,6 +33,14 @@ struct CreateUseColor {
static ManagedStatic, CreateUseColor> UseColor;
void llvm::initWithColorOptions() { *UseColor; }
+static bool DefaultAutoDetectFunction(const raw_ostream &OS) {
+ return *UseColor == cl::BOU_UNSET ? OS.has_colors()
+: *UseColor == cl::BOU_TRUE;
+}
+
+WithColor::AutoDetectFunctionType WithColor::AutoDetectFunction =
+DefaultAutoDetectFunction;
+
WithColor::WithColor(raw_os
[Lldb-commits] [PATCH] D120595: [WIP][trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
in terms of functionality, everything is right, but you need to do some
improvements
Comment at: lldb/docs/lldb-gdb-remote.txt:454-479
+//}],
+//... other parameters specific to the provided tracing type
// }
//
// NOTES
// - "traceThreads" includes all thread traced by both "process tracing" and
// "thread tracing".
you have to improve the documentation and formatting following the existing
documentation's format
Comment at: lldb/include/lldb/Target/Trace.h:306
+ /// \param[in] json_string
+ /// String representation of the jLLDBTraceGetState response.
+ ///
JSON string representing the jLLDBTraceGetState response. It might be invalid.
Comment at: lldb/include/lldb/Target/Trace.h:310
+ /// Unique pointer to the packet response, nullptr if response parsing
failed.
+ virtual std::unique_ptr
+ DoRefreshLiveProcessState(llvm::Expected json_string) = 0;
you don't need to return a unique_ptr for this. There's nothing that you want
to forbid copies of. Just return
llvm::Optional
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:14
+
/// See docs/lldb-gdb-remote.txt for more information.
undo this
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:43
+
+/// TSC to nanosecond conversion.
+class TimestampCounterRate {
TSC is not a very well known concept, so better be very explanatory in the
documentation
Class that can be extended to represent different CPU Time Stamp Counter
(TSC) frequency rates, which can be used to convert TSCs to common units of
time, such as nanoseconds. More on TSCs can be found here
https://en.wikipedia.org/wiki/Time_Stamp_Counter.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:47
+virtual ~TimestampCounterRate() = default;
+/// Convert TSC value to nanoseconds. This represents the number of
nanoseconds since
+/// the TSC register's reset.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:53
+
+typedef std::shared_ptr TimestampCounterRateSP;
+
modern c++ uses the `using` keyword.
using TimestampCounterRateSP = std::shared_ptr;
use the version you want
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:55-56
+
+/// TSC to nanoseconds conversion values defined in struct
perf_event_mmap_page.
+/// See https://man7.org/linux/man-pages/man2/perf_event_open.2.html for more
information.
+class PerfTimestampCounterRate : public TimestampCounterRate {
You have to make the documentation very friendly to newcomers.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:59
+ public:
+ PerfTimestampCounterRate() = default;
+ PerfTimestampCounterRate(uint32_t time_mult, uint16_t time_shift, uint64_t
time_zero) :
do you need this?
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:61
+ PerfTimestampCounterRate(uint32_t time_mult, uint16_t time_shift, uint64_t
time_zero) :
+m_time_mult(time_mult), m_time_shift(time_shift),
m_time_zero(static_cast(time_zero)) {}
+ uint64_t ToNanos(uint64_t tsc) override;
why do you do static_cast?
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:66-68
+ uint32_t m_time_mult;
+ uint16_t m_time_shift;
+ uint64_t m_time_zero;
having the correct types here is the right thing to do
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:72
+/// jLLDBTraceGetState gdb-remote packet
+/// Contains additional information related to TSC -> nanosecond conversion.
+struct TraceIntelPTGetStateResponse : TraceGetStateResponse {
The response contains the frequency rate. Whether it's used to convert to nanos
or not is a different topic.
Just delete this line.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:74
+struct TraceIntelPTGetStateResponse : TraceGetStateResponse {
+ /// `nullptr` if no tsc conversion rate exists.
+ llvm::Optional tsc_rate;
avoid using nullptr when you have an Optional.
Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:51
+ int64_t m_time_shift;
+ int64_t m_time_zero;
+};
jj10306 wrote:
> wallace wrote:
> > jj10306 wrote:
> > > What is the suggested way to serialize a u64 into JSON? I thought of two
> > > approaches:
> > > 1. Store two separate u32
> > > 2. store the integer as a string
> > >
> > > @wallace wdyt?
> > you don't need to use int64_t here. You can us
