Type spreads of regular variants in patterns

[zth]

Solves part of https://github.com/rescript-lang/rescript-compiler/discussions/6273 Closes https://github.com/rescript-lang/rescript-compiler/issues/6562 Closes https://github.com/rescript-lang/rescript-compiler/issues/6277

This PoC adds support for type spreads of regular variants in patterns. This is already a thing with polyvariants, and now it's also a thing for regular variants.

Rationale is we already have variant spreads and coercion, which lets you go from small type to big type. Having this will let you go from big type to small type. This is a missing piece.

Specification:

• Any variant X that is a subtype of variant Y can be spread in a pattern matching of a value of type Y
• Using an alias ...subtypeVariant as value will give the type subTypeVariant to value
• Syntax: Spreads of regular variants are done like spreads of polyvariants, but without the leading #: | ...subtypeVariant as value => doSomethingWithSubTypeVariant(value)

type a = One | Two | Three
type b = | ...a | Four | Five

let doWithA = (a: a) => {
  switch a {
  | One => Js.log("aaa")
  | Two => Js.log("twwwoooo")
  | Three => Js.log("threeeee")
  }
}

let lookup = (b: b) =>
  switch b {
  | ...a as a => doWithA(a)
  | Four => Js.log("four")
  | Five => Js.log("five")
  }

Should be a well isolated feature, since it's dependent on the parser adding an attribute when the spread is a regular variant (no leading #) and no regular pre-existing code path is touched.

[kayhayen]

Nuitka 1.6 is not too relevant anymore.

[n-bes]

@kayhayen Read more carefully please - example 2 based on fresh version: 2.3

[kayhayen]

@n-bes You are right, but 1.6 stopped me from reading on.

[kayhayen]

At some point a commercial customer provided information about Nuitka and these kinds of reports. I do not recall them being very actual. I think it was copying unused memory and such, resulting in the micro-optimization of the dictionary copy, but it wasn't a real bug in that sense.

I still have what I used to check it, from those days:

ASAN_OPTIONS=detect_leaks=0 LDFLAGS="-fsanitize=address" CCFLAGS="-Og -fno-omit-frame-pointer -fsanitize=address -w -DNDEBUG" NUITKA_EXTRA_OPTIONS="--clang --lto=no --unstripped --static-libpython=no" python3.10 tests/basics/run_all.py search

I am currently re-running that, with 3.10 and then 3.12 maybe. It seems that HWAsan is something different, can you provide me with the flags to use to get that, I am not all that familiar with any of this.

[n-bes]

I do not recall them being very actual

False-positive reports are very rare, please, do not ignore them.

I think it was copying unused memory and such, resulting in the micro-optimization of the dictionary copy, but it wasn't a real bug in that sense.

Coping unused memory is not marked as a problem by ASAN/HWasan, only incorrect memory access (out-of-bounds, use of uninitialized memory, etc.)

It seems that HWAsan is something different

Clang docs describes it.

Can you provide me with the flags to use to get that, I am not all that familiar with any of this.

All information available in the repo, useful links:

• https://github.com/n-bes/nuitka-crash-poc/blob/init/Dockerfile#L65
• https://github.com/n-bes/nuitka-crash-poc/blob/init/Dockerfile#L90
• https://github.com/n-bes/nuitka-crash-poc/blob/init/docker-compose.yaml#L136

[n-bes]

@kayhayen reopen?

[kayhayen]

I actually resolved them all, what I was saying is that they were not observable issues, like allocating or copying more memory than necessary, but never in a way that causes overflow.

[kayhayen]

I am not sure, if I have AArch64 hardware, does my M1 mac quality for that? Otherwise I have 64 bit ARM, but not too much. The code you compiled looks very small-ish. Can you maybe provide the lines around the one where your stack trace points to, if I am never going to have it?

Also, I only ever checked programs like that, never modules, I might have to do that too.

[kayhayen]

