Skip to content

Conversation

@guitargeek
Copy link
Contributor

According to the gfortran argument passing conventions, for any Fortran procedure, the compiler will automatically define a C prototype. This is what we use in h2root.

Note that for procedures like HROPEN that takes string arguments, the signature of the C prototype will have extra arguments for the string lengths, which we also have to include in our forward declaration and usage. However, the type of these was changed with GCC 8 to size_t, so we have to also account for that as recommended in [1]. Otherwise, we get undefined behavor, which causes the h2root test to fail on ARM64 with GCC 14.

[1] https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html

See also this PR that tests this commit on Alma 10 ARM, with GCC 14:

According to the `gfortran` argument passing conventions, for any
Fortran procedure, the compiler will automatically define a C prototype.
This is what we use in `h2root`.

Note that for procedures like `HROPEN` that takes string arguments, the
signature of the C prototype will have extra arguments for the string
lengths, which we also have to include in our forward declaration and
usage. However, the type of these was changed with GCC 8 to size_t, so
we have to also account for that as recommended in [1]. Otherwise, we
get undefined behavor, which causes the `h2root` test to fail on ARM64
with GCC 14.

[1] https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html
@guitargeek guitargeek requested a review from pcanal as a code owner November 26, 2025 13:26
@guitargeek guitargeek added the bug label Nov 26, 2025
@guitargeek guitargeek self-assigned this Nov 26, 2025
Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@ferdymercury
Copy link
Collaborator

Thanks a lot for this!
Could you remove the -O0 flag in
set_target_properties(minicern PROPERTIES COMPILE_FLAGS "-O0 -w")

to see if it's now good or if there are still issues somewhere?

As well as locally add:

set_target_properties(minicern PROPERTIES COMPILE_FLAGS "-fsanitize=undefined -fsanitize=address")
target_link_options(minicern BEFORE PUBLIC -fsanitize=undefined PUBLIC -fsanitize=address)

@guitargeek
Copy link
Contributor Author

Ok! I'll try in a separate PR after this one is merged, so we can see what happens

@ferdymercury
Copy link
Collaborator

ferdymercury commented Nov 26, 2025

Just tried your changes locally on Ubuntu 22, with sanitizer enabled, and I still get:

/opt/root_src/misc/minicern/src/zebra.f:6746:72: runtime error: load of address 0x555556df372c with insufficient space for an object of type 'integer(kind=4)'
0x555556df372c: note: pointer points here
  24 02 00 00 18 2f 0f 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
              ^ 
/opt/root_src/misc/minicern/src/zebra.f:6747:72: runtime error: load of address 0x555556df372c with insufficient space for an object of type 'integer(kind=4)'
0x555556df372c: note: pointer points here
  24 02 00 00 18 2f 0f 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
              ^ 
/opt/root_src/misc/minicern/src/zebra.f:6753:72: runtime error: store to address 0x555556df372c with insufficient space for an object of type 'integer(kind=4)'
0x555556df372c: note: pointer points here
  24 02 00 00 18 2f 0f 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
              ^ 
 Converting directory //example

but I guess that's an unrelated issue.

Copy link
Collaborator

@ferdymercury ferdymercury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "mac14-fortran=OFF" line could be removed now from the CI-config-file.

@guitargeek guitargeek merged commit aafddfb into root-project:master Nov 26, 2025
30 of 32 checks passed
@guitargeek guitargeek deleted the h2root branch November 26, 2025 16:18
ferdymercury added a commit to ferdymercury/root that referenced this pull request Nov 26, 2025
guitargeek pushed a commit that referenced this pull request Nov 26, 2025
guitargeek pushed a commit to guitargeek/root that referenced this pull request Nov 27, 2025
guitargeek pushed a commit that referenced this pull request Nov 28, 2025
KoljaFrahm pushed a commit to KoljaFrahm/root that referenced this pull request Nov 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants