[Lldb-commits] [PATCH] D146536: SymbolFile: ensure that we have a value before invoking `getBitWidth`

Wed Mar 22 05:35:46 PDT 2023

compnerd added a comment.

In D146536#4210062 <https://reviews.llvm.org/D146536#4210062>, @sgraenitz wrote:

> Thanks for tackling this! Yes, I hit it as well during development and wondered what is the right thing to do. The effect of the proposed change will be log messages like this right?
>
>   ... which resolves to a constant value of mismatched width (-1 bits). Ignoring constant.

No, it should be something like 4294967295 bits.  Assuming that you have that log enabled.

> I think it would be great to log the actual issue (constant width not accessible) and bail out if `value.Type == llvm::pdb::Empty`. Even before requesting `qual_type`. What do you think?

Sure, I can test that and see if there are any adverse effects from that.

================
Comment at: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:1307
+        unsigned constant_width =
+            value.Type == llvm::pdb::Empty ? - 1 : value.getBitWidth();

----------------
sgraenitz wrote:
> Is the `-1` intended to express an invalid value? Maybe this should become a signed `int` then?
Yeah, by means of it being impossibly large.  I suppose I could do an explicit cast (i.e. `static_cast<unsigned>(-1)` to make this clearer.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146536