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