Repository navigation
C++: Introduce getBitWidth predicate for Type - #22830
Conversation
Generally this will just be `8 * getSize()` however this is not the case for `_BitInt`s.
paldepind
left a comment
paldepind
left a comment
There was a problem hiding this comment.
Looks good and makes sense to me.
There was a problem hiding this comment.
🟡 Changes recommended
Incorrect type widths and conversion handling can produce missed or spurious analysis results.
4 open findings
What changed in this PR
Adds Type.getBitWidth() to distinguish _BitInt precision from storage size across C++ analyses.
Changes:
- Generalizes bit-width access from
BitPreciseIntTypetoType. - Updates analyses to use bit widths where appropriate.
- Adds a library change note.
| File | Description |
|---|---|
| cpp/ql/src/Likely Bugs/Memory Management/Padding/Suboptimal64BitType.ql | Uses wide-character bit width. |
| cpp/ql/src/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql | Uses target width for bitfield narrowing. |
| cpp/ql/src/Likely Bugs/Conversion/ConversionChangesSign.ql | Updates the constant-fit bound. |
| cpp/ql/src/Likely Bugs/AmbiguouslySignedBitField.ql | Checks declared bits against type width. |
| cpp/ql/src/jsf/4.23 Type Conversions/AV Rule 180.ql | Uses bit widths in conversion checks. |
| cpp/ql/src/jsf/4.20 Unions and Bit Fields/AV Rule 154.ql | Updates full-width bitfield detection. |
| cpp/ql/lib/semmle/code/cpp/Type.qll | Exposes getBitWidth() on Type. |
| cpp/ql/lib/semmle/code/cpp/rangeanalysis/RangeAnalysisUtils.qll | Calculates integral bounds using bit widths. |
| cpp/ql/lib/semmle/code/cpp/controlflow/internal/ConstantExprs.qll | Updates constant representability checks. |
| cpp/ql/lib/semmle/code/cpp/commons/Printf.qll | Updates numeric output-length estimates. |
| cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/extensions/ConstantShiftExprRange.qll | Validates shifts against operand bit width. |
| cpp/ql/lib/change-notes/2026-10-15-bitwidth.md | Documents the new predicate. |
🧠 Review effort: Max
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| not bit_precise_int_types(underlyingElement(this), _) and | ||
| result = this.getSize() * 8 | ||
| or | ||
| bit_precise_int_types(underlyingElement(this), result) |
There was a problem hiding this comment.
That's fair. Let me fix that (in the correct way).
| if val >= 0 and val.bitShiftRight(t.getBitWidth() - 1) = 0 | ||
| then result = val | ||
| else ( | ||
| (-(val + 1)).bitShiftRight(t.getSize() * 8 - 1) = 0 and result = val | ||
| (-(val + 1)).bitShiftRight(t.getBitWidth() - 1) = 0 and result = val |
There was a problem hiding this comment.
QL's
int.bitShiftRightreduces the shift count modulo 32
I see no evidence of this in the documentation, but if this is the case, this was already a problem for larger types like __int128. So, won't fix here.
There was a problem hiding this comment.
I think QL ints are 32-bit, so it's kind of right.
There was a problem hiding this comment.
That's a weird way of phrasing things by Copilot then. In any case, pre-existing problem.
| ) and | ||
| fi.getNumBits() > fct.getSize() * 8 and | ||
| fi.getNumBits() > fct.getBitWidth() and | ||
| va.getExplicitlyConverted().getType().getSize() > fct.getSize() and |
|
|
||
| int getSourceSize() { | ||
| if this instanceof IntConstantExpr | ||
| then result = this.(IntConstantExpr).getRequiredSizeBits() |
There was a problem hiding this comment.
That has a FIXME with limitations on it in any case, so won't fix for now.
|
DCA looks good, merging. |
Generally this will just be
8 * getSize()however this is not the case for_BitInts. Use where this seems appropriate.Follow-up to #22816 (comment)