Question on -fwrapv and -fwrapv-pointer

Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

Hi,

I have several questions on these options:

1.are pointers treated as signed integers in general? (I thought that pointers are addresses to the memory, should be treated as unsigned integer…)
2. If Yes, why? 
3. why a separate option for pointesr -fwrapv-pointer in addition to -fwrapv if they are treated as signed integers?

Thanks for your help.

Qing

Re: Question on -fwrapv and -fwrapv-pointer

[Richard Biener]

On Thu, Sep 14, 2023 at 3:42 PM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> I have several questions on these options:
>
> 1.are pointers treated as signed integers in general? (I thought that pointers are addresses to the memory, should be treated as unsigned integer…)
> 2. If Yes, why?
> 3. why a separate option for pointesr -fwrapv-pointer in addition to -fwrapv if they are treated as signed integers?

Pointers are unsigned, they might sign-extend to Pmode though.
-fwrapv-pointer is to enable wrapping over zero,
we don't have many places using this, ISTR kernel folks requested to
disable specific folding - digging in history
might reveal the case/PR.

Richard.

> Thanks for your help.
>
> Qing
>

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

Thanks for the info.

> On Sep 14, 2023, at 10:06 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Thu, Sep 14, 2023 at 3:42 PM Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> Hi,
>> 
>> I have several questions on these options:
>> 
>> 1.are pointers treated as signed integers in general? (I thought that pointers are addresses to the memory, should be treated as unsigned integer…)
>> 2. If Yes, why?
>> 3. why a separate option for pointesr -fwrapv-pointer in addition to -fwrapv if they are treated as signed integers?
> 
> Pointers are unsigned, they might sign-extend to Pmode though.
If they are unsigned, why they are sign-extend to Pmode? Is there any special reason for this? 
In another word, can we consistently treat pointers as unsigned? 

> -fwrapv-pointer is to enable wrapping over zero,

If we always treat pointers are unsigned, then we don’t need the -fwrapv-pointer anymore, right? 

> we don't have many places using this, ISTR kernel folks requested to
> disable specific folding - digging in history
> might reveal the case/PR.

Do you mean that this -fwrapv-pointer was introduced for kernel?

I will try to dig a little bit here.

thanks.

Qing
> 
> Richard.
> 
>> Thanks for your help.
>> 
>> Qing
>> 

Re: Question on -fwrapv and -fwrapv-pointer

[Richard Biener]

> Am 14.09.2023 um 17:01 schrieb Qing Zhao <qing.zhao@oracle.com>:
> 
> Thanks for the info.
> 
>> On Sep 14, 2023, at 10:06 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> 
>>> On Thu, Sep 14, 2023 at 3:42 PM Qing Zhao via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>> 
>>> Hi,
>>> 
>>> I have several questions on these options:
>>> 
>>> 1.are pointers treated as signed integers in general? (I thought that pointers are addresses to the memory, should be treated as unsigned integer…)
>>> 2. If Yes, why?
>>> 3. why a separate option for pointesr -fwrapv-pointer in addition to -fwrapv if they are treated as signed integers?
>> 
>> Pointers are unsigned, they might sign-extend to Pmode though.
> If they are unsigned, why they are sign-extend to Pmode? Is there any special reason for this? 

Some targets require this.  See POINTERS_EXTEND_UNSIGNED

> In another word, can we consistently treat pointers as unsigned? 

We do, but on GIMPLE it doesn’t matter.

>> -fwrapv-pointer is to enable wrapping over zero,
> 
> If we always treat pointers are unsigned, then we don’t need the -fwrapv-pointer anymore, right? 

No, the naming is just ‚bad‘

> 
>> we don't have many places using this, ISTR kernel folks requested to
>> disable specific folding - digging in history
>> might reveal the case/PR.
> 
> Do you mean that this -fwrapv-pointer was introduced for kernel?

I think it was introduced when removing the separate fstrict-overflow flag and since that covered also some pointer transforms the wraps-pointer flag was introduced.

> 
> I will try to dig a little bit here.
> 
> thanks.
> 
> Qing
>> 
>> Richard.
>> 
>>> Thanks for your help.
>>> 
>>> Qing
>>> 
> 

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 14, 2023, at 11:12 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> 
> 
>> Am 14.09.2023 um 17:01 schrieb Qing Zhao <qing.zhao@oracle.com>:
>> 
>> Thanks for the info.
>> 
>>> On Sep 14, 2023, at 10:06 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>>> On Thu, Sep 14, 2023 at 3:42 PM Qing Zhao via Gcc-patches
>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> I have several questions on these options:
>>>> 
>>>> 1.are pointers treated as signed integers in general? (I thought that pointers are addresses to the memory, should be treated as unsigned integer…)
>>>> 2. If Yes, why?
>>>> 3. why a separate option for pointesr -fwrapv-pointer in addition to -fwrapv if they are treated as signed integers?
>>> 
>>> Pointers are unsigned, they might sign-extend to Pmode though.
>> If they are unsigned, why they are sign-extend to Pmode? Is there any special reason for this? 
> 
> Some targets require this.  See POINTERS_EXTEND_UNSIGNED

Okay, I see.
> 
>> In another word, can we consistently treat pointers as unsigned? 
> 
> We do, but on GIMPLE it doesn’t matter.

Currently, GCC behaves as following:

/* True if overflow wraps around for the given integral or pointer type.  That
   is, TYPE_MAX + 1 == TYPE_MIN.  */
#define TYPE_OVERFLOW_WRAPS(TYPE) \
  (POINTER_TYPE_P (TYPE)                                        \
   ? flag_wrapv_pointer                                         \
   : (ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag  \
      || flag_wrapv))

/* True if overflow is undefined for the given integral or pointer type.
   We may optimize on the assumption that values in the type never overflow.

   IMPORTANT NOTE: Any optimization based on TYPE_OVERFLOW_UNDEFINED
   must issue a warning based on warn_strict_overflow.  In some cases
   it will be appropriate to issue the warning immediately, and in
   other cases it will be appropriate to simply set a flag and let the
   caller decide whether a warning is appropriate or not.  */
#define TYPE_OVERFLOW_UNDEFINED(TYPE)                           \
  (POINTER_TYPE_P (TYPE)                                        \
   ? !flag_wrapv_pointer                                        \
   : (!ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag \
      && !flag_wrapv && !flag_trapv))

The logic above seems treating the pointer default as signed integer, right?

Qing

> 
>>> -fwrapv-pointer is to enable wrapping over zero,
>> 
>> If we always treat pointers are unsigned, then we don’t need the -fwrapv-pointer anymore, right? 
> 
> No, the naming is just ‚bad‘
> 
>> 
>>> we don't have many places using this, ISTR kernel folks requested to
>>> disable specific folding - digging in history
>>> might reveal the case/PR.
>> 
>> Do you mean that this -fwrapv-pointer was introduced for kernel?
> 
> I think it was introduced when removing the separate fstrict-overflow flag and since that covered also some pointer transforms the wraps-pointer flag was introduced.
> 
>> 
>> I will try to dig a little bit here.
>> 
>> thanks.
>> 
>> Qing
>>> 
>>> Richard.
>>> 
>>>> Thanks for your help.
>>>> 
>>>> Qing
>>>> 
>> 

Re: Question on -fwrapv and -fwrapv-pointer

[Xi Ruoyao]

On Thu, 2023-09-14 at 15:57 +0000, Qing Zhao via Gcc-patches wrote:
> Currently, GCC behaves as following:
> 
> /* True if overflow wraps around for the given integral or pointer type.  That
>    is, TYPE_MAX + 1 == TYPE_MIN.  */
> #define TYPE_OVERFLOW_WRAPS(TYPE) \
>   (POINTER_TYPE_P (TYPE)                                        \
>    ? flag_wrapv_pointer                                         \
>    : (ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag  \
>       || flag_wrapv))
> 
> /* True if overflow is undefined for the given integral or pointer type.
>    We may optimize on the assumption that values in the type never overflow.
> 
>    IMPORTANT NOTE: Any optimization based on TYPE_OVERFLOW_UNDEFINED
>    must issue a warning based on warn_strict_overflow.  In some cases
>    it will be appropriate to issue the warning immediately, and in
>    other cases it will be appropriate to simply set a flag and let the
>    caller decide whether a warning is appropriate or not.  */
> #define TYPE_OVERFLOW_UNDEFINED(TYPE)                           \
>   (POINTER_TYPE_P (TYPE)                                        \
>    ? !flag_wrapv_pointer                                        \
>    : (!ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag \
>       && !flag_wrapv && !flag_trapv))
> 
> The logic above seems treating the pointer default as signed integer, right?

It only says the pointers cannot overflow, not the pointers are signed.

printf("%d\n", (char *)(intptr_t)-1 > (char *)(intptr_t)1);

produces 1 instead of 0.  Technically this is invoking undefined
behavior and a conforming implementation can output anything.  But
consider a 32-bit bare metal target where the linker can locate a "char
x[512]" at [0x7fffff00, 0x80000100).  The standard then requires &x[512]
> &x[0], but if we do a signed comparison here we'll end up "&x[512] <
&x[0]", this is non-conforming.

IIUC, pointers are not integers, at all.  If we treat them as integers
in the brain we'll end up invoking undefined behavior sooner or later. 
Thus the wrapping/overflowing behavior of pointer is controlled by a
different option than integers.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 14, 2023, at 12:18 PM, Xi Ruoyao <xry111@xry111.site> wrote:
> 
> On Thu, 2023-09-14 at 15:57 +0000, Qing Zhao via Gcc-patches wrote:
>> Currently, GCC behaves as following:
>> 
>> /* True if overflow wraps around for the given integral or pointer type.  That
>>    is, TYPE_MAX + 1 == TYPE_MIN.  */
>> #define TYPE_OVERFLOW_WRAPS(TYPE) \
>>   (POINTER_TYPE_P (TYPE)                                        \
>>    ? flag_wrapv_pointer                                         \
>>    : (ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag  \
>>       || flag_wrapv))
>> 
>> /* True if overflow is undefined for the given integral or pointer type.
>>    We may optimize on the assumption that values in the type never overflow.
>> 
>>    IMPORTANT NOTE: Any optimization based on TYPE_OVERFLOW_UNDEFINED
>>    must issue a warning based on warn_strict_overflow.  In some cases
>>    it will be appropriate to issue the warning immediately, and in
>>    other cases it will be appropriate to simply set a flag and let the
>>    caller decide whether a warning is appropriate or not.  */
>> #define TYPE_OVERFLOW_UNDEFINED(TYPE)                           \
>>   (POINTER_TYPE_P (TYPE)                                        \
>>    ? !flag_wrapv_pointer                                        \
>>    : (!ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag \
>>       && !flag_wrapv && !flag_trapv))
>> 
>> The logic above seems treating the pointer default as signed integer, right?
> 
> It only says the pointers cannot overflow, not the pointers are signed.
> 
> printf("%d\n", (char *)(intptr_t)-1 > (char *)(intptr_t)1);
> 
> produces 1 instead of 0.  Technically this is invoking undefined
> behavior and a conforming implementation can output anything.  But
> consider a 32-bit bare metal target where the linker can locate a "char
> x[512]" at [0x7fffff00, 0x80000100).  The standard then requires &x[512]
>> &x[0], but if we do a signed comparison here we'll end up "&x[512] <
> &x[0]", this is non-conforming.

So, are both the above examples showing that pointer based comparisons are similar as unsigned integer comparison?  -:)
Do we have examples on treating the pointer arithmetic as signed integer arithmetic? (Really curious on this….)

But anyway, if we cannot treat pointer type consistently as signed or unsigned, shall we still need to catch pointer overflow? 

Currently, In GCC, we have -fsanitize=signed-integer-overflow to catch signed integer overflow.
But we don’t have options to catch unsigned integer overflow and pointer overflow. 

Shall we add two more options to catch unsigned integer overflow and pointer overflow, like:

-fsanitize=unsigned-integer-overflow
-fsanitize=pointer-overflow

CLANG already provided -fsanitize=unsigned-integer-overflow. GCC might need to do the same.

And both Clang and GCC might also need to add -fsanitize=pointer-overflow?

> IIUC, pointers are not integers, at all.  If we treat them as integers
> in the brain we'll end up invoking undefined behavior sooner or later. 
> Thus the wrapping/overflowing behavior of pointer is controlled by a
> different option than integers.

However, the wrapping/overflowing behavior of pointers is still based on the corresponding integer(or unsigned integer) wrapping/overflowing, right? 
Do we have special pointer wrapping/overflowing?

Qing

> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Andrew Pinski]

On Thu, Sep 14, 2023 at 1:50 PM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> > On Sep 14, 2023, at 12:18 PM, Xi Ruoyao <xry111@xry111.site> wrote:
> >
> > On Thu, 2023-09-14 at 15:57 +0000, Qing Zhao via Gcc-patches wrote:
> >> Currently, GCC behaves as following:
> >>
> >> /* True if overflow wraps around for the given integral or pointer type.  That
> >>    is, TYPE_MAX + 1 == TYPE_MIN.  */
> >> #define TYPE_OVERFLOW_WRAPS(TYPE) \
> >>   (POINTER_TYPE_P (TYPE)                                        \
> >>    ? flag_wrapv_pointer                                         \
> >>    : (ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag  \
> >>       || flag_wrapv))
> >>
> >> /* True if overflow is undefined for the given integral or pointer type.
> >>    We may optimize on the assumption that values in the type never overflow.
> >>
> >>    IMPORTANT NOTE: Any optimization based on TYPE_OVERFLOW_UNDEFINED
> >>    must issue a warning based on warn_strict_overflow.  In some cases
> >>    it will be appropriate to issue the warning immediately, and in
> >>    other cases it will be appropriate to simply set a flag and let the
> >>    caller decide whether a warning is appropriate or not.  */
> >> #define TYPE_OVERFLOW_UNDEFINED(TYPE)                           \
> >>   (POINTER_TYPE_P (TYPE)                                        \
> >>    ? !flag_wrapv_pointer                                        \
> >>    : (!ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag \
> >>       && !flag_wrapv && !flag_trapv))
> >>
> >> The logic above seems treating the pointer default as signed integer, right?
> >
> > It only says the pointers cannot overflow, not the pointers are signed.
> >
> > printf("%d\n", (char *)(intptr_t)-1 > (char *)(intptr_t)1);
> >
> > produces 1 instead of 0.  Technically this is invoking undefined
> > behavior and a conforming implementation can output anything.  But
> > consider a 32-bit bare metal target where the linker can locate a "char
> > x[512]" at [0x7fffff00, 0x80000100).  The standard then requires &x[512]
> >> &x[0], but if we do a signed comparison here we'll end up "&x[512] <
> > &x[0]", this is non-conforming.
>
> So, are both the above examples showing that pointer based comparisons are similar as unsigned integer comparison?  -:)
> Do we have examples on treating the pointer arithmetic as signed integer arithmetic? (Really curious on this….)
>
> But anyway, if we cannot treat pointer type consistently as signed or unsigned, shall we still need to catch pointer overflow?
>
> Currently, In GCC, we have -fsanitize=signed-integer-overflow to catch signed integer overflow.
> But we don’t have options to catch unsigned integer overflow and pointer overflow.
>
> Shall we add two more options to catch unsigned integer overflow and pointer overflow, like:
>
> -fsanitize=unsigned-integer-overflow
> -fsanitize=pointer-overflow
>
> CLANG already provided -fsanitize=unsigned-integer-overflow. GCC might need to do the same.

