[all-commits] [llvm/llvm-project] 035d98: [CodeGen] Fix -fsanitize=array-bounds for __sized_...

[Dan Liew via All-commits]

  Branch: refs/heads/users/delcypher/fix-or_null_sized_by-codegen-bugs-part-3
  Home:   https://github.com/llvm/llvm-project
  Commit: 035d983848c57a05f136cc766d0aab2c1831f5b3
      https://github.com/llvm/llvm-project/commit/035d983848c57a05f136cc766d0aab2c1831f5b3
  Author: Dan Liew <dliew at apple.com>
  Date:   2026-09-09 (Wed, 09 Sep 2026)

  Changed paths:
    M clang/lib/CodeGen/CGExpr.cpp
    M clang/test/CodeGen/attr-counted-by-or-null-for-pointers.c
    M clang/test/CodeGen/attr-sized-by-for-pointers.c
    M clang/test/CodeGen/attr-sized-by-or-null-for-pointers.c

  Log Message:
  -----------
  [CodeGen] Fix -fsanitize=array-bounds for __sized_by / _or_null pointers

`EmitCountedByBoundsChecking()` assumed a CountAttributedType is always
a __counted_by pointer. That isn't true; there are four versions of the
attribute:

* `__counted_by`: Already handled correctly.
* `__counted_by_or_null`: Incorrectly handled.
* `__sized_by`: Incorrectly handled.
* `__sized_by_or_null`: Incorrectly handled.

In particular:

* __sized_by / __sized_by_or_null: the loaded bound is a byte count, but the
  element index was compared against it directly, so an access was only
  flagged once the index exceeded the byte count -- missing out-of-bounds
  accesses for a pointee larger than one byte. Convert the loaded byte count
  into an element count by dividing it by the element size (`bound /
  sizeof(element)`) so the check can compare the (element) index directly.
  `counted_by` already counts elements and is unchanged; a void (or otherwise
  zero-/unknown-sized) pointee uses the GNU convention of element size 1, i.e.
  no conversion. Signed division is used for a signed count field so a negative
  byte count stays non-positive and is still rejected by the negative-bounds
  guard in EmitBoundsCheckImpl (unsigned division would turn it into a large
  positive count).

* __counted_by_or_null / __sized_by_or_null: a null pointer describes no
  accessible memory, but the null case was ignored, so an access through a
  null pointer with a non-zero count was not trapped. Treat the bound as 0
  when the pointer is null.

Both adjustments are applied to the loaded bound in
EmitCountedByBoundsChecking before it is handed to EmitBoundsCheckImpl;
the bounds-checking machinery and the runtime handler are left unchanged
and continue to work in terms of the element index. Plain `__counted_by`
and the general array-bounds path are unaffected.

I did try an alternative implementation where the comparison was done in
terms of bytes (rather than elements which this patch does), but this
had several disadvantages:

* It was less correct. Because the element-count implementation uses
  truncating division, a `__sized_by()` whose size is not a multiple of the
  pointee size rounds the bound down, so an access to a partially in-bounds
  object is caught. A byte-level check would instead only verify that the
  first byte of the object is in bounds, which would allow accessing a
  partially in-bounds object.

* When benchmarking (small microbenchmarks targetting arm64e), it
  appeared that the division implementation was faster. Note that an
  unsigned count field and an element size that is a power 2 produces
  the best LLVM IR because unsigned division by a power of 2 can be
  turned by the optimizer into a `lshr` instruction.

* It required changes in `CodeGenFunction::EmitBoundsCheckImpl` as well,
  which made the change more invasive.

Note while working on this I did discover an existing bug where if the
base (e.g. `side_effect()->ptr[0]`) has a side-effect it is incorrectly
evaluated multiple times. Fixing that is out-of-scope for this patch and
will be handled later.

Assisted-by: Claude Code

rdar://184139694


  Commit: 8bb425fa00159836c4faeed9fb0b455da38b3365
      https://github.com/llvm/llvm-project/commit/8bb425fa00159836c4faeed9fb0b455da38b3365
  Author: Dan Liew <dan at su-root.co.uk>
  Date:   2026-09-14 (Mon, 14 Sep 2026)

  Changed paths:
    M clang/lib/CodeGen/CGExpr.cpp

  Log Message:
  -----------
  Refactor the bounds value computation for
`__sized_by`/`__sized_by_or_null` pointers for readability.


Compare: https://github.com/llvm/llvm-project/compare/5151d293b0df...8bb425fa0015

To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications