Re: [dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well

[Jason Green]

On Jan 22, 2008 4:00 PM, Eric Pouech  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

[dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well

[John Klehm]

On Jan 23, 2008 8:14 AM, Jason Green  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

[dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well

[Jason Green]

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  wrote:
> On Jan 23, 2008 8:14 AM, Jason Green  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

[dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well

[Eric Pouech]

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;

[dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well

[Jason Green]

On Jan 23, 2008 3:38 PM, Eric Pouech  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?

[dbghelp 4/10] Rewrite much of the symbol lookup method to work with Optimized PDB files as well

[Eric Pouech]

Jason Green a écrit :
> On Jan 23, 2008 3:38 PM, Eric Pouech  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+