NO. There is no such thing as unsigned integer overflow. That option
is badly designed and the GCC community has rejected a few times now
having that sanitizer before. It is bad form to have a sanitizer for
well defined code.

Now -fsanitize=pointer-overflow is already there for GCC which was
added in r8-2238-gc9b39a4955f56fe609ef5478 . LLVM/clang also provides
it in the same timeframe too .
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80998

Thanks,
Andrew

>
> And both Clang and GCC might also need to add -fsanitize=pointer-overflow?
>
> > IIUC, pointers are not integers, at all.  If we treat them as integers
> > in the brain we'll end up invoking undefined behavior sooner or later.
> > Thus the wrapping/overflowing behavior of pointer is controlled by a
> > different option than integers.
>
> However, the wrapping/overflowing behavior of pointers is still based on the corresponding integer(or unsigned integer) wrapping/overflowing, right?
> Do we have special pointer wrapping/overflowing?
>
> Qing
>
> >
> > --
> > Xi Ruoyao <xry111@xry111.site>
> > School of Aerospace Science and Technology, Xidian University
>

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 14, 2023, at 4:57 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> 
> On Thu, Sep 14, 2023 at 1:50 PM Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> 
>> 
>>> On Sep 14, 2023, at 12:18 PM, Xi Ruoyao <xry111@xry111.site> wrote:
>>> 
>>> On Thu, 2023-09-14 at 15:57 +0000, Qing Zhao via Gcc-patches wrote:
>>>> Currently, GCC behaves as following:
>>>> 
>>>> /* True if overflow wraps around for the given integral or pointer type.  That
>>>>   is, TYPE_MAX + 1 == TYPE_MIN.  */
>>>> #define TYPE_OVERFLOW_WRAPS(TYPE) \
>>>>  (POINTER_TYPE_P (TYPE)                                        \
>>>>   ? flag_wrapv_pointer                                         \
>>>>   : (ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag  \
>>>>      || flag_wrapv))
>>>> 
>>>> /* True if overflow is undefined for the given integral or pointer type.
>>>>   We may optimize on the assumption that values in the type never overflow.
>>>> 
>>>>   IMPORTANT NOTE: Any optimization based on TYPE_OVERFLOW_UNDEFINED
>>>>   must issue a warning based on warn_strict_overflow.  In some cases
>>>>   it will be appropriate to issue the warning immediately, and in
>>>>   other cases it will be appropriate to simply set a flag and let the
>>>>   caller decide whether a warning is appropriate or not.  */
>>>> #define TYPE_OVERFLOW_UNDEFINED(TYPE)                           \
>>>>  (POINTER_TYPE_P (TYPE)                                        \
>>>>   ? !flag_wrapv_pointer                                        \
>>>>   : (!ANY_INTEGRAL_TYPE_CHECK(TYPE)->base.u.bits.unsigned_flag \
>>>>      && !flag_wrapv && !flag_trapv))
>>>> 
>>>> The logic above seems treating the pointer default as signed integer, right?
>>> 
>>> It only says the pointers cannot overflow, not the pointers are signed.
>>> 
>>> printf("%d\n", (char *)(intptr_t)-1 > (char *)(intptr_t)1);
>>> 
>>> produces 1 instead of 0.  Technically this is invoking undefined
>>> behavior and a conforming implementation can output anything.  But
>>> consider a 32-bit bare metal target where the linker can locate a "char
>>> x[512]" at [0x7fffff00, 0x80000100).  The standard then requires &x[512]
>>>> &x[0], but if we do a signed comparison here we'll end up "&x[512] <
>>> &x[0]", this is non-conforming.
>> 
>> So, are both the above examples showing that pointer based comparisons are similar as unsigned integer comparison?  -:)
>> Do we have examples on treating the pointer arithmetic as signed integer arithmetic? (Really curious on this….)
>> 
>> But anyway, if we cannot treat pointer type consistently as signed or unsigned, shall we still need to catch pointer overflow?
>> 
>> Currently, In GCC, we have -fsanitize=signed-integer-overflow to catch signed integer overflow.
>> But we don’t have options to catch unsigned integer overflow and pointer overflow.
>> 
>> Shall we add two more options to catch unsigned integer overflow and pointer overflow, like:
>> 
>> -fsanitize=unsigned-integer-overflow
>> -fsanitize=pointer-overflow
>> 
>> CLANG already provided -fsanitize=unsigned-integer-overflow. GCC might need to do the same.
> 
> NO. There is no such thing as unsigned integer overflow. That option
> is badly designed and the GCC community has rejected a few times now
> having that sanitizer before. It is bad form to have a sanitizer for
> well defined code.

Even though unsigned integer overflow is well defined, it might be unintentional, shall we warn user about this?

> 
> Now -fsanitize=pointer-overflow is already there for GCC which was
> added in r8-2238-gc9b39a4955f56fe609ef5478 . LLVM/clang also provides
> it in the same timeframe too .
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80998

Got it. Thanks a lot for the info.

Qing
> 
> Thanks,
> Andrew
> 
>> 
>> And both Clang and GCC might also need to add -fsanitize=pointer-overflow?
>> 
>>> IIUC, pointers are not integers, at all.  If we treat them as integers
>>> in the brain we'll end up invoking undefined behavior sooner or later.
>>> Thus the wrapping/overflowing behavior of pointer is controlled by a
>>> different option than integers.
>> 
>> However, the wrapping/overflowing behavior of pointers is still based on the corresponding integer(or unsigned integer) wrapping/overflowing, right?
>> Do we have special pointer wrapping/overflowing?
>> 
>> Qing
>> 
>>> 
>>> --
>>> Xi Ruoyao <xry111@xry111.site>
>>> School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Kees Cook]

On Thu, Sep 14, 2023 at 01:57:41PM -0700, Andrew Pinski wrote:
> Now -fsanitize=pointer-overflow is already there for GCC which was
> added in r8-2238-gc9b39a4955f56fe609ef5478 . LLVM/clang also provides
> it in the same timeframe too .
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80998

Ah, thanks for the link! And checking it just now it seems like Clang's
implementation doesn't work. Fun times.

-Kees

-- 
Kees Cook

Re: Question on -fwrapv and -fwrapv-pointer

[Xi Ruoyao]

