Improve portability of building libprism

[hasumikin]

Background

I'm investigating the possibility of making a compiler for mruby and PicoRuby using Prism.
To do that, I think some additional considerations in the build process of libprism will be helpful.

Summary of the patch

• Define PRISM_NO_MEMORY_MAP constant macro to exclude the memory map interface such as mmap(2). This makes libprism more portable, especially for a non-POSIX platform
• Include <inttypes.h> and use their macros to cover format specifiers of printf() among various platforms. Without this, building libprism against 32bit architecture raises errors that look like the following:

  src/prettyprint.c:6759:59: error: format '%d' expects argument of type 'int', but argument 3 has type 'uint32_t' {aka 'long unsigned int'} [-Werror=format=]
   6759 |                 pm_buffer_append_format(output_buffer, " %d\n", cast->number);
        |                                                          ~^     ~~~~~~~~~~~~
        |                                                           |         |
        |                                                           int       uint32_t {aka long unsigned int}
        |                                                          %ld
  

[hasumikin]

CFLAGS='-fanalyzer' bundle exec rake compile fails. I'll consider a way to resolve this successfully

[hasumikin]

It looks like CFLAGS='-fanalyzer' bundle exec rake compile was not working as originally intended in the main branch.
This way (CFLAGS='-fanalyzer' bundle...) cannot override the definition of CFLAGS in Makefile and the analysis doesn't work.
I will investigate more

[kddnewton]

Thank you very much for your contributions here!

First, I think you are correct and we should enhance our build system. I've opened #2189 for context and tracking.

Second, thank you for finding the issue with our clang-analyzer and gcc-analyzer. I'll get those things fixed.

Third, I am fine with the inttypes.h change, that is great. As for the PRISM_NO_MEMORY_MAP macro, I was wondering if you could take the approach we had previously where if mmap(2) didn't exist we would instead malloc/free the associated memory by reading the file? You can see it was removed here: #1224. You can see how it was done in this file: https://github.com/ruby/prism/blob/f8a231e0acdd4f5b5f6deeef18ea877169ddc7bd/src/util/yp_string.c.

[eregon]

• Define PRISM_NO_MEMORY_MAP constant macro to exclude the memory map interface such as mmap(2). This makes libprism more portable, especially for a non-POSIX platform

Do you have an example platform which is neither POSIX nor Windows?

[eregon]

I think this should move to https://github.com/ruby/prism/blob/main/include/prism/defines.h to ensure it's defined before any other system #include (otherwise these macros could be missing and the second include is noop).
I think it makes sense to also move the #include <inttypes.h> there.

[kddnewton]

• Define PRISM_NO_MEMORY_MAP constant macro to exclude the memory map interface such as mmap(2). This makes libprism more portable, especially for a non-POSIX platform

Do you have an example platform which is neither POSIX nor Windows?

He mentions this in build_system.md

[eregon]

Right, I see:

Arm Cortex-M0+ embedded system

but that sounds like a processor/arch and doesn't tell me which operating system/libc it is, and why that would not have mmap.
I know rather little about embedded systems though.

[hasumikin]

@kddnewton

I was wondering if you could take the approach we had previously where if mmap(2) didn't exist we would instead malloc/free the associated memory by reading the file?

I think that way works. (Additionally, we will need to wrap #ifdef HAVE_MMAP ... #endif around these lines:

prism/src/util/yp_string.c

Lines 7 to 10 in f8a231e

	#include <fcntl.h>
	#include <sys/mman.h>
	#include <sys/stat.h>
	#include <unistd.h>

)

[edit] Sorry, I was wrong. It wouldn't work because some systems don't have only MMAP but also a filesystem.
In general, loading Ruby scripts from a file needs to be an optional way in Prism (if it supports embedded systems)

[eregon]

[edit] Sorry, I was wrong. It wouldn't work because some systems don't have only MMAP but also a filesystem.
In general, loading Ruby scripts from a file needs to be an optional way in Prism (if it supports embedded systems)

Systems without a filesystem cannot run CRuby and so cannot run the Prism test suite, so then it seems very hard/impossible to test if Prism works correctly there.
Does MRuby or PicoRuby support running on embedded systems without a filesystem?

On such systems, is read(2) not even declared in headers? Or does it fail at runtime?

[kddnewton]

@eregon Arm Cortex-M0+ does not typically have a general purpose OS. It sometimes will have something like RTX or some other real-time operating system. picoruby supports compiling to these platforms using the standard C library, using a libc like glibc, Newlib or Newlib-nano. This information comes from the docs for the platform and runtime, respectively. You can learn more by reading them yourself.

@hasumikin you shouldn't have to load the contents directly from a Ruby file. This should be enough to get you started:

void parse(const uint8_t *source, size_t length) {
    pm_parser_t parser;
    pm_parser_init(&parser, source, length, NULL);

    pm_node_t *root = pm_parse(&parser);
    // do something here with root node

    pm_node_destroy(&parser, root);
    pm_parser_free(&parser);
}

I'm happy to support a PRISM_EMBEDDED define, HAVE_MMAP define, or something similar where we do not try to include the headers for mmap.

[hasumikin]

@eregon

(Let me add some to Kevin's comment)

Does MRuby or PicoRuby support running on embedded systems without a filesystem?

Yes, so the CORE implementation of mruby and PicoRuby doesn't depend on a filesystem.
But of course, a command line tool like mrbc, which compiles an mruby script file to an mruby VM code, uses the Unix filesystem.
When you build an mruby app for Unix, you selectively specify mruby-io mrbgem to support the filesystem and system call like read(2).

On such systems, is read(2) not even declared in headers?

No,

Or does it fail at runtime?

Generally, the compilation by gcc fails due to the lack of some functions.
Sometimes, a runtime error happens: eg) In Newlib, as far as I know, the regex.h exists while the regex.c (substance of the Regex) doesn't exist

---

@kddnewton

This should be enough to get you started:

Thanks, I could successfully compile my mruby/PicoRuby script on my Ubuntu :)

I'm happy to support a PRISM_EMBEDDED define, HAVE_MMAP define, or something similar where we do not try to include the headers for mmap.

Glad to hear that!
Now that I think:

• For now, I'm going to close this PR without merging
• Open a new Discussion to discuss the direction of embedded support in detail

(I mean this issue is kind of a decision of design including #2189 . So I think it would be better to have a meticulous discussion.)

What do you say?

[kddnewton]

@hasumikin yes, that makes sense. I've opened #2215 to take the inttypes.h work you did here, so we will at least have that merged. In this case I'll close this ticket.

[kddnewton]

I wonder if we can just check _POSIX_VERSION to determine if we can use mmap, and otherwise not.