> On Aug 18, 2014, at 3:18 PM, Todd Fiala <[email protected]> wrote: > > On Mon, Aug 18, 2014 at 2:47 PM, <[email protected]> wrote: > > > On Aug 18, 2014, at 12:45 PM, Todd Fiala <[email protected]> wrote: > > > > On Mon, Aug 18, 2014 at 11:22 AM, <[email protected]> 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 <[email protected]> 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 > > > [email protected] > > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > _______________________________________________ > > lldb-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > > > > > -- > > Todd Fiala | Software Engineer | [email protected] | 650-943-3180 > > > > > -- > Todd Fiala | Software Engineer | [email protected] | 650-943-3180 _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
