RFC: Y2038 check for lossy time_t conversions

Title: [clang-tidy] RFC: Y2038 check for lossy time_t conversions

Hi,

This RFC proposes a new clang-tidy check for Y2038-related bugs. The goal is to warn when a value involving time_t is converted to a type that may not be able to represent it, for example storing or passing time_t through a narrower integer type, or through a type with incompatible signedness.

The motivation is to catch code that is valid C/C++, but may truncate or otherwise lose information when time_t is 64-bit. For example:

void f(void)
{
  int i = (int)time(NULL);

  int i2;
  i2 = time(NULL);

  struct S { int sec; };
  struct S s;
  s.sec = time(NULL);

  time_t t = time(NULL);
  uint32_t u32 = t; // warn
  int32_t i32 = t; // warn
  uint64_t u64 = t; // warn
  int64_t i64 = t; // no warning
}

The check would produce diagnostics such as:

warning: conversion from 'time_t' to 'int' may lose information [portability-y2038-lossy]
  int i = (int)time(NULL);
          ^
warning: conversion from 'time_t' to 'int' may lose information [portability-y2038-lossy]
  i2 = time(NULL);
       ^
warning: conversion from 'time_t' to 'int' may lose information [portability-y2038-lossy]
     s.i = time(NULL);
           ^

The check should also catch cases where the conversion happens as part of a larger expression, such as function arguments, return statements, conditions, and conditional expressions.

The current prototype works by matching cast expressions in the AST, instead of trying to handle assignments, variable declarations, function calls, etc. separately. After finding a cast, the check looks at whether the source expression involves time_t or something in its typedef chain, for example __time_t, and then checks whether the destination type may be narrower or otherwise lossy.

Very roughly, the matcher looks like this:

Finder->addMatcher(
    traverse(TK_AsIs,
             castExpr(unless(isExpansionInSystemHeader()),
                      anyOf(hasCastKind(CK_IntegralCast),
                            hasCastKind(CK_IntegralToBoolean)))
                 .bind("cast")),
    this);

Most of the logic is then in the check callback: inspect the source expression, desugar typedefs where needed, and compare the source and destination types.

For fix-its, the intention is to keep them conservative. In straightforward local cases, the check can walk up the AST until it finds the related VarDecl and suggest changing a too-narrow destination type. For example:

- int i = (int)time(NULL);
+ time_t i = time(NULL);

- int i2;
+ time_t i2;
  i2 = time(NULL);

- uint32_t u32 = t;
+ time_t u32 = t;

Fix-its would be avoided where the change affects an API boundary. For example, struct S { int sec; }; would not be changed by default just because of s.sec = time(NULL), since that changes the layout/API of the struct.

Feedback on the intended scope and diagnostic behavior would be appreciated. In particular: does matching cast expressions sound like the right approach here?

Thanks!

Thank you for proposing this!
As I understand, we generally look for is narrowing conventions from time_t to other narrowing types. We have a check that does exactly this: clang-tidy - bugprone-narrowing-conversions — Extra Clang Tools 23.0.0git documentation. But it will warn on all source types when narrowing occurs, but if we make a new option to allow only sertain types to emit diagnostic, will it be sufficient for your case?

Thanks for pointing this out! I was not aware of bugprone-narrowing-conversions; I’ll take a closer look at it.

There is definitely overlap for simple implicit narrowing cases. However, I think the Y2038 case is a bit different because the check is specifically interested in conversions where the source expression involves time_t.

One important difference is explicit casts. For Y2038-related code, I think those are still important to diagnose. For example:

int i = (int)time(NULL);
f_int((int)time(NULL));

Both of these can hide the exact kind of issue the check is trying to find. From the documentation, bugprone-narrowing-conversions intentionally does not warn when the narrowing conversion is marked by an explicit cast.

There are also cases where the current check does not seem to warn in my testing, for example:

uint32_t u32 = time(NULL);

So I think adding an option to bugprone-narrowing-conversions to restrict diagnostics to certain source types may cover part of the use case, but probably not all of it unless it also supports diagnosing explicit casts.

That said, if the preferred direction is to extend bugprone-narrowing-conversions rather than add a separate check, I’m open to exploring that. For example, this could perhaps be modeled as an option that enables diagnostics only for selected source types such as time_t, with Y2038-specific handling for explicit casts.

Hi @vbvictor, gentle ping on this.

I tried the suggested direction and have a prototype that extends bugprone-narrowing-conversions with a time_t-focused mode. When enabled, it limits diagnostics to conversions involving time_t, while also handling cases such as explicit casts and fixed-width integer types from <stdint.h>.

I also added tests for the relevant cases.

Rather than pasting a larger patch in this thread, would it make sense for me to open an upstream PR and continue the discussion there?