labath added inline comments.
================
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166
+ // The dlopen succeeded!
+ if (token != 0x0)
+ return process->AddImageToken(token);
----------------
jingham wrote:
> labath wrote:
> > Use LLDB_INVALID_IMAGE_TOKEN here?
> That wouldn't be right. LLDB_INVALID_IMAGE_TOKEN is the invalid token in
> lldb's namespace for image loading tokens, which has no relationship to any
> given platform's invalid token specifier. In fact, LLDB_INVALID_IMAGE_TOKEN
> is UINT32_MAX, so it is actually different from the POSIX invalid image token.
I see. Thanks for the explanation.
================
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+ return nullptr;
----------------
jingham wrote:
> labath wrote:
> > jingham wrote:
> > > labath wrote:
> > > > Will this ever be true? I would not expect one to be able to change the
> > > > platform of a process mid-session.
> > > Reading through the code I could not convince myself that it could NOT
> > > happen. Target::SetPlatform is a function anybody can call at any time.
> > > I agree that it would be odd to do so, and we might think about
> > > prohibiting it (Target::SetPlatform could return an error, and forbid
> > > changing the Platform, if the Target has a live process.)
> > >
> > > If everybody agrees that that is a good idea, I can do that and remove
> > > this check.
> > I see three call in the codebase to SetPlatform. All of them seem to be in
> > the initialization code, though some of them seem to happen after the
> > process is launched (DynamicLoaderDarwinKernel constructor).
> >
> > So it may not be possible to completely forbid setting the platform this
> > way, but I think we can at least ban changing the platform once it has
> > already been set in a pretty hard way (lldb_assert or even assert). I think
> > a lot of things would currently break if someone started changing the
> > platforms of a running process all of a sudden.
> Not sure. It seems reasonable to make a target, run it against one platform,
> then switch the platform and run it again against the new platform. I'm not
> sure I'm comfortable saying that a target can only ever use one platform.
Yeah, you're right. I suppose that makes sense. In my mind a target was
associated with a given platform from the moment it got created (that is how I
always do things: `platform select`, `platform connect`, and only then `target
create`). I never tried what would happen if I reversed that. So I guess the
platform can change during the lifetime of a Target, but I hope that is not the
case for the lifetime of a Process.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
