jingham added a comment.

I added a comment, I'll upload the diff with that in a sec.



================
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166
+  // The dlopen succeeded!
+  if (token != 0x0)
+    return process->AddImageToken(token);
----------------
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.


================
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+  if (platform != GetTarget().GetPlatform().get())
+    return nullptr;
----------------
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.


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

Reply via email to