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

[Todd Fiala]

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).

-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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140818/414605e1/attachment.html>