Re: [dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well
On Jan 22, 2008 4:00 PM, Eric Pouech <eric.pouech(a)orange.fr> wrote:
Jason Green a écrit :
Re-responses from Eric van Beurden:
hmm I still don't get how, in a generic way symbols could overlap the only think I could come up with if when static functions get automatically inlined by the compiler, but that's rather a different story (as I'd suspect the inlined function to be a single memory range within a container => and in that case returning the shortest matching symbol should be fine is there an existing exe/pdb file available for further testings
in testing with one of our projects, i was seeing symbols lookups fail 85-90% of the time with the initial versions of dbghelp. The PDB files that were being used were generated along with the release build of the project (it was a C++ project as well). On further investigation i found that the lookups were failing because many symbol addresses and ranges were overlapping. This seemed to occur on even unrelated objects and symbols. I would suspect it was the result of the optimizations the compiler performed.
I was able to generate PDBs that exhibited this behaviour with every test app i built under Visual Studio (v7.1 and 8.0Express); even apps that were only 25 lines long. Since it was very repeatable and the project in question would always be using PDBs generated from optimized release builds i decided to change the symbol lookup method to take the range into account and to be able to store a sublist of all symbols whose addresses fall into the same range (15-25% of the lookups still failed with just the range addition).
I'm still not convinced that the fix proposed is the correct one. Do you have one of those examples handy so that I can have a look on it (src file + .exe + .pdb) ? that would be great TIA
Re-re-responses from Eric van Beurden. I'll send the sample app to you off-list (and anyone else who wants a copy, just email me directly).: I tracked down one of my sample apps that showed the overlapping symbols issue in its PDB file. Many of the symbols in it are either coincident or overlapping. In some cases the overlap occurs between a C++ decorated name and its undecorated counterpart, but in several cases the overlapping symbols are unrelated. For some symbols it could be possible that their sizes are being read incorrectly from the PDB. The original symbol lookup method used a binary search on a symbol list that was sorted by starting address. The issue was that during the search it was skipping over the correct symbol because its starting address was the same as a previous symbol's address. This sent the search in the wrong direction after that and the lookup would fail. There was no fallback for handling the case of the symbol being coincident with another or the symbol being within the address space of another symbol. The attached archive contains 4 files: - 'emptyTest.exe': the optimized release build executable linked to 'emptyTest.pdb'. This should crash. - 'emptyTest.pdb': the PDB file - 'main.cpp': the source file for the test app. Some of the code is unused as the app has morphed over time. - 'emptyTest.vcproj': the VC++7.1 project file for the test app
On Jan 23, 2008 8:14 AM, Jason Green <jave27(a)gmail.com> wrote:
The attached archive contains 4 files: - 'emptyTest.exe': the optimized release build executable linked to 'emptyTest.pdb'. This should crash. - 'emptyTest.pdb': the PDB file - 'main.cpp': the source file for the test app. Some of the code is unused as the app has morphed over time. - 'emptyTest.vcproj': the VC++7.1 project file for the test app
Missing the attachment? --John
See the first paragraph: :) Re-re-responses from Eric van Beurden. I'll send the sample app to you off-list (and anyone else who wants a copy, just email me directly).: I'll send it to you off-list, too, John. On Jan 23, 2008 10:31 AM, John Klehm <xixsimplicityxix(a)gmail.com> wrote:
On Jan 23, 2008 8:14 AM, Jason Green <jave27(a)gmail.com> wrote:
The attached archive contains 4 files: - 'emptyTest.exe': the optimized release build executable linked to 'emptyTest.pdb'. This should crash. - 'emptyTest.pdb': the PDB file - 'main.cpp': the source file for the test app. Some of the code is unused as the app has morphed over time. - 'emptyTest.vcproj': the VC++7.1 project file for the test app
Missing the attachment?
--John
Jason Green a écrit : thanks for the sample files how does this patch solve the issue ? A+ diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h index 58627c0..8a22dfd 100644 --- a/include/wine/mscvpdb.h +++ b/include/wine/mscvpdb.h @@ -343,9 +343,9 @@ union codeview_type { unsigned short int len; short int id; - unsigned this_type; - unsigned int class_type; unsigned int rvtype; + unsigned int class_type; + unsigned this_type; unsigned char call; unsigned char reserved; unsigned short params; -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)
On Jan 23, 2008 3:38 PM, Eric Pouech <eric.pouech(a)orange.fr> wrote:
Jason Green a écrit : thanks for the sample files how does this patch solve the issue ? A+
diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h index 58627c0..8a22dfd 100644 --- a/include/wine/mscvpdb.h +++ b/include/wine/mscvpdb.h @@ -343,9 +343,9 @@ union codeview_type { unsigned short int len; short int id; - unsigned this_type; - unsigned int class_type; unsigned int rvtype; + unsigned int class_type; + unsigned this_type; unsigned char call; unsigned char reserved; unsigned short params;
Hey, Eric - I actually don't see that part of a patch anywhere in what I submitted... Are you sure this was what you meant to paste?
Jason Green a écrit :
On Jan 23, 2008 3:38 PM, Eric Pouech <eric.pouech(a)orange.fr> wrote:
Jason Green a écrit : thanks for the sample files how does this patch solve the issue ? A+
diff --git a/include/wine/mscvpdb.h b/include/wine/mscvpdb.h index 58627c0..8a22dfd 100644 --- a/include/wine/mscvpdb.h +++ b/include/wine/mscvpdb.h @@ -343,9 +343,9 @@ union codeview_type { unsigned short int len; short int id; - unsigned this_type; - unsigned int class_type; unsigned int rvtype; + unsigned int class_type; + unsigned this_type; unsigned char call; unsigned char reserved; unsigned short params;
Hey, Eric - I actually don't see that part of a patch anywhere in what I submitted... Are you sure this was what you meant to paste?
my point was to revert the patch #10 (the one about overlapping symbols and the likes), and to replace it simply with this one A+ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)
participants (3)
-
Eric Pouech -
Jason Green -
John Klehm