dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

gcc ubsan reports pointer overflow in bitsetasshortlong perhaps a false positive #73964

Open RobertHenry6bev opened 2 years ago

RobertHenry6bev commented 2 years ago

Description

ubuntu linux x64 all gcc's report a runtime error with https://github.com/dotnet/runtime/blob/a85b7826d6b7d120c827b5350a962b13b095a8b8/src/coreclr/jit/bitsetasshortlong.h#L492

When ubsan complains The bs is a reference to a pointer; the pointer has value 0xff*fYULL. Adding 1 to the pointer adds sizeof (uint64)_t) to the bit value which wraps around in 64 bit integer, resulting in a new pointer with value 0x7ULL assigned to m_bsEnd.

It seems that this is OK because a while later in the code ++m_bs; is used, which will wrap in presumably the same way, and so compare == with m_bsEnd.

The code understandably works hard to save storage. Perhaps the code could be cleaned up using unions, and that would also silence ubsan.

Reproduction Steps

Compile gcc -fsanitize=undefined and run

Expected behavior

no runtime errors

Actual behavior

runtime errors from ubsan about pointer arithmetic overflowing and wrapping aound.

Regression?

No response

Known Workarounds

One can dynamically or statically suppress the ubsan error in a variety of ways

Configuration

No response

Other information

/cc @BruceForstall

category:implementation theme:bitset skill-level:beginner cost:small impact:small

dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost commented 2 years ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

Issue Details
### Description ubuntu linux x64 all gcc's report a runtime error with https://github.com/dotnet/runtime/blob/a85b7826d6b7d120c827b5350a962b13b095a8b8/src/coreclr/jit/bitsetasshortlong.h#L492 When ubsan complains The bs is a reference to a pointer; the pointer has value 0xff*fYULL. Adding 1 to the pointer adds sizeof (uint64)_t) to the bit value which wraps around in 64 bit integer, resulting in a new pointer with value 0x7ULL assigned to m_bsEnd. It seems that this is OK because a while later in the code ++m_bs; is used, which will wrap in presumably the same way, and so compare == with m_bsEnd. The code understandably works hard to save storage. Perhaps the code could be cleaned up using unions, and that would also silence ubsan. ### Reproduction Steps Compile gcc -fsanitize=undefined and run ### Expected behavior no runtime errors ### Actual behavior runtime errors from ubsan about pointer arithmetic overflowing and wrapping aound. ### Regression? _No response_ ### Known Workarounds One can dynamically or statically suppress the ubsan error in a variety of ways ### Configuration _No response_ ### Other information /cc @bruce
Author: RobertHenry6bev
Assignees: BruceForstall
Labels: `area-CodeGen-coreclr`
Milestone: -
BruceForstall commented 2 years ago

A simple fix would be to initialize m_bs=nullptr in the short case (before setting m_bsEnd) so the tool doesn't get upset about rounding.

BruceForstall commented 2 years ago

(Fixed reference to wrong Bruce)