Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use runtime_CPPFLAGS also to build .*pic.o files #11

Closed
wants to merge 57 commits into from
Closed

Conversation

shym
Copy link
Owner

@shym shym commented Sep 4, 2024

In commit 31cdf41 from ocaml#12589, the runtime_CPPFLAGS were dropped from the OC_CPPFLAGS for .*pic.o object files by accident, as at least -DCAMLDLLIMPORT= is necessary to build the Cygwin port without warnings.
Without that PR, the build with -Werror ends up with:

gcc -O2 -fno-strict-aliasing -fwrapv -g -Wall -Wint-conversion -Wstrict-prototypes -Wold-style-definition -Wundef -Wold-style-declaration -Wimplicit-fallthrough=5 -Werror -fno-common -fexcess-precision=standard -Wvla -fno-tree-vrp  -pthread -I ./runtime -I ./flexdll  -D_FILE_OFFSET_BITS=64 -U_WIN32   -o runtime/alloc.bpic.o -c runtime/alloc.c
runtime/alloc.c:33:18: error: 'caml_alloc' redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes]
   33 | CAMLexport value caml_alloc (mlsize_t wosize, tag_t tag)
      |                  ^~~~~~~~~~
...

A complete run is currently available here.

I assume it was accidental as I didn’t find any mention of the fact that -DCAMLDLLIMPORT and -DIN_CAML_RUNTIME were dropped on purpose, either in the commit message or in the PR.

I’ve added a quick mashup of the Unixy CI workflow with the workflow we use in multicoretests to torture Cygwin to test this fix, but it’s not intended to be part of what is merged, if I understood correctly the plans for the Cygwin CI return.

gasche and others added 30 commits July 31, 2024 21:55
… default

This un-does a small pessimization of ocaml#13076, which was analyzed in
details in
ocaml#13076 (comment), but
at the time was believed to not affect any program in the wild.

Since then Nick Roberts found instances of this pattern involving
or-patterns and polymorphic variants.

Thanks to the new presentation of partiality information that
distingues the "global" and "current" information, the pessimization
is now easy to undo, as done in this commit.
The [check_partial] heuristics are a coarse-grained approach degrade
matching totality information in certain cases -- when clauses contain
both reads of mutable fields and either guards or forcing of lazy
patterns.

