On Wed, May 31, 2017 at 4:13 PM, Martin Liška <mli...@suse.cz> wrote:
> On 05/31/2017 03:31 PM, Richard Biener wrote:
>> On Wed, May 31, 2017 at 2:28 PM, Martin Liška <mli...@suse.cz> wrote:
>>> On 05/31/2017 02:04 PM, Richard Biener wrote:
>>>> On Wed, May 31, 2017 at 1:51 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>>>>> On Wed, May 31, 2017 at 01:46:00PM +0200, Richard Biener wrote:
>>>>>> Just wanting to add that "ab-"using options/variables to implement
>>>>>> what are really
>>>>>> function attributes doesn't look very clean. Unless the plan is to get
>>>>>> rid of
>>>>>> function attributes in favor of per-function options.
>>>>>
>>>>> Function attribute here is one thing (the way user writes it) and that
>>>>> combined with the command line options determines the sanitization
>>>>> performed
>>>>> (the function attributes only say what sanitization flags should be
>>>>> ignored). The proposed per-function variable is just a cache of this
>>>>> information, because parsing function attributes every time is way too
>>>>> expensive.
>>>>
>>>> True, but isn't that just an excuse to not improve attribute list
>>>> representation?
>>>>
>>>> Ideally we'd have sth like attributes.def and a sorted vector of
>>>> integer id, args
>>>> pairs. Using a sorted vector of the existing stuff (compared to the tree
>>>> list)
>>>> might also help.
>>>
>>> Then it would be tree-wise very similar to CONSTRUCTOR which also contains
>>> vector
>>> of (index, value) pairs?
>>>
>>>>
>>>> Yes, we'd get (quite?) a bit less attribute list sharing this way but
>>>> we can still
>>>> share the actual tree-whatever thing that represents the args.
>>>
>>> Any estimation how difficult such transformation would be?
>>
>> attribute lists are dealt with in quite some places (with or without
>> helpers) so I guess it would be somewhat invasive but largely
>> mechanical. Using a .def file vs. the current strings can be
>> done separately -- after all we can also sort strings. I suspect
>> doing the string -> ID transform pays off faster (still linear search
>> but integer comparison instead of string compare).
>
> Ok, I'm ready to do the transformation in this stage1. That said, will you be
> Jakub fine with the original patch (rebase will be needed) as it is, using
> DECL_ATTRIBUTE?
It looks ok to me though I miss __attribute__((no_sanitize("all"))) (or is no
argument equal to 'all' -- spelling out all those opts in the testcases looks
awkward).
I'd appreciate a 2nd eye though, the patch is large.
The use of sanitize_flags_p (...) in pass_ubsan::execute would appreciate
sth like
flags = sanitize_flags (..., fun->decl);
so it can cache across different flag settings.
Thanks,
Richard.
> Thanks,
> Martin
>
>>
>> Richard.
>>
>>> Martin
>>>
>>>>
>>>> Richard.
>>>>
>>>>>
>>>>> Jakub
>>>
>
