[PATCH v3 0/6] MR73: Improvement in PDB's types record handling

[eric pouech (＠epo)]

This serie tackles several issues encountered while loading PDB files through dbghelp.

It happens that PDB allows to have several type records for user defined types (struct / enums)
with the same name.
This seems to be generated by a) compiler (when modifying a structure definition between several
compilation phases), b) the incremental linker (likely an optimisation of previous case, but
not 100% certain when it's triggered).
Wine's dbghelp ended up not picking the right record, hence generating errneous outputs.
This series:
- cleans up some PDB related code
- update winedump to display the PDB hash table internals
- fixes dbghelp to preserve in Wine's dbghelp hash table the partial order between type
  records of same name.

v2:
- rebased
- removed a bunch of stray comments

--
  v3: dlls/dbghelp{pdb}: use remap table from PDB hash stream
      dlls/dbghelp{pdb}: ensure dbghelp's list order in hash table matches PDB's
      dlls/dbghelp{pdb}: clearly separate the type loading into two passes
      include/wine/mscvpdb.h: redefine property with bitfields
      dlls/dbghelp{pdb}: rely on first/last index from type header
      tools/winedump{pdb}: explore a bit more TPI hash elements

https://gitlab.winehq.org/wine/wine/-/merge_requests/73

[PATCH v3 1/6] tools/winedump{pdb}: explore a bit more TPI hash elements

[Eric Pouech]

From: Eric Pouech

Renamed of couple of fields in PDB structures for clarity

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/msc.c     |   2 +-
 include/wine/mscvpdb.h |  10 +--
 tools/winedump/pdb.c   | 188 +++++++++++++++++++++++++++++++++++------
 3 files changed, 167 insertions(+), 33 deletions(-)

diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c
index 5b45c474160..2be648ca578 100644
--- a/dlls/dbghelp/msc.c
+++ b/dlls/dbghelp/msc.c
@@ -2954,7 +2954,7 @@ static void pdb_convert_types_header(PDB_TYPES* types, const BYTE* image)
         types->type_size   = old->type_size;
         types->first_index = old->first_index;
         types->last_index  = old->last_index;
-        types->file        = old->file;
+        types->hash_file   = old->hash_file;
     }
     else
     {
diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h
index 1772e6d10df..f0fbfa92b39 100644
--- a/include/wine/mscvpdb.h
+++ b/include/wine/mscvpdb.h
@@ -2403,7 +2403,7 @@ typedef struct _PDB_TYPES_OLD
     unsigned short first_index;
     unsigned short last_index;
     unsigned int   type_size;
-    unsigned short file;
+    unsigned short hash_file;
     unsigned short pad;
 } PDB_TYPES_OLD, *PPDB_TYPES_OLD;

@@ -2414,16 +2414,16 @@ typedef struct _PDB_TYPES
     unsigned int   first_index;
     unsigned int   last_index;
     unsigned int   type_size;
-    unsigned short file;
+    unsigned short hash_file;
     unsigned short pad;
     unsigned int   hash_size;
-    unsigned int   hash_base;
+    unsigned int   hash_num_buckets;
     unsigned int   hash_offset;
     unsigned int   hash_len;
     unsigned int   search_offset;
     unsigned int   search_len;
-    unsigned int   unknown_offset;
-    unsigned int   unknown_len;
+    unsigned int   type_remap_offset;
+    unsigned int   type_remap_len;
 } PDB_TYPES, *PPDB_TYPES;

typedef struct _PDB_SYMBOL_RANGE
diff --git a/tools/winedump/pdb.c b/tools/winedump/pdb.c
index 2277e6556f9..db5cea6dc2b 100644
--- a/tools/winedump/pdb.c
+++ b/tools/winedump/pdb.c
@@ -608,18 +608,152 @@ static void pdb_dump_symbols(struct pdb_reader* reader, PDB_STREAM_INDEXES* sidx
     free(filesimage);
 }

-static void pdb_dump_types_hash(struct pdb_reader* reader, unsigned file, const char* strmname)
+static BOOL is_bit_set(const unsigned* dw, unsigned len, unsigned i)
 {
-    void*  hash = NULL;
-    DWORD  size;
+    if (i >= len * sizeof(unsigned) * 8) return FALSE;
+    return (dw[i >> 5] & (1u << (i & 31u))) != 0;
+}

-    hash = reader->read_file(reader, file);
-    if (!hash) return;
+static void pdb_dump_hash_value(const BYTE* ptr, unsigned len)
+{
+    int i;

-    size = pdb_get_file_size(reader, file);
+    printf("[");
+    for (i = len - 1; i >= 0; i--)
+        printf("%02x", ptr[i]);
+    printf("]");
+}
+
+static struct
+{
+    const BYTE* hash;
+    unsigned hash_size;
+} collision_arg;
+
+static int collision_compar(const void *p1, const void *p2)
+{
+    unsigned idx1 = *(unsigned*)p1;
+    unsigned idx2 = *(unsigned*)p2;
+    return memcmp(collision_arg.hash + idx1 * collision_arg.hash_size,
+                  collision_arg.hash + idx2 * collision_arg.hash_size,
+                  collision_arg.hash_size);
+}
+
+static void pdb_dump_types_hash(struct pdb_reader* reader, const PDB_TYPES* types, const char* strmname)
+{
+    void*  hash = NULL;
+    unsigned i, strmsize;
+    const unsigned* table;
+    char* strbase;
+    unsigned *collision;
+    hash = reader->read_file(reader, types->hash_file);
+    if (!hash) return;

printf("Types (%s) hash:\n", strmname);
-    dump_data(hash, size, "    ");
+    strmsize = pdb_get_file_size(reader, types->hash_file);
+    if (types->hash_offset + types->hash_len > strmsize ||
+        (types->last_index - types->first_index) * types->hash_size != types->hash_len ||
+        types->search_offset + types->search_len > strmsize ||
+        types->type_remap_offset + types->type_remap_len > strmsize)
+    {
+        printf("\nIncoherent sizes... skipping\n");
+        return;
+    }
+    printf("\n\tIndexes => hash value:\n");
+    for (i = types->first_index; i < types->last_index; i++)
+    {
+        printf("\t\t%08x => ", i);
+        pdb_dump_hash_value((const BYTE*)hash + types->hash_offset + (i - types->first_index) * types->hash_size, types->hash_size);
+        printf("\n");
+    }
+    /* print collisions in hash table (if any) */
+    collision = malloc((types->last_index - types->first_index) * sizeof(unsigned));
+    if (collision)
+    {
+        collision_arg.hash = (const BYTE*)hash + types->hash_offset;
+        collision_arg.hash_size = types->hash_size;
+        unsigned head_printed = 0;
+        for (i = 0; i < types->last_index - types->first_index; i++) collision[i] = i;
+        qsort(collision, types->last_index - types->first_index, sizeof(unsigned), collision_compar);
+        for (i = 0; i < types->last_index - types->first_index; i++)
+        {
+            unsigned j;
+            for (j = i + 1; j < types->last_index - types->first_index; j++)
+                if (memcmp((const BYTE*)hash + types->hash_offset + collision[i] * types->hash_size,
+                           (const BYTE*)hash + types->hash_offset + collision[j] * types->hash_size,
+                           types->hash_size))
+                    break;
+            if (j > i + 1)
+            {
+                unsigned k;
+                if (!head_printed)
+                {
+                    printf("\n\t\tCollisions:\n");
+                    head_printed = 1;
+                }
+                printf("\t\t\tHash ");
+                pdb_dump_hash_value((const BYTE*)hash + types->hash_offset + collision[i] * types->hash_size, types->hash_size);
+                printf(":");
+                for (k = i; k < j; k++)
+                    printf(" %x", types->first_index + collision[k]);
+                printf("\n");
+                i = j - 1;
+            }
+        }
+        free(collision);
+    }
+    printf("\n\tIndexes => offsets:\n");
+    table = (const unsigned*)((const BYTE*)hash + types->search_offset);
+    for (i = 0; i < types->search_len / (2 * sizeof(unsigned)); i += 2)
+    {
+        printf("\t\t%08x => %08x\n", table[2 * i + 0], table[2 * i + 1]);
+    }
+    if (types->type_remap_len && (strbase = read_string_table(reader)))
+    {
+        unsigned num, capa, count_present, count_deleted;
+        const unsigned* present_bitset;
+        const unsigned* deleted_bitset;
+
+        printf("\n\tType remap:\n");
+        table = (const unsigned*)((const BYTE*)hash + types->type_remap_offset);
+        /* dump_data((const BYTE*)table, types->type_remap_len, "\t\t"); */
+        num = *table++;
+        capa = *table++;
+        count_present = *table++;
+        present_bitset = table;
+        table += count_present;
+        count_deleted = *table++;
+        deleted_bitset = table;
+        table += count_deleted;
+        printf("\t\tNumber of present entries: %u\n", num);
+        printf("\t\tCapacity: %u\n", capa);
+        printf("\t\tBitset present:\n");
+        printf("\t\t\tCount: %u\n", count_present);
+        printf("\t\t\tBitset: ");
+        pdb_dump_hash_value((const BYTE*)present_bitset, count_present * sizeof(unsigned));
+        printf("\n");
+        printf("\t\tBitset deleted:\n");
+        printf("\t\t\tCount: %u\n", count_deleted);
+        printf("\t\t\tBitset: ");
+        pdb_dump_hash_value((const BYTE*)deleted_bitset, count_deleted * sizeof(unsigned));
+        printf("\n");
+        for (i = 0; i < capa; ++i)
+        {
+            printf("\t\t%2u) %c",
+                   i,
+                   is_bit_set(present_bitset, count_present, i) ? 'P' :
+                   is_bit_set(deleted_bitset, count_deleted, i) ? 'D' : '_');
+            if (is_bit_set(present_bitset, count_present, i))
+            {
+                printf(" %s => ", strbase + 12 + *table++);
+                pdb_dump_hash_value((const BYTE*)table, types->hash_size);
+                table = (const unsigned*)((const BYTE*)table + types->hash_size);
+            }
+            printf("\n");
+        }
+        free(strbase);
+        printf("\n");
+    }
     free(hash);
 }

@@ -657,39 +791,39 @@ static void pdb_dump_types(struct pdb_reader* reader, unsigned strmidx, const ch

/* Read type table */
     printf("Types (%s):\n"
-           "\tversion:        %u\n"
-           "\ttype_offset:    %08x\n"
-           "\tfirst_index:    %x\n"
-           "\tlast_index:     %x\n"
-           "\ttype_size:      %x\n"
-           "\tfile:           %x\n"
-           "\tpad:            %x\n"
-           "\thash_size:      %x\n"
-           "\thash_base:      %x\n"
-           "\thash_offset:    %x\n"
-           "\thash_len:       %x\n"
-           "\tsearch_offset:  %x\n"
-           "\tsearch_len:     %x\n"
-           "\tunknown_offset: %x\n"
-           "\tunknown_len:    %x\n",
+           "\tversion:           %u\n"
+           "\ttype_offset:       %08x\n"
+           "\tfirst_index:       %x\n"
+           "\tlast_index:        %x\n"
+           "\ttype_size:         %x\n"
+           "\thash_file:         %x\n"
+           "\tpad:               %x\n"
+           "\thash_size:         %x\n"
+           "\thash_buckets       %x\n"
+           "\thash_offset:       %x\n"
+           "\thash_len:          %x\n"
+           "\tsearch_offset:     %x\n"
+           "\tsearch_len:        %x\n"
+           "\ttype_remap_offset: %x\n"
+           "\ttype_remap_len:    %x\n",
            strmname,
            types->version,
            types->type_offset,
            types->first_index,
            types->last_index,
            types->type_size,
-           types->file,
+           types->hash_file,
            types->pad,
            types->hash_size,
-           types->hash_base,
+           types->hash_num_buckets,
            types->hash_offset,
            types->hash_len,
            types->search_offset,
            types->search_len,
-           types->unknown_offset,
-           types->unknown_len);
+           types->type_remap_offset,
+           types->type_remap_len);
     codeview_dump_types_from_block((const char*)types + types->type_offset, types->type_size);
-    pdb_dump_types_hash(reader, types->file, strmname);
+    pdb_dump_types_hash(reader, types, strmname);
     free(types);
 }

[PATCH v3 2/6] dlls/dbghelp{pdb}: rely on first/last index from type header

[Eric Pouech]

From: Eric Pouech

(mainly code cleanup)
code now follows these guidelines:
- first type index comes from type header (instead of being hard coded as
  FIRST_DEFINABLE_TYPE)
- define PDB & Codeview internals in cvconst.h and mscvinfo.h (instead
  of having definitions in .c files, some of them being duplicate of .h
  content, and their "duplicate" values eventually diverged over time)
- use last_type index from type header (instead of guessing the right value
  when parsing types)

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/msc.c     | 105 ++++++++++++++++++-----------------------
 include/wine/mscvpdb.h |   3 +-
 2 files changed, 48 insertions(+), 60 deletions(-)

diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c
index 2be648ca578..cec4602e0b3 100644
--- a/dlls/dbghelp/msc.c
+++ b/dlls/dbghelp/msc.c
@@ -132,15 +132,13 @@ static void dump(const void* ptr, unsigned len)
  * Process CodeView type information.
  */

-#define MAX_BUILTIN_TYPES	0x06FF
-#define FIRST_DEFINABLE_TYPE    0x1000
-
-static struct symt*     cv_basic_types[MAX_BUILTIN_TYPES];
+static struct symt*     cv_basic_types[T_MAXPREDEFINEDTYPE];

struct cv_defined_module
 {
     BOOL                allowed;
-    unsigned int        num_defined_types;
+    unsigned int        first_type_index;
+    unsigned int        last_type_index;
     struct symt**       defined_types;
 };
 /* FIXME: don't make it static */
@@ -532,15 +530,12 @@ static struct symt*  codeview_get_type(unsigned int typeno, BOOL quiet)

/*
      * Convert Codeview type numbers into something we can grok internally.
-     * Numbers < FIRST_DEFINABLE_TYPE are all fixed builtin types.
-     * Numbers from FIRST_DEFINABLE_TYPE and up are all user defined (structs, etc).
+     * Numbers < T_MAXPREDEFINEDTYPE all fixed builtin types.
+     * Numbers from T_FIRSTDEFINABLETYPE and up are all user defined (structs, etc).
      */
-    if (typeno < FIRST_DEFINABLE_TYPE)
-    {
-        if (typeno < MAX_BUILTIN_TYPES)
-	    symt = cv_basic_types[typeno];
-    }
-    else
+    if (typeno < T_MAXPREDEFINEDTYPE)
+        symt = cv_basic_types[typeno];
+    else if (typeno >= T_FIRSTDEFINABLETYPE)
     {
         unsigned        mod_index = typeno >> 24;
         unsigned        mod_typeno = typeno & 0x00FFFFFF;
@@ -548,12 +543,12 @@ static struct symt*  codeview_get_type(unsigned int typeno, BOOL quiet)

mod = (mod_index == 0) ? cv_current_module : &cv_zmodules[mod_index];

-        if (mod_index >= CV_MAX_MODULES || !mod->allowed) 
+        if (mod_index >= CV_MAX_MODULES || !mod->allowed)
             FIXME("Module of index %d isn't loaded yet (%x)\n", mod_index, typeno);
         else
         {
-            if (mod_typeno - FIRST_DEFINABLE_TYPE < mod->num_defined_types)
-                symt = mod->defined_types[mod_typeno - FIRST_DEFINABLE_TYPE];
+            if (mod_typeno >= mod->first_type_index && mod_typeno < mod->last_type_index)
+                symt = mod->defined_types[mod_typeno - mod->first_type_index];
         }
     }
     if (!quiet && !symt && typeno) FIXME("Returning NULL symt for type-id %x\n", typeno);
@@ -563,22 +558,20 @@ static struct symt*  codeview_get_type(unsigned int typeno, BOOL quiet)
 struct codeview_type_parse
 {
     struct module*      module;
+    PDB_TYPES           header;
     const BYTE*         table;
     const DWORD*        offset;
-    DWORD               num;
 };

static inline const void* codeview_jump_to_type(const struct codeview_type_parse* ctp, DWORD idx)
 {
-    if (idx < FIRST_DEFINABLE_TYPE) return NULL;
-    idx -= FIRST_DEFINABLE_TYPE;
-    return (idx >= ctp->num) ? NULL : (ctp->table + ctp->offset[idx]); 
+    return (idx >= ctp->header.first_index && idx < ctp->header.last_index) ?
+        ctp->table + ctp->offset[idx - ctp->header.first_index] : NULL;
 }

static int codeview_add_type(unsigned int typeno, struct symt* dt)
 {
-    if (typeno < FIRST_DEFINABLE_TYPE)
-        FIXME("What the heck\n");
+    unsigned idx;
     if (!cv_current_module)
     {
         FIXME("Adding %x to non allowed module\n", typeno);
@@ -587,31 +580,18 @@ static int codeview_add_type(unsigned int typeno, struct symt* dt)
     if ((typeno >> 24) != 0)
         FIXME("No module index while inserting type-id assumption is wrong %x\n",
               typeno);
-    if (typeno - FIRST_DEFINABLE_TYPE >= cv_current_module->num_defined_types)
+    if (typeno < cv_current_module->first_type_index || typeno >= cv_current_module->last_type_index)
     {
-        if (cv_current_module->defined_types)
-        {
-            cv_current_module->num_defined_types = max( cv_current_module->num_defined_types * 2,
-                                                        typeno - FIRST_DEFINABLE_TYPE + 1 );
-            cv_current_module->defined_types = HeapReAlloc(GetProcessHeap(),
-                            HEAP_ZERO_MEMORY, cv_current_module->defined_types,
-                            cv_current_module->num_defined_types * sizeof(struct symt*));
-        }
-        else
-        {
-            cv_current_module->num_defined_types = max( 256, typeno - FIRST_DEFINABLE_TYPE + 1 );
-            cv_current_module->defined_types = HeapAlloc(GetProcessHeap(),
-                            HEAP_ZERO_MEMORY,
-                            cv_current_module->num_defined_types * sizeof(struct symt*));
-        }
-        if (cv_current_module->defined_types == NULL) return FALSE;
+        FIXME("Adding type index %x out of bounds\n", typeno);
+        return FALSE;
     }
-    if (cv_current_module->defined_types[typeno - FIRST_DEFINABLE_TYPE])
+    idx = typeno - cv_current_module->first_type_index;
+    if (cv_current_module->defined_types[idx])
     {
-        if (cv_current_module->defined_types[typeno - FIRST_DEFINABLE_TYPE] != dt)
+        if (cv_current_module->defined_types[idx] != dt)
             FIXME("Overwriting at %x\n", typeno);
     }
-    cv_current_module->defined_types[typeno - FIRST_DEFINABLE_TYPE] = dt;
+    cv_current_module->defined_types[idx] = dt;
     return TRUE;
 }

@@ -622,10 +602,11 @@ static void codeview_clear_type_table(void)
     for (i = 0; i < CV_MAX_MODULES; i++)
     {
         if (cv_zmodules[i].allowed)
-            HeapFree(GetProcessHeap(), 0, cv_zmodules[i].defined_types);
+            free(cv_zmodules[i].defined_types);
         cv_zmodules[i].allowed = FALSE;
         cv_zmodules[i].defined_types = NULL;
-        cv_zmodules[i].num_defined_types = 0;
+        cv_zmodules[i].first_type_index = 0;
+        cv_zmodules[i].last_type_index = 0;
     }
     cv_current_module = NULL;
 }
@@ -1381,10 +1362,15 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,