From what I am reading, ASAN should be good enough and not cost more than HWASAN, just at a lower cost, so I don't have to worry about using it. Should issues be apparent in either? Can you confirm to me, that my theory holds for the 2.3 issue you are seeing?

[kayhayen]

It seems that I was getting linker errors for module builds. I updated from llvm-15 to llvm-19, and now it no longer works at all for me locally on my Ubuntu 22.04. So I downgraded to llvm-14 as in Ubuntu, and now it works again. The LLVM snapshots appears to be only so good.

However, it appears I need to build a Python that is built with ASAN in mind. I cannot quite see that in your Dockerfile, though, and maybe I need to try hwsanitize, maybe it doesn't need it.

[kayhayen]

So, I think I convinced it to compile semi-properly for packages

LD_LIBRARY_PATH=/usr/lib/llvm-14/lib/clang/14.0.0/lib/linux/ ASAN_OPTIONS=detect_leaks=0 LDFLAGS="-fsanitize=hwaddress -g -shared-libasan -fuse-ld=lld -Wl,--no-relax" CCFLAGS="-Og -fno-omit-frame-pointer -fsanitize=hwaddress -w -DNDEBUG" NUITKA_EXTRA_OPTIONS="--clang --lto=yes --unstripped" python3.10 tests/packages/run_all.py resume

However, getting FATAL: HWAddressSanitizer requires a kernel with tagged address ABI. in my WSL. Guess I need to spin up some Ubuntu it seems.

[n-bes]

I actually resolved them all, what I was saying is that they were not observable issues, like allocating or copying more memory than necessary, but never in a way that causes overflow.

I found a lot of null dereferences on generated code by static analysis tool and it is the main reason why I just run it with ASAN/HWasan and prove it.

I actually resolved them all, what I was saying is that they were not observable issues, like allocating or copying more memory than necessary, but never in a way that causes overflow.

I'll try to reproduce example 1 on latest nuitka version.

I am not sure, if I have AArch64 hardware, does my M1 mac quality for that? Otherwise I have 64 bit ARM, but not too much.

I don't know about m1, my local setup is M3 Max with Docker Desktop (kernel version is Linux d3e7e642290b 6.6.26-linuxkit)

It seems that I was getting linker errors for module builds. I updated from llvm-15 to llvm-19, and now it no longer works at all for me locally on my Ubuntu 22.04. So I downgraded to llvm-14 as in Ubuntu, and now it works again. The LLVM snapshots appears to be only so good.

I prepared Ubuntu 22.04 with clang-19, and it works (and crashes): https://github.com/n-bes/nuitka-crash-poc/blob/init/Dockerfile#L65

Can you maybe provide the lines around the one where your stack trace points to, if I am never going to have it? Also, I only ever checked programs like that, never modules, I might have to do that too.

See:

• source code: https://github.com/n-bes/nuitka-crash-poc/tree/init/ubuntu-2204-hwasan-env-with-nuitka/module.build
• stack trace: https://github.com/n-bes/nuitka-crash-poc?tab=readme-ov-file#ubuntu-2204-hwasan-env-with-nuitka

However, it appears I need to build a Python that is built with ASAN in mind. I cannot quite see that in your Dockerfile, though, and maybe I need to try hwsanitize, maybe it doesn't need it.

I think that compiled python interpreter from sources with ASAN should be enough to detect problems. It is my first HWAsan launch, so ... ASan also replace allocator and can't detect the problem, for now we may only guess about results.

[kayhayen]

The code in question is this:

    if (_Py_PackageContext != NULL) {
        module_full_name = _Py_PackageContext;
    }

For 3.12 that is using this:

#if PYTHON_VERSION >= 0x3c0
#define _Py_PackageContext (_PyRuntime.imports.pkgcontext)
#endif

and

static char const *module_full_name = %(module_name_cstr)s;

I am not getting what that should mean in terms of a illegal write. It would appear that in libpython it's defined similar,

