Bug 96570 - Warnings desired for time_t to int coversions
Summary: Warnings desired for time_t to int coversions
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 10.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 118326 (view as bug list)
Depends on:
Blocks: new-warning, new_warning
  Show dependency treegraph
 
Reported: 2020-08-11 15:46 UTC by M Welinder
Modified: 2025-01-12 04:08 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
testcase (110 bytes, text/plain)
2025-01-11 04:26 UTC, Bernhard M. Wiedemann
Details
actual testcase (286 bytes, text/plain)
2025-01-11 04:29 UTC, Bernhard M. Wiedemann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description M Welinder 2020-08-11 15:46:18 UTC
This is a wishlist item.  It's filed for C++ but would apply to C too.

It would be useful to have some mechanism to cause warnings to be emitted at compile-time when values of type time_t are cut down in size.  For example:

int now = time(nullptr); // Not good!

int now = int(time(nullptr)); // Not good!
// (This one occurs not only when someone wanted the compiler to shut up,
// but also occurs when using casts to resolve overload resolution.)


void foo(int);
...
time_t now = time(nullptr);
foo(now); // Not good

Note: this is desired for explicit casts too.  Hence -Wconversion in its current form is not what I am looking for.  I am not sure how, if at all, the warnings should be suppressed, but a cast via a 64-bit type, int(int64_t(time(null))), is an option.
Comment 1 Jonathan Wakely 2020-08-11 16:12:13 UTC
(In reply to M Welinder from comment #0)
> Note: this is desired for explicit casts too.

Why? If somebody wants to be explicitly stupid, that's their prerogative.
Comment 2 M Welinder 2020-08-11 17:17:29 UTC
> Why? If somebody wants to be explicitly stupid, that's their prerogative.

I agree with the second sentence.

However, casts are not a clear indication that somebody wants to be explicitly stupid, at least not in C++.  If you were looking for such an indication, my int(int64_t(...)) suggestion is probably closer.

Casts occur also in (e.g.) overload resolution and entirely too often in template soup.  And in macros too, I guess.

The goal here is to find time handling bugs.  We are by definition talking about code that should not have been written that way in the first place.
Comment 3 Jonathan Wakely 2020-08-11 17:26:54 UTC
(In reply to M Welinder from comment #2)
> Casts occur also in (e.g.) overload resolution and entirely too often in
> template soup.  And in macros too, I guess.

Explicit casts don't, and that's what I was questioning.

-Wconversion will warn about implicit conversions. What explicit conversions happen inadvertently?
Comment 4 M Welinder 2020-08-12 01:05:54 UTC
> Explicit casts don't, and that's what I was questioning.

They most certainly do.  That's an empirical statement from having gone over a fairly large code base.  It is not a statement that they should occur there and there is likely nothing "inadvertent[ly]" about their presence.  "Mistaken" and "ill-advised" are probably better descriptions, but the reasons have long since been forgotten.  I.e., the code is buggy.

Look, you are being a bit defensive here which is strange as no-one is attacking you.  Please try looking at the goal:

*** The goal here is to find time handling bugs.

Is that a worthy goal for gcc?

I will assert that there is a lot of buggy time handling code out there and that fixing it will receive more and more attention over the next 15 years.

The compiler may or may not be the right tool to help find and fix these, but gcc has in the past taken it upon itself to warn about other classes of likely-wrong code and it is fairly well positioned to do so given its access to a parse tree and type information.
Comment 5 Jonathan Wakely 2020-08-12 07:12:48 UTC
(In reply to M Welinder from comment #4)
> > Explicit casts don't, and that's what I was questioning.
> 
> They most certainly do.

I think I understand what you mean now, cases like:

int i = std::max(t, int(time(nullptr)));

?

That's more compelling than:

int now = int(time(nullptr)); // Not good!

There seems absolutely no reason to warn here. The user clearly wants to create an int *and* has used an explicit cast.

Since you're asking for new checks to detect special cases involving time_t, why not make it only warn about problem cases? Requiring int(int64_t(time(nullptr))) here is not acceptable IMHO.
Comment 6 Eric Gallager 2020-08-12 15:27:10 UTC
(In reply to M Welinder from comment #2)
> > Why? If somebody wants to be explicitly stupid, that's their prerogative.
> 
> I agree with the second sentence.
> 
> However, casts are not a clear indication that somebody wants to be
> explicitly stupid, at least not in C++.  If you were looking for such an
> indication, my int(int64_t(...)) suggestion is probably closer.
> 
> Casts occur also in (e.g.) overload resolution and entirely too often in
> template soup.  And in macros too, I guess.
> 

This reminds me of bug 69818
Comment 7 Andrew Pinski 2025-01-07 17:03:31 UTC
*** Bug 118326 has been marked as a duplicate of this bug. ***
Comment 8 Andrew Pinski 2025-01-09 18:56:40 UTC
This reminds me of PR 85861 where size_t can either be 32bit or 64bit.
Comment 9 Bernhard M. Wiedemann 2025-01-11 04:26:39 UTC
Created attachment 60104 [details]
testcase
Comment 10 Bernhard M. Wiedemann 2025-01-11 04:29:07 UTC
Created attachment 60105 [details]
actual testcase
Comment 11 Xi Ruoyao 2025-01-12 04:08:18 UTC
Also note that

srand(time(nullptr));

should be perfectly valid code even after 2038.  Now to suppress the warning we can write

srand((int)time(nullptr));

So if we start to warn even with an explicitly cast, how would we handle this case now?