static BOOL codeview_parse_type_table(struct codeview_type_parse* ctp)
 {
-    unsigned int                curr_type = FIRST_DEFINABLE_TYPE;
+    unsigned int                curr_type;
     const union codeview_type*  type;

-    for (curr_type = FIRST_DEFINABLE_TYPE; curr_type < FIRST_DEFINABLE_TYPE + ctp->num; curr_type++)
+    cv_current_module->first_type_index = ctp->header.first_index;
+    cv_current_module->last_type_index = ctp->header.last_index;
+    cv_current_module->defined_types = calloc(ctp->header.last_index - ctp->header.first_index,
+                                              sizeof(*cv_current_module->defined_types));
+
+    for (curr_type = ctp->header.first_index; curr_type < ctp->header.last_index; curr_type++)
     {
         type = codeview_jump_to_type(ctp, curr_type);

@@ -3053,15 +3039,14 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
                                 struct codeview_type_parse* ctp,
                                 BYTE* image)
 {
-    PDB_TYPES types;
-    DWORD total;
     const BYTE* ptr;
     DWORD* offset;
+    int i;

-    pdb_convert_types_header(&types, image);
+    pdb_convert_types_header(&ctp->header, image);

/* Check for unknown versions */
-    switch (types.version)
+    switch (ctp->header.version)
     {
     case 19950410:      /* VC 4.0 */
     case 19951122:
@@ -3070,22 +3055,22 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
     case 20040203:      /* VC 8.0 */
         break;
     default:
-        ERR("-Unknown type info version %d\n", types.version);
+        ERR("-Unknown type info version %d\n", ctp->header.version);
         return FALSE;
     }

ctp->module = msc_dbg->module;
     /* reconstruct the types offset...
-     * FIXME: maybe it's present in the newest PDB_TYPES structures
+     * Note: the hash subfile of the PDB_TYPES only contains a partial table
+     * (not all the indexes are present, so it requires search in table +
+     * linear search from previous index...)
      */
-    total = types.last_index - types.first_index + 1;
-    offset = HeapAlloc(GetProcessHeap(), 0, sizeof(DWORD) * total);
+    offset = HeapAlloc(GetProcessHeap(), 0, sizeof(DWORD) * (ctp->header.last_index - ctp->header.first_index));
     if (!offset) return FALSE;
-    ctp->table = ptr = image + types.type_offset;
-    ctp->num = 0;
-    while (ptr < ctp->table + types.type_size && ctp->num < total)
+    ctp->table = ptr = image + ctp->header.type_offset;
+    for (i = ctp->header.first_index; i < ctp->header.last_index; i++)
     {
-        offset[ctp->num++] = ptr - ctp->table;
+        offset[i - ctp->header.first_index] = ptr - ctp->table;
         ptr += ((const union codeview_type*)ptr)->generic.len + 2;
     }
     ctp->offset = offset;
@@ -3882,7 +3867,9 @@ static BOOL codeview_process_info(const struct process* pcs,
                 types = (const OMFGlobalTypes*)(msc_dbg->root + ent->lfo);
                 ctp.module = msc_dbg->module;
                 ctp.offset = (const DWORD*)(types + 1);
-                ctp.num    = types->cTypes;
+                memset(&ctp.header, 0, sizeof(ctp.header));
+                ctp.header.first_index = T_FIRSTDEFINABLETYPE;
+                ctp.header.last_index = ctp.header.first_index + types->cTypes;
                 ctp.table  = (const BYTE*)(ctp.offset + types->cTypes);

cv_current_module = &cv_zmodules[0];
diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h
index f0fbfa92b39..e0f524ca46f 100644
--- a/include/wine/mscvpdb.h
+++ b/include/wine/mscvpdb.h
@@ -1172,7 +1172,8 @@ union codeview_fieldtype
 #define T_64PCHAR8          0x067c  /* 64 near pointer to 8-bit unicode char */

/* counts, bit masks, and shift values needed to access various parts of the built-in type numbers */
-#define T_MAXPREDEFINEDTYPE 0x0580  /* maximum type index for all built-in types */
+#define T_FIRSTDEFINABLETYPE 0x1000 /* first type index that's not predefined */
+#define T_MAXPREDEFINEDTYPE 0x0680  /* maximum type index for all built-in types */
 #define T_MAXBASICTYPE      0x0080  /* maximum type index all non-pointer built-in types */
 #define T_BASICTYPE_MASK    0x00ff  /* mask of bits that can potentially identify a non-pointer basic type */
 #define T_BASICTYPE_SHIFT   8       /* shift count to push out the basic type bits from a type number */

[PATCH v3 3/6] include/wine/mscvpdb.h: redefine property with bitfields

[Eric Pouech]

From: Eric Pouech

stopping hard coding the bitfield values

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/msc.c     | 10 +++++-----
 include/wine/mscvpdb.h | 37 ++++++++++++++++++++++++++++---------
 tools/winedump/msc.c   | 38 +++++++++++++++++++-------------------
 3 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c
index cec4602e0b3..583f75aed2f 100644
--- a/dlls/dbghelp/msc.c
+++ b/dlls/dbghelp/msc.c
@@ -1014,7 +1014,7 @@ static struct symt* codeview_add_type_enum(struct codeview_type_parse* ctp,
 static struct symt* codeview_add_type_struct(struct codeview_type_parse* ctp,
                                              struct symt* existing,
                                              const char* name, int structlen,
-                                             enum UdtKind kind, unsigned property)
+                                             enum UdtKind kind, cv_property_t property)
 {
     struct symt_udt*    symt;

@@ -1044,7 +1044,7 @@ static struct symt* codeview_add_type_struct(struct codeview_type_parse* ctp,
     {
         if (!(symt = codeview_cast_symt(existing, SymTagUDT))) return NULL;
         /* should also check that all fields are the same */
-        if (!(property & 0x80)) /* 0x80 = forward declaration */
+        if (!property.is_forward_defn)
         {
             if (!symt->size) /* likely prior forward declaration, set UDT size */
                 symt_set_udt_size(ctp->module, symt, structlen);
@@ -1194,7 +1194,7 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
         if (details)
         {
             codeview_add_type(curr_type, symt);
-            if (!(type->struct_v1.property & 0x80)) /* 0x80 = forward declaration */
+            if (!type->struct_v1.property.is_forward_defn)
                 codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
                                                     type->struct_v1.fieldlist);
         }
@@ -1210,7 +1210,7 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
         if (details)
         {
             codeview_add_type(curr_type, symt);
-            if (!(type->struct_v2.property & 0x80)) /* 0x80 = forward declaration */
+            if (!type->struct_v2.property.is_forward_defn)
                 codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
                                                     type->struct_v2.fieldlist);
         }
@@ -1226,7 +1226,7 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
         if (details)
         {
             codeview_add_type(curr_type, symt);
-            if (!(type->struct_v3.property & 0x80)) /* 0x80 = forward declaration */
+            if (!type->struct_v3.property.is_forward_defn)
                 codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
                                                     type->struct_v3.fieldlist);
         }
diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h
index e0f524ca46f..8fb30d25cc6 100644
--- a/include/wine/mscvpdb.h
+++ b/include/wine/mscvpdb.h
@@ -112,6 +112,25 @@ typedef unsigned short  cv_typ16_t;
 typedef unsigned int    cv_typ_t;
 typedef cv_typ_t        cv_itemid_t;

+typedef struct cv_property_t
+{
+    unsigned short  is_packed               : 1;
+    unsigned short  has_ctor                : 1;
+    unsigned short  has_overloaded_operator : 1;
+    unsigned short  is_nested               : 1;
+    unsigned short  has_nested              : 1;
+    unsigned short  has_overloaded_assign   : 1;
+    unsigned short  has_operator_cast       : 1;
+    unsigned short  is_forward_defn         : 1;
+    unsigned short  is_scoped               : 1;
+    unsigned short  has_decorated_name      : 1; /* follows name field */
+    unsigned short  is_sealed               : 1; /* not usage as base class */
+    unsigned short  hfa                     : 2;
+    unsigned short  is_intrinsic            : 1;
+    unsigned short  mocom                   : 2;
+}
+cv_property_t;
+
 /* ======================================== *
  *             Type information
  * ======================================== */
@@ -201,7 +220,7 @@ union codeview_type
         unsigned short int      id;
         short int               n_element;
         cv_typ16_t              fieldlist;
-        short int               property;
+        cv_property_t           property;
         cv_typ16_t              derived;
         cv_typ16_t              vshape;
         unsigned short int      structlen;  /* numeric leaf */
@@ -215,7 +234,7 @@ union codeview_type
         unsigned short int      len;
         unsigned short int      id;
         short int               n_element;
-        short int               property;
+        cv_property_t           property;
         cv_typ_t                fieldlist;
         cv_typ_t                derived;
         cv_typ_t                vshape;
@@ -230,7 +249,7 @@ union codeview_type
         unsigned short int      len;
         unsigned short int      id;
         short int               n_element;
-        short int               property;
+        cv_property_t           property;
         cv_typ_t                fieldlist;
         cv_typ_t                derived;
         cv_typ_t                vshape;
@@ -246,7 +265,7 @@ union codeview_type
         unsigned short int      id;
         short int               count;
         cv_typ16_t              fieldlist;
-        short int               property;
+        cv_property_t           property;
         unsigned short int      un_len;     /* numeric leaf */
 #if 0
         struct p_string         p_name;
@@ -258,7 +277,7 @@ union codeview_type
         unsigned short int      len;
         unsigned short int      id;
         short int               count;
-        short int               property;
+        cv_property_t           property;
         cv_typ_t                fieldlist;
         unsigned short int      un_len;     /* numeric leaf */
 #if 0
@@ -271,7 +290,7 @@ union codeview_type
         unsigned short int      len;
         unsigned short int      id;
         short int               count;
-        short int               property;
+        cv_property_t           property;
         cv_typ_t                fieldlist;
         unsigned short int      un_len;     /* numeric leaf */
 #if 0
@@ -286,7 +305,7 @@ union codeview_type
         short int               count;
         cv_typ16_t              type;
         cv_typ16_t              fieldlist;
-        short int               property;
+        cv_property_t           property;
         struct p_string         p_name;
     } enumeration_v1;

@@ -295,7 +314,7 @@ union codeview_type
         unsigned short int      len;
         unsigned short int      id;
         short int               count;
-        short int               property;
+        cv_property_t           property;
         cv_typ_t                type;
         cv_typ_t                fieldlist;
         struct p_string         p_name;
@@ -306,7 +325,7 @@ union codeview_type
         unsigned short int      len;
         unsigned short int      id;
         short int               count;
-        short int               property;
+        cv_property_t           property;
         cv_typ_t                type;
         cv_typ_t                fieldlist;
         char                    name[1];
diff --git a/tools/winedump/msc.c b/tools/winedump/msc.c
index a7e3df961bb..8c38dd1819e 100644
--- a/tools/winedump/msc.c
+++ b/tools/winedump/msc.c
@@ -227,28 +227,28 @@ static const char* get_attr(unsigned attr)
     return tmp;
 }

-static const char* get_property(unsigned prop)
+static const char* get_property(cv_property_t prop)
 {
     static char tmp[1024];
     unsigned    pos = 0;

-    if (!prop) return "none";
 #define X(s) {if (pos) tmp[pos++] = ';'; strcpy(tmp + pos, s); pos += strlen(s);}
-    if (prop & 0x0001) X("packed");
-    if (prop & 0x0002) X("w/{cd}tor");
-    if (prop & 0x0004) X("w/overloaded-ops");
-    if (prop & 0x0008) X("nested-class");
-    if (prop & 0x0010) X("has-nested-classes");
-    if (prop & 0x0020) X("w/overloaded-assign");
-    if (prop & 0x0040) X("w/casting-methods");
-    if (prop & 0x0080) X("forward");
-    if (prop & 0x0100) X("scoped");
-    if (prop & 0x0200) X("decorated-name");
-    if (prop & 0x0400) X("sealed-name");
-    if (prop & 0x1800) pos += sprintf(tmp, "hfa%x", (prop >> 11) & 3);
-    if (prop & 0x2000) X("intrinsic");
-    if (prop & 0xC000) pos += sprintf(tmp, "mocom%x", prop >> 14);
+    if (prop.is_packed)                 X("packed");
+    if (prop.has_ctor)                  X("w/{cd}tor");
+    if (prop.has_overloaded_operator)   X("w/overloaded-ops");
+    if (prop.is_nested)                 X("nested-class");
+    if (prop.has_nested)                X("has-nested-classes");
+    if (prop.has_overloaded_assign)     X("w/overloaded-assign");
+    if (prop.has_operator_cast)         X("w/casting-methods");
+    if (prop.is_forward_defn)           X("forward");
+    if (prop.is_scoped)                 X("scoped");
+    if (prop.has_decorated_name)        X("decorated-name");
+    if (prop.is_sealed)                 X("sealed");
+    if (prop.hfa)                       pos += sprintf(tmp, "hfa%x", prop.hfa);
+    if (prop.is_intrinsic)              X("intrinsic");
+    if (prop.mocom)                     pos += sprintf(tmp, "mocom%x", prop.mocom);
 #undef X
+    if (!pos) return "none";

tmp[pos] = '\0';
     assert(pos < sizeof(tmp));
@@ -902,7 +902,7 @@ static void codeview_dump_one_type(unsigned curr_type, const union codeview_type
                str, type->struct_v3.n_element, get_property(type->struct_v3.property),
                type->struct_v3.fieldlist, type->struct_v3.derived,
                type->struct_v3.vshape, value);
-        if (type->union_v3.property & 0x200)
+        if (type->union_v3.property.has_decorated_name)
             printf("\t\tDecorated name:%s\n", str + strlen(str) + 1);
         break;

@@ -929,7 +929,7 @@ static void codeview_dump_one_type(unsigned curr_type, const union codeview_type
                curr_type, str, type->union_v3.count,
                get_property(type->union_v3.property),
                type->union_v3.fieldlist, value);
-        if (type->union_v3.property & 0x200)
+        if (type->union_v3.property.has_decorated_name)
             printf("\t\tDecorated name:%s\n", str + strlen(str) + 1);
         break;

@@ -958,7 +958,7 @@ static void codeview_dump_one_type(unsigned curr_type, const union codeview_type
                type->enumeration_v3.fieldlist,
                type->enumeration_v3.count,
                get_property(type->enumeration_v3.property));
-        if (type->union_v3.property & 0x200)
+        if (type->union_v3.property.has_decorated_name)
             printf("\t\tDecorated name:%s\n", type->enumeration_v3.name + strlen(type->enumeration_v3.name) + 1);
         break;

[PATCH v3 4/6] dlls/dbghelp{pdb}: clearly separate the type loading into two passes

[Eric Pouech]

From: Eric Pouech

- removed the details parameter from load_one_type
  (the two passes split didn't even correspond to the details split)
- correctly move forward handling for unions & enum in first pass

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/msc.c | 486 ++++++++++++++++++++++-----------------------
 1 file changed, 241 insertions(+), 245 deletions(-)

diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c
index 583f75aed2f..dfb5cac94aa 100644
--- a/dlls/dbghelp/msc.c
+++ b/dlls/dbghelp/msc.c
@@ -613,7 +613,7 @@ static void codeview_clear_type_table(void)

static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
                                             unsigned curr_type,
-                                            const union codeview_type* type, BOOL details);
+                                            const union codeview_type* type);

static void* codeview_cast_symt(struct symt* symt, enum SymTagEnum tag)
 {
@@ -626,7 +626,7 @@ static void* codeview_cast_symt(struct symt* symt, enum SymTagEnum tag)
 }

static struct symt* codeview_fetch_type(struct codeview_type_parse* ctp,
-                                        unsigned typeno, BOOL details)
+                                        unsigned typeno)
 {
     struct symt*                symt;
     const union codeview_type*  p;
@@ -634,29 +634,40 @@ static struct symt* codeview_fetch_type(struct codeview_type_parse* ctp,
     if (!typeno) return NULL;
     if ((symt = codeview_get_type(typeno, TRUE))) return symt;

-    /* forward declaration */
-    if (!(p = codeview_jump_to_type(ctp, typeno)))
+    if ((p = codeview_jump_to_type(ctp, typeno)))
+        symt = codeview_parse_one_type(ctp, typeno, p);
+    if (!symt) FIXME("Couldn't load type %x\n", typeno);
+    return symt;
+}
+
+static BOOL codeview_is_remapable_type(const union codeview_type* type)
+{
+    switch (type->generic.id)
     {
-        FIXME("Cannot locate type %x\n", typeno);
-        return NULL;
+    case LF_STRUCTURE_V1:
+    case LF_CLASS_V1:
+    case LF_STRUCTURE_V2:
+    case LF_CLASS_V2:
+    case LF_STRUCTURE_V3:
+    case LF_CLASS_V3:
+    case LF_UNION_V1:
+    case LF_UNION_V2:
+    case LF_UNION_V3:
+    case LF_ENUM_V1:
+    case LF_ENUM_V2:
+    case LF_ENUM_V3:
+        return TRUE;
+    default: return FALSE;
     }
-    symt = codeview_parse_one_type(ctp, typeno, p, details);
-    if (!symt) FIXME("Couldn't load forward type %x\n", typeno);
-    return symt;
 }

static struct symt* codeview_add_type_pointer(struct codeview_type_parse* ctp,
-                                              struct symt* existing,
                                               unsigned int pointee_type)
 {
     struct symt* pointee;

-    if (existing)
-    {
-        existing = codeview_cast_symt(existing, SymTagPointerType);
-        return existing;
-    }
-    pointee = codeview_fetch_type(ctp, pointee_type, FALSE);
+    pointee = codeview_fetch_type(ctp, pointee_type);
+    if (!pointee) return NULL;
     return &symt_new_pointer(ctp->module, pointee, ctp->module->cpu->word_size)->symt;
 }

@@ -666,8 +677,8 @@ static struct symt* codeview_add_type_array(struct codeview_type_parse* ctp,
                                             unsigned int indextype,
                                             unsigned int arr_len)
 {
-    struct symt*        elem = codeview_fetch_type(ctp, elemtype, FALSE);
-    struct symt*        index = codeview_fetch_type(ctp, indextype, FALSE);
+    struct symt*        elem = codeview_fetch_type(ctp, elemtype);
+    struct symt*        index = codeview_fetch_type(ctp, indextype);
     DWORD64             elem_size;
     DWORD               count = 0;

@@ -680,14 +691,20 @@ static struct symt* codeview_add_type_array(struct codeview_type_parse* ctp,
     return &symt_new_array(ctp->module, 0, count, elem, index)->symt;
 }

-static BOOL codeview_add_type_enum_field_list(struct module* module,
+static BOOL codeview_add_type_enum_field_list(struct codeview_type_parse* ctp,
                                               struct symt_enum* symt,
-                                              const union codeview_reftype* ref_type)
+                                              unsigned typeno)
 {
-    const unsigned char*                ptr = ref_type->fieldlist.list;
-    const unsigned char*                last = (const BYTE*)ref_type + ref_type->generic.len + 2;
+    const union codeview_reftype*       ref_type;
+    const unsigned char*                ptr;
+    const unsigned char*                last;
     const union codeview_fieldtype*     type;

+    if (!typeno) return TRUE;
+    if (!(ref_type = codeview_jump_to_type(ctp, typeno))) return FALSE;
+    ptr = ref_type->fieldlist.list;
+    last = (const BYTE*)ref_type + ref_type->generic.len + 2;
+
     while (ptr < last)
     {
         if (*ptr >= 0xf0)       /* LF_PAD... */
@@ -705,7 +722,7 @@ static BOOL codeview_add_type_enum_field_list(struct module* module,
             int value, vlen = numeric_leaf(&value, &type->enumerate_v1.value);
             const struct p_string* p_name = (const struct p_string*)((const unsigned char*)&type->enumerate_v1.value + vlen);

-            symt_add_enum_element(module, symt, terminate_string(p_name), value);
+            symt_add_enum_element(ctp->module, symt, terminate_string(p_name), value);
             ptr += 2 + 2 + vlen + (1 + p_name->namelen);
             break;
         }
@@ -714,7 +731,7 @@ static BOOL codeview_add_type_enum_field_list(struct module* module,
             int value, vlen = numeric_leaf(&value, &type->enumerate_v3.value);
             const char* name = (const char*)&type->enumerate_v3.value + vlen;

-            symt_add_enum_element(module, symt, name, value);
+            symt_add_enum_element(ctp->module, symt, name, value);
             ptr += 2 + 2 + vlen + (1 + strlen(name));
             break;
         }
@@ -740,19 +757,19 @@ static void codeview_add_udt_element(struct codeview_type_parse* ctp,
         {
         case LF_BITFIELD_V1:
             symt_add_udt_element(ctp->module, symt, name,
-                                 codeview_fetch_type(ctp, cv_type->bitfield_v1.type, FALSE),
+                                 codeview_fetch_type(ctp, cv_type->bitfield_v1.type),
                                  value, cv_type->bitfield_v1.bitoff,
                                  cv_type->bitfield_v1.nbits);
             return;
         case LF_BITFIELD_V2:
             symt_add_udt_element(ctp->module, symt, name,
-                                 codeview_fetch_type(ctp, cv_type->bitfield_v2.type, FALSE),
+                                 codeview_fetch_type(ctp, cv_type->bitfield_v2.type),
                                  value, cv_type->bitfield_v2.bitoff,
                                  cv_type->bitfield_v2.nbits);
             return;
         }
     }
-    subtype = codeview_fetch_type(ctp, type, FALSE);
+    subtype = codeview_fetch_type(ctp, type);

if (subtype)
     {
@@ -984,60 +1001,30 @@ static int codeview_add_type_struct_field_list(struct codeview_type_parse* ctp,
     return TRUE;
 }

-static struct symt* codeview_add_type_enum(struct codeview_type_parse* ctp,
-                                           struct symt* existing,
-                                           const char* name,
-                                           unsigned fieldlistno,
-                                           unsigned basetype)
-{
-    struct symt_enum*   symt;
-
-    if (existing)
-    {
-        if (!(symt = codeview_cast_symt(existing, SymTagEnum))) return NULL;
-        /* should also check that all fields are the same */
-    }
-    else
-    {
-        symt = symt_new_enum(ctp->module, name,
-                             codeview_fetch_type(ctp, basetype, FALSE));
-        if (fieldlistno)
-        {
-            const union codeview_reftype* fieldlist;
-            fieldlist = codeview_jump_to_type(ctp, fieldlistno);
-            codeview_add_type_enum_field_list(ctp->module, symt, fieldlist);
-        }
-    }
-    return &symt->symt;
-}
-
 static struct symt* codeview_add_type_struct(struct codeview_type_parse* ctp,
-                                             struct symt* existing,
                                              const char* name, int structlen,
                                              enum UdtKind kind, cv_property_t property)
 {
     struct symt_udt*    symt;
+    struct symt*        existing = NULL;

/* if we don't have an existing type, try to find one with same name
      * FIXME: what to do when several types in different CUs have same name ?
      */
-    if (!existing)
+    void*                       ptr;
+    struct symt_ht*             type;
+    struct hash_table_iter      hti;
+
+    hash_table_iter_init(&ctp->module->ht_types, &hti, name);
+    while ((ptr = hash_table_iter_up(&hti)))
     {
-        void*                       ptr;
-        struct symt_ht*             type;
-        struct hash_table_iter      hti;
+        type = CONTAINING_RECORD(ptr, struct symt_ht, hash_elt);

-        hash_table_iter_init(&ctp->module->ht_types, &hti, name);
-        while ((ptr = hash_table_iter_up(&hti)))
+        if (type->symt.tag == SymTagUDT &&
+            type->hash_elt.name && !strcmp(type->hash_elt.name, name))
         {
-            type = CONTAINING_RECORD(ptr, struct symt_ht, hash_elt);
-
-            if (type->symt.tag == SymTagUDT &&
-                type->hash_elt.name && !strcmp(type->hash_elt.name, name))
-            {
-                existing = &type->symt;
-                break;
-            }
+            existing = &type->symt;
+            break;
         }
     }
     if (existing)
@@ -1057,21 +1044,12 @@ static struct symt* codeview_add_type_struct(struct codeview_type_parse* ctp,
     return &symt->symt;
 }

-static struct symt* codeview_new_func_signature(struct codeview_type_parse* ctp, 
-                                                struct symt* existing,
+static struct symt* codeview_new_func_signature(struct codeview_type_parse* ctp,
                                                 enum CV_call_e call_conv)
 {
     struct symt_function_signature*     sym;

-    if (existing)
-    {
-        sym = codeview_cast_symt(existing, SymTagFunctionType);
-        if (!sym) return NULL;
-    }
-    else
-    {
-        sym = symt_new_function_signature(ctp->module, NULL, call_conv);
-    }
+    sym = symt_new_function_signature(ctp->module, NULL, call_conv);
     return &sym->symt;
 }

@@ -1082,7 +1060,7 @@ static void codeview_add_func_signature_args(struct codeview_type_parse* ctp,
 {
     const union codeview_reftype*       reftype;

-    sym->rettype = codeview_fetch_type(ctp, ret_type, FALSE);
+    sym->rettype = codeview_fetch_type(ctp, ret_type);
     if (args_list && (reftype = codeview_jump_to_type(ctp, args_list)))
     {
         unsigned int i;
@@ -1091,12 +1069,12 @@ static void codeview_add_func_signature_args(struct codeview_type_parse* ctp,
         case LF_ARGLIST_V1:
             for (i = 0; i < reftype->arglist_v1.num; i++)
                 symt_add_function_signature_parameter(ctp->module, sym,
-                                                      codeview_fetch_type(ctp, reftype->arglist_v1.args[i], FALSE));
+                                                      codeview_fetch_type(ctp, reftype->arglist_v1.args[i]));
             break;
         case LF_ARGLIST_V2:
             for (i = 0; i < reftype->arglist_v2.num; i++)
                 symt_add_function_signature_parameter(ctp->module, sym,
-                                                      codeview_fetch_type(ctp, reftype->arglist_v2.args[i], FALSE));
+                                                      codeview_fetch_type(ctp, reftype->arglist_v2.args[i]));
             break;
         default:
             FIXME("Unexpected leaf %x for signature's pmt\n", reftype->generic.id);
@@ -1106,15 +1084,12 @@ static void codeview_add_func_signature_args(struct codeview_type_parse* ctp,

static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
                                             unsigned curr_type,
-                                            const union codeview_type* type, BOOL details)
+                                            const union codeview_type* type)
 {
-    struct symt*                symt;
+    struct symt*                symt = NULL;
     int                         value, leaf_len;
     const struct p_string*      p_name;
     const char*                 c_name;
-    struct symt*                existing;
-
-    existing = codeview_get_type(curr_type, TRUE);

switch (type->generic.id)
     {
@@ -1128,7 +1103,7 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
              type->modifier_v1.attribute & 0x02 ? "volatile " : "",
              type->modifier_v1.attribute & 0x04 ? "unaligned " : "",
              type->modifier_v1.attribute & ~0x07 ? "unknown " : "");
-        symt = codeview_fetch_type(ctp, type->modifier_v1.type, details);
+        symt = codeview_fetch_type(ctp, type->modifier_v1.type);
         break;
     case LF_MODIFIER_V2:
         /* FIXME: we don't handle modifiers, but readd previous type on the curr_type */
@@ -1138,214 +1113,141 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
              type->modifier_v2.attribute & 0x02 ? "volatile " : "",
              type->modifier_v2.attribute & 0x04 ? "unaligned " : "",
              type->modifier_v2.attribute & ~0x07 ? "unknown " : "");
-        symt = codeview_fetch_type(ctp, type->modifier_v2.type, details);
+        symt = codeview_fetch_type(ctp, type->modifier_v2.type);
         break;

case LF_POINTER_V1:
-        symt = codeview_add_type_pointer(ctp, existing, type->pointer_v1.datatype);
+        symt = codeview_add_type_pointer(ctp, type->pointer_v1.datatype);
         break;
     case LF_POINTER_V2:
-        symt = codeview_add_type_pointer(ctp, existing, type->pointer_v2.datatype);
+        symt = codeview_add_type_pointer(ctp, type->pointer_v2.datatype);
         break;

case LF_ARRAY_V1:
-        if (existing) symt = codeview_cast_symt(existing, SymTagArrayType);
-        else
-        {
-            leaf_len = numeric_leaf(&value, &type->array_v1.arrlen);
-            p_name = (const struct p_string*)((const unsigned char*)&type->array_v1.arrlen + leaf_len);
-            symt = codeview_add_type_array(ctp, terminate_string(p_name),
-                                           type->array_v1.elemtype,
-                                           type->array_v1.idxtype, value);
-        }
+        leaf_len = numeric_leaf(&value, &type->array_v1.arrlen);
+        p_name = (const struct p_string*)((const unsigned char*)&type->array_v1.arrlen + leaf_len);
+        symt = codeview_add_type_array(ctp, terminate_string(p_name),
+                                       type->array_v1.elemtype,
+                                       type->array_v1.idxtype, value);
         break;
     case LF_ARRAY_V2:
-        if (existing) symt = codeview_cast_symt(existing, SymTagArrayType);
-        else
-        {
-            leaf_len = numeric_leaf(&value, &type->array_v2.arrlen);
-            p_name = (const struct p_string*)((const unsigned char*)&type->array_v2.arrlen + leaf_len);
+        leaf_len = numeric_leaf(&value, &type->array_v2.arrlen);
+        p_name = (const struct p_string*)((const unsigned char*)&type->array_v2.arrlen + leaf_len);

-            symt = codeview_add_type_array(ctp, terminate_string(p_name),
-                                           type->array_v2.elemtype,
-                                           type->array_v2.idxtype, value);
-        }
+        symt = codeview_add_type_array(ctp, terminate_string(p_name),
+                                       type->array_v2.elemtype,
+                                       type->array_v2.idxtype, value);
         break;
     case LF_ARRAY_V3:
-        if (existing) symt = codeview_cast_symt(existing, SymTagArrayType);
-        else
-        {
-            leaf_len = numeric_leaf(&value, &type->array_v3.arrlen);
-            c_name = (const char*)&type->array_v3.arrlen + leaf_len;
+        leaf_len = numeric_leaf(&value, &type->array_v3.arrlen);
+        c_name = (const char*)&type->array_v3.arrlen + leaf_len;

-            symt = codeview_add_type_array(ctp, c_name,
-                                           type->array_v3.elemtype,
-                                           type->array_v3.idxtype, value);
-        }
+        symt = codeview_add_type_array(ctp, c_name,
+                                       type->array_v3.elemtype,
+                                       type->array_v3.idxtype, value);
         break;

case LF_STRUCTURE_V1:
     case LF_CLASS_V1:
-        leaf_len = numeric_leaf(&value, &type->struct_v1.structlen);
-        p_name = (const struct p_string*)((const unsigned char*)&type->struct_v1.structlen + leaf_len);
-        symt = codeview_add_type_struct(ctp, existing, terminate_string(p_name), value,
-                                        type->generic.id == LF_CLASS_V1 ? UdtClass : UdtStruct,
-                                        type->struct_v1.property);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            if (!type->struct_v1.property.is_forward_defn)
-                codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
-                                                    type->struct_v1.fieldlist);
-        }
+        if (!type->struct_v1.property.is_forward_defn)
+            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)codeview_get_type(curr_type, TRUE),
+                                                type->struct_v1.fieldlist);
         break;

case LF_STRUCTURE_V2:
     case LF_CLASS_V2:
-        leaf_len = numeric_leaf(&value, &type->struct_v2.structlen);
-        p_name = (const struct p_string*)((const unsigned char*)&type->struct_v2.structlen + leaf_len);
-        symt = codeview_add_type_struct(ctp, existing, terminate_string(p_name), value,
-                                        type->generic.id == LF_CLASS_V2 ? UdtClass : UdtStruct,
-                                        type->struct_v2.property);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            if (!type->struct_v2.property.is_forward_defn)
-                codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
-                                                    type->struct_v2.fieldlist);
-        }
+        if (!type->struct_v2.property.is_forward_defn)
+            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)codeview_get_type(curr_type, TRUE),
+                                                type->struct_v2.fieldlist);
         break;

case LF_STRUCTURE_V3:
     case LF_CLASS_V3:
-        leaf_len = numeric_leaf(&value, &type->struct_v3.structlen);
-        c_name = (const char*)&type->struct_v3.structlen + leaf_len;
-        symt = codeview_add_type_struct(ctp, existing, c_name, value,
-                                        type->generic.id == LF_CLASS_V3 ? UdtClass : UdtStruct,
-                                        type->struct_v3.property);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            if (!type->struct_v3.property.is_forward_defn)
-                codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
-                                                    type->struct_v3.fieldlist);
-        }
+        if (!type->struct_v3.property.is_forward_defn)
+            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)codeview_get_type(curr_type, TRUE),
+                                                type->struct_v3.fieldlist);
         break;