On Thu, 2023-09-14 at 21:41 +0000, Qing Zhao wrote:
> > > CLANG already provided -fsanitize=unsigned-integer-overflow. GCC
> > > might need to do the same.
> > 
> > NO. There is no such thing as unsigned integer overflow. That option
> > is badly designed and the GCC community has rejected a few times now
> > having that sanitizer before. It is bad form to have a sanitizer for
> > well defined code.
> 
> Even though unsigned integer overflow is well defined, it might be
> unintentional, shall we warn user about this?

*Everything* could be unintentional and should be warned then.  GCC is a
compiler, not an advanced AI educating the programmers.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Arsen Arsenović]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]


Qing Zhao <qing.zhao@oracle.com> writes:

> Even though unsigned integer overflow is well defined, it might be
> unintentional, shall we warn user about this?

This would be better addressed by providing operators or functions that
do overflow checking in the language, so that they can be explicitly
used where overflow is unexpected.  One could easily imagine a scenario
where overflow is not expected in some region of code but is in the
larger application.
-- 
Arsen Arsenović

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 381 bytes --]

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 15, 2023, at 3:43 AM, Xi Ruoyao <xry111@xry111.site> wrote:
> 
> On Thu, 2023-09-14 at 21:41 +0000, Qing Zhao wrote:
>>>> CLANG already provided -fsanitize=unsigned-integer-overflow. GCC
>>>> might need to do the same.
>>> 
>>> NO. There is no such thing as unsigned integer overflow. That option
>>> is badly designed and the GCC community has rejected a few times now
>>> having that sanitizer before. It is bad form to have a sanitizer for
>>> well defined code.
>> 
>> Even though unsigned integer overflow is well defined, it might be
>> unintentional, shall we warn user about this?
> 
> *Everything* could be unintentional and should be warned then.  GCC is a
> compiler, not an advanced AI educating the programmers.

Well, you are right in some sense. -:)

However, overflow is one important source for security flaws, it’s important  for compilers to detect
overflows in the programs in general. 

Qing
> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Andrew Pinski]

On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao <qing.zhao@oracle.com> wrote:
>
>
>
> > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao <xry111@xry111.site> wrote:
> >
> > On Thu, 2023-09-14 at 21:41 +0000, Qing Zhao wrote:
> >>>> CLANG already provided -fsanitize=unsigned-integer-overflow. GCC
> >>>> might need to do the same.
> >>>
> >>> NO. There is no such thing as unsigned integer overflow. That option
> >>> is badly designed and the GCC community has rejected a few times now
> >>> having that sanitizer before. It is bad form to have a sanitizer for
> >>> well defined code.
> >>
> >> Even though unsigned integer overflow is well defined, it might be
> >> unintentional, shall we warn user about this?
> >
> > *Everything* could be unintentional and should be warned then.  GCC is a
> > compiler, not an advanced AI educating the programmers.
>
> Well, you are right in some sense. -:)
>
> However, overflow is one important source for security flaws, it’s important  for compilers to detect
> overflows in the programs in general.

Except it is NOT an overflow. Rather it is wrapping. That is a big
point here. unsigned wraps and does NOT overflow. Yes there is a major
difference.

>
> Qing
> >
> > --
> > Xi Ruoyao <xry111@xry111.site>
> > School of Aerospace Science and Technology, Xidian University
>

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
> 
> 
> Qing Zhao <qing.zhao@oracle.com> writes:
> 
>> Even though unsigned integer overflow is well defined, it might be
>> unintentional, shall we warn user about this?
> 
> This would be better addressed by providing operators or functions that
> do overflow checking in the language, so that they can be explicitly
> used where overflow is unexpected.

Yes, that will be very helpful to prevent unexpected overflow in the program in general.
However, this will mainly benefit new codes. 

For the existing C codes, especially large applications, we still need to identify all the places 
Where the overflow is unexpected, and fix them. 

One good example is linux kernel. 

>  One could easily imagine a scenario
> where overflow is not expected in some region of code but is in the
> larger application.

Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and 
expected wrap-around are mixed together inside one module. 
It’s hard to detect the unexpected overflow under such situation based on the current GCC. 

Thanks.

Qing
> -- 
> Arsen Arsenović

Re: Question on -fwrapv and -fwrapv-pointer

[Richard Biener]

> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
> 
> 
> 
>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
>> 
>> 
>> Qing Zhao <qing.zhao@oracle.com> writes:
>> 
>>> Even though unsigned integer overflow is well defined, it might be
>>> unintentional, shall we warn user about this?
>> 
>> This would be better addressed by providing operators or functions that
>> do overflow checking in the language, so that they can be explicitly
>> used where overflow is unexpected.
> 
> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
> However, this will mainly benefit new codes.
> 
> For the existing C codes, especially large applications, we still need to identify all the places 
> Where the overflow is unexpected, and fix them. 
> 
> One good example is linux kernel. 
> 
>> One could easily imagine a scenario
>> where overflow is not expected in some region of code but is in the
>> larger application.
> 
> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and 
> expected wrap-around are mixed together inside one module. 
> It’s hard to detect the unexpected overflow under such situation based on the current GCC. 

But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.

Richard 

> Thanks.
> 
> Qing
>> -- 
>> Arsen Arsenović
> 

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 15, 2023, at 11:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> 
> 
>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
>> 
>> 
>> 
>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
>>> 
>>> 
>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>> 
>>>> Even though unsigned integer overflow is well defined, it might be
>>>> unintentional, shall we warn user about this?
>>> 
>>> This would be better addressed by providing operators or functions that
>>> do overflow checking in the language, so that they can be explicitly
>>> used where overflow is unexpected.
>> 
>> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
>> However, this will mainly benefit new codes.
>> 
>> For the existing C codes, especially large applications, we still need to identify all the places 
>> Where the overflow is unexpected, and fix them. 
>> 
>> One good example is linux kernel. 
>> 
>>> One could easily imagine a scenario
>>> where overflow is not expected in some region of code but is in the
>>> larger application.
>> 
>> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and 
>> expected wrap-around are mixed together inside one module. 
>> It’s hard to detect the unexpected overflow under such situation based on the current GCC. 
> 
> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.

Right, compiler cannot fix this. 
But can provide some tools to help the user to detect this more conveniently. 

Right now, GCC provides two set of options for different types:

 A. Turn the overflow to expected wrap-around (remove UB);
 B. Detect overflow;

			A				B
	  	 remove UB		-fsanitize=…
signed	   -fwrapv			signed-integer-overflow
pointer	   -fwrapv-pointer	pointer-overflow (broken in Clang)

However, Options in A and B excluded with each other. They cannot mix together for a single file.

What’s requested from Kernel is:

compiler needs to provide a functionality that can mix these two together for a file. 

i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part of the program.  And then add -fsnaitize=*overflow to detect all other
Unexpected overflows in the program.

This is currently missing from GCC, I guess?

Qing





> 
> Richard 
> 
>> Thanks.
>> 
>> Qing
>>> -- 
>>> Arsen Arsenović

Re: Question on -fwrapv and -fwrapv-pointer

[Xi Ruoyao]

On Fri, 2023-09-15 at 15:37 +0000, Qing Zhao wrote:
> 
> 
> > On Sep 15, 2023, at 11:29 AM, Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > 
> > 
> > 
> > > Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
> > > 
> > > 
> > > 
> > > > On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me>
> > > > wrote:
> > > > 
> > > > 
> > > > Qing Zhao <qing.zhao@oracle.com> writes:
> > > > 
> > > > > Even though unsigned integer overflow is well defined, it
> > > > > might be
> > > > > unintentional, shall we warn user about this?
> > > > 
> > > > This would be better addressed by providing operators or
> > > > functions that
> > > > do overflow checking in the language, so that they can be
> > > > explicitly
> > > > used where overflow is unexpected.
> > > 
> > > Yes, that will be very helpful to prevent unexpected overflow in
> > > the program in general.
> > > However, this will mainly benefit new codes.
> > > 
> > > For the existing C codes, especially large applications, we still
> > > need to identify all the places 
> > > Where the overflow is unexpected, and fix them. 
> > > 
> > > One good example is linux kernel. 
> > > 
> > > > One could easily imagine a scenario
> > > > where overflow is not expected in some region of code but is in
> > > > the
> > > > larger application.
> > > 
> > > Yes, that’s exactly the same situation Linux kernel faces now, the
> > > unexpected Overflow and 
> > > expected wrap-around are mixed together inside one module. 
> > > It’s hard to detect the unexpected overflow under such situation
> > > based on the current GCC. 
> > 
> > But that’s hardly GCCs fault nor can GCC fix that in any way.  Only
> > the programmer can distinguish both cases.
> 
> Right, compiler cannot fix this. 
> But can provide some tools to help the user to detect this more
> conveniently. 
> 
> Right now, GCC provides two set of options for different types:
> 
>  A. Turn the overflow to expected wrap-around (remove UB);
>  B. Detect overflow;
> 
>                         A                               B
>                  remove UB              -fsanitize=…
> signed     -fwrapv                      signed-integer-overflow
> pointer    -fwrapv-pointer      pointer-overflow (broken in Clang)
> 
> However, Options in A and B excluded with each other. They cannot mix
> together for a single file.
> 
> What’s requested from Kernel is:
> 
> compiler needs to provide a functionality that can mix these two
> together for a file. 
> 
> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part
> of the program.  And then add -fsnaitize=*overflow to detect all other
> Unexpected overflows in the program.
> 
> This is currently missing from GCC, I guess?

If overflow is really so rare, we should just enable -fsanitize=signed-
integer-overflow globally and special case the code paths where we want
wrapping.  It's easy in 2023:

/* b + c may wrap here because ... ... */
ckd_add(&a, b, c);

Or

/* if b + c overflows, we have a severe issue, let's panic even if
   sanitizer disabled */
if (chk_add(&a, b, c))
  panic("b + c overflows but it shouldn't (b = %d, c = %d)", b, c);

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Richard Biener]

> Am 15.09.2023 um 17:37 schrieb Qing Zhao <qing.zhao@oracle.com>:
> 
> 
> 
>>> On Sep 15, 2023, at 11:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>> 
>>> 
>>>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
>>> 
>>> 
>>> 
>>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
>>>> 
>>>> 
>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>> 
>>>>> Even though unsigned integer overflow is well defined, it might be
>>>>> unintentional, shall we warn user about this?
>>>> 
>>>> This would be better addressed by providing operators or functions that
>>>> do overflow checking in the language, so that they can be explicitly
>>>> used where overflow is unexpected.
>>> 
>>> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
>>> However, this will mainly benefit new codes.
>>> 
>>> For the existing C codes, especially large applications, we still need to identify all the places 
>>> Where the overflow is unexpected, and fix them. 
>>> 
>>> One good example is linux kernel. 
>>> 
>>>> One could easily imagine a scenario
>>>> where overflow is not expected in some region of code but is in the
>>>> larger application.
>>> 
>>> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and 
>>> expected wrap-around are mixed together inside one module. 
>>> It’s hard to detect the unexpected overflow under such situation based on the current GCC. 
>> 
>> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.
> 
> Right, compiler cannot fix this. 
> But can provide some tools to help the user to detect this more conveniently. 
> 
> Right now, GCC provides two set of options for different types:
> 
> A. Turn the overflow to expected wrap-around (remove UB);
> B. Detect overflow;
> 
>            A                B
>           remove UB        -fsanitize=…
> signed       -fwrapv            signed-integer-overflow
> pointer       -fwrapv-pointer    pointer-overflow (broken in Clang)
> 
> However, Options in A and B excluded with each other. They cannot mix together for a single file.
> 
> What’s requested from Kernel is:
> 
> compiler needs to provide a functionality that can mix these two together for a file. 
> 
> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part of the program.  And then add -fsnaitize=*overflow to detect all other
> Unexpected overflows in the program.
> 
> This is currently missing from GCC, I guess?

How can GCC know which part of the program wants wrapping and which sanitizing?

Richard 

> Qing
> 
> 
> 
> 
> 
>> 
>> Richard 
>> 
>>> Thanks.
>>> 
>>> Qing
>>>> -- 
>>>> Arsen Arsenović
> 

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 15, 2023, at 1:26 PM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> 
> 
>> Am 15.09.2023 um 17:37 schrieb Qing Zhao <qing.zhao@oracle.com>:
>> 
>> 
>> 
>>>> On Sep 15, 2023, at 11:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>>> 
>>>> 
>>>> 
>>>>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
>>>> 
>>>> 
>>>> 
>>>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
>>>>> 
>>>>> 
>>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>>> 
>>>>>> Even though unsigned integer overflow is well defined, it might be
>>>>>> unintentional, shall we warn user about this?
>>>>> 
>>>>> This would be better addressed by providing operators or functions that
>>>>> do overflow checking in the language, so that they can be explicitly
>>>>> used where overflow is unexpected.
>>>> 
>>>> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
>>>> However, this will mainly benefit new codes.
>>>> 
>>>> For the existing C codes, especially large applications, we still need to identify all the places 
>>>> Where the overflow is unexpected, and fix them. 
>>>> 
>>>> One good example is linux kernel. 
>>>> 
>>>>> One could easily imagine a scenario
>>>>> where overflow is not expected in some region of code but is in the
>>>>> larger application.
>>>> 
>>>> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and 
>>>> expected wrap-around are mixed together inside one module. 
>>>> It’s hard to detect the unexpected overflow under such situation based on the current GCC. 
>>> 
>>> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.
>> 
>> Right, compiler cannot fix this. 
>> But can provide some tools to help the user to detect this more conveniently. 
>> 
>> Right now, GCC provides two set of options for different types:
>> 
>> A. Turn the overflow to expected wrap-around (remove UB);
>> B. Detect overflow;
>> 
>>           A                B
>>          remove UB        -fsanitize=…
>> signed       -fwrapv            signed-integer-overflow
>> pointer       -fwrapv-pointer    pointer-overflow (broken in Clang)
>> 
>> However, Options in A and B excluded with each other. They cannot mix together for a single file.
>> 
>> What’s requested from Kernel is:
>> 
>> compiler needs to provide a functionality that can mix these two together for a file. 
>> 
>> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part of the program.  And then add -fsnaitize=*overflow to detect all other
>> Unexpected overflows in the program.
>> 
>> This is currently missing from GCC, I guess?
> 
> How can GCC know which part of the program wants wrapping and which sanitizing?

GCC doesn’t know, but the user knows. 

Then just provide the user a way to mark part of the program to be wrapping around and excluded from sanitizing? 

Currently, GCC provides 

__attribute__(optimize ("wrapv"))  

To mark the specific function to be wrapped around.

However, this attribute does not work for linux kernel due to the following reason:

