DynamoRIO / DynamoRIO/dynamorio

Default and unspecified values for offline_file_type_t

Open
#7,228 2 comments 0 reactions 0 assignees View on GitHub
Component-DrMemtrace
Dominant language
C
Stars
3.2k
Forks
629
Avg merge
2d 15h
Merged PRs (30d)
31

Description

Today, offline_file_type_t has OFFLINE_FILE_TYPE_DEFAULT which perhaps at some point previously was intended for real trace files with no other "special" bit. But as of today, it seems that we would expect atleast one of the arch-related bits (OFFLINE_FILE_TYPE_ARCH_*).

https://github.com/DynamoRIO/dynamorio/blob/f007194c734b769c7af69df332c9d36583b97281/clients/drcachesim/common/trace_entry.h#L992

Some existing code in opcode_mix does assume that no existing trace may use OFFLINE_FILE_TYPE_DEFAULT-only. https://github.com/DynamoRIO/dynamorio/blob/f007194c734b769c7af69df332c9d36583b97281/clients/drcachesim/tools/opcode_mix.cpp#L184

Also, the scheduler initializes the get_filetype value to 0 (which is essentially OFFLINE_FILE_TYPE_DEFAULT): https://github.com/DynamoRIO/dynamorio/blob/f007194c734b769c7af69df332c9d36583b97281/clients/drcachesim/scheduler/scheduler.h#L1202

But then there are various unit tests that use 0 as a real value for the test trace: https://github.com/DynamoRIO/dynamorio/blob/f007194c734b769c7af69df332c9d36583b97281/clients/drcachesim/tests/view_test.cpp#L307

and also existing code that uses -1 to mean uninitialized: https://github.com/DynamoRIO/dynamorio/blob/f007194c734b769c7af69df332c9d36583b97281/clients/drcachesim/tools/view.cpp#L227

Goal: If it is indeed so that OFFLINE_FILE_TYPE_DEFAULT cannot be present on its own in real traces, perhaps we can use that as the uninitialized sentinel value, which is much cleaner. In that case, we should rename it (keeping the original enum value for backward compatibility) to OFFLINE_FILE_TYPE_UNINITIALIZED.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.