8000 CG_SIZE_MAX not enclosed in protecting CGNS_ENUMV · Issue #816 · CGNS/CGNS · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

CG_SIZE_MAX not enclosed in protecting CGNS_ENUMV #816

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.

Sign up for GitHub

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

Jump to bottom
Open
pavanakumar opened this issue Jan 7, 2025 · 14 comments
Open

CG_SIZE_MAX not enclosed in protecting CGNS_ENUMV #816

pavanakumar opened this issue Jan 7, 2025 · 14 comments
Assignees
@brtnfld
Labels
Component - Build Priority - Minor 🔽 Type - Bug
Milestone
v4.6.0

Comments

@pavanakumar
Copy link

CG_SIZE_MAX must be protected using the macro CGNS_ENUMV. Otherwise it throws compile errors in gcc version 14.2.0 (Homebrew GCC 14.2.0_1).

Giving below the patch (diff)

--- a/src/cgnslib.c
+++ b/src/cgnslib.c
@@ -6920,7 +6920,7 @@ int cg_elements_general_write(int fn, int B, int Z, int S,

         /* create new element connectivity array */

-        if (newsize > CG_SIZE_MAX / sizeof(cgsize_t)) {
+        if (newsize > CGNS_ENUMV(SIZE_MAX) / sizeof(cgsize_t)) {
             cgi_error("Error in allocation size for new connectivity data");
             return CG_ERROR;
         }
@@ -7038,7 +7038,7 @@ int cg_elements_general_write(int fn, int B, int Z, int S,

         if (read_parent_data(section)) return CG_ERROR;

-        if((cnt*newsize) > CG_SIZE_MAX/sizeof(cgsize_t) ) {
+        if((cnt*newsize) > CGNS_ENUMV(SIZE_MAX)/sizeof(cgsize_t) ) {
             cgi_error("Error in allocation size for new ParentElements data");
             return CG_ERROR;
         }
@@ -7321,7 +7321,7 @@ int cg_poly_elements_general_write(int fn, int B, int Z, int S,

             if (m_trail_size > 0){
                 /* partial load trailing elements that will be relocated */
-                if (m_trail_size > CG_SIZE_MAX / sizeof(cgsize_t)) {
+                if (m_trail_size > CGNS_ENUMV(SIZE_MAX) / sizeof(cgsize_t)) {
                   cgi_error("Error in allocation size for trail_elements");
                   return CG_ERROR;
                 }
@@ -7484,7 +7484,7 @@ int cg_poly_elements_general_write(int fn, int B, int Z, int S,
             }
         }
         /* create new element connectivity array and offsets*/
-        if (newsize > CG_SIZE_MAX / sizeof(cgsize_t)) {
+        if (newsize > CGNS_ENUMV(SIZE_MAX) / sizeof(cgsize_t)) {
             cgi_error("Error in allocation size for new connectivity data");
             return CG_ERROR;
         }
@MicK7
Copy link
Contributor
MicK7 commented Jan 7, 2025 •
edited
Loading

Hello CG_SIZE_MAX should be a MACRO value defined in cgnstypes.h .
It does not need any protection.
Can you check that cgnstypes.h was generated ? What compilation error do you get ?

@pavanakumar
Copy link
Author

cgnstypes.h is indeed being generated in the cmake build/src folder. But I get the following compilation error,

CGNS/src/cgnslib.c:6923:23: error: 'CG_SIZE_MAX' undeclared (first use in this function); did you mean 'SIZE_MAX'?
 6923 |         if (newsize > CG_SIZE_MAX / sizeof(cgsize_t)) {
      |                       ^~~~~~~~~~~
      |                       SIZE_MAX

@MicK7
Copy link
Contributor
MicK7 commented Jan 7, 2025 •
edited
Loading

Have you check that CG_SIZE_MAX is or is not present in the cgnstype.h of your cmake build folder ? It may be a cmake or toolchain issue since the code has been tested with gcc 14 without any issue. You should try to clean up the build and do a new install.

@pavanakumar
Copy link
Author

I did a clean configure/compile but this does not change the compile errors. Protecting with CGNS_ENUMV removes the errors. I guess this maybe a Mac specific issue with gcc-14.

@MicK7
Copy link
Contributor
MicK7 commented Jan 7, 2025

As already mentioned CG_SIZE_MAX is a MACRO defined value, it is not an enum. Thus, good for you if it works with your modification but we won't do this kind of incoherent modification to the CGNS MLL code.

@brtnfld
Copy link
Member
brtnfld commented Jan 7, 2025

@pavanakumar, I can reproduce your error. I'll look into an alternative solution. Thanks.

@brtnfld brtnfld self-assigned this Jan 7, 2025
@brtnfld brtnfld added this to the v4.5.0 milestone Jan 7, 2025
@brtnfld brtnfld added this to v4.5.0 Jan 7, 2025
@MicK7
Copy link
Contributor
MicK7 commented Jan 7, 2025

@brtnfld does the "inttypes.h" header exists on MACOS or was it removed ?

@brtnfld
Copy link
Member
brtnfld commented Jan 7, 2025

I'm still investigating, but it seems to be a CMake issue. An Autotools build does not have any issues.

I did reproduce the error before but did a fresh install, and I can no longer reproduce the reported error. With scoping off, it always seems to work. But with scoping on, it does fail to compile the Fortran tests.

/Users/brtnfld/packages/cgns/src/tests/cgwrite_f03.F90:213:36:

  213 |           CGNS_ENUMV(Vertex), CGNS_ENUMV(Abutting1to1), CGNS_ENUMV(PointList), npnts, pnts, 'zone#2', &
      |                                    1

@brtnfld
Copy link
Member
brtnfld commented Jan 7, 2025 •
edited
Loading

I think it has to do with doing an Autotools build in the source directory and then a CMake build using that source directory, at least for me.

If you clone the source, do a CMake build; it has no issues with scoping on or off.

@pavanakumar, can you try that?

@brtnfld
Copy link
Member
brtnfld commented Jan 7, 2025

Using a fresh CGNS install, I have no issues with CMake or Autotools, with and without scoping using GCC 14.2.0 on Macs. If you still do, then can you provide your CGNS build options?

@pavanakumar
Copy link
Author

Here are my build options

-DCGNS_ENABLE_HDF5=ON 
-DCGNS_ENABLE_PARALLEL=ON 
-DHDF5_NEED_MPI=ON 
-DCGNS_ENABLE_MEM_DEBUG=ON 
-DCGNS_ENABLE_SCOPING=ON 
-DCGNS_ENABLE_64BIT=ON 
-DCGNS_BUILD_SHARED=ON

I tried with/without scoping and also disabled mem debug. I get the same compiler error. Note that I use HDF5 v1.14.3 compiled against MPICH v4.3.0b1 and Cmake v3.31.2.

@MicK7
Copy link
Contributor
MicK7 commented Jan 8, 2025 •
edited
Loading

@pavanakumar The develop branch has been updated to help cmake identify your issue.
@brtnfld How did you manage to reproduce the issue (was it an inplace build?) ?

@MicK7 8000
Copy link
Contributor
MicK7 commented Jan 8, 2025

@brtnfld I don't mind keeping this issue open. But as we currently do not manage to reproduce the issue and the MacOS CI build works perfectly well I suggest removing the blocking flag and add a low priority.

@brtnfld
Copy link
Member
brtnfld commented Jan 8, 2025

I'm sorry. I had to rebuild mpich with GCC 14.2, which takes forever. The original reproducer was from a CGNS directory that I use for a playground. Once I used a clean CGNS checkout, I could not reproduce the error regardless of whether an Autotools build was also there. It always works for parallel or serial.

I should have asked what version of CGNS this is.

I agree; we will push this to version 4.6.0 with a lower status. I'll go ahead and merge your recent update to master for the release.

@brtnfld brtnfld modified the milestones: v4.5.0, v4.6.0 Jan 8, 2025
@brtnfld brtnfld removed this from v4.5.0 Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brtnfld brtnfld

Labels
Component - Build Priority - Minor 🔽 Type - Bug
Projects
None yet
Milestone
v4.6.0
Development

No branches or pull requests
3 participants
@pavanakumar @MicK7 @brtnfld
2A61
0