[all-commits] [llvm/llvm-project] 5c25ff: [OCaml] Fix unsafe uses of Store_field

Josh Berdine via All-commits all-commits at lists.llvm.org
Mon Apr 5 02:59:08 PDT 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 5c25ff8739e013fec39bc8c6fc1df16e0e5041ca
      https://github.com/llvm/llvm-project/commit/5c25ff8739e013fec39bc8c6fc1df16e0e5041ca
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/analysis/CMakeLists.txt
    M llvm/bindings/ocaml/analysis/analysis_ocaml.c
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c
    M llvm/bindings/ocaml/llvm/llvm_ocaml.h
    M llvm/bindings/ocaml/target/CMakeLists.txt
    M llvm/bindings/ocaml/target/target_ocaml.c

  Log Message:
  -----------
  [OCaml] Fix unsafe uses of Store_field

Using `Store_field` to initialize fields of blocks allocated with
`caml_alloc_small` is unsafe. The fields of blocks allocated by
`caml_alloc_small` are not initialized, and `Store_field` calls the
OCaml GC write barrier. If the uninitialized value of a field happens
to point into the OCaml heap, then it will e.g. be added to a conflict
set or followed and have what the GC thinks are color bits
changed. This leads to crashes or memory corruption.

This diff fixes a few (I think all) instances of this problem. Some of
these are creating option values. OCaml 4.12 has a dedicated
`caml_alloc_some` function for this, so this diff adds a compatible
function with a version check to avoid conflict. With that, macros for
accessing option values are also added.

Differential Revision: https://reviews.llvm.org/D99471


  Commit: 58bb9222dd298a2a38b76817df18323167f095f7
      https://github.com/llvm/llvm-project/commit/58bb9222dd298a2a38b76817df18323167f095f7
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c

  Log Message:
  -----------
  [OCaml] Minor optimizations by avoiding double initialization

In several functions an OCaml block is allocated and no further OCaml
allocation functions (or other functions that might trigger allocation
or collection) are performed before the block is fully initialized. In
these cases, it is safe and slightly more efficient to allocate an
uninitialized block.

Also, the code does not become more complex after the non-initializing
allocation, since in the case that a non-small allocation is made, the
initial values stored are definitely not pointers to OCaml young
blocks, and so initializing via direct assignment is still safe. That
is, in general if `caml_alloc_small` is called, initializing it with
direct assignments is safe, but if `caml_alloc_shr` is
called (e.g. for a block larger than `Max_young_wosize`), then
`caml_initialize` should be called to inform the GC of a potential
major to minor pointer. But if the initial value is definitely not a
young OCaml block, direct assignment is safe.

Differential Revision: https://reviews.llvm.org/D99472


  Commit: e5b7fedc573c8f7977c8a4800144df6d341d8887
      https://github.com/llvm/llvm-project/commit/e5b7fedc573c8f7977c8a4800144df6d341d8887
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c

  Log Message:
  -----------
  [OCaml] Code simplification using option allocation functions

Using the `caml_alloc_some` and `ptr_to_option` functions that
allocate OCaml `option` values enables simplifications in many
cases. These simplifications also result in avoiding unnecessary
double initialization in many cases, so yield a minor optimization as
well.

Also, change to avoid using the old unprefixed functions such as
`alloc_small` and instead use the current `caml_alloc_small`.

A few of the changed functions were slightly rewritten in the
early-return style.

Differential Revision: https://reviews.llvm.org/D99473


  Commit: 2c82ea1849dc77eedeedb59a73c870717229ed37
      https://github.com/llvm/llvm-project/commit/2c82ea1849dc77eedeedb59a73c870717229ed37
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c

  Log Message:
  -----------
  [OCaml] Code simplification using string allocation functions

Using the `cstr_to_string` function that allocates and initializes an
OCaml `string` value enables simplifications in several cases. This
change also has the effect of avoiding calling `memcpy` on NULL
pointers even if only 0 bytes are to be copied.

