Re: [Xen-devel] [PATCH v2 for-4.10] ubsan: add clang 5.0 support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.10.17 at 11:42, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Oct 18, 2017 at 03:23:20AM -0600, Jan Beulich wrote:
>> >>> On 18.10.17 at 09:45, <roger.pau@xxxxxxxxxx> wrote:
>> > +void __ubsan_handle_pointer_overflow(struct pointer_overflow_data *data,
>> > +                          unsigned long base, unsigned long result)
>> > +{
>> > +  unsigned long flags;
>> > +
>> > +  if (suppress_report(&data->location))
>> > +          return;
>> > +
>> > +  ubsan_prologue(&data->location, &flags);
>> > +
>> > +  if (((long)base >= 0) == ((long)result >= 0))
>> > +          pr_err("pointer operation %s %p to %p\n",
>> > +                  base > result ? "underflowed" : "overflowed",
>> > +                  (void *)base, (void *)result);
>> > +  else
>> > +          pr_err("pointer index expression with base %p overflowed to 
>> > %p\n",
>> > +                  (void *)base, (void *)result);
>> 
>> Would you mind explaining the difference between if and else
>> branches? (I do realize I should have asked this on v1 already,
>> but I didn't pay enough attention.) Whatever the idea behind
>> this, it should probably be explained in a comment, as it looks
>> to be heuristic.
> 
> The upstream commit is:
> 
> https://github.com/llvm-mirror/compiler-rt/commit/079b7657767dcc0fb284225c277d
>  
> 2b9ce73e423b
> 
> However it's lacking a proper commit message. It seems to me like it's
> there to detect addition of signed + unsigned values when an overflow
> happens, but I don't really see it's value rather than just using the
> first message.

Right - me too. I'd therefore like to simply drop the "if" and the "else"
branch (likely easily done while committing), and then the change is
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel