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

Header order dependency for build_target_arch_type() #7236

Open
abhinav92003 opened this issue Jan 30, 2025 · 0 comments
Open

Header order dependency for build_target_arch_type() #7236

abhinav92003 opened this issue Jan 30, 2025 · 0 comments

Comments

@abhinav92003
Copy link
Contributor

build_target_arch_type() is defined in trace_entry.h

build_target_arch_type()

But it is defined conditionally only if dr_defines.h has already defined certain symbols, specifically IF_X64_ELSE

# define IF_X64_ELSE(x, y) x

Otherwise this shows up as a build failure to find build_target_arch_type, which may seem mysterious as the order between trace_entry.h and dr_defines.h (both of which may be transitively included) may not be immediately clear. It is hard to ensure that for all builds dr_defines.h is included before trace_entry.h.

One way to get around this is to wrap build_target_arch_type in a new function implemented in some non-header file like what #7233 did. But eventually we should clean this up so the build_target_arch_type in trace_entry.h is itself usable.

abhinav92003 added a commit that referenced this issue Jan 30, 2025
Moves the check that compares the current build enviroment with the arch
bit of the trace file to decode_cache_t::init.

Due to some complexities in how build_target_arch_type() is declared in
trace_entry.h, it had to be wrapped by a new API in decode_cache_base_t.
build_target_arch_type() is defined conditionally in trace_entry.h only
if dr_defines.h has already defined certain symbols. It is hard to
ensure that for all builds dr_defines.h is included before
trace_entry.h, so we ensure it just for decode_cache.cpp which is
packaged as a separate lib unit drmemtrace_decode_cache. This also means
that the new API could not be in decode_cache_t which is a template
class that has to be defined in the header.

Issue: #7113, #7236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants