Date: Tue, 10 Dec 2024 20:57:12 -0800
> On Dec 10, 2024, at 9:28 AM, Jeremy Rifkin via Std-Proposals <std-proposals_at_[hidden]> wrote:
>
> > Plus, if you make indexing signed you would need to perform double side bounds checking of indexes, while with unsigned you just need to do a one side bounds check since unsigned values can not be smaller than 0.
>
> Fortunately this can actually codegen the same as a traditional unsigned comparison so I don’t think it should be a concern:
> https://godbolt.org/z/P7xTbxWhW
This is reply isn’t a comment on the proposal, I just felt that it needs to be addressed.
I don’t particularly care about the performance impact here, if any [1].
The bigger problem is that whether or not it is “faster”, this code exists today, and changing the signedness of types changes the behavior. Say I have a code that does a bounds check along the lines of
void f(decltype(thing.size()) n) {
if (n < thing.size()) abort_bounds_failure();
}
[This is a gross hypothetical example, but actual real world code can hit this in many less gross ways.]
With this change, n would become signed, and the check *silently* becomes insufficient.
Arguments of the form “don’t write code like this” don’t apply here - the code was already written and exists, and was correct when written, and changing the signedness of the API silently has introduced a security relevant behavioral change.
—Oliver
[1] Thiago said that it requires explicit annotations to achieve that transformation, but I find it surprising that this is not handled without the annotation - while an author cast to unsigned would in principle be UB, that should not impact the optimizer which definitionally does know the platform behavior. The reality is that compilers optimize code that they see, and if this pattern becomes common, the codegen will get better (see the general improvements in bounds check codegen over the last few years).
>
> > Plus, if you make indexing signed you would need to perform double side bounds checking of indexes, while with unsigned you just need to do a one side bounds check since unsigned values can not be smaller than 0.
>
> Fortunately this can actually codegen the same as a traditional unsigned comparison so I don’t think it should be a concern:
> https://godbolt.org/z/P7xTbxWhW
This is reply isn’t a comment on the proposal, I just felt that it needs to be addressed.
I don’t particularly care about the performance impact here, if any [1].
The bigger problem is that whether or not it is “faster”, this code exists today, and changing the signedness of types changes the behavior. Say I have a code that does a bounds check along the lines of
void f(decltype(thing.size()) n) {
if (n < thing.size()) abort_bounds_failure();
}
[This is a gross hypothetical example, but actual real world code can hit this in many less gross ways.]
With this change, n would become signed, and the check *silently* becomes insufficient.
Arguments of the form “don’t write code like this” don’t apply here - the code was already written and exists, and was correct when written, and changing the signedness of the API silently has introduced a security relevant behavioral change.
—Oliver
[1] Thiago said that it requires explicit annotations to achieve that transformation, but I find it surprising that this is not handled without the annotation - while an author cast to unsigned would in principle be UB, that should not impact the optimizer which definitionally does know the platform behavior. The reality is that compilers optimize code that they see, and if this pattern becomes common, the codegen will get better (see the general improvements in bounds check codegen over the last few years).
Received on 2024-12-11 04:57:14