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

Provide single-threaded (no-locking) implementation of IMetadataEmit family of APIs. #52

Merged
merged 52 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
b04ad89
Add C++ RAII helper for row adds. Add declaration of IMetaDataEmit an…
jkoritzinsky Aug 14, 2023
3e967bc
Implement saving.
jkoritzinsky Aug 30, 2023
253b9f4
Implement more methods.
jkoritzinsky Aug 30, 2023
cefd384
Change md_apply_delta to take a delta image handle instead of the raw…
jkoritzinsky Sep 13, 2023
9132aed
Implement IMetadataEmit::ApplyEditAndContinue
jkoritzinsky Sep 13, 2023
3d42493
Implement most of DefineImportType
jkoritzinsky Sep 14, 2023
d978f36
Implement DefineMemberRef and fix QIs in DefineImportType to use the …
jkoritzinsky Sep 14, 2023
4ed2f5f
Stub out EnC-related methods and implement a few more methods
jkoritzinsky Sep 14, 2023
c8821b8
Implement DefineField
jkoritzinsky Sep 18, 2023
538d8bc
Implement SetFieldProps
jkoritzinsky Sep 18, 2023
108317a
Implement most of the remaining APIs on IMetaDataEmit/IMetaDataEmit2/…
jkoritzinsky Nov 29, 2023
6136ed5
Implement Param methods
jkoritzinsky Dec 12, 2023
f5cff9d
Provide a no-op implementation of SetHandler as we don't do any token…
jkoritzinsky Dec 12, 2023
0b7966d
Fix errors and correctly handle names in SetParamProps
jkoritzinsky Dec 12, 2023
48f79b6
Remove TranslateSigWithScope helper and delegate back to the correct …
jkoritzinsky Dec 13, 2023
ac00eb1
Merge metadataemithelpers and importhelpers
jkoritzinsky Jan 16, 2024
84520b6
Use helpers instead of re-implementing the algorithms and implement r…
jkoritzinsky Jan 16, 2024
0195a85
Don't use UINT32 type as it's not defined by dncp
jkoritzinsky Jan 16, 2024
51c83ff
Update dncp
jkoritzinsky Jan 17, 2024
3eceff0
Fix Clang failures
jkoritzinsky Jan 17, 2024
cdc4e8c
Define IMetaDataAssemblyEmit IID
jkoritzinsky Jan 17, 2024
69d4d30
Add override specifier
jkoritzinsky Jan 17, 2024
b14b2db
Make DefineScope result editable
jkoritzinsky Jan 18, 2024
34d3111
Correctly handle null heap entries as per the ECMA-335 spec (emit the…
jkoritzinsky Jan 20, 2024
2e24a1a
Zero-initialize each column in the ::DefineX methods when the user do…
jkoritzinsky Jan 20, 2024
8820812
Initialize currentImplementation
jkoritzinsky Jan 20, 2024
d78baa3
Fix incorrect dereference
jkoritzinsky Jan 20, 2024
3323d11
Fix bugs in DefineScope path and add first emit unit test
jkoritzinsky Jan 26, 2024
b65cda2
Add tests for some corner case scenarios.
jkoritzinsky Jan 26, 2024
c83191e
Add CMake option to enable msvc profiling to enable code coverage in …
jkoritzinsky Jan 27, 2024
ef58687
Add tests for SetModuleProps
jkoritzinsky Jan 29, 2024
df8f518
Implement tests for defining typedefs and fix bugs found in the process.
jkoritzinsky Jan 30, 2024
3c75715
Add test for SetTypeDefProps
jkoritzinsky Jan 30, 2024
061e4d5
Flip include order for non-Windows
jkoritzinsky Jan 30, 2024
28229cf
Add basic "define MethodDef" test and fix bugs.
jkoritzinsky Jan 30, 2024
9491f51
Add more methoddef tests and standalonesig tests
jkoritzinsky Jan 31, 2024
027ece6
Add tests for MemberRef
jkoritzinsky Jan 31, 2024
b611631
Add tests for TypeSpec
jkoritzinsky Jan 31, 2024
374ebda
Add Assembly test and update DNMD APIs to behave better around zero-l…
jkoritzinsky Feb 13, 2024
858aac7
Add AssemblyRef test
jkoritzinsky Feb 15, 2024
361b5e8
Add some parameter tests that also set constants
jkoritzinsky Feb 15, 2024
dbc497d
Add a test for out of order parameters at the start of the parameter …
jkoritzinsky Feb 16, 2024
ef6747d
Add tests for FieldMarshal and FieldRva table-related APIs and fix un…
jkoritzinsky Mar 12, 2024
1d4c98f
Just avoid using max
jkoritzinsky Mar 12, 2024
bc23799
Include cstddef for size_t
jkoritzinsky Mar 12, 2024
ce744d3
Fix incorrect wide-char spec
jkoritzinsky Mar 13, 2024
4661260
Add necessary const casts
jkoritzinsky Mar 13, 2024
061faf5
PR feedback excluding east-const
jkoritzinsky Jun 13, 2024
e940d12
Fix #US physical blob construction and make editor creation a little …
jkoritzinsky Jun 13, 2024
2ede98b
East const
jkoritzinsky Jun 13, 2024
8996494
Produce position-independent code to enable linking with the shared e…
jkoritzinsky Jun 13, 2024
a68f4fb
Add TestResults to gitignore so VS results from coverage/profiling ar…
jkoritzinsky Jun 18, 2024
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
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
jkoritzinsky marked this conversation as resolved.
Outdated
Show resolved Hide resolved
Binary file not shown.
8 changes: 7 additions & 1 deletion configure.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,10 @@ endif()
#
# Agnostic compiler/platform settings
#
add_compile_definitions(__STDC_WANT_LIB_EXT1__=1) # https://en.cppreference.com/w/c/error#Bounds_checking
add_compile_definitions(__STDC_WANT_LIB_EXT1__=1) # https://en.cppreference.com/w/c/error#Bounds_checking

option(DNMD_ENABLE_PROFILING OFF)

if (DNMD_ENABLE_PROFILING AND MSVC)
add_link_options(/PROFILE)
endif()
2 changes: 1 addition & 1 deletion external/dncp
19 changes: 16 additions & 3 deletions src/dnmd/editor.c
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,9 @@ uint32_t add_to_string_heap(mdcxt_t* cxt, char const* str)
mdeditor_t* editor = get_editor(cxt);
if (editor == NULL)
return 0;

if (str[0] == '\0')
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
return 0;

// TODO: Deduplicate heap
uint32_t str_len = (uint32_t)strlen(str);
Expand All @@ -649,9 +652,13 @@ uint32_t add_to_blob_heap(mdcxt_t* cxt, uint8_t const* data, uint32_t length)
mdeditor_t* editor = get_editor(cxt);
if (editor == NULL)
return 0;

if (length == 0)
return 0;

// TODO: Deduplicate heap
uint8_t compressed_length[4];
size_t compressed_length_size = 0;
size_t compressed_length_size = 4;
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
if (!compress_u32(length, compressed_length, &compressed_length_size))
return 0;

Expand Down Expand Up @@ -713,8 +720,11 @@ uint32_t add_to_user_string_heap(mdcxt_t* cxt, char16_t const* str)
}
}