const char * pkgcontext;

So, can you explain what this means. Is it complaining that I am copying a pointer into a DLL (the extension module) that is exported from the main process?

[kayhayen]

I didn't see the use of llvm.sh. Unfortunately my Virtualbox hates even after an update the Ubuntu 24.04 installation so hard, it hurts, kind of surprising. I avoid working with Docker when I can, I have had issues on macOS with architecture mismatches, but you probably are using arm there.

[kayhayen]

I found a lot of null dereferences on generated code by static analysis tool and it is the main reason why I just run it with ASAN/HWasan and prove it.

These would probably be of interest, but lets just say, the stacktraces you provided, are clearly not one of those. It does a NULL check after all, but even if it didn't that would copy a NULL pointer and that would be an issue only later, most definitely a crash with segfault.

[kayhayen]

I gave macOS a try, and for packages, it refuses to work, because the Python is not instrumented, but for programs, it seems to not find any stuff with -fsanitize=address and that's good and all. The hwaddress is not support for arm macOS proper it seems.

So, I think you reproduced the results I once had with 1.6 and I don't feel like their is new evidence. The report given I would have to read up what it could even mean, but I am like 100% sure that for pre-3.12 it is definitely not a bug, and for 3.12 it is working pretty well for getting the right name to use. Also, in your example, it probably should be NULL, I can check that.

Something that might be wrong, now that I think of it, that it might have to do a strdup if it was to use it that much later, but I think it doesn't do that, i.e. when the module returns, the usage is stopped or should be. But I don't think that is what the message means, what does it mean?

[kayhayen]

I have pushed to the factory branch a change, where the assignment only happens when the names differ and now it is done via strcpy so your stacktrace should change. I am still not sure how valid it is, but checking PyModule_Create doesn't see to remember the string other than as Python objects, but the md_def remains in the module, but it seems the name is never used, but it's better to be safe than sorry, dangling pointers are not a good thing.

Please retest with current factory state, and lets see what pops up. I would expect nothing, since I didn't understand hwaddress to be better. Yet, I didn't have a chance to see module mode myself. But I am not entirely inclined right now to reproduce what I feel is a false alarm. Also, does it only give a single report at a time?

[n-bes]

python3.10

In Ubuntu 22.04 python version is 3.10

I'll try to reproduce example 1 on latest nuitka version.

Do not reproduced with gcc (ASAN) and factory version of nutika. I'll try with clang and HWASAN.

So, can you explain what this means. Is it complaining that I am copying a pointer into a DLL (the extension module) that is exported from the main process?

I think that something happens while trying to load module in memory, not related with copying of pointer.

Please retest with current factory state, and lets see what pops up.

Ready:

• https://github.com/n-bes/nuitka-crash-poc/tree/init?tab=readme-ov-file#ubuntu-2404-hwasan-factory-env-with-nuitka
• https://github.com/n-bes/nuitka-crash-poc/blob/init/docker-compose.yaml#L187

Also, does it only give a single report at a time?

Yes, it is stopped on first error.

[kayhayen]

I am not convinced that your report is not an issue with hwsanitizer at all. And I would ask you to use not module mode, but executable mode and find something. It seems I wasn't able to do that, and it seems while it links an extension module, that doesn't mean, it likes being run inside the DLL only while talking to the outside. So until there is new evidence that doesn't just look like a false alarm, I am closing this, sorry.

[kayhayen]

I would of course be interested in your static analysis that claims there might be errors in the code, even if providing the evidence for it is not as easy as you hope to be, it could be something and I can of course decide if it's true.

[n-bes]

@kayhayen I added results to the repository in two formats:

• plain-text
• sarif

[kayhayen]

So, the ones about unreachable code in generated code make no sense:

* UNREACHABLE_CODE: The condition in ternary operator is always true at HelpersComparisonGe.c:364.
    ternary operator at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersComparisonGe.c:364

This is generated from a Jinja2 template, that is using a bool to nuitka_bool conversion with the explicit intent to let the C compiler resolve this at its compile time rather than creating unnecessarily complex template code. If you can, ignore these things in those files.

Similarily this one makes no sense to me either.

    default:
        NUITKA_CANNOT_GET_HERE("invalid PYGEN_ result");
    }

I have those to catch implementation errors at runtime. Having a default is a good practice, is it not.

Then those

* TAINTED_PTR.MIGHT: Variable 'home_path' is obtained from an untrusted source at HelpersFilesystemPaths.c:951 by calling function 'getenv'. The variable is used in a vulnerable operation at HelpersSafeStrings.c:57 by passing as 2nd parameter to function 'appendStringSafe' at HelpersFilesystemPaths.c:963. Please double check if input string is properly validated in error-prone manner
    [vulnerable operation] Variable 'home_path' is passed to function 'appendStringSafe' as 2nd parameter at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersFilesystemPaths.c:963
    [tainted pointer] Call of 'getenv' at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersFilesystemPaths.c:951

These functions e.g. appendStringSafe will abort on overflows, and thus the usage is safe.

And this

            coercion c2 =
                (type2->tp_as_number != NULL && NEW_STYLE_NUMBER_TYPE(type2)) ? type2->tp_as_number->nb_coerce : NULL;

Again generated code, NEW_STYLE_NUMBER makes it so that this doesn't happen on Python3.12, but it's not the only version this code is used with. Similar story, not making generated code do things, the C compiler will do for us.

This one is actually good:

* REDUNDANT_COMPARISON.ALWAYS_FALSE: Redundant comparison '0' > 'res' (0 > {[0, 1]}) is always false at HelpersBuiltin.c:624.
    comparison at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersBuiltin.c:624

I probably changed this function to be bool result at some point, and code still treats it as int result in a few places, I found at least one more spot.

Also this

* NO_VA_END: Function 'va_end' was not called before HelpersHeapStorage.c:31 inside function 'Nuitka_PreserveHeap'. (Function 'va_start' was called at HelpersHeapStorage.c:17.)
    [no va_end] no va_end inside function 'Nuitka_PreserveHeap' at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersHeapStorage.c:31

Seems not too relevant for the compilers and code I have, but the standard has a "must call" there, so doing it will probably just be something the C compiler knows if it needs it or not.

* NEGATIVE_CODE_ERROR.EX: Variable 'wb.len', which might receive a negative value at HelpersOperationInplaceAddUtils.c:71, is used without checking at HelpersOperationInplaceAddUtils.c:91 by calling function 'memcpy'.
    [value use] Variable 'wb.len' is passed to function 'memcpy' as 3rd parameter at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersOperationInplaceAddUtils.c:91
    [error code] Assign the negative value to variable 'wb.len' at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersOperationInplaceAddUtils.c:71

In this specific case, the function is known to always work, and in fact wb.len may not make sense to be set. Adding an assert(wb.len >= 0); can of course affirm that.

This one is in line with what Python does:

* HANDLE_LEAK.EX: The handle 'handle' is created at MetaPathBasedLoader.c:766 by calling function 'dlopen' and lost at MetaPathBasedLoader.c:823.
    [acquire] Call of 'dlopen' at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/MetaPathBasedLoader.c:766
    [leaked] leak at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/MetaPathBasedLoader.c:823

Since we use the code from the DLL with dlsym, releasing it seems not possible.

This one

* BUFFER_SIZE_MISMATCH: Pointer 'args', referencing a memory location of size 8 bytes, is passed as a parameter to function 'memcpy' at HelpersCallingGenerated.c:1577 by calling function 'memcpy' with a size parameter that is always equal to 16. This can lead to a buffer overflow.
    Variable 'args' is passed to function 'memcpy' as 2nd parameter at /usr/local/lib/python3.12/dist-packages/nuitka/build/static_src/HelpersCallingGenerated.c:1577

Seems to be about a false alarm, the memcpy is done on a PyObject *const *args = &PyTuple_GET_ITEM(pos_args, 0); and two array elements are intended to be copied here memcpy(python_pars, args, 2 * sizeof(PyObject *)); which would be fine. I am not sure if helping the tool via &args[0] can help it, this is just a wrong detection. The size is statically known to be 2 and even asserted in this generated code assert(PyTuple_GET_SIZE(pos_args) == 2); so that is false alarm.

There is one on Nuitka_Generator_send which checks for NULL, but only once. That is indeed unnecessary, because it's used as a single argument function, so the one check there is can actually be removed and replaced by an assertion I guess.

[kayhayen]

I have a bunch of changes on factory, I invite you to re-run this once it hits develop or now. Factory is unusually a bit unstable to run, but I guess your analysis need not care. If possible ignore list the generated C helper issues, that I addressed. For the LOOKUP_ATTRIBUTE, I mean to add an asserting variation, as for some values, it's known they will not raise an error. For the UNICODE_APPEND one I need to find a strategy, but it seemed the lines didn't quite match, and it's not important yet.

[n-bes]

Seems to be about a false alarm

Some memory errors definitely exist, I trust HWasan.

I invite you to re-run this once it hits develop or now.

Ready: https://github.com/n-bes/nuitka-crash-poc/tree/init/ubuntu-2404-factory-gcc-env

python3 -m nuitka --module --show-scons --include-package=module --debug module --jobs=1 --lto=no

If possible ignore list the generated C helper issues

Filter the report as you want with jq:

cat nuitka-poc-ubuntu-2404-factory-gcc-env.svres.json  | jq '.WARNS[] | select(.FILE | startswith("/usr") | not)'

{
  "WARN_ID": 1027,
  "WARN_CLASS": "DEREF_OF_NULL.COND",
  "TAGS": "",
  "MTID": "SvEng.ND.20",
  "FILE": "/code/module.build/module.module.c",
  "LINE": 245,
  "FUNC": "MAKE_FUNCTION_module$$$function__1_print_variable",
  "ORIG_FUNC": "MAKE_FUNCTION_module$$$function__1_print_variable",
  "DETAILS": "'0';10th;Nuitka_Function_New;",
  "MSG": "Pointer '0', which has a NULL value at module.module.c:245, is passed as 10th parameter in call to function 'Nuitka_Function_New' at module.module.c:245, where it may be dereferenced at CompiledFunctionType.c:1280.",
  "STATUS": "Default",
  "COMMENT": "",
  "LANG": "C_Cpp",
  "TOOL": "SvEng",
  "TRACES": []
}

[kayhayen]

The example you gave is a memcpy with size 0 on a NULL pointer, and I don't want to make it slower by adding a check. However, since it's common not to have a closure, I consider specializing this case, as I have done before, or just outright attaching the closure to the function after it has been created. The memcpy(NULL, NULL, 0) is not defined behavior according to C standards, but I won't care much about that. I am adding an assertion that actually confirms that like assert(closure_given == 0 || closure != NULL); and maybe it's going to help.

[kayhayen]

I don't think I looked at HWSan results with findings yet. The module mode is not suited. Have you produced results that I have not discovered yet. I only looked at the static analysis results recently.

This static analysis tool is not reliable and doesn't know enough about the code. I am done with looking at it. What remains are false alarms to me. The LOOKUP_ATTRIBUTE stuff, I have taken note of to be dealt with when I write Jinja2 template code to generate the existing code helpers, such that a expected-not-possible-to-fail one will be easy to add, but that's not now. It is however not a possible issue to not check for NULL there.

Part of the changes are in the hotfix, part are in develop, and a few minor things were added for factory just now.

[kayhayen]

This is part of the stable release 2.4 that I just made.