Allow the use of debugging symbols in a separate file #96
Conversation
src/cv2pdb.cpp
Outdated
| CV2PDB::CV2PDB(PEImage& image, DebugLevel debug_) | ||
| : img(image), pdb(0), dbi(0), tpi(0), ipi(0), libraries(0), rsds(0), rsdsLen(0), modules(0), globmod(0) | ||
| CV2PDB::CV2PDB(PEImage& image, PEImage* imageDWARF, DebugLevel debug_) | ||
| : img(image), imgDWARF(imageDWARF ? imageDWARF : &image), pdb(0), dbi(0), tpi(0), ipi(0), libraries(0), rsds(0), rsdsLen(0), modules(0), globmod(0) |
There was a problem hiding this comment.
I think it is a bit confusing that imgDWARF is also used for the CodeView part. I haven't checked how often img is still used now, but couldn't it be initialized with img(imageDWARF ? *imageDWARF : image) instead, and image be kept in some exeImage reference/pointer?
There was a problem hiding this comment.
imgDWARF will always be NULL for CodeView.
If exe image has either CV entries or DWARF entries, it will be assigned to img and imgDWARF will be NULL. This logic remains as in the original code.
If there is a debug link, either specified by the switch or present in the exe image, code will attempt to load the linked DWARF image. In this case, img will be the exe image and imgDWARF will be the linked DWARF image.
Otherwise a separate .dbg is loaded (CodeView) and assigned to img and imgDWARF will be NULL.
There was a problem hiding this comment.
In CV2PDB initialization, imageDWARF will refer to img if the pointer was NULL.
Perhaps renaming imageDWARF is the correct choice because it can point to img which in turn can be either exe image or CV image.
There was a problem hiding this comment.
img is what code modifies while imgDWARF is a "read-only" image. Just trying to come up with a good name for it.
There was a problem hiding this comment.
I renamed imgDWARF member to cimgDebugInfo and made it const
There was a problem hiding this comment.
Ok, img is still used for other purposes. cimgDebugInfo is fine with me, though a bit long. Maybe just imgDI or imgDbg with a commit at the declaration site?
There was a problem hiding this comment.
with a commit at the declaration site
Don't understand the lingo. Did you mean "comment"?
There was a problem hiding this comment.
Don't understand the lingo. Did you mean "comment"?
Oops. Indeed that's what I meant ;)
| @@ -180,18 +180,18 @@ bool CV2PDB::openPDB(const TCHAR* pdbname, const TCHAR* pdbref) | |||
| printf("DBI::QueryImplementationVersion() = %d\n", dbi->QueryImplementationVersion()); | |||
| #endif | |||
|
|
|||
| rc = pdb->OpenTpi("", &tpi); | |||
| // The default is "r" mode. We need "rw" mode so TPI gets created if it does not exist | |||
| rc = pdb->OpenTpi("rw", &tpi); | |||
There was a problem hiding this comment.
Sounds good. Is there official documentation for this by now?
There was a problem hiding this comment.
I looked at their source code to see what it does.
There was a problem hiding this comment.
This might fix #87
| struct _stat buffer; | ||
|
|
||
| if (debug_link || exe.hasDebugLink()) |
There was a problem hiding this comment.
Can this part be extracted to some function?
There was a problem hiding this comment.
In what way? One checks a command line switch and the other checks if the image has a debug link section. Or do you mean the whole block?
src/readDwarf.cpp
Outdated
| @@ -359,7 +359,7 @@ Location decodeLocation(const DWARF_Attribute& attr, const Location* frameBase, | |||
| } | |||
| } | |||
|
|
|||
| assert(stackDepth > 0); | |||
| //assert(stackDepth > 0); | |||
There was a problem hiding this comment.
I was hitting this and another assert while testing with a Golang CGO DLL. I do not really know what is the problem being detected by this (and the other one I commented out) asserts.
I can roll these back as I have not touched this code.
There was a problem hiding this comment.
Probably better to keep the asserts, so it can be investigated if it comes up again.
There was a problem hiding this comment.
Removed the commit
michailf
force-pushed
the
feature/use-separate-debugging-symbols
branch
3 times, most recently
from
c7c6ca0 to
c5e103f
Compare
michailf
force-pushed
the
feature/use-separate-debugging-symbols
branch
from
c5e103f to
82d192b
Compare
|
Thanks. |
This PR resolves issue #17
It introduces a new option
-l(lower case L) to allow specifying a debug link to an external debug information file. If the the option is not specified, the image file is checked for.gnu_debuglinksection, and that link is used. The option has precedence over the link in the image file.The debug information file will be loaded from the same directory as the image file. If the file is not found, it will be loaded from the
.debugsubdirectory. If the debug link is an absolute path, it will be used as-is.