case LF_UNION_V1:
-        leaf_len = numeric_leaf(&value, &type->union_v1.un_len);
-        p_name = (const struct p_string*)((const unsigned char*)&type->union_v1.un_len + leaf_len);
-        symt = codeview_add_type_struct(ctp, existing, terminate_string(p_name),
-                                        value, UdtUnion, type->union_v1.property);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
+        if (!type->union_v1.property.is_forward_defn)
+            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)codeview_get_type(curr_type, TRUE),
                                                 type->union_v1.fieldlist);
-        }
         break;

case LF_UNION_V2:
-        leaf_len = numeric_leaf(&value, &type->union_v2.un_len);
-        p_name = (const struct p_string*)((const unsigned char*)&type->union_v2.un_len + leaf_len);
-        symt = codeview_add_type_struct(ctp, existing, terminate_string(p_name),
-                                        value, UdtUnion, type->union_v2.property);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
+        if (!type->union_v2.property.is_forward_defn)
+            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)codeview_get_type(curr_type, TRUE),
                                                 type->union_v2.fieldlist);
-        }
         break;

case LF_UNION_V3:
-        leaf_len = numeric_leaf(&value, &type->union_v3.un_len);
-        c_name = (const char*)&type->union_v3.un_len + leaf_len;
-        symt = codeview_add_type_struct(ctp, existing, c_name,
-                                        value, UdtUnion, type->union_v3.property);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)symt,
+        if (!type->union_v3.property.is_forward_defn)
+            codeview_add_type_struct_field_list(ctp, (struct symt_udt*)codeview_get_type(curr_type, TRUE),
                                                 type->union_v3.fieldlist);
