[Lldb-commits] [PATCH] Add process launch --enable-aslr option, tweak handling of flag

[jingham at apple.com]

> On Aug 18, 2014, at 3:18 PM, Todd Fiala <tfiala at google.com> wrote:
> 
> On Mon, Aug 18, 2014 at 2:47 PM,  <jingham at apple.com> wrote:
> 
> > On Aug 18, 2014, at 12:45 PM, Todd Fiala <tfiala at google.com> wrote:
> >
> > On Mon, Aug 18, 2014 at 11:22 AM,  <jingham at apple.com> wrote:
> > I don't think we have any other instances where we use two flags to express "do x" and "don't do x".
> > For the long options is isn't such a big deal but we try not to use up more short options than necessary, since this gets to be a crowded space.
> >
> > I'd be happy to use long-option only here.
> >
> >   Maybe "process launch --enable-aslr <true/false>", which mirrors the setting anyway?
> >
> >
> > Sounds good.  But the fallback behavior when process launch doesn't specify anything is currently to disable ASLR if the target.disable-aslr setting is true.  Are you interested in reversing that setting to enable-aslr?  If not, then maybe we go with just extending 'process launch --disable-aslr <true/false>'.
> >
> > In sum, I'm for either
> >
> > (1) changing to either `process launch --enable-aslr <true/false>` and change the `settings target.disable-aslr' to match the name as the fallback setting, or
> >
> > (2) keeping the target.disable-aslr setting, and extending `process launch --disable-aslr` to take true/false.  (But - note - this does get into what Chandler called out before as being somewhat long in the tooth - `process launch --disable-aslr false` when you want ASLR.)
> 
> I like option 2.  You generally aren't turning on aslr, that's something the system does or does not have on.  You're disabling the system's aslr for this launch.  
> 
> Yeah, I started having pangs of a potential misleading option that requires more explaining when I wrote this comment in the patch:
> 
> // Determine whether we will disable ASLR or leave it in the default state (i.e. enabled if the platform supports it).
> 
> So I think disable is right.  
> 
> I am logically behind that argument.  I talked to Ed Maste earlier today as I had done some research on this over the weekend on FreeBSD and found that it was still in patch state (i.e. not part of FreeBSD proper).  So there it definitely would be always disabled, because the default state is "not there".  Enable would certainly be misleading there.  Chandler made the argument for emitting a warning/error in that case (i.e. you're asking for something that cannot be).
> 
> Probably the conceptual dislike here is we have a disable mechanism but the disable is the default.  So shutting off the disable is saying disable = no.  Just a bit wordy.  But accurate.
>  
> 
> I like option 2.  You generally aren't turning on aslr, that's something the system does or does not have on.  You're disabling the system's aslr for this launch. 
> 
> Let's just leave it at that.  I'll adjust.  So, to make sure I understand, are we keeping the -A for that as the short option?  (I think yes, and we're just getting rid of the --enable-aslr since this will be --disable-aslr false).
> 

Yes, that seems right to me.

Jim



> -Todd
> 
> As for the length of the command, either this isn't going to be something you type often (and even if you are you'll type:
> 
> pr la --d 0
> 
> ) or if you use it a lot, you'll make an alias for it.
> 
> 
> >
> > Thoughts on that?  I'll code up whatever we decide on.
> >
> > If backward  compat for option (1) is a concern, we could continue to accept `settings set disable-aslr <true/false>` and just have it do the right thing.
> >
> > -Todd
> >
> > Jim
> >
> > > On Aug 17, 2014, at 10:48 PM, Todd Fiala <todd.fiala at gmail.com> wrote:
> > >
> > > This change modifies the logic used to set the eLaunchFlagDisableASLR ProcessLaunchInfo setting for inferior process launching.  Now, if 'process launch' is provided with either --disable-aslr or --enable-aslr, then the launch flag is set accordingly.  If niether --disable-aslr or --enable-aslr are specified, then the setting for target.disable-aslr is used to determine the setting or clearing of the eLaunchFlagDisableASLR setting.  The target.disable-aslr setting currently defaults to true, so the default behavior when nothing is specified on the 'process launch' (i.e. 'run' command) is to disable ASLR.
> > >
> > > --
> > > -Todd
> > > <tfiala_enable-aslr.diff>_______________________________________________
> > > lldb-commits mailing list
> > > lldb-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> >
> >
> >
> > --
> > Todd Fiala |   Software Engineer |     tfiala at google.com |     650-943-3180
> 
> 
> 
> 
> -- 
> Todd Fiala |	 Software Engineer |	 tfiala at google.com |	 650-943-3180