Handle AArch32 immediates and literal/displacement ranges by fhunleth · Pull Request #23 · stritzinger/otp

fhunleth · 2026-06-27T15:47:51Z

To see a crash that this fixes, run memory(atom). This was the key change that got the ARM32 JIT running on a small Nerves project.

ziopio · 2026-06-30T08:14:14Z

I do not experience the crash on my emulator, but this could be something really dependant on your call args and maybe also the fact that is a real system.

Is this happening only on the hardware or did it happen also in the emulator?
What is your development branch? I might inspect it to see further differences in your build config.
Can you give me the command line you used to trigger the error?
also if you did use another configuration file, could you give it to me?

I tried to build with debug and I cannot reproduce the error, I get instead:

1> memory(atom).
** exception error: notsup
     in function  erlang:memory/1
        called as erlang:memory(atom)
        *** argument 1: invalid memory type option
2>

This is happening because many allocators are disabled by default with -DSMALL_MEMORY in the configure settings

So I tried to add -Mea max -MTe true to the boot args to enable everything and I got it to work:

Eshell V15.0 (press Ctrl+G to abort, type help(). for help)
1> memory(atom).
209005

ziopio

There are multiple changes that hide the real FIX, most of them are good addition, but need to be separated:
Ideally we need to separate:

actual fix
eventual refactoring if needed
optimizations/enhancements
cleanups

See my comments, I can help of course to move code and refactor the code where necessary, I would really like to reproduce the crash you are getting...

ziopio · 2026-06-30T09:06:55Z

        }
    }

+    void safe_str(a32::Gp gp, arm::Mem mem) {


Are these safe_* wrappers really needed in BeamAssembler? in case they are, I would like to reduce code repetition as much as possible.

ziopio · 2026-06-30T09:08:07Z

+    void safe_str(a32::Gp gp, arm::Mem mem) {
+        size_t abs_offset = std::abs(mem.offset());
+        auto offset = mem.offset();
+        constexpr size_t max_disp = 4095;


Check the Displacement enum, it is better to use disp4KB, I know it is not defined for the BeamAssembler, if needed we would have to refactor this to avoid code repetitions.

ziopio · 2026-06-30T09:30:08Z

    /* Save our return address in c_p->i so we can tell where we crashed if we
     * did so during GC. */
-    a.str(a32::lr, arm::Mem(c_p, offsetof(Process, i)));
+    safe_str(a32::lr, arm::Mem(c_p, offsetof(Process, i)));


You do not need safe_str here, offsetof(Process, i) is a predictable integer and we know it can be rappresented by an immediate with 12bits.

It is really important that the safe_str, safe_ldr ecc... functions are NOT spammed through the code base, I should probably comment a disclaimer on them.

They make the LDR/STR OP safe regarding the displacement handling. But they are also very dangerous if abused.

These functions use either TMP or VAR as temporary storage to compute and load the address. TMP and VAR are not an esclusive of these wrappers. This means that you could have callers of these functions storing important data on TMP and/or VAR and then use LDR and STR. In these cases, if you swap a a.ldr with a safe_ldr to fix a bug you could introduce a very HARD to detect register corruption...

So in your case I do not know which safe_* function solved your problem.
I really doubt every change here is necessary, so I would like to pin point the real fix and the check it is safe. It probably is safe since it works, but we cannot give it for granted...

I should rename it like: dynamic_str or something like that... on arm64 they have a really safe version, I kept the name, but did not get the luxury of safety...

Ah also sub and add can overwrite TMP and must be used with caution

ziopio · 2026-06-30T09:35:19Z

    a.orr(TMP, TMP, imm(F_DELAY_GC));
    a.str(TMP, flags);
-    a.str(ARG1, arm::Mem(c_p, offsetof(Process, i)));
+    safe_str(ARG1, arm::Mem(c_p, offsetof(Process, i)));


This is a perfect example: using safe_str here is not needed, if you would actually need it, it would corrupt TMP...

ziopio · 2026-06-30T10:02:53Z

+    bool rhs_is_arm_literal = RHS.isSmall() &&
+                              canEncodeAArch32Imm(
+                                      RHS.as<ArgSmall>().get() &
+                                      ~_TAG_IMMED1_MASK);


This is very cool! This enlarges and widely corrects the behaviour, but it is an optimization right? This allows to emit the faster branch in many more cases. Actually, since the tag is 0xF, the jit was always outputting suboptimal code.

But this would not fix a crash, I would like it to be a different commit since it looks like an optimization fix.

ziopio · 2026-06-30T10:04:53Z

+static bool canEncodeAArch32Imm(T value) {
+    uint32_t encoded;
+    return arm::Utils::encodeAArch32Imm(value, &encoded);
+}


ziopio · 2026-06-30T10:09:26Z

This file looks like a cleanup to make the configure script more coherent. This should be a different commit. I see this file is generated by autoconf, does autoconf needs to be updated too or is this just a configure update that you committed while the configure.ac is already fixed ?

ziopio · 2026-06-30T10:17:57Z

        a.align(AlignMode::kCode, 4);
        a.bind(pointer);
-        a.embedLabel(veneer.target);
+        a.embedLabel(veneer.target, 4);


this is defensive, as the asmjit should deduce that a label is 4 bytes by the target architecture, I prefer to not write it as it should not fix anything.

ziopio · 2026-06-30T10:18:04Z

        a.embedUInt32(value.as<ArgWord>().get());
    } else if (value.isLabel()) {
-        a.embedLabel(rawLabels.at(value.as<ArgLabel>().get()));
+        a.embedLabel(rawLabels.at(value.as<ArgLabel>().get()), 4);


ziopio · 2026-06-30T10:27:21Z

+                                 Constant{.latestOffset = std::max<ssize_t>(
+                                                   currOffset,
+                                                   maxOffset -
+                                                           STUB_CHECK_INTERVAL),


This is a safer and less optimal offset that will embed constants sooner and more often. This may or may not be part of the fix. It is not bad but I would like to know.

fhunleth · 2026-06-30T11:23:39Z

@ziopio Thanks for the super thorough review. I think it's best if you just pluck out what you like and then we throw the rest away.

The command that broke for me was running runtime_info at the IEx prompt. I really thought it was the call to get atom memory, but maybe it was a following one? https://github.com/elixir-lang/elixir/blob/b680bea383f93318ebebf275a0865d9f20c807bd/lib/iex/lib/iex/helpers.ex#L661

Handle AArch32 immediates and literal/displacement ranges correctly

41142ef

ziopio requested changes Jun 30, 2026

View reviewed changes

Uh oh!

Conversation

fhunleth commented Jun 27, 2026

Uh oh!

ziopio commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ziopio left a comment

Choose a reason for hiding this comment

Uh oh!

ziopio Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziopio Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhunleth commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ziopio commented Jun 30, 2026 •

edited

Loading

ziopio Jun 30, 2026 •

edited

Loading

ziopio Jun 30, 2026 •

edited

Loading