Attribute optimize should be only used for debugging purpose;
The kernel has banned its usage;

So, a new attribute was requested from Linux kernel security: 

 request wrap-around behavior for specific function (PR102317)
__attribute__((wrapv)) 

Is this request reasonable?

Qing

> Richard 
> 
>> Qing
>> 
>> 
>> 
>> 
>> 
>>> 
>>> Richard 
>>> 
>>>> Thanks.
>>>> 
>>>> Qing
>>>>> -- 
>>>>> Arsen Arsenović

Re: Question on -fwrapv and -fwrapv-pointer

[Qing Zhao]

> On Sep 15, 2023, at 12:53 PM, Xi Ruoyao <xry111@xry111.site> wrote:
> 
> On Fri, 2023-09-15 at 15:37 +0000, Qing Zhao wrote:
>> 
>> 
>>> On Sep 15, 2023, at 11:29 AM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>> 
>>> 
>>> 
>>>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
>>>> 
>>>> 
>>>> 
>>>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me>
>>>>> wrote:
>>>>> 
>>>>> 
>>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>>> 
>>>>>> Even though unsigned integer overflow is well defined, it
>>>>>> might be
>>>>>> unintentional, shall we warn user about this?
>>>>> 
>>>>> This would be better addressed by providing operators or
>>>>> functions that
>>>>> do overflow checking in the language, so that they can be
>>>>> explicitly
>>>>> used where overflow is unexpected.
>>>> 
>>>> Yes, that will be very helpful to prevent unexpected overflow in
>>>> the program in general.
>>>> However, this will mainly benefit new codes.
>>>> 
>>>> For the existing C codes, especially large applications, we still
>>>> need to identify all the places 
>>>> Where the overflow is unexpected, and fix them. 
>>>> 
>>>> One good example is linux kernel. 
>>>> 
>>>>> One could easily imagine a scenario
>>>>> where overflow is not expected in some region of code but is in
>>>>> the
>>>>> larger application.
>>>> 
>>>> Yes, that’s exactly the same situation Linux kernel faces now, the
>>>> unexpected Overflow and 
>>>> expected wrap-around are mixed together inside one module. 
>>>> It’s hard to detect the unexpected overflow under such situation
>>>> based on the current GCC. 
>>> 
>>> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only
>>> the programmer can distinguish both cases.
>> 
>> Right, compiler cannot fix this. 
>> But can provide some tools to help the user to detect this more
>> conveniently. 
>> 
>> Right now, GCC provides two set of options for different types:
>> 
>>  A. Turn the overflow to expected wrap-around (remove UB);
>>  B. Detect overflow;
>> 
>>                         A                               B
>>                  remove UB              -fsanitize=…
>> signed     -fwrapv                      signed-integer-overflow
>> pointer    -fwrapv-pointer      pointer-overflow (broken in Clang)
>> 
>> However, Options in A and B excluded with each other. They cannot mix
>> together for a single file.
>> 
>> What’s requested from Kernel is:
>> 
>> compiler needs to provide a functionality that can mix these two
>> together for a file. 
>> 
>> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part
>> of the program.  And then add -fsnaitize=*overflow to detect all other
>> Unexpected overflows in the program.
>> 
>> This is currently missing from GCC, I guess?
> 
> If overflow is really so rare, we should just enable -fsanitize=signed-
> integer-overflow globally and special case the code paths where we want
> wrapping.  It's easy in 2023:
> 
> /* b + c may wrap here because ... ... */
> ckd_add(&a, b, c);
> 
> Or
> 
> /* if b + c overflows, we have a severe issue, let's panic even if
>   sanitizer disabled */
> if (chk_add(&a, b, c))
>  panic("b + c overflows but it shouldn't (b = %d, c = %d)", b, c);

Yes, this might be exactly the situation that Linux kernel faces and asks for help:

In some part of the program, it tried to detect the overflow itself and handled specifically,  therefore, 
It want this part of the program to be excluded from automagical overflow detection applied by 
the -fsanitize=signed-integer-overflow. 

Please see my reply to Richard’s email for more info on this.

Thanks.

Qing

> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Re: Question on -fwrapv and -fwrapv-pointer

[Kees Cook]

On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote:
> On Fri, Sep 15, 2023 at 8:12 AM Qing Zhao <qing.zhao@oracle.com> wrote:
> >
> >
> >
> > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao <xry111@xry111.site> wrote:
> > >
> > > On Thu, 2023-09-14 at 21:41 +0000, Qing Zhao wrote:
> > >>>> CLANG already provided -fsanitize=unsigned-integer-overflow. GCC
> > >>>> might need to do the same.
> > >>>
> > >>> NO. There is no such thing as unsigned integer overflow. That option
> > >>> is badly designed and the GCC community has rejected a few times now
> > >>> having that sanitizer before. It is bad form to have a sanitizer for
> > >>> well defined code.
> > >>
> > >> Even though unsigned integer overflow is well defined, it might be
> > >> unintentional, shall we warn user about this?
> > >
> > > *Everything* could be unintentional and should be warned then.  GCC is a
> > > compiler, not an advanced AI educating the programmers.
> >
> > Well, you are right in some sense. -:)
> >
> > However, overflow is one important source for security flaws, it’s important  for compilers to detect
> > overflows in the programs in general.
> 
> Except it is NOT an overflow. Rather it is wrapping. That is a big
> point here. unsigned wraps and does NOT overflow. Yes there is a major
> difference.

Right, yes. I will try to pick my language very carefully. :)

The practical problem I am trying to solve in the 30 million lines of
Linux kernel code is that of catching arithmetic wrap-around. The
problem is one of evolving the code -- I can't just drop -fwrapv and
-fwrapv-pointer because it's not possible to fix all the cases at once.
(And we really don't want to reintroduce undefined behavior.)

So, for signed, pointer, and unsigned types, we need:

a) No arithmetic UB -- everything needs to have deterministic behavior.
   The current solution here is "-fno-strict-overflow", which eliminates
   the UB and makes sure everything wraps.

b) A way to run-time warn/trap on overflow/underflow/wrap-around. This
   would work with -fsanitize=[signed-integer|pointer]-overflow except
   due to "a)" we always wrap. And there isn't currently coverage like
   this for unsigned (in GCC).

Our problem is that the kernel is filled with a mix of places where there
is intended wrap-around and unintended wrap-around. We can chip away at
fixing the intended wrap-around that we can find with static analyzers,
etc, but at the end of the day there is a long tail of finding the places
where intended wrap-around is hiding. But when the refactoring is
sufficiently completely, we can move the wrap-around warning to a trap,
and the kernel will not longer have this class of security flaw.

As a real-world example, here is a bug where a u8 wraps around causing
an under-allocation that allowed for a heap overwrite:

https://git.kernel.org/linus/6311071a0562
https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L5422

If there were more than 255 elements in a linked list, the allocation
would be too small, and the second loop would write past the end of the
allocation. This is a pretty classic allocation underflow and linear
heap write overflow security flaw. (And it would be trivially stopped by
trapping on the u8 wrap around.)

So, I want to be able to catch that at run-time. But we also have code
doing things like "if (ulong + offset < ulong) { ... }":

https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec6_crypto.c#L1187

This is easy for a static analyzer to find and we can replace it with a
non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find
them all immediately, especially for the signed and pointer cases.

So, I need to retain the "everything wraps" behavior while still being
able to detect when it happens.

-- 
Kees Cook

Re: Question on -fwrapv and -fwrapv-pointer

[Kees Cook]

On Fri, Sep 15, 2023 at 05:47:08PM +0000, Qing Zhao wrote:
> 
> 
> > On Sep 15, 2023, at 1:26 PM, Richard Biener <richard.guenther@gmail.com> wrote:
> > 
> > 
> > 
> >> Am 15.09.2023 um 17:37 schrieb Qing Zhao <qing.zhao@oracle.com>:
> >> 
> >> 
> >> 
> >>>> On Sep 15, 2023, at 11:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >>>> 
> >>>> 
> >>>> 
> >>>>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
> >>>> 
> >>>> 
> >>>> 
> >>>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
> >>>>> 
> >>>>> 
> >>>>> Qing Zhao <qing.zhao@oracle.com> writes:
> >>>>> 
> >>>>>> Even though unsigned integer overflow is well defined, it might be
> >>>>>> unintentional, shall we warn user about this?
> >>>>> 
> >>>>> This would be better addressed by providing operators or functions that
> >>>>> do overflow checking in the language, so that they can be explicitly
> >>>>> used where overflow is unexpected.
> >>>> 
> >>>> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
> >>>> However, this will mainly benefit new codes.
> >>>> 
> >>>> For the existing C codes, especially large applications, we still need to identify all the places 
> >>>> Where the overflow is unexpected, and fix them. 
> >>>> 
> >>>> One good example is linux kernel. 
> >>>> 
> >>>>> One could easily imagine a scenario
> >>>>> where overflow is not expected in some region of code but is in the
> >>>>> larger application.
> >>>> 
> >>>> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and 
> >>>> expected wrap-around are mixed together inside one module. 
> >>>> It’s hard to detect the unexpected overflow under such situation based on the current GCC. 
> >>> 
> >>> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.
> >> 
> >> Right, compiler cannot fix this. 
> >> But can provide some tools to help the user to detect this more conveniently. 
> >> 
> >> Right now, GCC provides two set of options for different types:
> >> 
> >> A. Turn the overflow to expected wrap-around (remove UB);
> >> B. Detect overflow;
> >> 
> >>           A                B
> >>          remove UB        -fsanitize=…
> >> signed       -fwrapv            signed-integer-overflow
> >> pointer       -fwrapv-pointer    pointer-overflow (broken in Clang)
> >> 
> >> However, Options in A and B excluded with each other. They cannot mix together for a single file.
> >> 
> >> What’s requested from Kernel is:
> >> 
> >> compiler needs to provide a functionality that can mix these two together for a file. 
> >> 
> >> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part of the program.  And then add -fsnaitize=*overflow to detect all other
> >> Unexpected overflows in the program.
> >> 
> >> This is currently missing from GCC, I guess?
> > 
> > How can GCC know which part of the program wants wrapping and which sanitizing?
> 
> GCC doesn’t know, but the user knows. 
> 
> Then just provide the user a way to mark part of the program to be wrapping around and excluded from sanitizing? 
> 
> Currently, GCC provides 
> 
> __attribute__(optimize ("wrapv"))  
> 
> To mark the specific function to be wrapped around.
> 
> However, this attribute does not work for linux kernel due to the following reason:
> 
> Attribute optimize should be only used for debugging purpose;
> The kernel has banned its usage;
> 
> So, a new attribute was requested from Linux kernel security: 
> 
>  request wrap-around behavior for specific function (PR102317)
> __attribute__((wrapv)) 
> 
> Is this request reasonable?

After working through this discussion, I'd say it's likely more helpful
to have a way to disable the sanitizers for a given function (or
variable). i.e. The goal for the kernel would that untrapped wrap-around
would be the very rare exception. e.g. our refcount_t implementation:
https://elixir.bootlin.com/linux/v6.5/source/include/linux/refcount.h#L200

Then we can continue to build the kernel with -fno-strict-overflow (to
avoid UB), but gain sanitizer coverage for all run-time wraps, except
for the very few places where we depend on it. Getting there will also
take some non-trivial refactoring on our end, but without the sanitizers
we're unlikely to find them all.

-- 
Kees Cook

Re: Question on -fwrapv and -fwrapv-pointer

[Fangrui Song]

On Fri, Sep 15, 2023 at 11:43 AM Kees Cook via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Fri, Sep 15, 2023 at 05:47:08PM +0000, Qing Zhao wrote:
> >
> >
> > > On Sep 15, 2023, at 1:26 PM, Richard Biener <richard.guenther@gmail.com> wrote:
> > >
> > >
> > >
> > >> Am 15.09.2023 um 17:37 schrieb Qing Zhao <qing.zhao@oracle.com>:
> > >>
> > >> 
> > >>
> > >>>> On Sep 15, 2023, at 11:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> > >>>>
> > >>>>
> > >>>>
> > >>>>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
> > >>>>
> > >>>> 
> > >>>>
> > >>>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
> > >>>>>
> > >>>>>
> > >>>>> Qing Zhao <qing.zhao@oracle.com> writes:
> > >>>>>
> > >>>>>> Even though unsigned integer overflow is well defined, it might be
> > >>>>>> unintentional, shall we warn user about this?
> > >>>>>
> > >>>>> This would be better addressed by providing operators or functions that
> > >>>>> do overflow checking in the language, so that they can be explicitly
> > >>>>> used where overflow is unexpected.
> > >>>>
> > >>>> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
> > >>>> However, this will mainly benefit new codes.
> > >>>>
> > >>>> For the existing C codes, especially large applications, we still need to identify all the places
> > >>>> Where the overflow is unexpected, and fix them.
> > >>>>
> > >>>> One good example is linux kernel.
> > >>>>
> > >>>>> One could easily imagine a scenario
> > >>>>> where overflow is not expected in some region of code but is in the
> > >>>>> larger application.
> > >>>>
> > >>>> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and
> > >>>> expected wrap-around are mixed together inside one module.
> > >>>> It’s hard to detect the unexpected overflow under such situation based on the current GCC.
> > >>>
> > >>> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.
> > >>
> > >> Right, compiler cannot fix this.
> > >> But can provide some tools to help the user to detect this more conveniently.
> > >>
> > >> Right now, GCC provides two set of options for different types:
> > >>
> > >> A. Turn the overflow to expected wrap-around (remove UB);
> > >> B. Detect overflow;
> > >>
> > >>           A                B
> > >>          remove UB        -fsanitize=…
> > >> signed       -fwrapv            signed-integer-overflow
> > >> pointer       -fwrapv-pointer    pointer-overflow (broken in Clang)
> > >>
> > >> However, Options in A and B excluded with each other. They cannot mix together for a single file.
> > >>
> > >> What’s requested from Kernel is:
> > >>
> > >> compiler needs to provide a functionality that can mix these two together for a file.
> > >>
> > >> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part of the program.  And then add -fsnaitize=*overflow to detect all other
> > >> Unexpected overflows in the program.

Yes, I believe combining A and B should be allowed.

