[analyzer] Don't crash on printing ConcreteInt of size >64 bits

[a.sidorin]

Summary

Printing of ConcreteInts with size >64 bits resulted in assertion failure in get[Z|S]ExtValue() :getActiveBits() <= 64 && "Too many bits for uint64_t"' failed'. This patch fixes the issue.

Initial patch by Ivan Sidorenko (modified by A. Sidorin).

[george.karpenkov]

lib/StaticAnalyzer/Core/SVals.cpp

304

Given that getValue is accessed 6 times now, I would put it into a variable

305

Makes sense, another option would be to add a fast path to APInt::print?

test/Analysis/egraph-dump-int128.c

3 ↗

(On Diff #141557)

At least on a mac, ViewExplodedGraph launches GraphViz.app.
Can we have a less invasive check? Wouldn't just dumping a value be sufficient?

[NoQ]

test/Analysis/egraph-dump-int128.c

3 ↗

(On Diff #141557)

Yeah, it just runs xdg-open on linux and open on mac, which often opens up a GUI viewer. That's not the behavior we want from automated tests.

I think you should use ExprInspection instead, and either dump the whole state with clang_analyzer_printState() or, even better, dump only the particular value with clang_analyzer_dump(...) - that way you'll be also capable of verifying the output.

Note that we actually don't have much tests for our dumps.

[a.sidorin]

After thinking a bit more, I have removed the FIXME at all: dumping SVal is an extremely rare event so this shouldn't affect the performance.

[a.sidorin]

Renamed the test file.

[george.karpenkov]

I have removed the FIXME at all

👍

[a.sidorin]

Closed with https://reviews.llvm.org/rC330605. Forgot to mention the Differential Revision, sorry.

lib/StaticAnalyzer/Core/SVals.cpp

 void NonLoc::dumpToStream(raw_ostream &os) const {
   switch (getSubKind()) {
     case nonloc::ConcreteIntKind: {
-      const nonloc::ConcreteInt& C = castAs<nonloc::ConcreteInt>();
-      if (C.getValue().isUnsigned())
-        os << C.getValue().getZExtValue();
-      else
-        os << C.getValue().getSExtValue();
-      os << ' ' << (C.getValue().isUnsigned() ? 'U' : 'S')
-         << C.getValue().getBitWidth() << 'b';
+      const auto &Value = castAs<nonloc::ConcreteInt>().getValue();
+      os << Value << ' ' << (Value.isSigned() ? 'S' : 'U')
+         << Value.getBitWidth() << 'b';
       break;
     }
     case nonloc::SymbolValKind:

test/Analysis/sval-dump-int128.c

+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection %s -verify
+
+void clang_analyzer_dump(unsigned __int128 x);
+
+void testDumpInt128() {
+  clang_analyzer_dump((unsigned __int128)5 << 64); // expected-warning{{92233720368547758080 U128b}}
+}