Page MenuHomePhabricator

Support member function types in PdbAstBuilder
AcceptedPublic

Authored by zloyrobot on Apr 25 2019, 6:34 AM.

Details

Summary

This path implements missing case in PdbAstBuilder::CreateType for LF_MFUNCTION. This is necessary, for example, in stack unwinding of struct methods.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

zloyrobot created this revision.Apr 25 2019, 6:34 AM

Please add some detail to the commit message and consider re-titling it. It looks like this patch is actually adding missing functionality rather than "fixing" a bug in how structs are handled. If that's correct, then please make the commit message reflect that.

Sorry I didn't get a chance to finish reviewing this today, but I'll try to get it done tomorrow.

zloyrobot retitled this revision from Fix stack unwinding for struct methods to Support member function types in PdbAstBuilder.Apr 26 2019, 8:06 AM
zloyrobot edited the summary of this revision. (Show Details)
amccarth accepted this revision.Apr 29 2019, 11:26 AM

Thanks for the improved commit message. Again, sorry about the delay.

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
183

Suggestion: Consider CreateMemberFunctionType or CreateMethodType to avoid confusion with CreateProcedureType.

This revision is now accepted and ready to land.Apr 29 2019, 11:26 AM

Thanks for the improved commit message. Again, sorry about the delay.

No problem, thanks for review!

Thanks for the improved commit message. Again, sorry about the delay.

No problem, thanks for review!

I afraid I don't have commit access to merge this path. Could somebody help me?

labath added a subscriber: labath.EditedMay 13 2019, 6:37 AM

Looks like this is failing on the windows bot http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/4635/steps/test/logs/stdio.

Looking at the test, it seems that you are asserting that you will unwind into a particular offset in ntdll.dll. That sounds like an incredibly fragile assertion, which will only be true for people running the exact same version of ntdll that you happen to have...

Looks like this is failing on the windows bot http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/4635/steps/test/logs/stdio.

Looking at the test, it seems that you are asserting that you will unwind into a particular offset in ntdll.dll. That sounds like an incredibly fragile assertion, which will only be true for people running the exact same version of ntdll that you happen to have...

My bad, these assertions (ntdll and kernel32) should be removed from test. I already asked @aleksandr.urakov to do it.