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

Filipe Cabecinhas filcab+llvm.phabricator at gmail.com
Thu Jun 4 16:25:58 PDT 2015


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.

 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/61e98337/attachment.html>


More information about the llvm-commits mailing list