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

[Filipe Cabecinhas]

I can live with test case churn.
I'll do it soon and submit the patches for:
 - test churn
 - default to vs-style if defined(_MSC_VER)

Thanks,

 Filipe

On Thu, Jun 4, 2015 at 4:35 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:

>
> 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/9c459dc0/attachment.html>