if (str_len == 0)
return 0;

uint8_t compressed_length[sizeof(str_len)];
size_t compressed_length_size = 0;
size_t compressed_length_size = 4;
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
if (!compress_u32(str_len, compressed_length, &compressed_length_size))
return 0;

Expand All @@ -738,6 +748,9 @@ uint32_t add_to_guid_heap(mdcxt_t* cxt, mdguid_t guid)
if (editor == NULL)
return 0;
// TODO: Deduplicate heap
static const mdguid_t empty_guid = { 0 };
if (memcmp(&guid, &empty_guid, sizeof(mdguid_t)) == 0)
return 0;

uint32_t heap_offset;
if (!reserve_heap_space(editor, sizeof(mdguid_t), mdtc_hguid, false, &heap_offset))
Expand All @@ -746,7 +759,7 @@ uint32_t add_to_guid_heap(mdcxt_t* cxt, mdguid_t guid)
}

memcpy(editor->guid_heap.heap.ptr + heap_offset, &guid, sizeof(mdguid_t));
return heap_offset / sizeof(mdguid_t);
return heap_offset / sizeof(mdguid_t) + 1;
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
}

bool append_heap(mdcxt_t* cxt, mdcxt_t* delta, mdtcol_t heap_id)
Expand Down
13 changes: 4 additions & 9 deletions src/dnmd/entry.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ static bool initialize_minimal_table_rows(mdcxt_t* cxt)
if (1 != md_set_column_value_as_utf8(global_type_cursor, mdtTypeDef_TypeNamespace, 1, &namespace))
return false;

