rainers / cv2pdb

converter of DMD CodeView/DWARF debug information to PDB files
Artistic License 2.0
472 stars 109 forks source link

Bad things happen running cv2pdb on the msys runtime .dll. #35

Closed dakotahawkins closed 4 years ago

dakotahawkins commented 6 years ago

Note: I had updated the runtime's build process to set -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer in CFLAGS and CXXFLAGS, by default they used -ggdb.

Specifically, this is happening with cygwin/msys DLLs. The Git for Windows project has been trying to supply a PDB for the msys runtime (see git-for-windows/git/issues/356).

The x64 runtime DLL, msys-2.0.dll, is built with an image base address of 0x180040000 (Cygwin x64 default, AFAIK). That's too big for an int, but I see a lot of the cv2pdb types used to do math with offsets and the image base address are ints or unsigned ints. I suspect there are several places where this math overflows or underflows because of the large base address. There's also the call in main.cpp of exe.relocateDebugLineInfo(0x400000) where the windows default base address is hardcoded.

Attached is a .zip with enough unmodified and modified DLLs for reproduction or inspection. Contents are:

I'm not sure how many types in cv2pdb would need to change to handle this, but from looking in to it some it seems like maybe a lot. It might also be necessary to add an optional command-line option to specify the base address if it can't be looked up before it's required in main.cpp.

dakotahawkins commented 6 years ago

Update: I've tried using objcopy to change various addresses, but it doesn't have any apparent effect on the resulting DLL generated by cv2pdb. My assumption that this problem was caused by the image base address may have been incorrect.

dakotahawkins commented 6 years ago

Here is my build log for the MSYS runtime in case it helps (search for "-shared -o msys0.dll" to get to the linker step).

rainers commented 6 years ago

I don't think the large image base address is an issue (it might if the image crosses a 32-bit boundary), because it usually works with clipped addresses, too.

Line numbers didn't translate because the line info data uses 8-byte pointer addresses. I've committet a fix for that.

dakotahawkins commented 6 years ago

Awesome! I'll give it a try! Will take several minutes :(

dakotahawkins commented 6 years ago

Still getting "truncated .dll" error from objdump -h on the cv2pdb produce .DLL -- in case it helps here's the "regular" stripped .DLL and the cv2pdb .DLL and .PDB:

tuncated-dll-2.zip

rainers commented 6 years ago

Hmm, the problem seems to be that the image sections are not in ascending order.

dakotahawkins commented 6 years ago
$ objdump -h msys-2.0.dll

msys-2.0.dll:     file format pei-x86-64

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .text         001c0ac8  0000000180041000  0000000180041000  00000400  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA
  1 .autoload_text 00003120  0000000180202000  0000000180202000  001c1000  2**4
                  CONTENTS, ALLOC, LOAD, CODE
  2 .data         00026700  0000000180206000  0000000180206000  001c4200  2**5
                  CONTENTS, ALLOC, LOAD, DATA
  3 .rdata        0005f2e0  000000018022d000  000000018022d000  001eaa00  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 .buildid      00000035  000000018028d000  000000018028d000  00249e00  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 .pdata        0000fb1c  000000018028e000  000000018028e000  0024a000  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .xdata        0001392c  000000018029e000  000000018029e000  00259c00  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  7 .bss          00047b6c  00000001802b2000  00000001802b2000  00000000  2**5
                  ALLOC
  8 .edata        000086cb  00000001802fa000  00000001802fa000  0026d600  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  9 .reloc        00004cf8  0000000180303000  0000000180303000  00275e00  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 10 .cygwin_dll_common 0000c220  0000000180308000  0000000180308000  0027ac00  2**5
                  CONTENTS, ALLOC, LOAD, DATA, SHARED
 11 .idata        00003710  0000000180315000  0000000180315000  00287000  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 12 .gnu_debuglink 00000014  0000000180319000  0000000180319000  0028a800  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA, EXCLUDE
 13 .rsrc         00000410  000000018031a000  000000018031a000  0028aa00  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 14 .cygheap      00305000  000000018031b000  000000018031b000  00000000  2**2
                  ALLOC

