[PATCH] [sanitizer_common] Added VS-style output for source locations

[Alexey Samsonov]

On Thu, Jun 4, 2015 at 4:25 PM, Filipe Cabecinhas <
filcab+llvm.phabricator at gmail.com> wrote:

> On Thu, Jun 4, 2015 at 4:16 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
>
>>
>> On Thu, Jun 4, 2015 at 3:57 PM, Filipe Cabecinhas <
>> filcab+llvm.phabricator at gmail.com> wrote:
>>
>>> Sounds good to me. But there's a detail to be dealt with.
>>>
>>> For out of the box _testing_, having the VS style output be the default
>>> will force us to, either:
>>>  - use a script to run the commands (doable, but ugly (except if you
>>> need the script anyway, for remote testing. I'm ok with this))
>>>  - or change all the tests to expect one of the output styles (I don't
>>> really like this :-) )
>>>
>>> I would like to avoid both of those solutions.
>>>
>>> What do you think about doing something like this:
>>>
>>> Have options get their values in this order:
>>>  - Compiled-in defaults
>>>  - $SANITIZER_OPTIONS (new)
>>>  - __tool_default_options() (asan, ubsan, etc)
>>>  - $TOOL_OPTIONS (ASAN, UBSAN, etc)
>>>
>>> The rationale is:
>>>  - Compiled-in defaults should be sensible defaults for the platform.
>>>  - $SANITIZER_OPTIONS could set common flags (only, no tool-specific
>>> options) for the user/machine
>>>  - __tool_default_options() can be used in a program to override some
>>> flags because the test/program expects this
>>>  - $TOOL_OPTIONS is a final chance for the current user, running the
>>> program, to override.
>>>
>>> I'm ok with not doing this, and simply waiting until it's more necessary.
>>> For the PS4, I can just work around it and "hack" our remote run script
>>> to always add symbolize_vs_style=false to *_OPTIONS env vars when testing.
>>> But it wouldn't be nice for local Windows testing (non-remote), since we
>>> would default to vs-style (if _MSC_VER was defined) in some cases.
>>>
>>
>> I'd like to avoid it if possible :) I feel that there are far too many
>> overrides already, which occasionally confuse users. It would be really
>> untrivial to explain why one environment variable kicks off before
>> linked-in defaults,
>> and all the rest env vars kicks off afterwards.
>>
> As for testing, we can adjust ASAN_OPTIONS, UBSAN_OPTIONS etc. in lit
>> config and add "symbolize_vs_style=false" there. Then we can write all test
>> expectations for POSIX style (and have a couple of lit tests overriding
>> this value and testing VS-style output).
>>
> You can't simply do this. A bunch of tests will, in their run commands,
> define ASAN_OPTIONS and UBSAN_OPTIONS. This will override anything that lit
> might be setting, throwing our override out the window. Unless I'm missing
> something.
>

Well, you can fix those tests to call "env
ASAN_OPTIONS=$ASAN_OPTIONS:my_override=false" so that you preserve global
ASAN_OPTIONS specified in lit config. Note that after your recent change we
won't pick up ASAN_OPTIONS from the actual environment.


>
>  Filipe
>
>
>>
>>
>>>
>>>
>>
>>> Any ideas?
>>>
>>> Thanks,
>>>
>>>  Filipe
>>>
>>>
>>> On Thu, Jun 4, 2015 at 2:57 PM, Reid Kleckner <rnk at google.com> wrote:
>>>
>>>> REPOSITORY
>>>>   rL LLVM
>>>>
>>>> ================
>>>> Comment at: lib/sanitizer_common/sanitizer_flags.inc:149
>>>> @@ -148,1 +148,3 @@
>>>>              "Print inlined frames in stacktraces. Defaults to true.")
>>>> +COMMON_FLAG(bool, symbolize_vs_style, false,
>>>> +            "Print file locations in Visual Studio style (e.g: "
>>>> ----------------
>>>> samsonov wrote:
>>>> > filcab wrote:
>>>> > > This should probably default to SANITIZER_WINDOWS instead of false.
>>>> > > I think it would be the option that better matches what a user of
>>>> that platform would expect, no?
>>>> > I'd let Reid comment on this. I don't have strong preference about
>>>> Windows defaults.
>>>> Let's default this to `defined(_MSC_VER)`. Thankfully it is very easy
>>>> to override the default sanitizer options with __asan_default_options(), so
>>>> I'd rather optimize for first time users of visual studio rather than
>>>> people like us with big cross-platform projects.
>>>>
>>>> http://reviews.llvm.org/D10113
>>>>
>>>> EMAIL PREFERENCES
>>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> Alexey Samsonov
>> vonosmas at gmail.com
>>
>
>


-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/2f19a036/attachment.html>