mdToken nil_typedef = CreateTokenType(mdtTypeDef);
mdToken nil_typedef = CreateTokenType(mdtid_TypeDef);
if (1 != md_set_column_value_as_token(global_type_cursor, mdtTypeDef_Extends, 1, &nil_typedef))
return false;

Expand Down Expand Up @@ -361,26 +361,21 @@ mdhandle_t md_create_new_pdb_handle()
}
#endif // DNMD_PORTABLE_PDB

bool md_apply_delta(mdhandle_t handle, void const* data, size_t data_len)
bool md_apply_delta(mdhandle_t handle, mdhandle_t delta_handle)
{
mdcxt_t* base = extract_mdcxt(handle);
if (base == NULL)
return false;

mdhandle_t h;
if (!md_create_handle(data, data_len, &h))
mdcxt_t* delta = extract_mdcxt(delta_handle);
if (delta == NULL)
return false;

mdcxt_t* delta = extract_mdcxt(h);
assert(delta != NULL);

// Verify the supplied delta is actually a delta file
bool result = false;
if (delta->context_flags & mdc_minimal_delta)
result = merge_in_delta(base, delta);

// Free all data for the delta
md_destroy_handle(h);
return result;
}

Expand Down
16 changes: 16 additions & 0 deletions src/dnmd/streams.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ bool try_get_string(mdcxt_t* cxt, size_t offset, char const** str)
assert(cxt != NULL && str != NULL);

mdstream_t* h = &cxt->strings_heap;

if (h->size == 0 && offset == 0)
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
{
*str = "\0"; // II.24.2.3 'The first character must be the '\0' character.
return true;
}

if (h->size <= offset)
return false;

