rainers / cv2pdb

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

Access violation fixed. Better aligning of debug section. #27

Closed AlexWhiter closed 6 years ago

AlexWhiter commented 6 years ago
  1. If exe-file parameter consisted of just a disk letter and a file name (i.e. c:file.exe), AV occurred.
  2. Aligning the debug section offset to the closest 16-byte boundary.
rainers commented 6 years ago

Is alignment important? A section is aligned to the section alignment anyway (usually 0x200), and the debug directory length seems unaligned with images built with the MS linker.

AlexWhiter commented 6 years ago

Without any alignment, IMAGE_DEBUG_DIRECTORY structure will too reside unaligned in memory. It's doesn't seem to be a big problem for dbghelp.dll, but a couple of rather old versions if IDA Pro complained about broken files when I've tried to load an EXE with unaligned debug directory. Some exotic debuggers can theoretically do the same.

In all files build with MSVC I've checked, IMAGE_DEBUG_DIRECTORY was always aligned to 16 bytes.

rainers commented 6 years ago

Without any alignment, IMAGE_DEBUG_DIRECTORY structure will too reside unaligned in memory.

IIUC your change does not align the offset of the struct, but the length. I noticed multiple debug entries in some system DLLs (e.g. kernel32.dll), so it might be even harmful if another entry might be expected in the padded space.

AlexWhiter commented 6 years ago

IMAGE_DEBUG_DIRECTORY resides after RSDS structure, and changing RSDS length changes the alignment of IMAGE_DEBUG_DIRECTORY, isn't it?

Yes, there can be more than one debug entry, since IMAGE_DEBUG_DIRECTORY is just an array. But this array shouldn't necessarily start with .debug section. It's OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG] that specify the array start and size. And the code I've proposed aligns OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress.

rainers commented 6 years ago

Ah sorry, you're right. I think it would be more obvious if the alignment happens inside replaceDebugSection when calculating xdatalen and dbgdir.

AlexWhiter commented 6 years ago

Ok.

rainers commented 6 years ago

Looks good now. Thanks.