core icon indicating copy to clipboard operation
core copied to clipboard

Build fails with PHASTA code

Open jrwrigh opened this issue 2 years ago • 16 comments

Trying to build core, I get:

In destructor ‘virtual ph::PhastaSharing::~PhastaSharing()’,
    inlined from ‘virtual ph::PhastaSharing::~PhastaSharing()’ at /home/jrwrigh/software/core/phasta/phLinks.cc:31:3,
    inlined from ‘virtual ph::PhastaSharing::~PhastaSharing()’ at /home/jrwrigh/software/core/phasta/phLinks.cc:29:12,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:134:1:
/home/jrwrigh/software/core/phasta/phLinks.cc:29:12: error: array subscript ‘ph::PhastaSharing[0]’ is partly outside array bounds of ‘unsigned char [16]’ [-Werror=array-bounds=]
   29 |     delete helperN;
      |            ^~~~~~~
In constructor ‘ph::PhastaSharing::PhastaSharing(apf::Mesh*)’,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:101:22:
/home/jrwrigh/software/core/phasta/phLinks.cc:24:39: note: object of size 16 allocated by ‘operator new’
   24 |     helperN = new apf::NormalSharing(m);
      |                                       ^
In destructor ‘virtual ph::PhastaSharing::~PhastaSharing()’,
    inlined from ‘virtual ph::PhastaSharing::~PhastaSharing()’ at /home/jrwrigh/software/core/phasta/phLinks.cc:31:3,
    inlined from ‘virtual ph::PhastaSharing::~PhastaSharing()’ at /home/jrwrigh/software/core/phasta/phLinks.cc:29:12,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:134:1:
/home/jrwrigh/software/core/phasta/phLinks.cc:30:12: error: array subscript ‘ph::PhastaSharing[0]’ is partly outside array bounds of ‘unsigned char [16]’ [-Werror=array-bounds=]
   30 |     delete helperM;
      |            ^~~~~~~
In constructor ‘ph::PhastaSharing::PhastaSharing(apf::Mesh*)’,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:101:22:
/home/jrwrigh/software/core/phasta/phLinks.cc:24:39: note: object of size 16 allocated by ‘operator new’
   24 |     helperN = new apf::NormalSharing(m);
      |                                       ^
In destructor ‘virtual ph::PhastaSharing::~PhastaSharing()’,
    inlined from ‘virtual ph::PhastaSharing::~PhastaSharing()’ at /home/jrwrigh/software/core/phasta/phLinks.cc:31:3,
    inlined from ‘virtual ph::PhastaSharing::~PhastaSharing()’ at /home/jrwrigh/software/core/phasta/phLinks.cc:29:12,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:134:1:
/home/jrwrigh/software/core/phasta/phLinks.cc:31:3: error: array subscript ‘ph::PhastaSharing::__as_base [0]’ is partly outside array bounds of ‘unsigned char [16]’ [-Werror=array-bounds=]
   31 |   }
      |   ^
In constructor ‘ph::PhastaSharing::PhastaSharing(apf::Mesh*)’,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:101:22:
/home/jrwrigh/software/core/phasta/phLinks.cc:24:39: note: object of size 16 allocated by ‘operator new’
   24 |     helperN = new apf::NormalSharing(m);
      |                                       ^
In member function ‘virtual bool ph::PhastaSharing::isOwned(apf::MeshEntity*)’,
    inlined from ‘virtual bool ph::PhastaSharing::isOwned(apf::MeshEntity*)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:43:30,
    inlined from ‘virtual bool ph::PhastaSharing::isOwned(apf::MeshEntity*)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:40:8,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:112:23:
/home/jrwrigh/software/core/phasta/phLinks.cc:42:9: error: array subscript ‘ph::PhastaSharing[0]’ is partly outside array bounds of ‘unsigned char [16]’ [-Werror=array-bounds=]
   42 |     if (isDG)
      |         ^~~~
In constructor ‘ph::PhastaSharing::PhastaSharing(apf::Mesh*)’,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:101:22:
/home/jrwrigh/software/core/phasta/phLinks.cc:24:39: note: object of size 16 allocated by ‘operator new’
   24 |     helperN = new apf::NormalSharing(m);
      |                                       ^
In member function ‘virtual bool ph::PhastaSharing::isOwned(apf::MeshEntity*)’,
    inlined from ‘virtual bool ph::PhastaSharing::isOwned(apf::MeshEntity*)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:43:30,
    inlined from ‘virtual bool ph::PhastaSharing::isOwned(apf::MeshEntity*)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:40:8,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:112:23:
/home/jrwrigh/software/core/phasta/phLinks.cc:44:12: error: array subscript ‘ph::PhastaSharing[0]’ is partly outside array bounds of ‘unsigned char [16]’ [-Werror=array-bounds=]
   44 |     return helperM->isOwned(e);
      |            ^~~~~~~
In constructor ‘ph::PhastaSharing::PhastaSharing(apf::Mesh*)’,
    inlined from ‘void ph::getLinks(apf::Mesh*, int, Links&, BCs&)’ at /home/jrwrigh/software/core/phasta/phLinks.cc:101:22:
/home/jrwrigh/software/core/phasta/phLinks.cc:24:39: note: object of size 16 allocated by ‘operator new’
   24 |     helperN = new apf::NormalSharing(m);
      |                                       ^
cc1plus: all warnings being treated as errors

This is with GCC 13.1.1, which might have something to do with it. I'm not super comfortable with C++, so I'm not quite sure what the issue is (other than something is being accessed outside array bounds). I'll take a look at it, but I'm not sure why I'd suddenly be running into issues.

jrwrigh avatar Jul 20 '23 20:07 jrwrigh

