Skip to content
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

src,etw: fix event 9 on 64 bit Windows #15563

Closed
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/node_win32_etw_provider-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@
#include "node_win32_etw_provider.h"
#include "node_etw_provider.h"

#if defined(_WIN64)
# define ETW_WRITE_INTPTR_DATA ETW_WRITE_INT64_DATA
#else
# define ETW_WRITE_INTPTR_DATA ETW_WRITE_INT32_DATA
#endif

namespace node {

// From node_win32_etw_provider.cc
Expand Down Expand Up @@ -100,7 +94,7 @@ extern int events_enabled;
ETW_WRITE_ADDRESS_DATA(descriptors, &context); \
ETW_WRITE_ADDRESS_DATA(descriptors + 1, &startAddr); \
ETW_WRITE_INT64_DATA(descriptors + 2, &size); \
ETW_WRITE_INTPTR_DATA(descriptors + 3, &id); \
ETW_WRITE_INT32_DATA(descriptors + 3, &id); \
ETW_WRITE_INT16_DATA(descriptors + 4, &flags); \
ETW_WRITE_INT16_DATA(descriptors + 5, &rangeId); \
ETW_WRITE_INT64_DATA(descriptors + 6, &sourceId); \
Expand Down Expand Up @@ -253,7 +247,11 @@ void NODE_V8SYMBOL_ADD(LPCSTR symbol,
}
void* context = nullptr;
INT64 size = (INT64)len;
INT_PTR id = (INT_PTR)addr1;
#if defined(_WIN64)
INT32 id = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we not just use the lower 32 bits on 64 bit? That is, why not just have the non-64 bit version on 64 bit too?

Also agree with @refack that using addr1 as the id is somewhat arbitrary but I don't know if v8 has a better (and more stable/deterministic) id that we can use here- in ChakraCore we use our "function id" which I believe is somewhat stable across runs of node because it's incremented in order of in which the function is compiled- this allows us to collect a trace, and then run with additional tracing flags for just that function id to get more details on that function. Does v8 have something stable like that that we can use here? (cc @fhinkel)

#else
INT32 id = (INT32)addr1;
#endif
INT16 flags = 0;
INT16 rangeid = 1;
INT32 col = 1;
Expand Down