[[clang::unsafe_buffer_usage]] Source range in unsafe pointer arithmetic warning doesn't show entire unsafe operation

[tsepez]

Given a file range0.cc:

int lookup(int *ptr, unsigned idx) {
  return ptr[idx];
}

int* predecrement(int* ptr) {
  return --ptr;   // not actually useful, only for example
}

range0.cc:3:10:{3:10-3:13}: warning: unsafe buffer access [-Wunsafe-buffer-usage]
    3 |    return ptr[idx];
      |           ^~~
range0.cc:3:10: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
range0.cc:7:12:{7:12-7:15}: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
    7 |    return --ptr;   // not actually useful, only for example
      |             ^~~

The returned range indicates only the unsafe operand rather than the range of the unsafe operation.

We seek to automate applying the _Pragma("clang unsafe_buffer_usage begin") / _Pragma("clang unsafe_buffer_usage end") form of suppression around the ranges as exactly indicated by the compiler,
and using some macro trickery, i.e.

#define UNSAFE_BUFFERS(...)                  \
  _Pragma("clang unsafe_buffer_usage begin") \
  __VA_ARGS__                                \
  _Pragma("clang unsafe_buffer_usage end")

int lookup(int *ptr, unsigned idx) {
  return UNSAFE_BUFFERS(ptr)[idx];
}

int* predecrement2(int* ptr) {
  return --UNSAFE_BUFFERS(ptr);   // not actually useful, only for example
}

The ptr[idx] looks unusual but does suppress the error. The --ptr case, however gives

range1.cc:12:27: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
   12 |   return --UNSAFE_BUFFERS(ptr);   // not actually useful, only for example
      |                           ^~~

The returned range should cover the entire unsafe operation, not the unsafe argument allowing us to write

int lookup(int *ptr, unsigned idx) {
  return UNSAFE_BUFFERS(ptr[idx]);
}

int* predecrement(int* ptr) {
  return UNSAFE_BUFFERS(--ptr);   // not actually useful, only for example
}

which compiles cleanly

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Thomas Sepez (tsepez)

Given a file range0.cc: ``` int lookup(int *ptr, unsigned idx) { return ptr[idx]; }

int* predecrement(int* ptr) {
return --ptr; // not actually useful, only for example
}

range0.cc:3:10:{3:10-3:13}: warning: unsafe buffer access [-Wunsafe-buffer-usage]
3 | return ptr[idx];
| ^~~
range0.cc:3:10: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
range0.cc:7:12:{7:12-7:15}: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
7 | return --ptr; // not actually useful, only for example
| ^~~

The returned range indicates only the unsafe operand rather than the range of the unsafe operation.

We seek to automate applying the _Pragma("clang unsafe_buffer_usage begin")  / _Pragma("clang unsafe_buffer_usage end") form of suppression around the ranges as exactly indicated by the compiler,
and using some macro trickery,  i.e. 

#define UNSAFE_BUFFERS(...)
_Pragma("clang unsafe_buffer_usage begin")
VA_ARGS
_Pragma("clang unsafe_buffer_usage end")

int lookup(int *ptr, unsigned idx) {
return UNSAFE_BUFFERS(ptr)[idx];
}

int* predecrement2(int* ptr) {
return --UNSAFE_BUFFERS(ptr); // not actually useful, only for example
}

The ptr[idx]  looks unusual but does suppress the error.  The --ptr case, however gives

range1.cc:12:27: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
12 | return --UNSAFE_BUFFERS(ptr); // not actually useful, only for example
| ^~~

The returned range should cover the entire unsafe operation, not the unsafe argument allowing us to write

int lookup(int *ptr, unsigned idx) {
return UNSAFE_BUFFERS(ptr[idx]);
}

int* predecrement(int* ptr) {
return UNSAFE_BUFFERS(--ptr); // not actually useful, only for example
}

which compiles cleanly

</details>

[tsepez]

Minor details

$ clang++ --version
Debian clang version 19.1.7 (3+build1)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-19/bin

[tsepez]

Notes for self: sniffing through the code, it seems like there is extra work done to produce the current behaviour, in AnalysisBasedWarnings.cpp:

  void handleUnsafeOperation(const Stmt *Operation, bool IsRelatedToDecl,
                             ASTContext &Ctx) override {
    SourceLocation Loc;
    SourceRange Range;
    unsigned MsgParam = 0;
    NamedDecl *D = nullptr;
    if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Operation)) {
      Loc = ASE->getBase()->getExprLoc();
      Range = ASE->getBase()->getSourceRange();
      MsgParam = 2;
    }
...

in that I'd like the range of the entire ASE, methinks, instead of its base.