Differential Revision: https://reviews.llvm.org/D99474


  Commit: d9bbd9864578204fb0bdeea685d0bcfda2f0aecf
      https://github.com/llvm/llvm-project/commit/d9bbd9864578204fb0bdeea685d0bcfda2f0aecf
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c

  Log Message:
  -----------
  [OCaml] Omit unnecessary GC root registrations

The current code does not follow the simple interface to the OCaml GC,
where GC roots are registered conservatively, only initializing
allocations are performed, etc. This is intentional, as stated in the
opening file comments. On the other hand, the current code does
register GC roots in many situations where it is not strictly
necessary. This diff omits many of them.

Differential Revision: https://reviews.llvm.org/D99475


  Commit: 8e4fc55a0e544d7d370a2d7689dbb622b2caefca
      https://github.com/llvm/llvm-project/commit/8e4fc55a0e544d7d370a2d7689dbb622b2caefca
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/all_backends/all_backends_ocaml.c
    M llvm/bindings/ocaml/analysis/analysis_ocaml.c
    M llvm/bindings/ocaml/backends/backend_ocaml.c
    M llvm/bindings/ocaml/bitreader/bitreader_ocaml.c
    M llvm/bindings/ocaml/bitwriter/bitwriter_ocaml.c
    M llvm/bindings/ocaml/debuginfo/debuginfo_ocaml.c
    M llvm/bindings/ocaml/executionengine/executionengine_ocaml.c
    M llvm/bindings/ocaml/irreader/irreader_ocaml.c
    M llvm/bindings/ocaml/linker/linker_ocaml.c
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c
    M llvm/bindings/ocaml/llvm/llvm_ocaml.h
    M llvm/bindings/ocaml/target/target_ocaml.c
    M llvm/bindings/ocaml/transforms/ipo/ipo_ocaml.c
    M llvm/bindings/ocaml/transforms/passmgr_builder/passmgr_builder_ocaml.c
    M llvm/bindings/ocaml/transforms/scalar_opts/scalar_opts_ocaml.c
    M llvm/bindings/ocaml/transforms/utils/transform_utils_ocaml.c
    M llvm/bindings/ocaml/transforms/vectorize/vectorize_ocaml.c

  Log Message:
  -----------
  [NFC][OCaml] Remove vestigial CAMLprim declarations

The CAMLprim macro has not been needed since OCaml 3.11, and is
defined to the empty string. This diff removes all instances of it.

Differential Revision: https://reviews.llvm.org/D99476


  Commit: f4d156aed0f8335a522a032f714b40d06449e720
      https://github.com/llvm/llvm-project/commit/f4d156aed0f8335a522a032f714b40d06449e720
  Author: Josh Berdine <josh at berdine.net>
  Date:   2021-04-05 (Mon, 05 Apr 2021)

  Changed paths:
    M llvm/bindings/ocaml/bitreader/bitreader_ocaml.c
    M llvm/bindings/ocaml/debuginfo/debuginfo_ocaml.c
    M llvm/bindings/ocaml/executionengine/executionengine_ocaml.c
    M llvm/bindings/ocaml/irreader/irreader_ocaml.c
    M llvm/bindings/ocaml/linker/linker_ocaml.c
    M llvm/bindings/ocaml/llvm/llvm_ocaml.c
    M llvm/bindings/ocaml/target/target_ocaml.c
    M llvm/bindings/ocaml/transforms/passmgr_builder/passmgr_builder_ocaml.c
    M llvm/bindings/ocaml/transforms/scalar_opts/scalar_opts_ocaml.c
    M llvm/bindings/ocaml/transforms/utils/transform_utils_ocaml.c

  Log Message:
  -----------
  [NFC][OCaml] Reformat to clean up following CAMLprim removal

The removal of CAMLprim left the code in need of an application of
clang-format. There are various other changes made by clang-format
which it seems ought to be rolled together into this diff.

Differential Revision: https://reviews.llvm.org/D99477


Compare: https://github.com/llvm/llvm-project/compare/162848654842...f4d156aed0f8


More information about the All-commits mailing list