Improve UBSan documentation

[DianeMeirowitz]

Summary

Add more checks, info on -fno-sanitize=..., and reference to 5/2021 UBSan Oracle blog.

[hctim]

LGTM with nits. Do you need me to submit this patch for you?

clang/docs/UndefinedBehaviorSanitizer.rst

15

nit: "Array subscript out of bounds, when the bounds can be statically determined"

17

nit: "Dereferencing misaligned or null pointers"

[DianeMeirowitz]

Mitch,

Thank you for reviewing my patch.

I don't agree with the phrasing : "Array subscript out of bounds, when the bounds can be statically determined". It is long, and I think it may confuse people who don't read language standards and also as far as I know, neither C nor C++ has a true dynamic array type. Dynamic arrays are declared as pointers, not arrays. So I suggest just keeping my original simple phrasing "Array subscript out of bounds". But if you feel strongly about this, go ahead.

I agree with the second nit, "Dereferencing misaligned or null pointers", since these pointers are fine until they are dereferenced, of course.

Yes, if you are willing to submit the patch, please do. And thank you!

Diane

On 7/27/21, 4:55 PM, "Mitch Phillips via Phabricator" <[e-mail]> wrote:

hctim accepted this revision.
hctim added a comment.
This revision is now accepted and ready to land.

LGTM with nits. Do you need me to submit this patch for you?



================
Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:15

-* Using misaligned or null pointer
+* Array subscript out of bounds
+* Bitwise shifts that are out of bounds for their data type
----------------
nit: "Array subscript out of bounds, when the bounds can be statically determined"


================
Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:17
+* Bitwise shifts that are out of bounds for their data type
+* Misaligned or null pointer
 * Signed integer overflow
----------------
nit: "Dereferencing misaligned or null pointers"


CHANGES SINCE LAST ACTION
  https://urldefense.com/v3/__https://reviews.llvm.org/D106908/new/__;!!ACWV5N9M2RV99hQ!fAT1f8xnLfUbRHWvjZ5u6KH3mZasWxxcgnq38vA1e9N4TyKUwqGrc4R8QLMDGrP0rzi1$ 

https://urldefense.com/v3/__https://reviews.llvm.org/D106908__;!!ACWV5N9M2RV99hQ!fAT1f8xnLfUbRHWvjZ5u6KH3mZasWxxcgnq38vA1e9N4TyKUwqGrc4R8QLMDGmhc9eVw$

[DianeMeirowitz]

Hi Mitch,

Do I need to do anything further with this, like upload a revised patch?

Diane

On 7/28/21, 10:14 AM, "Diane Meirowitz via Phabricator" <[e-mail]> wrote:

DianeMeirowitz added subscribers: eugenis, jkorous, kuhnel.
DianeMeirowitz added a comment.

Mitch,

Thank you for reviewing my patch.

I don't agree with the phrasing : "Array subscript out of bounds, when the bounds can be statically determined". It is long, and I think it may confuse people who don't read language standards and also as far as I know, neither C nor C++ has a true dynamic array type. Dynamic arrays are declared as pointers, not arrays. So I suggest just keeping my original simple phrasing "Array subscript out of bounds". But if you feel strongly about this, go ahead.

I agree with the second nit, "Dereferencing misaligned or null pointers", since these pointers are fine until they are dereferenced, of course.

Yes, if you are willing to submit the patch, please do. And thank you!

Diane

[hctim]

In D106908#2910131, @DianeMeirowitz wrote:

I don't agree with the phrasing : "Array subscript out of bounds, when the bounds can be statically determined". It is long, and I think it may confuse people who don't read language standards and also as far as I know, neither C nor C++ has a true dynamic array type. Dynamic arrays are declared as pointers, not arrays. So I suggest just keeping my original simple phrasing "Array subscript out of bounds". But if you feel strongly about this, go ahead.

Indirection can kill the bounds tracking, so we normally add the caveat that "the bounds can be statically determined". For example, this simple case escapes ubsan-bounds (but not asan):

int f(int y[]) {
  return y[1];
}

int main() {
  int x[1];
  return f(x);
}

I'll submit with the nit.

[DianeMeirowitz]

Mitch,

OK and thanks for submitting it!

Diane

On 8/2/21, 5:55 PM, "Mitch Phillips via Phabricator" <[e-mail]> wrote:

