support dllimport data about cv2pdb

[rainers]

I don't see this working when compiling dll and exe with the Microsoft compiler, so it's probably not so easy to implement. You would have to make the entry in the import table a reference to a Game instance and rename it "game" somehow...

[Trass3r]

Seems like my edit got lost. When compiling with msvc the pdb contains

S_GDATA32 `_imp__game`
           type = 0x1002 (Game), addr = 0002:0264
S_PUB32 `__imp__game`
           flags = none, addr = 0002:0264

Maybe they also do something special based on the naming convention (__imp__)? The Game type definition itself is also present in the cv2pdb produced pdb.

[rainers]

Indeed, the symbol is there, but it needs to be extern "C" to actually be able to evaluate it in the watch window because it needs to be spelled out as is (game doesn't work for me). Additionally it is typed badly, because the value in the import table is a pointer to Game, not the value itself.

Maybe the DWARF output makes more sense, though.

[Trass3r]

I did compile it as C. Indeed game is not recognized anymore when compiling with msvc. Odd, I'm pretty sure it worked before. Even a simple __declspec(dllimport) int i; doesn't work. I created a bug report for msvc.

del /Q *.pdb
cl -nologo -O1 -Z7 -LD dll.c -link -OPT:REF 
cl -nologo -O1 -Z7 main.c dll.lib -link -OPT:REF 

There's also a name and type difference between x86 and x64: void* __imp__game vs. Game __imp_game.

[rainers]

There's also a name and type difference between x86 and x64: void* __imp__game vs. Game __imp_game.

That's the usual underscore prepended to the game symbol on x86.

The latest commit 243f8da adds support for C like symbols, i.e.e that don't have additional mangling.

[Trass3r]

That fixes the sample code 👍
But in the real code which isn't really different it gets a null address in the debugger for whatever reason: <struct at NULL> | Game &
The pdb dumps look ok.

[rainers]

IIRC cv2pdb doesn't (fully) support DLLs that are relocated to an address other than the default address. If your "main" is actually in a DLL itself that might be the problem.

If not, a test case for reproduction might be useful.

[Trass3r]

Indeed the dll got relocated. But why would that matter? The pointer to the data is in the exe .idata section.
Edit: Yep forcing relocation via i686-w64-mingw32-gcc -Og -g dll.c -shared -o dll.dll -Wl,--no-dynamicbase,--image-base,0x400000 doesn't break the debug info in the minimal test case.
In the real code there seems to be a common offset of 1B8000 between correct address and the one given by DIA.

[Trass3r]

One unrelated problem in PEImage::findSymbol is it doesn't handle section number 0: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#section-number-values. See this dumpbin /symbols excerpt, but this affects only some variables depending on the entry order:

9383 00000000 UNDEF  notype       0x6a UNKNOWN SYMBOL CLASS | _split1at
...
9644 00001B04 SECT5  notype       External     | __imp__split1at

diff --git a/src/PEImage.cpp b/src/PEImage.cpp
index 1f916c2..6121a35 100644
--- a/src/PEImage.cpp
+++ b/src/PEImage.cpp
@@ -809,10 +809,11 @@ int PEImage::findSymbol(const char* name, unsigned long& off, bool& dllimport) c
 	{
 		IMAGE_SYMBOL* sym = (IMAGE_SYMBOL*) (symtable + i * sizeof_sym);
 		const char* symname = sym->N.Name.Short == 0 ? strtable + sym->N.Name.Long : (char*)sym->N.ShortName;
-		if(symbolMatches(name, symname, dllimport))
+		auto section = bigobj ? ((IMAGE_SYMBOL_EX*)sym)->SectionNumber : sym->SectionNumber;
+		if(section > 0 && symbolMatches(name, symname, dllimport))
 		{
 			off = sym->Value;
-			return bigobj ? ((IMAGE_SYMBOL_EX*)sym)->SectionNumber : sym->SectionNumber;
+			return section;
 		}
 		i += sym->NumberOfAuxSymbols;
 	}

[rainers]

Indeed the dll got relocated. But why would that matter? The pointer to the data is in the exe .idata section.

I meant the binary you are debugging. If that is an exe, there is no relocation and that's ok then.

One unrelated problem in PEImage::findSymbol is it doesn't handle section number 0:

I don't think these can appear in the executable. External symbols are only in object files and must be resolved by the linker.

[Trass3r]

Well see above, mingw did produce such entries in the coff symbol table.

I really don't know where that offset is coming from. IDiaSymbol gives

getVirtualAddress(): 0x00000000001b3b74 => should be 0x36BB74
getAddressSection(): 0x00000006 => seg + 1
getAddressOffset(): 0x00001b74 => correct

[rainers]

If you provide an example executable, I can have a look.

Well see above, mingw did produce such entries in the coff symbol table.

I guess you are referring to the object file.

BTW: The executable I tried with
g++.exe -g maingame.cpp game.lib (g++.exe (i686-posix-dwarf-rev0, Built by MinGW-W64 project) 8.1.0) isn't considered correct by dumpbin:

>dumpbin /symbols a.exe
Microsoft (R) COFF/PE Dumper Version 14.24.28316.0
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file a.exe

File Type: EXECUTABLE IMAGE
LINK : fatal error LNK1328: missing string table

[Trass3r]

No I do mean the exe as that's where cv2pdb reads the symbol address from. Mine is built with Linux MinGW (via WSL) and dumpbin works. I used cv2pdb -C, not sure if the -C option makes any difference.

_DK_game	<struct at NULL>	Game &
(Game**)0x0076b62c {dll!0x01540310 {unused_version=0x01540310 {...} ...}}

[rainers]

Your DLL/EXE still causes the same failure with dumpbin for me (from VS 2019 16.4.4 - edit: I suspect dumpbin is broken for the long section names). If I dump it with objdump -x keeperfx_hvlog.exe, you can find these lines:

There is an import table in .idata at 0x76a000

The Import Tables (interpreted .idata section contents)
 vma:            Hint    Time      Forward  DLL       First
                 Table   Stamp     Chain    Name      Thunk
 0036a000	0036a0dc 00000000 00000000 00370df0 0036af40

	DLL Name: IMAGEHLP.DLL
...
 0036a064	0036a364 00000000 00000000 00371b68 0036b1c8

	DLL Name: keeperfx.dll
	vma:  Hint/Ord Member-Name Bound-To
	36c5f8	    1  _DK_Border
...
	36e114	  945  _DK_game
...
Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  4 .idata        00007c04  0076a000  0076a000  001a8800  2**2
                  CONTENTS, ALLOC, LOAD, DATA
...
SYMBOL TABLE:
[  0](sec -2)(fl 0x00)(ty   0)(scl 103) (nx 1) 0x0000002b crtexe.c
...
[38492](sec  5)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x0000162c __imp___DK_game
...
[39014](sec  0)(fl 0x00)(ty   0)(scl 106) (nx 0) 0x00000000 __DK_game

Observations:

• 945*6+6 == 0x162c. I think the symbol information is wrong as it contains the offset in the import data for keeperfx.dll, but not in the .idata section.
• segment 6 in the debug info seems questionable, it should rather be 5?
• the "external" symbol with sec 0 seems to be some kind of alias to the import symbol and should rather be ignored.

[Trass3r]

My dumpbin is from the latest preview. Maybe they fixed something.

0x76b62c (the Game*) - 0x76a000 (.idata) is 162c. Seems correct.
And idk why the pdb has segment + 1 (

cv2pdb/src/dwarf2pdb.cpp

Line 1616 in 243f8da

appendGlobalVar(id.name, type, seg + 1, segOff);

) but it worked in the minimal test case.

[rainers]

0x76b62c (the Game*) - 0x76a000 (.idata) is 162c. Seems correct.

Indeed, not sure what I was looking at.

The missing offset is 0x1c0000, which is the size of the .bss section before .idata. This section does not exist in the VC compiled executable (although there is a .textbss section as the first section).

[Trass3r]

DIA (read via llvm-pdbutil pretty --globals) gives 0x01b362c (offset from imagebase). 76b62c - (1b362c + 400000) = 1B8000 which is only approx. the size of bss: 1BF3F0
But in the minimal test case there's the same section order and there it works.

[rainers]

The expected pointer is at 0x76b62c. You have to take the aligned sections sizes, or just the differences of the virtual addresses.

But in the minimal test case there's the same section order and there it works.

I suspect it works because the sections are all of size 0x1000, and the current debug info is off by one section.

[Trass3r]

I think I found the reason, the section map seems to be wrong. The bss section entry is set to size 0.
That plus the off-by-one error explains the difference.

diff --git a/src/dwarf2pdb.cpp b/src/dwarf2pdb.cpp
index 70b6fc9..daf2fa7 100644
--- a/src/dwarf2pdb.cpp
+++ b/src/dwarf2pdb.cpp
@@ -1613,8 +1613,8 @@ bool CV2PDB::createTypes()
 							cbDwarfTypes += addPointerType(dwarfTypes + cbDwarfTypes, type, pointerAttr | 0x20); // needs to be deduplicted?
 							type = nextDwarfType++;
 						}
-						appendGlobalVar(id.name, type, seg + 1, segOff);
-						int rc = mod->AddPublic2(id.name, seg + 1, segOff, type);
+						appendGlobalVar(id.name, type, seg, segOff);
+						int rc = mod->AddPublic2(id.name, seg, segOff, type);
 					}
 				}
 				break;
@@ -1652,7 +1652,7 @@ bool CV2PDB::createDWARFModules()
 	for (int s = 0; s < img.countSections(); s++)
 	{
 		const IMAGE_SECTION_HEADER& sec = img.getSection(s);
-		int rc = dbi->AddSec(s + 1, 0x10d, 0, sec.SizeOfRawData);
+		int rc = dbi->AddSec(s + 1, 0x10d, 0, sec.Misc.VirtualSize);
 		if (rc <= 0)
 			return setError("cannot add section");
 	}

[rainers]

Cool, looks good. I've pushed the fix (the segment should be changed in findSymbol, though).