Upgrading from (very) old unknown version of SafeInt to SafeInt 3.0.28p produces unexpected C2100 compilation failures with MSVC

[jacobl-at-ms]

Minimum repro case for the issue with Visual Studio 2022 was:

#include "SafeInt.hpp"

int main()
{
    char characters[7] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g' };

    char* ptr = &characters[0];
    SafeInt<int> x{ 4 };
#if USED_TO_WORK
    *(ptr + x) = 0; // produces C2100 illegal indirection compiler diagnostic
#else
    char* end = ptr;
    end += x;
    *(end) = 0;
#endif

    return 0;
}

I can see this being either an unexpected regression or intentional and I'm getting a bit of push-back about inserting casts or refactoring the code to use operator+= instead. Filing an issue for discussion/visibility.

[jacobl-at-ms]

Tagging @DanielJump as an interested party

[dcleblanc]

Hi Jacob,

I’m surprised that ever worked. I’m also curious how there could be an unknown version – everything from where it went up on CodePlex onward has been version 3.0.x, where I think minimum value of x was 12. There was a version that shipped with Visual Studio that’s been unsupported (by me at least) for a long time – IIRC, it had 2 files, one with private implementations.

The problem you’re hitting is that the compiler wants to find the appropriate overload, in this case it should be one of the friend methods defined after the main class. This assumes that one operand is a SafeInt, where T must be some numeric type (in some cases, bool is allowed), and the other is some type U, where U is a numeric type (usually an integer type).

Quite some time ago, the C++ standard gave me a way to detect whether a type is an integer type, and I started using that. You’d have to look through the revision history for when, could be as long as a decade back. Prior to that, I hoped you were using some integer type, but had no way to enforce it, but if you didn’t, then you just got a horrible mess of compiler errors.

In this specific case, you could cast the pointer to uintptr_t, and then it would work, but if it were a pointer to something other than char, it wouldn’t. I’d also point out that a pointer value is only defined by the standard within the buffer, and what used to work might get removed by impractical compilers such as gcc or clang. My suggested work-around wouldn’t get removed, but is in any case only a very partial check that rules out a wrap around, we still have no real idea whether ptr + x is within the buffer (or one past).

Your work-around is really the best fix, though I’d add additional checks to ensure you’re within the buffer. Also, if you’re really currently using the unsupported Visual Studio version, please update to current. There have been very, very few runtime fixes over the years, but quite a number of perf improvements and better leveraging of new language features. Once you’re on a current version, then I’ll support any issues you might hit.

If I recall correctly, you were in Office, and I’m not sure Office has been on a supported version for some time.

David

From: Jacob Langley @.> Sent: Tuesday, February 6, 2024 10:19 AM To: dcleblanc/SafeInt @.> Cc: Subscribed @.***> Subject: [dcleblanc/SafeInt] Upgrading from (very) old unknown version of SafeInt to SafeInt 3.0.28p produces unexpected C2100 compilation failures with MSVC (Issue #60)

Minimum repro case for the issue with Visual Studio 2022 was:

include "SafeInt.hpp"

int main()

{

char characters[7] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g' };

char* ptr = &characters[0];

SafeInt<int> x{ 4 };

if USED_TO_WORK

*(ptr + x) = 0; // produces C2100 illegal indirection compiler diagnostic

else

char* end = ptr;

end += x;

*(end) = 0;

endif

return 0;

}

I can see this being either an unexpected regression or intentional and I'm getting a bit of push-back about inserting casts or refactoring the code to use operator+= instead. Filing an issue for discussion/visibility.

— Reply to this email directly, view it on GitHubhttps://github.com/dcleblanc/SafeInt/issues/60, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHX6YL5JUQ3RKXLZRFHLOGDYSJXYVAVCNFSM6AAAAABC4O6USCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGEZDCNBRGMZDEMI. You are receiving this because you are subscribed to this thread.Message ID: @.**@.>>

[jacobl-at-ms]

Yea, this is definitely Office and its a heavily mucked with copy which is why I didn't just dump more info about it in a public repo. I'm trying to get us onto latest which is how it was discovered.

I was mostly after if this was an intentional design decision on the SafeInt<> side which it sounds like it is. I've got a couple workarounds already, depending on the context, so I'm not too worried about it. Just asking so that I have a place to point everyone who asks.

[jacobl-at-ms]

I just had a chance to dig back a decade across a source control change and the overload set that we're getting that allows the bad code to work may be an unmarked part of our mucking around with the original upstream which (for me) further cements the idea that the existing head version of SafeInt is the behavior thats intended and desired.

[dcleblanc]

Yep, Dan asked me for constexpr support a long while back, I did a bunch of work, and then he never cut over to the new version. Once you get on the newest, you should see perf/size improvements, especially on clang builds.

From: Jacob Langley @.> Sent: Tuesday, February 6, 2024 1:45 PM To: dcleblanc/SafeInt @.> Cc: David LeBlanc @.>; Comment @.> Subject: Re: [dcleblanc/SafeInt] Upgrading from (very) old unknown version of SafeInt to SafeInt 3.0.28p produces unexpected C2100 compilation failures with MSVC (Issue #60)

I just had a chance to dig back a decade across a source control change and the overload set that we're getting that allows the bad code to work may be an unmarked part of our mucking around with the original upstream which (for me) further cements the idea that the existing head version of SafeInt is the behavior thats intended and desired.

— Reply to this email directly, view it on GitHubhttps://github.com/dcleblanc/SafeInt/issues/60#issuecomment-1930802914, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHX6YLZDOPKB7V7F7KHRDDTYSKP7TAVCNFSM6AAAAABC4O6USCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZQHAYDEOJRGQ. You are receiving this because you commented.Message ID: @.**@.>>