-        }
         break;

case LF_ENUM_V1:
-        symt = codeview_add_type_enum(ctp, existing,
-                                      terminate_string(&type->enumeration_v1.p_name),
-                                      type->enumeration_v1.fieldlist,
-                                      type->enumeration_v1.type);
+        {
+            struct symt_enum* senum = (struct symt_enum*)codeview_get_type(curr_type, TRUE);
+            senum->base_type = codeview_fetch_type(ctp, type->enumeration_v1.type);
+            codeview_add_type_enum_field_list(ctp, senum, type->enumeration_v1.fieldlist);
+        }
         break;

case LF_ENUM_V2:
-        symt = codeview_add_type_enum(ctp, existing,
-                                      terminate_string(&type->enumeration_v2.p_name),
-                                      type->enumeration_v2.fieldlist,
-                                      type->enumeration_v2.type);
+        {
+            struct symt_enum* senum = (struct symt_enum*)codeview_get_type(curr_type, TRUE);
+            senum->base_type = codeview_fetch_type(ctp, type->enumeration_v2.type);
+            codeview_add_type_enum_field_list(ctp, senum, type->enumeration_v2.fieldlist);
+        }
         break;

case LF_ENUM_V3:
-        symt = codeview_add_type_enum(ctp, existing, type->enumeration_v3.name,
-                                      type->enumeration_v3.fieldlist,
-                                      type->enumeration_v3.type);
+        {
+            struct symt_enum* senum = (struct symt_enum*)codeview_get_type(curr_type, TRUE);
+            senum->base_type = codeview_fetch_type(ctp, type->enumeration_v3.type);
+            codeview_add_type_enum_field_list(ctp, senum, type->enumeration_v3.fieldlist);
+        }
         break;

case LF_PROCEDURE_V1:
-        symt = codeview_new_func_signature(ctp, existing, type->procedure_v1.callconv);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_func_signature_args(ctp,
-                                             (struct symt_function_signature*)symt,
-                                             type->procedure_v1.rvtype,
-                                             type->procedure_v1.arglist);
-        }
+        symt = codeview_new_func_signature(ctp, type->procedure_v1.callconv);
+        codeview_add_func_signature_args(ctp,
+                                         (struct symt_function_signature*)symt,
+                                         type->procedure_v1.rvtype,
+                                         type->procedure_v1.arglist);
         break;
     case LF_PROCEDURE_V2:
-        symt = codeview_new_func_signature(ctp, existing,type->procedure_v2.callconv);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_func_signature_args(ctp,
-                                             (struct symt_function_signature*)symt,
-                                             type->procedure_v2.rvtype,
-                                             type->procedure_v2.arglist);
-        }
+        symt = codeview_new_func_signature(ctp,type->procedure_v2.callconv);
+        codeview_add_func_signature_args(ctp,
+                                         (struct symt_function_signature*)symt,
+                                         type->procedure_v2.rvtype,
+                                         type->procedure_v2.arglist);
         break;

case LF_MFUNCTION_V1:
         /* FIXME: for C++, this is plain wrong, but as we don't use arg types
          * nor class information, this would just do for now
          */
-        symt = codeview_new_func_signature(ctp, existing, type->mfunction_v1.callconv);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_func_signature_args(ctp,
-                                             (struct symt_function_signature*)symt,
-                                             type->mfunction_v1.rvtype,
-                                             type->mfunction_v1.arglist);
-        }
+        symt = codeview_new_func_signature(ctp, type->mfunction_v1.callconv);
+        codeview_add_func_signature_args(ctp,
+                                         (struct symt_function_signature*)symt,
+                                         type->mfunction_v1.rvtype,
+                                         type->mfunction_v1.arglist);
         break;
     case LF_MFUNCTION_V2:
         /* FIXME: for C++, this is plain wrong, but as we don't use arg types
          * nor class information, this would just do for now
          */
-        symt = codeview_new_func_signature(ctp, existing, type->mfunction_v2.callconv);
-        if (details)
-        {
-            codeview_add_type(curr_type, symt);
-            codeview_add_func_signature_args(ctp,
-                                             (struct symt_function_signature*)symt,
-                                             type->mfunction_v2.rvtype,
-                                             type->mfunction_v2.arglist);
-        }
+        symt = codeview_new_func_signature(ctp, type->mfunction_v2.callconv);
+        codeview_add_func_signature_args(ctp,
+                                         (struct symt_function_signature*)symt,
+                                         type->mfunction_v2.rvtype,
+                                         type->mfunction_v2.arglist);
         break;

case LF_VTSHAPE_V1:
         /* this is an ugly hack... FIXME when we have C++ support */
-        if (!(symt = existing))
         {
             char    buf[128];
             snprintf(buf, sizeof(buf), "__internal_vt_shape_%x\n", curr_type);
@@ -1357,7 +1259,99 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
         dump(type, 2 + type->generic.len);
         return NULL;
     }
-    return codeview_add_type(curr_type, symt) ? symt : NULL;
+    return symt && codeview_add_type(curr_type, symt) ? symt : NULL;
+}
+
+static struct symt* codeview_parse_remapable_type(struct codeview_type_parse* ctp,
+                                                  unsigned curr_type,
+                                                  const union codeview_type* type)
+{
+    struct symt*                symt;
+    int                         value, leaf_len;
+    const struct p_string*      p_name;
+    const char*                 c_name;
+
+    switch (type->generic.id)
+    {
+    case LF_STRUCTURE_V1:
+    case LF_CLASS_V1:
+        leaf_len = numeric_leaf(&value, &type->struct_v1.structlen);
+        p_name = (const struct p_string*)((const unsigned char*)&type->struct_v1.structlen + leaf_len);
+        symt = codeview_add_type_struct(ctp, terminate_string(p_name), value,
+                                        type->generic.id == LF_CLASS_V1 ? UdtClass : UdtStruct,
+                                        type->struct_v1.property);
+        break;
+
+    case LF_STRUCTURE_V2:
+    case LF_CLASS_V2:
+        leaf_len = numeric_leaf(&value, &type->struct_v2.structlen);
+        p_name = (const struct p_string*)((const unsigned char*)&type->struct_v2.structlen + leaf_len);
+        symt = codeview_add_type_struct(ctp, terminate_string(p_name), value,
+                                        type->generic.id == LF_CLASS_V2 ? UdtClass : UdtStruct,
+                                        type->struct_v2.property);
+        break;
+
+    case LF_STRUCTURE_V3:
+    case LF_CLASS_V3:
+        leaf_len = numeric_leaf(&value, &type->struct_v3.structlen);
+        c_name = (const char*)&type->struct_v3.structlen + leaf_len;
+        symt = codeview_add_type_struct(ctp, c_name, value,
+                                        type->generic.id == LF_CLASS_V3 ? UdtClass : UdtStruct,
+                                        type->struct_v3.property);
+        break;
+
+    case LF_UNION_V1:
+        leaf_len = numeric_leaf(&value, &type->union_v1.un_len);
+        p_name = (const struct p_string*)((const unsigned char*)&type->union_v1.un_len + leaf_len);
+        symt = codeview_add_type_struct(ctp, terminate_string(p_name),
+                                        value, UdtUnion, type->union_v1.property);
+        break;
+
+    case LF_UNION_V2:
+        leaf_len = numeric_leaf(&value, &type->union_v2.un_len);
+        p_name = (const struct p_string*)((const unsigned char*)&type->union_v2.un_len + leaf_len);
+        symt = codeview_add_type_struct(ctp, terminate_string(p_name),
+                                        value, UdtUnion, type->union_v2.property);
+        break;
+
+    case LF_UNION_V3:
+        leaf_len = numeric_leaf(&value, &type->union_v3.un_len);
+        c_name = (const char*)&type->union_v3.un_len + leaf_len;
+        symt = codeview_add_type_struct(ctp, c_name,
+                                        value, UdtUnion, type->union_v3.property);
+        break;
+
+    case LF_ENUM_V1:
+        symt = &symt_new_enum(ctp->module, terminate_string(&type->enumeration_v1.p_name), NULL)->symt;
+        break;
+
+    case LF_ENUM_V2:
+        symt = &symt_new_enum(ctp->module, terminate_string(&type->enumeration_v2.p_name), NULL)->symt;
+        break;
+
+    case LF_ENUM_V3:
+        symt = &symt_new_enum(ctp->module, type->enumeration_v3.name, NULL)->symt;
+        break;
+
+    default: symt = NULL;
+    }
+    return symt && codeview_add_type(curr_type, symt) ? symt : NULL;
+}
+
+static BOOL codeview_is_top_level_type(const union codeview_type* type)
+{
+    /* type records we're interested in are the ones referenced by symbols
+     * The known ranges are (X mark the ones we want):
+     *   X  0000-0016       for V1 types
+     *      0200-020c       for V1 types referenced by other types
+     *      0400-040f       for V1 types (complex lists & sets)
+     *   X  1000-100f       for V2 types
+     *      1200-120c       for V2 types referenced by other types
+     *      1400-140f       for V1 types (complex lists & sets)
+     *   X  1500-150d       for V3 types
+     *      8000-8010       for numeric leafes
+     */
+    return !(type->generic.id & 0x8600) || (type->generic.id & 0x0100);
 }

static BOOL codeview_parse_type_table(struct codeview_type_parse* ctp)
@@ -1370,23 +1364,25 @@ static BOOL codeview_parse_type_table(struct codeview_type_parse* ctp)
     cv_current_module->defined_types = calloc(ctp->header.last_index - ctp->header.first_index,
                                               sizeof(*cv_current_module->defined_types));

+    /* phase I: only load remapable types (struct/class/union/enum), but without their content
+     *          handle also forward declarations
+     */
     for (curr_type = ctp->header.first_index; curr_type < ctp->header.last_index; curr_type++)
     {
         type = codeview_jump_to_type(ctp, curr_type);
-
-        /* type records we're interested in are the ones referenced by symbols
-         * The known ranges are (X mark the ones we want):
-         *   X  0000-0016       for V1 types
-         *      0200-020c       for V1 types referenced by other types
-         *      0400-040f       for V1 types (complex lists & sets)
-         *   X  1000-100f       for V2 types
-         *      1200-120c       for V2 types referenced by other types
-         *      1400-140f       for V1 types (complex lists & sets)
-         *   X  1500-150d       for V3 types
-         *      8000-8010       for numeric leafes
-         */
-        if (!(type->generic.id & 0x8600) || (type->generic.id & 0x0100))
-            codeview_parse_one_type(ctp, curr_type, type, TRUE);
+        if (codeview_is_top_level_type(type))
+            codeview_parse_remapable_type(ctp, curr_type, type);
+    }
+    /* phase II: non remapable types: load (but since they can be indirectly
+     *           loaded (from another type), don't load them twice)
+     *           rematable types: they can't be loaded indirectly, so load their content
+     */
+    for (curr_type = ctp->header.first_index; curr_type < ctp->header.last_index; curr_type++)
+    {
+        type = codeview_jump_to_type(ctp, curr_type);
+        if (codeview_is_top_level_type(type) &&
+            (!codeview_get_type(curr_type, TRUE) || codeview_is_remapable_type(type)))
+            codeview_parse_one_type(ctp, curr_type, type);
     }

return TRUE;

[PATCH v3 5/6] dlls/dbghelp{pdb}: ensure dbghelp's list order in hash table matches PDB's

[Eric Pouech]

From: Eric Pouech

The PDB types can contain several times the same type definition. It seems
to appear when modifying a type (like adding new fields to a struct):
- as the PDB file (generated from first compilation) is updated (and not
  fully rewritten), the debug information for the old type is not flushed;
  a new record (for the same struct name) is emitted, and inserted
  before the old one in the hash table (bucket list).

We must hence preserve that order in bucket list: even if dbghelp's hash
table is different from pdb's internal one (ie number of buckets & bucket
lists are different), this implies that new type information must appear
before the old on in the bucket list)

This order is also important as:
- forward definition always should be resolved with the new type
  (resolution is based on name)
- search by type name must return the new type

This patch fixes builtin dbghelp to implement this behavior.

This patch uses the pdb hash table and relies on it to preserve he partial
order described above. This pdb hash table is also used for resolving
forwards (now always picking the good one).

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/msc.c | 296 +++++++++++++++++++++++++++++++++------------
 1 file changed, 217 insertions(+), 79 deletions(-)

diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c
index dfb5cac94aa..e881c1d234e 100644
--- a/dlls/dbghelp/msc.c
+++ b/dlls/dbghelp/msc.c
@@ -555,12 +555,21 @@ static struct symt*  codeview_get_type(unsigned int typeno, BOOL quiet)
     return symt;
 }