hctim added a comment.

In D106908#2910131 <https://urldefense.com/v3/__https://reviews.llvm.org/D106908*2910131__;Iw!!ACWV5N9M2RV99hQ!ezsHkymW9n_oArVw48GURjA0PCnyK2zxkbZU2B9adp3WEpn3KvcVau7Y0fo6bxKxWNDu$ >, @DianeMeirowitz wrote:

> I don't agree with the phrasing : "Array subscript out of bounds, when the bounds can be statically determined". It is long, and I think it may confuse people who don't read language standards and also as far as I know, neither C nor C++ has a true dynamic array type. Dynamic arrays are declared as pointers, not arrays. So I suggest just keeping my original simple phrasing "Array subscript out of bounds". But if you feel strongly about this, go ahead.

Indirection can kill the bounds tracking, so we normally add the caveat that "the bounds can be statically determined". For example, this simple case escapes ubsan-bounds (but not asan):

  int f(int y[]) {
    return y[1];
  }

  int main() {
    int x[1];
    return f(x);
  }

I'll submit with the nit.


CHANGES SINCE LAST ACTION
  https://urldefense.com/v3/__https://reviews.llvm.org/D106908/new/__;!!ACWV5N9M2RV99hQ!ezsHkymW9n_oArVw48GURjA0PCnyK2zxkbZU2B9adp3WEpn3KvcVau7Y0fo6b0vh3Zht$ 

https://urldefense.com/v3/__https://reviews.llvm.org/D106908__;!!ACWV5N9M2RV99hQ!ezsHkymW9n_oArVw48GURjA0PCnyK2zxkbZU2B9adp3WEpn3KvcVau7Y0fo6b5X0CPtC$

clang/docs/UndefinedBehaviorSanitizer.rst

 ==========================
 UndefinedBehaviorSanitizer
 ==========================
 
 .. contents::
    :local:
 
 Introduction
 ============
 
 UndefinedBehaviorSanitizer (UBSan) is a fast undefined behavior detector.
 UBSan modifies the program at compile-time to catch various kinds of undefined
 behavior during program execution, for example:
 
-* Using misaligned or null pointer
+* Array subscript out of bounds
+* Bitwise shifts that are out of bounds for their data type
+* Misaligned or null pointer
 * Signed integer overflow
 * Conversion to, from, or between floating-point types which would
   overflow the destination
 
 See the full list of available :ref:`checks <ubsan-checks>` below.
 
 UBSan has an optional run-time library which provides better error reporting.
 The checks have small runtime cost and no impact on address space layout or ABI.
.. code-block:: console
   test.cc:3:5: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
 
 You can enable only a subset of :ref:`checks <ubsan-checks>` offered by UBSan,
 and define the desired behavior for each kind of check:
 
 * ``-fsanitize=...``: print a verbose error report and continue execution (default);
 * ``-fno-sanitize-recover=...``: print a verbose error report and exit the program;
 * ``-fsanitize-trap=...``: execute a trap instruction (doesn't require UBSan run-time support).
+* ``-fno-sanitize=...``: disable any check, e.g., -fno-sanitize=alignment.
 
 Note that the ``trap`` / ``recover`` options do not enable the corresponding
 sanitizer, and in general need to be accompanied by a suitable ``-fsanitize=``
 flag.
 
 For example if you compile/link your program as:
 
 .. code-block:: console

 * ``-fsanitize-undefined-strip-path-components=1``: ``code/library/file.cpp``
 * ``-fsanitize-undefined-strip-path-components=2``: ``library/file.cpp``
 * ``-fsanitize-undefined-strip-path-components=-1``: ``file.cpp``
 * ``-fsanitize-undefined-strip-path-components=-2``: ``library/file.cpp``
 
 More Information
 ================
 
+* From Oracle blog, including a discussion of error messages:
+  `Improving Application Security with UndefinedBehaviorSanitizer (UBSan) and GCC
+  <https://blogs.oracle.com/linux/improving-application-security-with-undefinedbehaviorsanitizer-ubsan-and-gcc>`_
 * From LLVM project blog:
   `What Every C Programmer Should Know About Undefined Behavior
   <http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html>`_
 * From John Regehr's *Embedded in Academia* blog:
   `A Guide to Undefined Behavior in C and C++
   <https://blog.regehr.org/archives/213>`_