Page MenuHomePhabricator

Add =shlibs-added/=shlibs-removed notifications (MI)
ClosedPublic

Authored by ki.stfu on Mar 10 2015, 7:12 AM.

Details

Summary

This patch adds =shlibs-added/=shlibs-removed notifications in lldb-mi. In more detail:

  1. Add Target::ModulesDidLoad/ModulesDidUnload notifications
  2. Improve Target::TargetEventData:
    1. Refactoring
    2. Move it back to include/lldb/Target/Target.h
    3. Add Target::{GetModuleListFromEvent,GetModuleList}; Add Target::m_module_list
  3. Add SBModule::{GetSymbolVendorMainFileSpec,GetObjectFileHeaderAddress}
  4. Add SBTarget::{EventIsTaretEvent,GetTargetFromEvent,GetNumModulesFromEvent,GetModuleAtIndexFromEvent}

All tests pass on OS X.

Diff Detail

Repository
rL LLVM

Event Timeline

ki.stfu updated this revision to Diff 21576.Mar 10 2015, 7:12 AM
ki.stfu retitled this revision from to Add =shlibs-added/=shlibs-removed notifications (MI).
ki.stfu updated this object.
ki.stfu edited the test plan for this revision. (Show Details)
ki.stfu added reviewers: abidh, clayborg.
ki.stfu added subscribers: abidh, clayborg, Unknown Object (MLST).
ki.stfu updated this object.Mar 10 2015, 7:16 AM
ki.stfu added reviewers: zturner, jingham.
ki.stfu added subscribers: zturner, jingham.
clayborg requested changes to this revision.Mar 10 2015, 12:57 PM
clayborg edited edge metadata.

Overall looks good, just a few things to fix. See inlined comments.