Writing down what (little) I've figured out, focusing on the first error message, it's complaining specifically about the destruction of shr at the end of the getLinks() function call: https://github.com/SCOREC/core/blob/97523f8dcbfca22f095813290e459b4877869ea5/phasta/phLinks.cc#L101

Things I still don't understand:

  • Where is the unsigned char [16] part coming from?
    • Well, I'm pretty sure it comes from /home/jrwrigh/software/core/phasta/phLinks.cc:24:39: note: object of size 16 allocated by ‘operator new’, but how "object of size 16" --> unsigned char [16] I don't know other than that's 16 bytes of data
  • From array subscript ‘ph::PhastaSharing[0]’, why is it trying to access the struct as an array?
    • Like, object[0] is equivalent to *object (dereferenced pointer), but shr isn't a pointer, it's just a struct.

I'm guessing the answer to most of these questions lies somewhere in C++ lore I've yet to become familiar with...

jrwrigh avatar Jul 20 '23 23:07 jrwrigh

Hi @jrwrigh. What repo/branch/commit is this?

cwsmith avatar Jul 21 '23 02:07 cwsmith

This is on master

jrwrigh avatar Jul 21 '23 02:07 jrwrigh

Thanks. To confirm, you are using SCOREC/core master @ 97523f8 (the current HEAD)?

cwsmith avatar Jul 21 '23 02:07 cwsmith

Actually, in double checking, it's the branch in https://github.com/SCOREC/core/pull/391 so that I can actually build it with CGNS.

If I run with CGNS enabled on master, it fails (which is what #391 addresses). If I disable CGNS, it builds fine.

On the #391 branch, CGNS disabled compiles fine, but running with CGNS causes the above error messages.

jrwrigh avatar Jul 21 '23 04:07 jrwrigh

Ooooh, I think I know what's happening. It's the change in the C++ standard. Currently CGNS "requires" C++14, whereas the rest is C++11. I'm guessing that change in standard is what's causing the issues.

Specifically looking at this part of the CMakeLists code: https://github.com/SCOREC/core/blob/97523f8dcbfca22f095813290e459b4877869ea5/CMakeLists.txt#L40-L45

Trying to compile with C++11 flags and CGNS enabled runs into different compiler errors, specifically with the CGNS source files (apfCGNS.cc and mdsCGNS.cc).

jrwrigh avatar Jul 21 '23 14:07 jrwrigh

@cwsmith who is the maintainer of phLinks? I see @seegyoung as possibly the original author. Has there been discussion of getting SCOREC/core to be C++14 compliant? If not, is it more practical to find a way to get CGNS to tolerate C++11?

While I was not familiar at first with phLinks, it looks like it is what builds all the part boundary stuff so it is rather critical.

KennethEJansen avatar Jul 21 '23 14:07 KennethEJansen

@jrwrigh @KennethEJansen I'm probably not going to have time to look at this in depth until later next week or the following week. I can provide brief inputs and answers until then. On that front, I'll first look into the C++ standards mentioned.

cwsmith avatar Jul 21 '23 15:07 cwsmith

Understood @cwsmith . Actually, I was hoping that we could get this delegated to someone else given your scarce time OR determine that C++14 compliance for SCOREC/core was not in the near term achievable horizon in which case we should try to get the CGNS back compatible to C++11.

To be clear, we have no expectation of you picking up support of CGNS issues and won't be bugging you about that. Further, we might ask @a-jp for help (though I expect his support time is also scarce since this was a very valuable volunteer project that should not come with expectations of ongoing support). We as the consumers of this development will have to shoulder the load to get it compatible with phLinks and other C++11 blocked components or find a way to bypass them (since PETSc will likely be building its own version of the part boundary interfaces and does not really need us to do that).

KennethEJansen avatar Jul 21 '23 15:07 KennethEJansen

Looking into it, there's nothing about the CGNS library that's C++, so I'm guessing that it's simply that the source code of apfCGNS.cc and mdsCGNS.cc are just written in C++14 compliant way.

I'll spend a bit of time just taking whatever the compiler's suggestion to fix the non-C++11 compliant issues are and see if that resolves the issues.

jrwrigh avatar Jul 21 '23 15:07 jrwrigh

Thanks. This seems the best approach and hopefully won't offend @a-jp too much. If it does, we can keep it on a branch that we use until SCOREC/core is ready to be C++14 compliant.

KennethEJansen avatar Jul 21 '23 16:07 KennethEJansen

@jrwrigh Would you please share the cmake configuration command (i.e., cmake /path/to/pumi -DOPTION=foo ...) you are using?

cwsmith avatar Jul 28 '23 14:07 cwsmith

cmake ../core \
  -DCMAKE_C_COMPILER="mpicc" \
  -DCMAKE_CXX_COMPILER="mpicxx" \
  -DCMAKE_C_FLAGS="-Wno-error" \
  -DCMAKE_CXX_FLAGS="-Wno-error" \
  -DMESHES="../core/pumi-meshes" \
  -DCMAKE_EXPORT_COMPILE_COMMANDS=1 \
  -DIS_TESTING=ON \
  -DENABLE_ZOLTAN=ON \
  -DENABLE_CGNS=ON

jrwrigh avatar Jul 28 '23 15:07 jrwrigh

HI I am using the version tag/2.2.7 and run into the same error as discussed here. I am using GCC/11.3.0. I am looking forward to updates:-)

daisy20170101 avatar Jan 10 '24 22:01 daisy20170101

PR https://github.com/SCOREC/core/pull/420 should fix the phLinks.cc build error. I didn't test with CGNS enabled.

cwsmith avatar Mar 05 '24 21:03 cwsmith

#420 was just merged into develop. If the nightly tests pass I'll mark this as closed.

cwsmith avatar Mar 06 '24 14:03 cwsmith