[tracing] Generalize {SetTraceValue} method

The {SetTraceValue} method was only defined for a set of integer types,
which sometimes lead to ambiguities when using types like {size_t},
{unsigned long} or the like (see https://crrev.com/c/1886912/1).
This CL fixes that by providing a method accepting any integer type.
It also changes the existing methods to avoid the "cast via union"
idiom, and uses memcpy instead.

R=petermarshall@chromium.org

Bug: v8:9810
Change-Id: I1530405640dc6cb0058153a8dbb860c7f3727ac5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1886918
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64710}
This commit is contained in:
Clemens Backes 2019-10-31 18:33:57 +01:00 committed by Commit Bot
parent 5e755c6ee6
commit 7f4a2ec4d9
4 changed files with 28 additions and 50 deletions

View File

@ -6943,8 +6943,7 @@ wasm::WasmCompilationResult ExecuteTurbofanWasmCompilation(
wasm::WasmFeatures* detected) {
TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("v8.wasm"),
"ExecuteTurbofanCompilation", "func_index", func_index,
"body_size",
static_cast<uint32_t>(func_body.end - func_body.start));
"body_size", func_body.end - func_body.start);
Zone zone(wasm_engine->allocator(), ZONE_NAME);
MachineGraph* mcgraph = new (&zone) MachineGraph(
new (&zone) Graph(&zone), new (&zone) CommonOperatorBuilder(&zone),

View File

@ -412,16 +412,6 @@ class TraceID {
uint64_t raw_id_;
};
// Simple union to store various types as uint64_t.
union TraceValueUnion {
bool as_bool;
uint64_t as_uint;
int64_t as_int;
double as_double;
const void* as_pointer;
const char* as_string;
};
// Simple container for const char* that should be copied instead of retained.
class TraceStringWithCopy {
public:
@ -479,42 +469,32 @@ static V8_INLINE uint64_t AddTraceEventWithTimestampImpl(
// Define SetTraceValue for each allowed type. It stores the type and
// value in the return arguments. This allows this API to avoid declaring any
// structures so that it is portable to third_party libraries.
#define INTERNAL_DECLARE_SET_TRACE_VALUE(actual_type, union_member, \
value_type_id) \
static V8_INLINE void SetTraceValue(actual_type arg, unsigned char* type, \
uint64_t* value) { \
TraceValueUnion type_value; \
type_value.union_member = arg; \
*type = value_type_id; \
*value = type_value.as_uint; \
}
// Simpler form for int types that can be safely casted.
#define INTERNAL_DECLARE_SET_TRACE_VALUE_INT(actual_type, value_type_id) \
static V8_INLINE void SetTraceValue(actual_type arg, unsigned char* type, \
uint64_t* value) { \
*type = value_type_id; \
*value = static_cast<uint64_t>(arg); \
}
// This is the base implementation for integer types (including bool) and enums.
template <typename T>
static V8_INLINE typename std::enable_if<
std::is_integral<T>::value || std::is_enum<T>::value, void>::type
SetTraceValue(T arg, unsigned char* type, uint64_t* value) {
*type = std::is_same<T, bool>::value
? TRACE_VALUE_TYPE_BOOL
: std::is_signed<T>::value ? TRACE_VALUE_TYPE_INT
: TRACE_VALUE_TYPE_UINT;
*value = static_cast<uint64_t>(arg);
}
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(uint64_t, TRACE_VALUE_TYPE_UINT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(unsigned int, TRACE_VALUE_TYPE_UINT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(uint16_t, TRACE_VALUE_TYPE_UINT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(unsigned char, TRACE_VALUE_TYPE_UINT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(int64_t, TRACE_VALUE_TYPE_INT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(int, TRACE_VALUE_TYPE_INT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(int16_t, TRACE_VALUE_TYPE_INT)
INTERNAL_DECLARE_SET_TRACE_VALUE_INT(signed char, TRACE_VALUE_TYPE_INT)
INTERNAL_DECLARE_SET_TRACE_VALUE(bool, as_bool, TRACE_VALUE_TYPE_BOOL)
INTERNAL_DECLARE_SET_TRACE_VALUE(double, as_double, TRACE_VALUE_TYPE_DOUBLE)
INTERNAL_DECLARE_SET_TRACE_VALUE(const void*, as_pointer,
TRACE_VALUE_TYPE_POINTER)
INTERNAL_DECLARE_SET_TRACE_VALUE(const char*, as_string,
TRACE_VALUE_TYPE_STRING)
INTERNAL_DECLARE_SET_TRACE_VALUE(const TraceStringWithCopy&, as_string,
#define INTERNAL_DECLARE_SET_TRACE_VALUE(actual_type, value_type_id) \
static V8_INLINE void SetTraceValue(actual_type arg, unsigned char* type, \
uint64_t* value) { \
*type = value_type_id; \
*value = 0; \
STATIC_ASSERT(sizeof(arg) <= sizeof(*value)); \
memcpy(value, &arg, sizeof(arg)); \
}
INTERNAL_DECLARE_SET_TRACE_VALUE(double, TRACE_VALUE_TYPE_DOUBLE)
INTERNAL_DECLARE_SET_TRACE_VALUE(const void*, TRACE_VALUE_TYPE_POINTER)
INTERNAL_DECLARE_SET_TRACE_VALUE(const char*, TRACE_VALUE_TYPE_STRING)
INTERNAL_DECLARE_SET_TRACE_VALUE(const TraceStringWithCopy&,
TRACE_VALUE_TYPE_COPY_STRING)
#undef INTERNAL_DECLARE_SET_TRACE_VALUE
#undef INTERNAL_DECLARE_SET_TRACE_VALUE_INT
static V8_INLINE void SetTraceValue(ConvertableToTraceFormat* convertable_value,
unsigned char* type, uint64_t* value) {

View File

@ -2185,8 +2185,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(AccountingAllocator* allocator,
WasmFeatures* detected) {
TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("v8.wasm"),
"ExecuteLiftoffCompilation", "func_index", func_index,
"body_size",
static_cast<uint32_t>(func_body.end - func_body.start));
"body_size", func_body.end - func_body.start);
Zone zone(allocator, "LiftoffCompilationZone");
const WasmModule* module = env ? env->module : nullptr;

View File

@ -1037,7 +1037,7 @@ bool ExecuteCompilationUnits(
auto publish_results = [&results_to_publish](
BackgroundCompileScope* compile_scope) {
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "PublishResults",
"num_results", uint64_t{results_to_publish.size()});
"num_results", results_to_publish.size());
if (results_to_publish.empty()) return;
WasmCodeRefScope code_ref_scope;
std::vector<WasmCode*> code_vector =
@ -2338,7 +2338,7 @@ void CompilationStateImpl::FinalizeJSToWasmWrappers(
// optimization we keep the code space unlocked to avoid repeated unlocking
// because many such wrapper are allocated in sequence below.
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "FinalizeJSToWasmWrappers",
"num_wrappers", uint64_t{js_to_wasm_wrapper_units_.size()});
"num_wrappers", js_to_wasm_wrapper_units_.size());
CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
for (auto& unit : js_to_wasm_wrapper_units_) {
Handle<Code> code = unit->Finalize(isolate);
@ -2357,7 +2357,7 @@ CompilationStateImpl::GetNextCompilationUnit(
void CompilationStateImpl::OnFinishedUnits(Vector<WasmCode*> code_vector) {
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "OnFinishedUnits",
"num_units", uint64_t{code_vector.size()});
"num_units", code_vector.size());
base::MutexGuard guard(&callbacks_mutex_);