Yeah, so .gnu_debuglink shouldn't be necessary (since I think it just stores the name of the .dbg file, and we're throwing it away. I think I could make that go away), but .cygheap is, I think, pretty important.

From what I can gather the .DLL is linked with no heap, or the heap off, or however that makes sense. --heap=0 is in the options in the earlier build log near the search string I provided. Then weird internal memory management gives you a heap as far as you know, but located inside of the .cygheap address space (maybe?). Presumably, all of that and more weird cygwin stuff combines to make that section.

rainers commented 6 years ago

For me objdump also shows the debug sections. It seems I was wrong about the bad order of sections, they now look ok to me. dumpbin /headers msys-2.0-pdb.dll gives a little more information:

Microsoft (R) COFF/PE Dumper Version 12.00.40629.0
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file msys-2.0-pdb.dll

PE signature found

File Type: DLL
msys-2.0-pdb.dll : fatal error LNK1106: invalid file or disk full: cannot seek to 0x18AA32C

but I have no clue where it gets that offset from.

I won't be able to do something about it in the next couple of weeks...

rainers commented 6 years ago

BTW: the DLL looks ok if you don't strip the debug sections by skipping this conditional: https://github.com/rainers/cv2pdb/blob/master/src/PEImage.cpp#L182

dakotahawkins commented 6 years ago

For me objdump also shows the debug sections.

My second attachment included a DLL that was stripped with objdump/objcopy, that's why the output I pasted didn't have debug sections.

I had seen that error in dumpbin, but figured the address wasn't very helpful without more context.

I'll try to think of something else to try to catch what's going wrong...

dscho commented 4 years ago
msys-2.0-pdb.dll : fatal error LNK1106: invalid file or disk full: cannot seek to 0x18AA32C

but I have no clue where it gets that offset from.

Seems that somebody else complained about this: https://github.com/git-for-windows/git/issues/2200. Their analysis say that this is the symbols table offset (and since there are no symbols any longer, it should probably be zeroed out?).

dscho commented 4 years ago

Their analysis say that this is the symbols table offset (and since there are no symbols any longer, it should probably be zeroed out?).

And indeed, if I apply this patch, it seems that even the old dumpbin (and current objdump) are happy with the resulting .exe:

diff --git a/src/PEImage.cpp b/src/PEImage.cpp
index d3651c8..cf13d65 100644
--- a/src/PEImage.cpp
+++ b/src/PEImage.cpp
@@ -246,6 +246,23 @@ bool PEImage::replaceDebugSection (const void* data, int datalen, bool initCV)
    dump_base = newdata;
    dump_total_len += fill + xdatalen;

+   // Invalidate the symbol table pointer
+   IMAGE_DOS_HEADER *dos = DPV<IMAGE_DOS_HEADER> (0);
+   if(dos && dos->e_magic == IMAGE_DOS_SIGNATURE)
+   {
+       IMAGE_NT_HEADERS32* hdr32 = DPV<IMAGE_NT_HEADERS32> (dos->e_lfanew);
+       if(hdr32->FileHeader.Machine == IMAGE_FILE_MACHINE_AMD64 ||
+          hdr32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64)
+       {
+           IMAGE_NT_HEADERS64* hdr64 = DPV<IMAGE_NT_HEADERS64> (dos->e_lfanew);
+           hdr64->FileHeader.PointerToSymbolTable = 0;
+           hdr64->FileHeader.NumberOfSymbols = 0;
+       } else {
+           hdr32->FileHeader.PointerToSymbolTable = 0;
+           hdr32->FileHeader.NumberOfSymbols = 0;
+       }
+   }
+
    return !initCV || initCVPtr(false);
 }

@rainers what do you think, is this the correct approach?

rainers commented 4 years ago

@rainers what do you think, is this the correct approach?

Indeed, looks correct.