> > >> This is currently missing from GCC, I guess?
> > >
> > > How can GCC know which part of the program wants wrapping and which sanitizing?
> >
> > GCC doesn’t know, but the user knows.
> >
> > Then just provide the user a way to mark part of the program to be wrapping around and excluded from sanitizing?
> >
> > Currently, GCC provides
> >
> > __attribute__(optimize ("wrapv"))
> >
> > To mark the specific function to be wrapped around.
> >
> > However, this attribute does not work for linux kernel due to the following reason:
> >
> > Attribute optimize should be only used for debugging purpose;
> > The kernel has banned its usage;
> >
> > So, a new attribute was requested from Linux kernel security:
> >
> >  request wrap-around behavior for specific function (PR102317)
> > __attribute__((wrapv))
> >
> > Is this request reasonable?
>
> After working through this discussion, I'd say it's likely more helpful
> to have a way to disable the sanitizers for a given function (or
> variable). i.e. The goal for the kernel would that untrapped wrap-around
> would be the very rare exception. e.g. our refcount_t implementation:
> https://elixir.bootlin.com/linux/v6.5/source/include/linux/refcount.h#L200
>
> Then we can continue to build the kernel with -fno-strict-overflow (to
> avoid UB), but gain sanitizer coverage for all run-time wraps, except
> for the very few places where we depend on it. Getting there will also
> take some non-trivial refactoring on our end, but without the sanitizers
> we're unlikely to find them all.
>
> --
> Kees Cook

I see a Clang patch that proposes -fsanitize=signed-integer-wrap,
which appears to be the same as signed-integer-overflow, but performs
the check in the -fwrapv mode.
I made a reply to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317#c13 that we
probably should just make -fsanitize=signed-integer-overflow work with
-fwrapv.

(This message is made so that interested folks are informed of the
Clang change.)


-- 
宋方睿

Re: Question on -fwrapv and -fwrapv-pointer

[Kees Cook]

On Thu, Feb 15, 2024 at 12:32:17AM -0800, Fangrui Song wrote:
> On Fri, Sep 15, 2023 at 11:43 AM Kees Cook via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Fri, Sep 15, 2023 at 05:47:08PM +0000, Qing Zhao wrote:
> > >
> > >
> > > > On Sep 15, 2023, at 1:26 PM, Richard Biener <richard.guenther@gmail.com> wrote:
> > > >
> > > >
> > > >
> > > >> Am 15.09.2023 um 17:37 schrieb Qing Zhao <qing.zhao@oracle.com>:
> > > >>
> > > >> 
> > > >>
> > > >>>> On Sep 15, 2023, at 11:29 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>> Am 15.09.2023 um 17:25 schrieb Qing Zhao <qing.zhao@oracle.com>:
> > > >>>>
> > > >>>> 
> > > >>>>
> > > >>>>> On Sep 15, 2023, at 8:41 AM, Arsen Arsenović <arsen@aarsen.me> wrote:
> > > >>>>>
> > > >>>>>
> > > >>>>> Qing Zhao <qing.zhao@oracle.com> writes:
> > > >>>>>
> > > >>>>>> Even though unsigned integer overflow is well defined, it might be
> > > >>>>>> unintentional, shall we warn user about this?
> > > >>>>>
> > > >>>>> This would be better addressed by providing operators or functions that
> > > >>>>> do overflow checking in the language, so that they can be explicitly
> > > >>>>> used where overflow is unexpected.
> > > >>>>
> > > >>>> Yes, that will be very helpful to prevent unexpected overflow in the program in general.
> > > >>>> However, this will mainly benefit new codes.
> > > >>>>
> > > >>>> For the existing C codes, especially large applications, we still need to identify all the places
> > > >>>> Where the overflow is unexpected, and fix them.
> > > >>>>
> > > >>>> One good example is linux kernel.
> > > >>>>
> > > >>>>> One could easily imagine a scenario
> > > >>>>> where overflow is not expected in some region of code but is in the
> > > >>>>> larger application.
> > > >>>>
> > > >>>> Yes, that’s exactly the same situation Linux kernel faces now, the unexpected Overflow and
> > > >>>> expected wrap-around are mixed together inside one module.
> > > >>>> It’s hard to detect the unexpected overflow under such situation based on the current GCC.
> > > >>>
> > > >>> But that’s hardly GCCs fault nor can GCC fix that in any way.  Only the programmer can distinguish both cases.
> > > >>
> > > >> Right, compiler cannot fix this.
> > > >> But can provide some tools to help the user to detect this more conveniently.
> > > >>
> > > >> Right now, GCC provides two set of options for different types:
> > > >>
> > > >> A. Turn the overflow to expected wrap-around (remove UB);
> > > >> B. Detect overflow;
> > > >>
> > > >>           A                B
> > > >>          remove UB        -fsanitize=…
> > > >> signed       -fwrapv            signed-integer-overflow
> > > >> pointer       -fwrapv-pointer    pointer-overflow (broken in Clang)
> > > >>
> > > >> However, Options in A and B excluded with each other. They cannot mix together for a single file.
> > > >>
> > > >> What’s requested from Kernel is:
> > > >>
> > > >> compiler needs to provide a functionality that can mix these two together for a file.
> > > >>
> > > >> i.e, apply A (convert UB to defined behavior WRAP-AROUND) only to part of the program.  And then add -fsnaitize=*overflow to detect all other
> > > >> Unexpected overflows in the program.
> 
> Yes, I believe combining A and B should be allowed.
> 
> > > >> This is currently missing from GCC, I guess?
> > > >
> > > > How can GCC know which part of the program wants wrapping and which sanitizing?
> > >
> > > GCC doesn’t know, but the user knows.
> > >
> > > Then just provide the user a way to mark part of the program to be wrapping around and excluded from sanitizing?
> > >
> > > Currently, GCC provides
> > >
> > > __attribute__(optimize ("wrapv"))
> > >
> > > To mark the specific function to be wrapped around.
> > >
> > > However, this attribute does not work for linux kernel due to the following reason:
> > >
> > > Attribute optimize should be only used for debugging purpose;
> > > The kernel has banned its usage;
> > >
> > > So, a new attribute was requested from Linux kernel security:
> > >
> > >  request wrap-around behavior for specific function (PR102317)
> > > __attribute__((wrapv))
> > >
> > > Is this request reasonable?
> >
> > After working through this discussion, I'd say it's likely more helpful
> > to have a way to disable the sanitizers for a given function (or
> > variable). i.e. The goal for the kernel would that untrapped wrap-around
> > would be the very rare exception. e.g. our refcount_t implementation:
> > https://elixir.bootlin.com/linux/v6.5/source/include/linux/refcount.h#L200
> >
> > Then we can continue to build the kernel with -fno-strict-overflow (to
> > avoid UB), but gain sanitizer coverage for all run-time wraps, except
> > for the very few places where we depend on it. Getting there will also
> > take some non-trivial refactoring on our end, but without the sanitizers
> > we're unlikely to find them all.
> >
> > --
> > Kees Cook
> 
> I see a Clang patch that proposes -fsanitize=signed-integer-wrap,
> which appears to be the same as signed-integer-overflow, but performs
> the check in the -fwrapv mode.
> I made a reply to
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317#c13 that we
> probably should just make -fsanitize=signed-integer-overflow work with
> -fwrapv.
> 
> (This message is made so that interested folks are informed of the
> Clang change.)

FWIW, I am fine with either -fsanitize=signed-integer-overflow working
with -fwrapv or creating the -wrap version of the sanitizer. I would
just like to get unblocked on the naming. :) If someone thinks there is
value in keep the UB-only sanitizer mode, then we need the -wrap mode.
Otherwise, let's get it working even without UB.

-Kees

-- 
Kees Cook