[PATCH] D70689: [analyzer] Fix SARIF column locations

[Joe Ranieri via Phabricator via cfe-commits]

jranieri-grammatech marked an inline comment as not done.
jranieri-grammatech added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:157
+  const MemoryBuffer *Buf = SM.getBuffer(LocInfo.first);
+  assert(Buf && "got a NULL buffer for the location's file");
+  assert(Buf->getBufferSize() >= (LocInfo.second + TokenLen) &&
----------------
aaron.ballman wrote:
> I don't think this API is able to return a null buffer, but it can return an invalid buffer under legitimate circumstances, I believe. For instance, if the source location is in the scratch space used for things like macros defined on the command line with `-D`. I think you should pass the optional `invalid` parameter to the call to `getBuffer()` and then do no adjustments if the buffer is invalid. WDYT?
Good catch that this can't return NULL.

I tested out defining things on the command line, but this code deals with expansion locations and wasn't affected by it. Unless you can think of a way to reproduce it, I think I'd rather pass in the `invalid` argument and assert that it isn't invalid.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70689/new/

https://reviews.llvm.org/D70689