It is not quite correct (it is not enough to prevent ocaml#7241), and is
not sufficient conceptually for OCaml Multicore. In a Multicore world,
other domains may race to update values concurrently, so we cannot
assume that only a fixed set of matching constructions may result in
side-effects.

This heuristic is subsumed by the recent changes to:
  - make context information accurate by binding mutable fields eagerly
  - make totality information accurate by tracking the mutability of
    the current position
and can now be retired.

Note: the [check_partial] function also contained hidden logic to deal
with the pattern-matching programs with no cases at all (more
precisely, those that have only refutation cases, like `function _ ->
.`), we retain this logic and move it to `toplevel_handler`.
gethostbyname is deprecated (replaced by getaddrinfo). The previous
stub had some mistakes: it was missing a return type, and a call to
caml_string_is_c_safe.
manual: use `rmdir` instead of `gethostbyname` in C stub example
Emit major slice counters in the runtime events
…zation-fix

Matching compilation: fix a pessimization from ocaml#13076
The Pattern-Matching Bug: fix totality information
…partial

The Pattern-Matching Bug: remove `check_partial`
The -pp option never behaved as described.
…caml#13390)

* Implement the --disable-ocamlobjinfo option for the configure script

This allows to disable the build of the ocamlobjinfo tool.
When used in conjunction with --disable-native-compiler,
it makes it possible to avoid the build of Flambda completely
while one wants to test only the bytecode compiler.

Indeed, when building only the byte code compiler, it's only because of
its use in ocamlobjinfo that Flambda gets built.
In trunk, all atomic functions exposed in the runtime are also exposed
as language primitives in our intermediate representations (lambda,
clambda). But except for `Patomic_load`, which benefits from
dedicated code generation, they are all transformed into C calls
on all backends.

The present PR simplifies the code noticeably by removing the
intermediate primitives, by producing C calls directly in
lambda/translprim.ml.

This reduces the amount of boilerplate to modify to implement
atomic record fields (ocaml/RFCs#39).

Co-authored-by: Clément Allain <[email protected]>
If the evaluation of one of the arguments to caml_alloc_3 triggers a
garbage collection, the other arguments will not be valid anymore.
It checks itself the value of err.
Its definition is already guarded by the DEBUG macro.
MisterDA and others added 27 commits August 26, 2024 08:51
Calling caml_c_thread_unregister() will (amongst many other things)
free the caml_thread_t signal_stack entry of the current thread.  The
OCaml runtime initializes the main thread using special code in
caml_thread_domain_initialize_hook, but this leaves the signal_stack
entry uninitialized.

If you use glibc tunables to enable malloc checking, and especially
use the malloc.perturb feature[1], then uninitialized areas of memory
are filled with a repeating pattern.  This causes the process to crash
if you free an uninitialized pointer.

Thus any program which calls caml_c_thread_unregister on the main
thread, and is using glibc malloc checking, will crash.

Fix this by initializing the signal_stack to NULL (since free(NULL) is
permitted and does nothing).

This was found when debugging a problem in the nbdkit OCaml plugin[2].

[1] https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html
[2] https://discuss.ocaml.org/t/free-uninitalized-data-when-calling-caml-c-thread-unregister-on-the-main-thread/15198

Fixes: ocaml#13400
Signed-off-by: Richard W.M. Jones <[email protected]>
otherlibs/systhreads/st_stubs.c: Avoid free of uninitialized pointer
There are now no ports/architectures which aren't available in 5.x
[refactor] simplify the definition of atomic functions
…te info

The Patomic_load primitive represents atomic loads (Atomic.get), and
it tracks whether the load type only contains immediate values, or
whether it contains both immediates and pointers. This comes at a
small cost in complexity in the compiler source, but is in fact
completely useless for two reasons:

1. We made a mistake in atomic.mli, which results in the information
   *never* being upgraded to Immediate: we declare
   `val get : 'a t -> 'a` instead of `external get : ...`,
   and as a consequence the compiler never specializes this primitive.

   (You can test this by compiling
     `let f (n : int Atomic.t) = Atomic.get n`
    and looking at the lambda or cmm output.)

2. The backend does not in fact depend on whether the value is
   immediate or not: even if we did track this information correctly,
   there would be no change in the generated code.
   (We checked this by reviewing atomic-load code production in all
   emit.mlp backends.)

This simplification will help (modestly) simplify the code of an
upcoming PR on atomic record fields.

Co-authored-by: Clément Allain <[email protected]>
Reviewed-by: KC Sivaramakrishnan <[email protected]>
[refactoring] Simplify the Patomic_load primitive by dropping immediate information
Fix "Warning: Element modname_from_source not found"
It only makes sense to confirm the domain interruptor is in running state
if there are interrupts to handle. There is a possibility of a domain
exiting while the backup thread, in BT_IN_BLOCKING_SECTION state, is
waiting for the domain lock; it will then proceed to invoke handle_incoming()
as there had been pending interrupts at the time it tried to acquire the
domain lock, but domain termination has taken care of them.

Therefore handle_incoming() should be safe to invoke with the interruptor in
non-running state, as long as there are indeed no pending interrupts.
The temporary r12 can be destroyed by a PLT shim.
Instead, use r27 (a non-temporary register, not used at OCaml function entry).
[runtime] Fix backup thread vs interrupt race
Remove the dependency of st_bt_lock_acquire() and st_bt_lock_release()
on st_masterlock.
In commit 31cdf41, the
`runtime_CPPFLAGS` were dropped from the `OC_CPPFLAGS` for .*pic.o
object files by accident, as at least `-DCAMLDLLIMPORT=` is necessary to
build the Cygwin port without warnings
@shym shym closed this Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.