include/lldb/API/SBModule.h
332–333 ↗(On Diff #21576)

Rename this to GetSymbolFileSpec(). The public API doesn't need to know anything about symbol vendors.

include/lldb/Target/Target.h
498–547 ↗(On Diff #21576)

Leave this in the Target.cpp and add static accessors to Target.h and Target.cpp much like the ones in SBTarget

source/API/SBModule.cpp
689 ↗(On Diff #21576)

Rename to GetSymbolFileSpec()

source/API/SBTarget.cpp
136 ↗(On Diff #21576)

Make static functions in Target.h/cpp so we don't expose the TargetEventData class to anyone.

142 ↗(On Diff #21576)

Make static functions in Target.h/cpp so we don't expose the TargetEventData class to anyone.

148 ↗(On Diff #21576)

Make static functions in Target.h/cpp so we don't expose the TargetEventData class to anyone.

155 ↗(On Diff #21576)

Make static functions in Target.h/cpp so we don't expose the TargetEventData class to anyone.

This revision now requires changes to proceed.Mar 10 2015, 12:57 PM

I have a question.

source/API/SBTarget.cpp
136 ↗(On Diff #21576)

Why you want to hide it in cpp file? It was done similarly to ThreadEventData/ProcessEventData/BreakpointEventData.

ki.stfu updated this revision to Diff 21640.Mar 10 2015, 2:39 PM
ki.stfu edited edge metadata.

Rename SBModule::GetSymbolVendorMainFileSpec to GetSymbolFileSpec

clayborg accepted this revision.Mar 10 2015, 2:52 PM
clayborg edited edge metadata.

Ok, I thought I remembers the ProcessEventData being in Process.cpp. So it seems consistent to put it in the header file.

Looks good.

This revision is now accepted and ready to land.Mar 10 2015, 2:52 PM
ki.stfu closed this revision.Mar 10 2015, 3:02 PM
This revision was automatically updated to reflect the committed changes.
abidh edited edge metadata.Mar 11 2015, 7:59 AM

I have some questions about this patch.

Gdb uses library-loaded to notify about the shared libraries. Why are you not using it?
Where can I get the documentation about the fields of shlibs-added?
This event is also being generated in case of main executable file. GDB only generates it for shared library. Is this intended behaviour.

In D8201#138864, @abidh wrote:

I have some questions about this patch.

Gdb uses library-loaded to notify about the shared libraries. Why are you not using it?
Where can I get the documentation about the fields of shlibs-added?
This event is also being generated in case of main executable file. GDB only generates it for shared library. Is this intended behaviour.

I looked how it works in GDB, and I have seen =shlibs-added/=shlibs-removed format. For example:

100=shlibs-added,shlib-info={num="2",name="nb-test",kind="-",dyld-addr="0x100000000",reason="exec",requested-state="Y",state="Y",path="/Users/bpontarelli/dev/inversoft/cleanspeak/cleanspeak-domain/projects/nb-test/dist/Debug/GNU-MacOSX/nb-test",description="/Users/bpontarelli/dev/inversoft/cleanspeak/cleanspeak-domain/projects/nb-test/dist/Debug/GNU-MacOSX/nb-test",loaded_addr="",slide="0x0",prefix=""}
100=shlibs-added,shlib-info={num="3",name="libgtest.0.0.0.dylib",kind="-",dyld-addr="0x1000f4000",reason="dyld",requested-state="Y",state="Y",path="/usr/local/lib/libgtest.0.0.0.dylib",description="/usr/local/lib/libgtest.0.0.0.dylib",loaded_addr="0x1000f4000",slide="-0xfffffffefff0c000",prefix=""}
100=shlibs-added,shlib-info={num="4",name="libmysqlclient.15.dylib",kind="-",dyld-addr="0x100152000",reason="dyld",requested-state="Y",state="Y",path="/usr/local/mysql-5.0.51b-osx10.5-x86_64/lib/libmysqlclient.15.dylib",description="/usr/local/mysql-5.0.51b-osx10.5-x86_64/lib/libmysqlclient.15.dylib",loaded_addr="0x100152000",slide="-0xfffffffeffeae000",prefix=""}
100=shlibs-added,shlib-info={num="5",name="libstdc++.6.0.4.dylib",kind="-",dyld-addr="0x7fff81e61000",reason="dyld",requested-state="Y",state="Y",path="/usr/lib/libstdc++.6.0.4.dylib",description="/usr/lib/libstdc++.6.0.4.dylib",loaded_addr="0x7fff81e61000",slide="-0xffff80007e19f000",prefix=""}
100=shlibs-added,shlib-info={num="6",name="libgcc_s.1.dylib",kind="-",dyld-addr="0x7fff80ef3000",reason="dyld",requested-state="Y",state="Y",path="/usr/lib/libgcc_s.1.dylib",description="/usr/lib/libgcc_s.1.dylib",loaded_addr="0x7fff80ef3000",slide="-0xffff80007f10d000",prefix=""}
100=shlibs-added,shlib-info={num="7",name="libSystem.B.dylib",kind="-",dyld-addr="0x7fff81728000",reason="dyld",requested-state="Y",state="Y",path="/usr/lib/libSystem.B.dylib",description="/usr/lib/libSystem.B.dylib",loaded_addr="0x7fff81728000",slide="-0xffff80007e8d8000",prefix="",commpage-objpath="/usr/lib/libSystem.B.dylib[LC_SEGMENT.__DATA.__commpage]"}
100=shlibs-added,shlib-info={num="8",name="libmathCommon.A.dylib",kind="-",dyld-addr="0x7fff824a4000",reason="dyld",requested-state="Y",state="Y",path="/usr/lib/system/libmathCommon.A.dylib",description="/usr/lib/system/libmathCommon.A.dylib",loaded_addr="0x7fff824a4000",slide="-0xffff80007db5c000",prefix=""}

Original link: https://bugzilla-attachments-169842.netbeans.org/bugzilla/attachment.cgi?id=85852

That probably was in Apple fork of gdb. @jasonmolenda may know the history. There is no such event in current gdb.

As you will see on the link below, "library-loaded" event is used.
https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Async-Records.html

I will dig more deeper to see how this is being used but it seems to me that you either have to use the "library-loaded" event or use this code for OSX only.

jingham edited edge metadata.Mar 11 2015, 10:13 AM

The Apple gdb implementation of the MI and the FSF version are different in quite a few respects.

The MI was added by Cygnus long ago for an project that died stillborn, and so its original form was never used for a real IDE. After that it languished for a while till we started using it for ProjectBuilder not quite so long ago. We added/changed many things to make it work IRL, but that was on a NeXT based fork of gdb that was pretty different from the then current FSF gdb, so these changes were not submitted back. Then when the FSF community revived the MI for Eclipse & the Emacs MI mode etc. they needed to make similar additions/modifications but since that was done independently the choices made were slightly different.

Did anybody outside ProjectBuilder use Apple's version of the MI interfaces? Unless there are important clients for that flavor, it seems like it would be better to use the FSF versions where they differ.

Jim

If you are on a system using ASLR you also might want to know where the main executable loaded, so it seems useful to send it for the main executable. shlibs-added is a little funny, for instance on OS X gdb used to read all the directly linked libraries when the main executable was loaded, so they weren't really "added". To that end we also had a shlibs-updated to say "you already knew about this but it either changed (binary changed) or its load addresses changed.

Jim

fwiw, I agree with Jim. The Apple gdb-mi style is a historical oddity at this point - Project Builder/Xcode were the only IDEs that interoperated with it as far as I know, and Xcode dropped MI support a few years ago altogether. There are many cases where we solved a problem in our MI one way, then it was either modified in the process of upstreaming the feature, or it was implemented independently by the FSF gdb folks as they completed the MI support for Eclipse. So there are lots of little divergences in the newer features that have been added to MI.

Eclipse may even have some old bitrotted support for the Apple variant of MI.

For lldb's MI support, it would be better to follow the current FSF gdb MI's style so we can interoperate with IDEs like Eclipse or emacs GUD mode seamlessly.