+struct hash_link
+{
+    unsigned            id;
+    struct hash_link*   next;
+};
+
 struct codeview_type_parse
 {
     struct module*      module;
     PDB_TYPES           header;
     const BYTE*         table;
-    const DWORD*        offset;
+    const DWORD*        offset;      /* typeindex => offset in 'table' */
+    BYTE*               hash_stream; /* content of stream header.hash_file */
+    struct hash_link**  hash;        /* hash_table (deserialized from PDB hash table) */
+    struct hash_link*   alloc_hash;  /* allocated hash_link (id => hash_link) */
 };

static inline const void* codeview_jump_to_type(const struct codeview_type_parse* ctp, DWORD idx)
@@ -615,16 +624,6 @@ static struct symt* codeview_parse_one_type(struct codeview_type_parse* ctp,
                                             unsigned curr_type,
                                             const union codeview_type* type);

-static void* codeview_cast_symt(struct symt* symt, enum SymTagEnum tag)
-{
-    if (symt->tag != tag)
-    {
-        FIXME("Bad tag. Expected %d, but got %d\n", tag, symt->tag);
-        return NULL;
-    }   
-    return symt;
-}
-
 static struct symt* codeview_fetch_type(struct codeview_type_parse* ctp,
                                         unsigned typeno)
 {
@@ -661,6 +660,139 @@ static BOOL codeview_is_remapable_type(const union codeview_type* type)
     }
 }