Expand Down Expand Up @@ -81,6 +88,15 @@ bool try_get_blob(mdcxt_t* cxt, size_t offset, uint8_t const** blob, uint32_t* b
assert(cxt != NULL && blob != NULL && blob_len != NULL);

mdstream_t* h = &cxt->blob_heap;

if (h->size == 0 && offset == 0)
{
// The first element must be the 0 - II.24.2.4
*blob = h->ptr;
*blob_len = 0;
return true;
}

if (h->size <= offset)
return false;

Expand Down
9 changes: 8 additions & 1 deletion src/dnmd/tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -772,11 +772,18 @@ bool initialize_new_table_details(
// Set the new table's row count temporarily to 1 to ensure that we initialize the table.
table_row_counts[id] = 1;

// We'll treat any new table that has keys as sorted.
// We only want to do this for tables with keys as tables without keys
// never use the is_sorted bit.
md_key_info_t const* keys;
uint8_t key_count = get_table_keys(id, &keys);
bool has_keys = key_count != 0;

if (!initialize_table_details(
table_row_counts,
cxt->context_flags,
id,
false,
has_keys,
table))
return false;

Expand Down
56 changes: 49 additions & 7 deletions src/dnmd/write.c
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,14 @@ int32_t md_set_column_value_as_blob(mdcursor_t c, col_index_t col_idx, uint32_t
int32_t written = 0;
do
{
if (blob_len[written] == 0)
{
if (!write_column_data(&acxt, 0))
return -1;
written++;
continue;
}

uint32_t heap_offset = add_to_blob_heap(CursorTable(&c)->cxt, blob[written], blob_len[written]);

if (heap_offset == 0)
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -415,6 +423,15 @@ int32_t md_set_column_value_as_guid(mdcursor_t c, col_index_t col_idx, uint32_t
int32_t written = 0;
do
{
static const mdguid_t empty_guid = { 0 };
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
if (memcmp(&guid[written], &empty_guid, sizeof(mdguid_t)) == 0)
{
if (!write_column_data(&acxt, 0))
return -1;
written++;
continue;
}

uint32_t index = add_to_guid_heap(CursorTable(&c)->cxt, guid[written]);

if (index == 0)
Expand Down Expand Up @@ -448,6 +465,14 @@ int32_t md_set_column_value_as_userstring(mdcursor_t c, col_index_t col_idx, uin
int32_t written = 0;
do
{
if (userstring[written][0] == 0)
{
if (!write_column_data(&acxt, 0))
return -1;
written++;
continue;
}

uint32_t index = add_to_user_string_heap(CursorTable(&c)->cxt, userstring[written]);

if (index == 0)
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -701,9 +726,10 @@ static bool add_new_row_to_list(mdcursor_t list_owner, col_index_t list_col, mdc
if (!md_get_column_value_as_range(list_owner, list_col, &range, &count))
return false;

// Assert that the insertion location is in our range.
// Assert that the insertion location is in our range or points to the first row of the next range.
// For a zero-length range, row_to_insert_before will be the first row of the next range, so we need to account for that.
assert(CursorTable(&range) == CursorTable(&row_to_insert_before));
assert(CursorRow(&range) <= CursorRow(&row_to_insert_before) && CursorRow(&row_to_insert_before) <= CursorRow(&range) + count);
assert(CursorRow(&range) <= CursorRow(&row_to_insert_before) && CursorRow(&row_to_insert_before) <= CursorRow(&range) + (count == 0 ? 1 : count));

mdcursor_t target_row;
// If the range is in an indirection table, we'll normalize our insert to the actual target table.
Expand All @@ -724,10 +750,11 @@ static bool add_new_row_to_list(mdcursor_t list_owner, col_index_t list_col, mdc
if (!md_set_column_value_as_cursor(new_indirection_row, index_to_col(0, CursorTable(&row_to_insert_before)->table_id), 1, new_row))
return false;

if (count == 0)
if (count == 0 || CursorRow(&range) == CursorRow(&row_to_insert_before))
{
// If our original count was zero, then this is the first element in the list for this parent.
// We need to update the parent's row column to point to the newly inserted row.
// If the start of our range is the same as the row we're inserting before, then we're inserting at the start of the list.
// In both of these cases, we need to update the parent's row column to point to the newly inserted row.
// Otherwise, this element would be associated with the entry before the parent row.
if (!md_set_column_value_as_cursor(list_owner, list_col, 1, &new_indirection_row))
return false;
Expand Down Expand Up @@ -869,10 +896,9 @@ bool md_add_new_row_to_sorted_list(mdcursor_t list_owner, col_index_t list_col,
// so start searching there to make this a little faster.
mdcursor_t row_to_check = row_to_insert_before;

// Move our cursor to the last row in the list.
// Move our cursor to the last row in the list and move back one more row each iteration.
// This can't return false as we got to row_to_insert_before by moving forward at least one row.
(void)md_cursor_move(&row_to_check, -1);
for (; CursorRow(&row_to_check) >= CursorRow(&existing_range); (void)md_cursor_move(&row_to_check, -1))
for (; md_cursor_move(&row_to_check, -1) && CursorRow(&row_to_check) >= CursorRow(&existing_range);)
{
// If the range is in an indirection table, we need to normalize to the target table to
// get the sort column value.
Expand All @@ -894,6 +920,15 @@ bool md_add_new_row_to_sorted_list(mdcursor_t list_owner, col_index_t list_col,
break;
}
}

// If we didn't find a row with a sort order less than or equal to the new row, we want to insert the new row at the beginning of the list.
// If our cursor is pointing at the first row, that means that our existing range starts at the first row.
if (CursorRow(&row_to_check) == 1 || CursorRow(&row_to_check) < CursorRow(&existing_range))
{
// We didn't find a row with a sort order less than or equal to the new row.
// So we want to insert the new row at the beginning of the list.
row_to_insert_before = existing_range;
}
}

if (!add_new_row_to_list(list_owner, list_col, row_to_insert_before, new_row))
Expand Down Expand Up @@ -954,6 +989,13 @@ static bool validate_row_sorted_within_table(mdcursor_t row)
void md_commit_row_add(mdcursor_t row)
{
mdtable_t* table = CursorTable(&row);

// If this method is called with a zero-initialized cursor,
// no-op. This helps make the C++ helper md_added_row_t function more easily.
// This also allows users to call this method in all cases, even if the row-add fails.
if (table == NULL)
return;

assert(table->is_adding_new_row);

// If the table was previously sorted,
Expand Down
2 changes: 1 addition & 1 deletion src/inc/dnmd.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ mdhandle_t md_create_new_pdb_handle();
#endif // DNMD_PORTABLE_PDB

// Apply delta data to the current metadata.
bool md_apply_delta(mdhandle_t handle, void const* data, size_t data_len);
bool md_apply_delta(mdhandle_t handle, mdhandle_t delta_handle);

// Destroy the metadata handle and free all associated memory.
void md_destroy_handle(mdhandle_t handle);
Expand Down
2 changes: 2 additions & 0 deletions src/interfaces/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ set(SOURCES
./dispenser.cpp
./symbinder.cpp
./metadataimport.cpp
./metadataemit.cpp
./hcorenum.cpp
./pal.cpp
./signatures.cpp
Expand All @@ -12,6 +13,7 @@ set(HEADERS
../inc/dnmd_interfaces.hpp
../inc/internal/span.hpp
./metadataimportro.hpp
./metadataemit.hpp
./hcorenum.hpp
./controllingiunknown.hpp
./tearoffbase.hpp
Expand Down
12 changes: 8 additions & 4 deletions src/interfaces/dispenser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "dnmd_interfaces.hpp"
#include "controllingiunknown.hpp"
#include "metadataimportro.hpp"
#include "metadataemit.hpp"

#include <cstring>

Expand Down Expand Up @@ -60,6 +61,7 @@ namespace
try
{
mdhandle_view handle_view{ obj->CreateAndAddTearOff<DNMDOwner>(std::move(md_ptr)) };
(void)obj->CreateAndAddTearOff<MetadataEmit>(handle_view);
(void)obj->CreateAndAddTearOff<MetadataImportRO>(std::move(handle_view));
}
catch(std::bad_alloc const&)
Expand Down Expand Up @@ -93,10 +95,6 @@ namespace
if (ppIUnk == nullptr)
return E_INVALIDARG;

// Only support the read-only state
if (!(dwOpenFlags & ofReadOnly))
return E_INVALIDARG;

dncp::cotaskmem_ptr<void> nowOwned;
if (dwOpenFlags & ofTakeOwnership)
nowOwned.reset((void*)pData);
Expand Down Expand Up @@ -126,6 +124,12 @@ namespace
try
{
mdhandle_view handle_view{ obj->CreateAndAddTearOff<DNMDOwner>(std::move(md_ptr), std::move(copiedMem), std::move(nowOwned)) };

if (!(dwOpenFlags & ofReadOnly))
{
(void)obj->CreateAndAddTearOff<MetadataEmit>(handle_view);
}
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved

(void)obj->CreateAndAddTearOff<MetadataImportRO>(std::move(handle_view));
}
catch(std::bad_alloc const&)
Expand Down
1 change: 1 addition & 0 deletions src/interfaces/iids.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ MIDL_DEFINE_GUID(IID_IMetaDataImport2,0xfce5efa0,0x8bba,0x4f8e,0xa0,0x36,0x8f,0x
MIDL_DEFINE_GUID(IID_IMetaDataAssemblyImport,0xee62470b,0xe94b,0x424e,0x9b,0x7c,0x2f,0x00,0xc9,0x24,0x9f,0x93);
MIDL_DEFINE_GUID(IID_IMetaDataEmit, 0xba3fee4c, 0xecb9, 0x4e41, 0x83, 0xb7, 0x18, 0x3f, 0xa4, 0x1c, 0xd8, 0x59);
MIDL_DEFINE_GUID(IID_IMetaDataEmit2, 0xf5dd9950, 0xf693, 0x42e6, 0x83, 0xe, 0x7b, 0x83, 0x3e, 0x81, 0x46, 0xa9);
MIDL_DEFINE_GUID(IID_IMetaDataAssemblyEmit, 0x211ef15b, 0x5317, 0x4438, 0xb1, 0x96, 0xde, 0xc8, 0x7b, 0x88, 0x76, 0x93);

// Define the ISymUnmanaged* IIDs here - corsym.h provides the declaration.
MIDL_DEFINE_GUID(IID_ISymUnmanagedBinder, 0xaa544d42, 0x28cb, 0x11d3, 0xbd, 0x22, 0x00, 0x00, 0xf8, 0x08, 0x49, 0xbd);
Expand Down
Loading
Loading