Skip to content

Commit 4c71498

Browse files
author
james
committed
docs: address comments on bad overflow guard slides
1 parent a9a0b9a commit 4c71498

File tree

2 files changed

+44
-24
lines changed

2 files changed

+44
-24
lines changed

docs/language/ql-training-rst/_static-training/slides-semmle-2/static/theme/css/default.css

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,9 @@ aside.gdbar img {
13161316

13171317
/* line 913, ../scss/default.scss */
13181318
.quote {
1319-
color: #e6e6e6;
1319+
color: black;
1320+
font-style: italic;
1321+
font-size: 1.2em;
13201322
}
13211323
/* line 916, ../scss/default.scss */
13221324
.quote .author {
@@ -1381,6 +1383,10 @@ em {
13811383
font-style: italic;
13821384
}
13831385

1386+
sup {
1387+
vertical-align: super;
1388+
font-size: 0.5em;
1389+
}
13841390

13851391

13861392
/* James slide backgrounds */
@@ -1413,6 +1419,11 @@ slide {
14131419
padding:0;
14141420
}
14151421

1422+
.background2 h1, .background2 p {
1423+
color: white;
1424+
}
1425+
1426+
14161427
/* Title slide styles */
14171428

14181429
.semmle-logo sup {
@@ -1502,6 +1513,7 @@ p.first.admonition-title {
15021513

15031514
.admonition.note > p {
15041515
width: inherit;
1516+
font-size: 1em;
15051517
}
15061518

15071519
.admonition.note ul {

docs/language/ql-training-rst/cpp/bad-overflow-guard.rst

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ More resources:
2929

3030
Alternatively, you can query any project (including ChakraCore) in the `query console on LGTM.com <https://lgtm.com/query/project:2034240708/lang:cpp/>`__.
3131

32-
Note that results generated in the query console are likely to differ to those generated in the QL plugin. LGTM.com analyzes the most recent revisions of each project that has been added–the snapshot available to download above is based on an historical version of the code base.
32+
Note that results generated in the query console are likely to differ to those generated in the QL plugin. LGTM.com analyzes the most recent revisions of each project that has been added–the snapshot available to download above is for an historical version of the code base.
3333

3434

3535
Checking for overflow in C
@@ -60,20 +60,22 @@ Integer promotion
6060

6161
From `https://en.cppreference.com/w/c/language/conversion <https://en.cppreference.com/w/c/language/conversion>`__:
6262

63-
*Integer promotion is the implicit conversion of a value of any integer type with rank less or equal to rank of* ``int`` *... to the value of type* ``int`` *or* ``unsigned int``.
63+
.. rst-class:: quote
64+
65+
Integer promotion is the implicit conversion of a value of any integer type with rank less or equal to rank of ``int`` ... to the value of type ``int`` or ``unsigned int``.
6466

6567
The arguments of the following arithmetic operators undergo implicit conversions:
6668

67-
- binary arithmetic (* / % + - )
68-
- relational operators (< > <= >= == !=)
69-
- binary bitwise operators (& ^ \|)
70-
- the conditional operator (?:)
69+
- binary arithmetic: ``* / % + -``
70+
- relational operators: ``< > <= >= == !=``
71+
- binary bitwise operators: ``& ^ |``
72+
- the conditional operator: ``?:``
7173

7274
.. note::
7375

7476
- Most of the time integer conversion works fine. However, the rules governing addition in C/C++ are complex, and it easy to get bitten.
75-
- CPUs generally prefer to perform arithmetic operations on 32 bit or larger integers, as the architectures are optimised to perform those efficiently.
76-
- The compiler therefore performs “integer promotion” for arguments to arithmetic operations that are smaller than 32 bit.
77+
- CPUs generally prefer to perform arithmetic operations on 32-bit or larger integers, as the architectures are optimized to perform those efficiently.
78+
- The compiler therefore performs “integer promotion” for arguments to arithmetic operations that are smaller than 32-bit.
7779

7880
Checking for overflow in C revisited
7981
====================================
@@ -101,6 +103,7 @@ Checking for overflow in C revisited
101103
Here is the example again, with the conversions made explicit:
102104

103105
.. code-block:: cpp
106+
:emphasize-lines: 4,7
104107
105108
uint16_t v = 65535;
106109
uint16_t b = 1;
@@ -116,10 +119,11 @@ Here is the example again, with the conversions made explicit:
116119
In this example the second branch is executed, even though there is a 16-bit overflow, and result is set to zero.
117120

118121
Explanation:
119-
- The two integer arguments to the addition, v and b, are promoted to 32 bit integers.
120-
- The comparison (<) is also an arithmetic operation, therefore it will also be completed on 32 bit integers.
121-
- This means that v + b < v will never be true, because v and b can hold at most 216.
122-
- Therefore, the second branch is executed, but the result of the addition is stored into the result variable. Overflow will still occur as result is a 16 bit integer.
122+
123+
- The two integer arguments to the addition, ``v`` and ``b``, are promoted to 32-bit integers.
124+
- The comparison (``<``) is also an arithmetic operation, therefore it will also be completed on 32-bit integers.
125+
- This means that ``v + b < v`` will never be true, because v and b can hold at most 2 :sup:`16`.
126+
- Therefore, the second branch is executed, but the result of the addition is stored into the result variable. Overflow will still occur as result is a 16-bit integer.
123127

124128
This happens even though the overflow check passed!
125129

@@ -146,9 +150,11 @@ Let’s look for overflow guards of the form ``v + b < v``, using the classes
146150
- When performing `variant analysis <https://semmle.com/variant-analysis>`__, it is usually helpful to write a simple query that finds the simple syntactic pattern, before trying to go on to describe the cases where it goes wrong.
147151
- In this case, we start by looking for all the *overflow* checks, before trying to refine the query to find all *bad overflow* checks.
148152
- The select clause defines what this query is looking for:
153+
149154
- an ``AddExpr``: the expression that is being checked for overflow.
150155
- a ``RelationalOperation``: the overflow comparison check.
151156
- a ``Variable``: used as an argument to both the addition and comparison.
157+
152158
- The where part of the query ties these three QL variables together using `predicates <https://help.semmle.com/QL/ql-handbook/predicates.html>`__ defined in the `standard QL for C/C++ library <https://help.semmle.com/qldoc/cpp/>`__.
153159

154160
QL query: bad overflow guards
@@ -174,7 +180,7 @@ We can get the size (in bytes) of a type using the ``getSize()`` method.
174180

175181
- An important part of the query is to determine whether a given expression has a “small” type that is going to trigger integer promotion.
176182
- We therefore write a helper predicate for small expressions.
177-
- This predicate effectively represents the set of all expressions in the database where the size of the type of the expression is less than 4 bytes, that is less than 32 bits.
183+
- This predicate effectively represents the set of all expressions in the database where the size of the type of the expression is less than 4 bytes, that is, less than 32-bits.
178184

179185
QL query: bad overflow guards
180186
=============================
@@ -190,23 +196,25 @@ Now our query becomes:
190196

191197
.. note::
192198

193-
- Recall from earlier that what makes an overflow check a “bad” check is that all the arguments to the addition are integers smaller than 32 bits.
194-
- We could write this by using our helper predicate ``isSmall`` to specify that each individual operand to the addition ``isSmall`` (that is under 32 bits):
199+
- Recall from earlier that what makes an overflow check a “bad” check is that all the arguments to the addition are integers smaller than 32-bits.
200+
- We could write this by using our helper predicate ``isSmall`` to specify that each individual operand to the addition ``isSmall`` (that is under 32-bits):
195201

196-
.. code-block:: ql
197-
198-
isSmall(a.getLeftOperand()) and
199-
isSmall(a.getRightOperand())
202+
.. code-block:: ql
203+
204+
isSmall(a.getLeftOperand()) and
205+
isSmall(a.getRightOperand())
200206
201207
- However, this is a little bit repetitive. What we really want to say is that: all the operands of the addition are small. Fortunately, QL provides a ``forall`` formula that we can use in these circumstances.
202208
- A ``forall`` has three parts:
209+
203210
- A declaration part, where we can introduce variables.
204211
- A “range” part, which allows us to restrict those variables.
205-
- A “condition” part. The ``foral`` as a whole holds if the condition holds for each of the values in the range.
212+
- A “condition” part. The ``forall`` as a whole holds if the condition holds for each of the values in the range.
206213
- In our case:
207-
- The declaration introduces a variable for Expressions, called ``op``. At this stage, this variable represents all the expressions in the program.
214+
215+
- The declaration introduces a variable for expressions, called ``op``. At this stage, this variable represents all the expressions in the program.
208216
- The “range” part, ``op = a.getAnOperand()``, restricts ``op`` to being one of the two operands to the addition.
209-
- The “condition” part, ``isSmall(op)``, says that the ``forall`` holds only if the condition - that the ``op`` is small - holds for everything in the range - that is both the arguments to the addition
217+
- The “condition” part, ``isSmall(op)``, says that the ``forall`` holds only if the condition - that the ``op`` is small - holds for everything in the range - that is both the arguments to the addition.
210218

211219
QL query: bad overflow guards
212220
=============================
@@ -225,4 +233,4 @@ The final query
225233
.. literalinclude:: ../query-examples/cpp/bad-overflow-guard-3.ql
226234
:language: ql
227235

228-
This query finds a single result, which is `a genuine bug in ChakraCore <https://github.com/Microsoft/ChakraCore/commit/2500e1cdc12cb35af73d5c8c9b85656aba6bab4d>`__.
236+
This query finds a single result in our historic snapshot, which was `a genuine bug in ChakraCore <https://github.com/Microsoft/ChakraCore/commit/2500e1cdc12cb35af73d5c8c9b85656aba6bab4d>`__.

0 commit comments

Comments
 (0)