+static BOOL codeview_type_is_forward(const union codeview_type* cvtype)
+{
+    cv_property_t property;
+
+    switch (cvtype->generic.id)
+    {
+    case LF_STRUCTURE_V1:
+    case LF_CLASS_V1:     property = cvtype->struct_v1.property;       break;
+    case LF_STRUCTURE_V2:
+    case LF_CLASS_V2:     property = cvtype->struct_v2.property;       break;
+    case LF_STRUCTURE_V3:
+    case LF_CLASS_V3:     property = cvtype->struct_v3.property;       break;
+    case LF_UNION_V1:     property = cvtype->union_v1.property;        break;
+    case LF_UNION_V2:     property = cvtype->union_v2.property;        break;
+    case LF_UNION_V3:     property = cvtype->union_v3.property;        break;
+    case LF_ENUM_V1:      property = cvtype->enumeration_v1.property;  break;
+    case LF_ENUM_V2:      property = cvtype->enumeration_v1.property;  break;
+    case LF_ENUM_V3:      property = cvtype->enumeration_v1.property;  break;
+    default:
+        return FALSE;
+    }
+    return property.is_forward_defn;
+}
+
+static BOOL codeview_type_extract_name(const union codeview_type* cvtype,
+                                       const char** name, unsigned* len, const char** decorated_name)
+{
+    int value, leaf_len;
+    const struct p_string* p_name = NULL;
+    const char* c_name = NULL;
+    BOOL decorated = FALSE;
+
+    switch (cvtype->generic.id)
+    {
+    case LF_STRUCTURE_V1:
+    case LF_CLASS_V1:
+        leaf_len = numeric_leaf(&value, &cvtype->struct_v1.structlen);
+        p_name = (const struct p_string*)((const unsigned char*)&cvtype->struct_v1.structlen + leaf_len);
+        break;
+    case LF_STRUCTURE_V2:
+    case LF_CLASS_V2:
+        leaf_len = numeric_leaf(&value, &cvtype->struct_v2.structlen);
+        p_name = (const struct p_string*)((const unsigned char*)&cvtype->struct_v2.structlen + leaf_len);
+        break;
+    case LF_STRUCTURE_V3:
+    case LF_CLASS_V3:
+        leaf_len = numeric_leaf(&value, &cvtype->struct_v3.structlen);
+        c_name = (const char*)&cvtype->struct_v3.structlen + leaf_len;
+        decorated = cvtype->struct_v3.property.has_decorated_name;
+        break;
+    case LF_UNION_V1:
+        leaf_len = numeric_leaf(&value, &cvtype->union_v1.un_len);
+        p_name = (const struct p_string*)((const unsigned char*)&cvtype->union_v1.un_len + leaf_len);
+        break;
+    case LF_UNION_V2:
+        leaf_len = numeric_leaf(&value, &cvtype->union_v2.un_len);
+        p_name = (const struct p_string*)((const unsigned char*)&cvtype->union_v2.un_len + leaf_len);
+        break;
+    case LF_UNION_V3:
+        leaf_len = numeric_leaf(&value, &cvtype->union_v3.un_len);
+        c_name = (const char*)&cvtype->union_v3.un_len + leaf_len;
+        decorated = cvtype->union_v3.property.has_decorated_name;
+        break;
+    case LF_ENUM_V1:
+        p_name = &cvtype->enumeration_v1.p_name;
+        break;
+    case LF_ENUM_V2:
+        p_name = &cvtype->enumeration_v2.p_name;
+        break;
+    case LF_ENUM_V3:
+        c_name = cvtype->enumeration_v3.name;
+        decorated = cvtype->union_v3.property.has_decorated_name;
+        break;
+    default:
+        return FALSE;
+    }
+    if (p_name)
+    {
+        *name = p_name->name;
+        *len = p_name->namelen;
+        *decorated_name = NULL;
+        return TRUE;
+    }
+    if (c_name)
+    {
+        *name = c_name;
+        *len = strlen(c_name);
+        *decorated_name = decorated ? (c_name + *len + 1) : NULL;
+        return TRUE;
+    }
+    return FALSE;
+}
+
+static unsigned pdb_read_hash_value(const struct codeview_type_parse* ctp, unsigned idx)
+{
+    const void* where = ctp->hash_stream + ctp->header.hash_offset + (idx - ctp->header.first_index) * ctp->header.hash_size;
+    switch (ctp->header.hash_size)
+    {
+    case 2: return *(unsigned short*)where;
+    case 4: return *(unsigned*)where;
+    }
+    return 0;
+}
+
+static BOOL codeview_resolve_forward_type(struct codeview_type_parse* ctp, const union codeview_type* cvref,
+                                          unsigned reftype, unsigned *realtype)
+{
+    const union codeview_type* cvdecl;
+    struct hash_link* hl;
+
+    for (hl = ctp->hash[pdb_read_hash_value(ctp, reftype)]; hl; hl = hl->next)
+    {
+        if (hl->id == reftype) break; /* no better choice */
+        cvdecl = codeview_jump_to_type(ctp, hl->id);
+        if (cvdecl && !codeview_type_is_forward(cvdecl) && cvref->generic.id == cvdecl->generic.id)
+        {
+            const char* nameref, * namedecl, * decoratedref, * decorateddecl;
+            unsigned lenref, lendecl;
+
+            if (codeview_type_extract_name(cvref,  &nameref,  &lenref,  &decoratedref) &&
+                codeview_type_extract_name(cvdecl, &namedecl, &lendecl, &decorateddecl) &&
+                ((decoratedref && decorateddecl && !strcmp(decoratedref, decorateddecl)) ||
+                 (!decoratedref && !decorateddecl && lenref == lendecl && !memcmp(nameref, namedecl, lenref))))
+            {
+                TRACE("mapping forward type %.*s (%s) %x into %x\n", lenref, nameref, debugstr_a(decoratedref), reftype, hl->id);
+                *realtype = hl->id;
+                return TRUE;
+            }
+        }
+    }
+    return FALSE;
+}
+
 static struct symt* codeview_add_type_pointer(struct codeview_type_parse* ctp,
                                               unsigned int pointee_type)
 {
@@ -1001,49 +1133,6 @@ static int codeview_add_type_struct_field_list(struct codeview_type_parse* ctp,
     return TRUE;
 }

-static struct symt* codeview_add_type_struct(struct codeview_type_parse* ctp,
-                                             const char* name, int structlen,
-                                             enum UdtKind kind, cv_property_t property)
-{
-    struct symt_udt*    symt;
-    struct symt*        existing = NULL;
-
-    /* if we don't have an existing type, try to find one with same name
-     * FIXME: what to do when several types in different CUs have same name ?
-     */
-    void*                       ptr;
-    struct symt_ht*             type;
-    struct hash_table_iter      hti;
-
-    hash_table_iter_init(&ctp->module->ht_types, &hti, name);
-    while ((ptr = hash_table_iter_up(&hti)))
-    {
-        type = CONTAINING_RECORD(ptr, struct symt_ht, hash_elt);
-
-        if (type->symt.tag == SymTagUDT &&
-            type->hash_elt.name && !strcmp(type->hash_elt.name, name))
-        {
-            existing = &type->symt;
-            break;
-        }
-    }
-    if (existing)
-    {
-        if (!(symt = codeview_cast_symt(existing, SymTagUDT))) return NULL;
-        /* should also check that all fields are the same */
-        if (!property.is_forward_defn)
-        {
-            if (!symt->size) /* likely prior forward declaration, set UDT size */
-                symt_set_udt_size(ctp->module, symt, structlen);
-            else /* different UDT with same name, create a new type */
-                existing = NULL;
-        }
-    }
-    if (!existing) symt = symt_new_udt(ctp->module, name, structlen, kind);
-
-    return &symt->symt;
-}
-
 static struct symt* codeview_new_func_signature(struct codeview_type_parse* ctp,
                                                 enum CV_call_e call_conv)
 {
@@ -1271,54 +1360,59 @@ static struct symt* codeview_parse_remapable_type(struct codeview_type_parse* ct
     const struct p_string*      p_name;
     const char*                 c_name;

+    if (codeview_type_is_forward(type))
+    {
+        unsigned impl_type;
+        if (codeview_resolve_forward_type(ctp, type, curr_type, &impl_type))
+        {
+            if (!(symt = codeview_get_type(impl_type, TRUE)))
+                FIXME("forward def of %x => %x doesn't have impl\n", curr_type, impl_type);
+            return codeview_add_type(curr_type, symt) ? symt : NULL;
+        }
+        /* else forward type definition without implementation, create empty Udt */
+    }
     switch (type->generic.id)
     {
     case LF_STRUCTURE_V1:
     case LF_CLASS_V1:
         leaf_len = numeric_leaf(&value, &type->struct_v1.structlen);
         p_name = (const struct p_string*)((const unsigned char*)&type->struct_v1.structlen + leaf_len);
-        symt = codeview_add_type_struct(ctp, terminate_string(p_name), value,
-                                        type->generic.id == LF_CLASS_V1 ? UdtClass : UdtStruct,
-                                        type->struct_v1.property);
+        symt = &symt_new_udt(ctp->module, terminate_string(p_name), value,
+                             type->generic.id == LF_CLASS_V1 ? UdtClass : UdtStruct)->symt;
         break;

case LF_STRUCTURE_V2:
     case LF_CLASS_V2:
         leaf_len = numeric_leaf(&value, &type->struct_v2.structlen);
         p_name = (const struct p_string*)((const unsigned char*)&type->struct_v2.structlen + leaf_len);
-        symt = codeview_add_type_struct(ctp, terminate_string(p_name), value,
-                                        type->generic.id == LF_CLASS_V2 ? UdtClass : UdtStruct,
-                                        type->struct_v2.property);
+        symt = &symt_new_udt(ctp->module, terminate_string(p_name), value,
+                             type->generic.id == LF_CLASS_V2 ? UdtClass : UdtStruct)->symt;
         break;

case LF_STRUCTURE_V3:
     case LF_CLASS_V3:
         leaf_len = numeric_leaf(&value, &type->struct_v3.structlen);
         c_name = (const char*)&type->struct_v3.structlen + leaf_len;
-        symt = codeview_add_type_struct(ctp, c_name, value,
-                                        type->generic.id == LF_CLASS_V3 ? UdtClass : UdtStruct,
-                                        type->struct_v3.property);
+        symt = &symt_new_udt(ctp->module, c_name, value,
+                             type->generic.id == LF_CLASS_V3 ? UdtClass : UdtStruct)->symt;
         break;

case LF_UNION_V1:
         leaf_len = numeric_leaf(&value, &type->union_v1.un_len);
         p_name = (const struct p_string*)((const unsigned char*)&type->union_v1.un_len + leaf_len);
-        symt = codeview_add_type_struct(ctp, terminate_string(p_name),
-                                        value, UdtUnion, type->union_v1.property);
+        symt = &symt_new_udt(ctp->module, terminate_string(p_name), value, UdtUnion)->symt;
         break;

case LF_UNION_V2:
         leaf_len = numeric_leaf(&value, &type->union_v2.un_len);
         p_name = (const struct p_string*)((const unsigned char*)&type->union_v2.un_len + leaf_len);
-        symt = codeview_add_type_struct(ctp, terminate_string(p_name),
-                                        value, UdtUnion, type->union_v2.property);
+        symt = &symt_new_udt(ctp->module, terminate_string(p_name), value, UdtUnion)->symt;
         break;

case LF_UNION_V3:
         leaf_len = numeric_leaf(&value, &type->union_v3.un_len);
         c_name = (const char*)&type->union_v3.un_len + leaf_len;
-        symt = codeview_add_type_struct(ctp, c_name,
-                                        value, UdtUnion, type->union_v3.property);
+        symt = &symt_new_udt(ctp->module, c_name, value, UdtUnion)->symt;
         break;

case LF_ENUM_V1:
@@ -1356,7 +1450,7 @@ static BOOL codeview_is_top_level_type(const union codeview_type* type)

static BOOL codeview_parse_type_table(struct codeview_type_parse* ctp)
 {
-    unsigned int                curr_type;
+    unsigned int                i, curr_type;
     const union codeview_type*  type;

cv_current_module->first_type_index = ctp->header.first_index;
@@ -1366,12 +1460,18 @@ static BOOL codeview_parse_type_table(struct codeview_type_parse* ctp)

/* phase I: only load remapable types (struct/class/union/enum), but without their content
      *          handle also forward declarations
+     *          - do it in the order generated from PDB hash table to preserve that order
+     *            (dbghelp hash table inserts new elements at the end of bucket's list)
      */
-    for (curr_type = ctp->header.first_index; curr_type < ctp->header.last_index; curr_type++)
+    for (i = 0; i < ctp->header.hash_num_buckets; i++)
     {
-        type = codeview_jump_to_type(ctp, curr_type);
-        if (codeview_is_top_level_type(type))
-            codeview_parse_remapable_type(ctp, curr_type, type);
+        struct hash_link* hl;
+        for (hl = ctp->hash[i]; hl; hl = hl->next)
+        {
+            type = codeview_jump_to_type(ctp, hl->id);
+            if (codeview_is_top_level_type(type))
+                codeview_parse_remapable_type(ctp, hl->id, type);
+        }
     }
     /* phase II: non remapable types: load (but since they can be indirectly
      *           loaded (from another type), don't load them twice)
@@ -3031,7 +3131,16 @@ static HANDLE map_pdb_file(const struct process* pcs,
     return hMap;
 }

+static void pdb_dispose_type_parse(struct codeview_type_parse* ctp)
+{
+    pdb_free(ctp->hash_stream);
+    free((DWORD*)ctp->offset);
+    free((DWORD*)ctp->hash);
+    free((DWORD*)ctp->alloc_hash);
+}
+
 static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
+                                const struct pdb_file_info* pdb_file,
                                 struct codeview_type_parse* ctp,
                                 BYTE* image)
 {
@@ -3039,6 +3148,10 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
     DWORD* offset;
     int i;

+    ctp->hash_stream = NULL;
+    ctp->offset = NULL;
+    ctp->hash = NULL;
+    ctp->alloc_hash = NULL;
     pdb_convert_types_header(&ctp->header, image);

/* Check for unknown versions */
@@ -3054,6 +3167,17 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
         ERR("-Unknown type info version %d\n", ctp->header.version);
         return FALSE;
     }
+    if (ctp->header.hash_size != 2 && ctp->header.hash_size != 4)
+    {
+        ERR("-Unsupported hash of size %u\n", ctp->header.hash_size);
+        return FALSE;
+    }
+    ctp->hash_stream = pdb_read_file(pdb_file, ctp->header.hash_file);
+    if (!ctp->hash_stream)
+    {
+        ERR("-Missing hash table in PDB file\n");
+        return FALSE;
+    }

ctp->module = msc_dbg->module;
     /* reconstruct the types offset...
@@ -3061,8 +3185,8 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
      * (not all the indexes are present, so it requires search in table +
      * linear search from previous index...)
      */
-    offset = HeapAlloc(GetProcessHeap(), 0, sizeof(DWORD) * (ctp->header.last_index - ctp->header.first_index));
-    if (!offset) return FALSE;
+    offset = malloc(sizeof(DWORD) * (ctp->header.last_index - ctp->header.first_index));
+    if (!offset) goto oom;
     ctp->table = ptr = image + ctp->header.type_offset;
     for (i = ctp->header.first_index; i < ctp->header.last_index; i++)
     {
@@ -3070,7 +3194,21 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
         ptr += ((const union codeview_type*)ptr)->generic.len + 2;
     }
     ctp->offset = offset;
+    ctp->hash = calloc(ctp->header.hash_num_buckets, sizeof(struct hash_link*));
+    if (!ctp->hash) goto oom;
+    ctp->alloc_hash = calloc(ctp->header.last_index - ctp->header.first_index, sizeof(struct hash_link));
+    if (!ctp->alloc_hash) goto oom;
+    for (i = ctp->header.first_index; i < ctp->header.last_index; i++)
+    {
+        unsigned hash_i = pdb_read_hash_value(ctp, i);
+        ctp->alloc_hash[i - ctp->header.first_index].id = i;
+        ctp->alloc_hash[i - ctp->header.first_index].next = ctp->hash[hash_i];
+        ctp->hash[hash_i] = &ctp->alloc_hash[i - ctp->header.first_index];
+    }
     return TRUE;
+oom:
+    pdb_dispose_type_parse(ctp);
+    return FALSE;
 }

static void pdb_process_types(const struct msc_debug_info* msc_dbg,
@@ -3081,11 +3219,11 @@ static void pdb_process_types(const struct msc_debug_info* msc_dbg,

if (types_image)
     {
-        if (pdb_init_type_parse(msc_dbg, &ctp, types_image))
+        if (pdb_init_type_parse(msc_dbg, pdb_file, &ctp, types_image))
         {
             /* Read type table */
             codeview_parse_type_table(&ctp);
-            HeapFree(GetProcessHeap(), 0, (DWORD*)ctp.offset);
+            pdb_dispose_type_parse(&ctp);
         }
         pdb_free(types_image);
     }
@@ -3369,7 +3507,7 @@ static BOOL pdb_process_internal(const struct process* pcs,
         pdb_process_types(msc_dbg, pdb_file);

ipi_image = pdb_read_file(pdb_file, 4);
-        ipi_ok = pdb_init_type_parse(msc_dbg, &ipi_ctp, ipi_image);
+        ipi_ok = pdb_init_type_parse(msc_dbg, pdb_file, &ipi_ctp, ipi_image);

/* Read global symbol table */
         globalimage = pdb_read_file(pdb_file, symbols.gsym_file);

[PATCH v3 6/6] dlls/dbghelp{pdb}: use remap table from PDB hash stream

[Eric Pouech]

From: Eric Pouech

The PDB hash stream from TPI header contains information to force a remap
to a given type record (whatever the order in the bucket hash list).
This is generated by the incremental linker in some occasions.

Use that information to remap the corresponding types.

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/msc.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/dlls/dbghelp/msc.c b/dlls/dbghelp/msc.c
index e881c1d234e..284cd49973d 100644
--- a/dlls/dbghelp/msc.c
+++ b/dlls/dbghelp/msc.c
@@ -3139,6 +3139,12 @@ static void pdb_dispose_type_parse(struct codeview_type_parse* ctp)
     free((DWORD*)ctp->alloc_hash);
 }

+static BOOL pdb_bitfield_is_bit_set(const unsigned* dw, unsigned len, unsigned i)
+{
+    if (i >= len * sizeof(unsigned) * 8) return FALSE;
+    return (dw[i >> 5] & (1u << (i & 31u))) != 0;
+}
+
 static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
                                 const struct pdb_file_info* pdb_file,
                                 struct codeview_type_parse* ctp,
@@ -3205,6 +3211,42 @@ static BOOL pdb_init_type_parse(const struct msc_debug_info* msc_dbg,
         ctp->alloc_hash[i - ctp->header.first_index].next = ctp->hash[hash_i];
         ctp->hash[hash_i] = &ctp->alloc_hash[i - ctp->header.first_index];
     }
+    /* parse the remap table
+     * => move listed type_id at first position of their hash buckets so that we force remap to them
+     */
+    if (ctp->header.type_remap_len)
+    {
+        const unsigned* remap = (const unsigned*)((const BYTE*)ctp->hash_stream + ctp->header.type_remap_offset);
+        unsigned i, capa, count_present;
+        const unsigned* present_bitset;
+        remap++; /* no need of num */
+        capa = *remap++;
+        count_present = *remap++;
+        present_bitset = remap;
+        remap += count_present;
+        remap += *remap + 1; /* skip deleted bit set */
+        for (i = 0; i < capa; ++i)
+        {
+            if (pdb_bitfield_is_bit_set(present_bitset, count_present, i))
+            {
+                unsigned hash_i;
+                struct hash_link** phl;
+                /* remap[0] is an offset for a string in /string stream, followed by type_id to force */
+                hash_i = pdb_read_hash_value(ctp, remap[1]);
+                for (phl = &ctp->hash[hash_i]; *phl; phl = &(*phl)->next)
+                    if ((*phl)->id == remap[1])
+                    {
+                        struct hash_link* hl = *phl;
+                        /* move hl node at first position of its hash bucket */
+                        *phl = hl->next;
+                        hl->next = ctp->hash[hash_i];
+                        ctp->hash[hash_i] = hl;
+                        break;
+                    }
+                remap += 2;
+            }
+        }
+    }
     return TRUE;
 oom:
     pdb_dispose_type_parse(ctp);

[eric pouech (＠epo)]

On Tue May 31 09:25:31 2022 +0000, eric pouech wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/73/diffs?diff_id=1630&start_sha=757dbf4d2f977b7c054bb3b562accc254bfa61de#65c8fe75b1ac2881f508d426ea498504d1aa131b_675_677)

v3 pushed (using qsort instead of qsort_r)